proposal: x/tools/.../shadow: add flag to skip reporting about "err"/specified identifiers #61566
Comments
|
Related #61574. |
|
While I think there may be some value in letting folks control
You can build a filter based on the json output of shadow that would be as accurate as grepping the output, and can have the right exit code. (I understand this is a less convenient suggestion.) |
|
I looked at the json output, but it doesn't seem more convenient than the regular output. When vetting multiple packages, it still prints the "#" lines for each package (so the output cannot directly be parsed as json!). And otherwise it just contains the vet output lines wrapped in a json object. It seems easier to parse the regular text output. As for annotations to suppress vet warnings, I don't think I would use them in my code. They would add too much noise (cure worse than disease), especially for suppressing warnings for all redeclarations of "err". At least the example you gave in the other issue requires the annotation to be valid Go code. When using comments you could end up with a language in a language, and could want to check them for syntax, etc (at least a mistyped annotation comment would simply not suppress a redeclaration warning, so you would notice soon enough). You mention errors in unrelated packages could be suppressed. How would that happen? I run vet on the packages in my repository, which I maintain and I want to find bugs in. If I would want to check other packages, like dependencies, I would do so explicitly, and wouldn't assume the same only-err-redeclarations policy would be appropriate. I can live with my grep-solution. I believed (still do) that skipping warnings specifically for "err" (not sure about other variables) could help other developers as well, but I also see that it is a specific tweak to a general tool and we may not want to go down that path. |
The shadow vet tool output is relatively noisy on my code, reporting many instances of "err" being redeclared. I believe this is common in Go code. I have found bugs with unintended redeclarations, so I have a policy of not allowing them, except for "err", because rewriting such redeclarations is itself error-prone and a lot of work. I currently filter out "err" redeclaration errors with grep. However, the exit code still indicates failure (I want to run shadow along with other checkers in a Makefile target, e.g. https://github.com/mjl-/mox/blob/main/Makefile#L32) and there is still a line of output for each package.
I'm proposing to add a flag "-skip" to the shadow tool, that takes a comma-separated list of identifier names that are skipped by the shadow tool. Identifiers like "ctx" could also be an option, though I have rewritten such cases.
To implement this, we would add the flag to golang.org/x/tools/go/analysis/passes/shadow/shadow.go.
And to run:
go vet -vettool=$(which shadow) -skip err,....Perhaps not all types of shadowed declarations should be skipped if they are "err". Instead of a generic option to skip errors for specified identifiers, we could consider adding a flag that only skips certain types of redeclarations if they are for "err" and of type "error".
I previously commented in issue #58917, but that is about making shadow good enough to enable by default in vet. This proposal is just about making shadow more easily usable by projects that want to check against their own mostly-no-redeclarations policy.
The text was updated successfully, but these errors were encountered: