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

Jump to bottom

cmd/vet: vet should warn on possible mis-shaddows caused by short variable declarations. #22582

Closed
go101 opened this issue Nov 5, 2017 · 10 comments
Closed

cmd/vet: vet should warn on possible mis-shaddows caused by short variable declarations. #22582

go101 opened this issue Nov 5, 2017 · 10 comments
Labels
FrozenDueToAge

Comments

@go101
Copy link

go101 commented Nov 5, 2017

Many new gophers give up using Go when they ever fall into the famous trap in Go.

func f() int {
	var a = 1
	if a != 0 {
		a, b := 2, 3 // vet should consider here the "a" might be mis-shaddowed.
		if b == 2 {
			return a
		}
	}
	return a
}

Go vet should make a warning for such cases to get better user experience.
@go101 go101 changed the title cmd/vet: vet should warn on possible mis-shaddowed caused by short variable declarations. cmd/vet: vet should warn on possible mis-shaddows caused by short variable declarations. Nov 5, 2017
@cznic
Copy link
Contributor

cznic commented Nov 5, 2017

This example cannot be vetted.

@go101
Copy link
Author

go101 commented Nov 5, 2017

@cznic
why?

@cznic
Copy link
Contributor

cznic commented Nov 5, 2017

Legitimate code.

@go101
Copy link
Author

go101 commented Nov 5, 2017

Yes, it is. But legitimate code can't be vetted?

@cznic
Copy link
Contributor

cznic commented Nov 5, 2017

What should vet say about it? 'Warning: legitimate code detected'?

@ghost
Copy link

ghost commented Nov 5, 2017

Many new gophers give up using Go ...

That seems hyperbolic.

@go101
Copy link
Author

go101 commented Nov 5, 2017

What should vet say about it? 'Warning: legitimate code detected'?

"possible mis-shaddow of a" is better.

@go101
Copy link
Author

go101 commented Nov 5, 2017 •
edited

That seems hyperbolic.

Not at all. I read many anti-Go articles, this trap is a must-have in them.
And personally, I agree with them on this specific point.
(BTW, I really wanted to give up Go when I first encountered this trap.
It is more a problem than lacking of general generic for me.
Even if I am aware of this trap, I still often fall into it again and again.)

@dominikh
Copy link
Member

dominikh commented Nov 5, 2017

This is already implemented and guarded by the -shadow flag:

$ go vet -shadow foo.go
foo.go:6: declaration of "a" shadows declaration at foo.go:4
exit status 1

This check cannot be enabled by default because it is noisy and produces false positives – not all shadowing is buggy, and a lot of shadowing is intentional.

@dominikh dominikh closed this as completed Nov 5, 2017
@go101
Copy link
Author

go101 commented Nov 5, 2017

Great! Glad to know it is supported.

@golang golang locked and limited conversation to collaborators Nov 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@dominikh @cznic @gopherbot @go101