Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/vet: make -all -shadow mean all default checks and also -shadow #13020

kostya-sh opened this issue Oct 22, 2015 · 4 comments

cmd/vet: make -all -shadow mean all default checks and also -shadow #13020

kostya-sh opened this issue Oct 22, 2015 · 4 comments


Copy link

@kostya-sh kostya-sh commented Oct 22, 2015

Currently the only way to run all vet checks in a single invocation is -test flag:

go tool vet -test *.go

However the -test flag documentation says that it is supposed to be used only for testing.

It is possible to achieve the same effect by invoking go tool vet multiple times:

go tool vet *.go
go tool vet -shadow *.go

The problem with this approach is that if more experimental checks are added in the future or shadow become a standard checker these commands will have to be changed.

I propose to

  • add -extra flag to go vet tool that enables all experimental checkers that are not enabled by -all flag (similar to -Wall and -Wextra in gcc).
  • remove (or deprecate) -test flag. Use combination of -all and -extra flags in the tests


  • Run all standard checks (exactly the same as now): go tool vet *.go or go tool vet -all *.go
  • Run all extra checks (currently the only extra check is shadow): go tool vet -extra *.go
  • Run all checks (including extra checks): go tool vet -all -extra *.go

This change will also allow integrating shadow vet check into Emacs flycheck package. See the following discussion for more details: flycheck/flycheck#765

Copy link

@robpike robpike commented Oct 22, 2015

I think you shouldn't turn on -shadow by default in your plugin. That is the underlying motivation for this change, but -shadow just isn't reliable enough to depend on. It doesn't work well at all. That's why it's disabled.

If you really want it enabled always (which is a very bad idea), just have the plugin turn it on.

Whether another experimental feature comes along, and what its properties, reliability, and safety might be are all unknown at this point, so planning for them in a way that others may depend on seems unwise.

I vote against this proposal.

Copy link
Contributor Author

@kostya-sh kostya-sh commented Oct 23, 2015

First of all while the shadow check is not accurate I still find it helpful especially when used interactively with an editor. From my experience accidental shadowing usually leads to unpredictable bugs that are hard to track (possibly because I am coming from Java where shadowed variables are not allowed) and any help from the tools is appreciated.

Secondly the main motivation for this proposal is not to enable shadow check by default but to make it easier to turn it on by plugins.

The only way to run all vet checks (including shadow) at the moment is to invoke go vet twice. It is possible to do it from the flycheck but the plugin author is reluctant to accept such change because it adds more complexity and such approach is different from the rest of the checkers. It would be good if it was possible to have a new flycheck configuration option "enable-shadow" that just translates to a go vet command line flag.

The concern about any upcoming experimental features is reasonable though. I'd like to propose an alternative approach to -extra flag: if -all flag is specified then any additional checks enabled via command line flags should run together with the checks enabled by -all flag. E.g.

  • go tool vet -all will run all standard checks (as it does now)
  • go tool vet -all -shadow will run all standard checks and shadow check (currently this command runs only shadow check)
  • go tool vet -shadow will run only shadow check (as it does now)
Copy link

@robpike robpike commented Oct 23, 2015

If go tool vet -all -shadow doesn't run -all, only runs -shadow, that's just a bug. Please file an issue.

@rsc rsc added this to the Proposal milestone Oct 24, 2015
@rsc rsc changed the title proposal: cmd/vet: make possible to run all (including shadow) vet checks in a single invocation cmd/vet: make -all -shadow mean all default checks and also -shadow Oct 24, 2015
@rsc rsc modified the milestones: Go1.6, Proposal Oct 24, 2015
Copy link

@gopherbot gopherbot commented Oct 26, 2015

CL mentions this issue.

@rsc rsc closed this in 3fb9e08 Dec 5, 2015
@golang golang locked and limited conversation to collaborators Dec 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.