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: vet should warn on possible mis-shaddows caused by short variable declarations. #22582

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

Comments

Projects
None yet
5 participants
@go101
Copy link

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

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2017

This example cannot be vetted.

@go101

This comment has been minimized.

Copy link
Author

commented Nov 5, 2017

@cznic
why?

@cznic

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2017

Legitimate code.

@go101

This comment has been minimized.

Copy link
Author

commented Nov 5, 2017

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

@cznic

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2017

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

@bontibon

This comment has been minimized.

Copy link
Contributor

commented Nov 5, 2017

Many new gophers give up using Go ...

That seems hyperbolic.

@go101

This comment has been minimized.

Copy link
Author

commented Nov 5, 2017

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

"possible mis-shaddow of a" is better.

@go101

This comment has been minimized.

Copy link
Author

commented Nov 5, 2017

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

This comment has been minimized.

Copy link
Member

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 Nov 5, 2017

@go101

This comment has been minimized.

Copy link
Author

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.
You can’t perform that action at this time.