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: "go vet --shadow" used to work, does not work #29260

Closed
kevinburke opened this issue Dec 14, 2018 · 7 comments
Closed

cmd/vet: "go vet --shadow" used to work, does not work #29260

kevinburke opened this issue Dec 14, 2018 · 7 comments

Comments

@kevinburke
Copy link
Contributor

@kevinburke kevinburke commented Dec 14, 2018

I have a test runner that calls go vet --all && go vet --shadow: https://github.com/kevinburke/go-bindata/blob/master/testdata/Makefile

This test runner breaks on tip:

$ go vet --shadow
vet: flag "--shadow" not defined
Run "go help vet" for more information

I'm not sure if the compatibility guarantee extends to flag arguments, but it's a little frustrating that this program broke and it took some searching to figure out how to fix it. Searching Google yields mostly results that expect "go vet --shadow" to work as previously written.

@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Dec 14, 2018

I suspect this is WAI. See the discussion in #28622, in particular Alan's comment at #28622 (comment).

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 16, 2018

@alandonovan Is there an existing shadow checker somewhere that uses the new framework that people can use?

@ianlancetaylor ianlancetaylor added this to the Go1.12 milestone Dec 16, 2018
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Dec 16, 2018

Marking for 1.12 because if nothing else this needs to be mentioned in the release notes.

@ianlancetaylor ianlancetaylor changed the title cmd/go: "go vet --shadow" used to work, does not work cmd/vet: "go vet --shadow" used to work, does not work Dec 16, 2018
@alandonovan

This comment has been minimized.

Copy link
Contributor

@alandonovan alandonovan commented Dec 17, 2018

Is there an existing shadow checker somewhere that uses the new framework that people can use?

Yes, see go help vet:

$ go help vet 
usage: go vet [-n] [-x] [-vettool prog] [build flags] [vet flags] [packages]

Vet runs the Go vet command on the packages named by the import paths.
...
The -vettool=prog flag selects a different analysis tool with alternative
or additional checks.
For example, the 'shadow' analyzer can be built and run using these commands:

  go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
  go vet -vettool=$(which shadow)

...
See also: go fmt, go fix.

The reason this checker is not included in the new vet is that it was the sole "experimental" checker, and I didn't think it warranted the introduction of the "experimental" concept to the new API. If we can improve the heuristics used by 'shadow' to the point where it would no longer be considered experimental then there's no reason not to add it back to the core vet suite.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Dec 17, 2018

Change https://golang.org/cl/154584 mentions this issue: doc: explain how to use "go vet -shadow"

@petar-dambovaliev

This comment was marked as off-topic.

Copy link

@petar-dambovaliev petar-dambovaliev commented Jun 27, 2019

Is there an existing shadow checker somewhere that uses the new framework that people can use?

Yes, see go help vet:

$ go help vet 
usage: go vet [-n] [-x] [-vettool prog] [build flags] [vet flags] [packages]

Vet runs the Go vet command on the packages named by the import paths.
...
The -vettool=prog flag selects a different analysis tool with alternative
or additional checks.
For example, the 'shadow' analyzer can be built and run using these commands:

  go install golang.org/x/tools/go/analysis/passes/shadow/cmd/shadow
  go vet -vettool=$(which shadow)

...
See also: go fmt, go fix.

The reason this checker is not included in the new vet is that it was the sole "experimental" checker, and I didn't think it warranted the introduction of the "experimental" concept to the new API. If we can improve the heuristics used by 'shadow' to the point where it would no longer be considered experimental then there's no reason not to add it back to the core vet suite.

You can come up with many fancy explanations to justify the decision.
The bottom line is that people used that feature and relied on it and it was taken out.
Everything else is noise.

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Jun 27, 2019

Quick reminder to only comment if you have an idea or constructive feedback. A new issue would probably be better too, as this issue is closed and isn't the best place to discuss.

In any case, personal opinions and criticism don't add to the thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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