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

go-vet: add support for -shadow and -shadowstrict options #897

Closed
wants to merge 1 commit into from
Closed

go-vet: add support for -shadow and -shadowstrict options #897

wants to merge 1 commit into from

Conversation

@kostya-sh
Copy link
Contributor

@kostya-sh kostya-sh commented Mar 1, 2016

Starting from Go 1.6 `go vet -all -shadow' means all default checkers and also
shadow checker. This allows to add options to enable shadow check.

The discussion in #765 has more details.

@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Mar 1, 2016

Thank you. Please add documentation for these options in doc/languages.texi.

(option "-printfuncs=" flycheck-go-vet-print-functions concat
flycheck-option-comma-separated-list)
(option-flag "-shadow" flycheck-go-vet-shadow)
(option-flag "-shadowstrict" flycheck-go-vet-shadowstrict)

This comment has been minimized.

@lunaryorn

lunaryorn Mar 1, 2016
Contributor

What is the difference between -shadow and -shadowstrict?


When non-nil, enable `go tool vet' shadowed variables check.

This option requires Go 1.6 and older."

This comment has been minimized.

@lunaryorn

lunaryorn Mar 1, 2016
Contributor

Older? Shouldn't this be "Go 1.6 or newer"?

When non-nil, make `go tool vet' shadowed variables check more
strict; can be noisy.

This option requires Go 1.6 and older."

This comment has been minimized.

@lunaryorn

lunaryorn Mar 1, 2016
Contributor

Likewise.

@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Mar 1, 2016

By the way, can't we merge this option into a single one that takes three values, nil, t (-shadow) and strict (-shadow -shadowstrict)?

@kostya-sh
Copy link
Contributor Author

@kostya-sh kostya-sh commented Mar 1, 2016

Single option is good idea. I've implemented it, addressed your other comments and updated the PR. Please let me know if anything else could be improved or should be changed.

Starting from Go 1.6 `go vet -all -shadow' means all default checkers and also
shadow checker. This allows to add flycheck option to enable shadow check.

The discussion in #765 has more details.
(option "-printfuncs=" flycheck-go-vet-print-functions concat
flycheck-option-comma-separated-list)
(option-flag "-shadow" flycheck-go-vet-shadow)
(eval (when (eq flycheck-go-vet-shadow 'strict) "-shadowstrict"))

This comment has been minimized.

@lunaryorn

lunaryorn Mar 1, 2016
Contributor

Nice.

(flycheck-define-checker go-vet
"A Go syntax checker using the `go tool vet' command.

See URL `http://golang.org/cmd/go/' and URL
`http://godoc.org/golang.org/x/tools/cmd/vet'."
:command ("go" "tool" "vet"
"-all"

This comment has been minimized.

@lunaryorn

lunaryorn Mar 1, 2016
Contributor

Why's that? I don't think that was in the first version of your PR?

This comment has been minimized.

@kostya-sh

kostya-sh Mar 1, 2016
Author Contributor

You will probably complain about weirdness of go command line tools again, but this is necessary ;)

vet -shadow runs only shadow check
vet -all runs all checks apart from shadow check (same as vet with no options)
vet -all -shadow runs all checks including shadow

-shadowstrict is a flag that is only used by shadow check.

This comment has been minimized.

@lunaryorn

lunaryorn Mar 2, 2016
Contributor

Well, that is weird. In other words, go vet and go vet -all are identical?

This comment has been minimized.

@kostya-sh

kostya-sh Mar 3, 2016
Author Contributor

Yes. Internally -all is a flag that is set to true by default. Full docs: https://godoc.org/golang.org/x/tools/cmd/vet

@lunaryorn lunaryorn closed this in b1e2572 Mar 2, 2016
@lunaryorn
Copy link
Contributor

@lunaryorn lunaryorn commented Mar 2, 2016

@kostya-sh I cherry-picked your change onto master, you may safely delete your branch. Thank you very much for your contribution, and for your patience with my questions and remarks 👍.

@kostya-sh
Copy link
Contributor Author

@kostya-sh kostya-sh commented Mar 3, 2016

@lunaryorn, thank you!

@adamcohen
Copy link

@adamcohen adamcohen commented Feb 27, 2019

Just came across this @kostya-sh and wanted to say thank you for adding this option, I've run into this issue so many times, it's great to finally have a solution!!

Also, thanks to @lunaryorn for your work on flycheck, it's helped save me hours of frustration!

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

Successfully merging this pull request may close these issues.

None yet

3 participants