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

Show warning, when overwriting variable #3674

Open
guettli opened this issue Jan 31, 2025 · 4 comments
Open

Show warning, when overwriting variable #3674

guettli opened this issue Jan 31, 2025 · 4 comments
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@guettli
Copy link

guettli commented Jan 31, 2025

Is your feature request related to a problem? Please describe.

I wrote that code, and I did not understand why path was empty, although findDev() returns a non-empty path:

func getDeviceFromPath(path string) (*evdev.InputDevice, error) {
	if path == "" {
		path, err := findDev()   // "path" of outer block gets overwritten
		if err != nil {
			return nil, err
		}
		fmt.Printf("Using device %q\n", path)
	}
	sourceDev, err := evdev.Open(path)

Describe the solution you'd like

I would like to see a warning in vscode, when I overwrite a variable from the outer scope.

Describe alternatives you've considered

I could use external linters, but I think that should be available "out of the box".

@gopherbot gopherbot added this to the Untriaged milestone Jan 31, 2025
@h9jiang
Copy link
Member

h9jiang commented Feb 3, 2025

Hi guettli,

Based on the go spec,

An identifier declared in a block may be redeclared in an inner block. While the identifier of the inner declaration is in scope, it denotes the entity declared by the inner declaration.

In this case, path got re-declared so the value returns from findDev() got assigned to path declare in the inner block (the if block).

I would like to see a warning in vscode, when I overwrite a variable from the outer scope.

If I understand your quest, you are looking for a warning when you re-declare a variable in the inner block. In this case, show a warning in line 3 path that tell the user this path is re-declaring in the inner scope (because there is another path in the outer block).

@findleyr to see if this is something worth implementing from the gopls side.

@h9jiang h9jiang added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Feb 3, 2025
@firelizzard18
Copy link
Contributor

@guettli redeclaring a variable in an inner scope is a pretty normal thing to do in Go. Given the Go spec and the language's approach to this situation I think an external linter is more appropriate than gopls itself issuing a warning for all inner scope redeclarations. That being said I think this particular scenario is well suited for go vet to the point where I'm almost surprised it isn't already a warning. I suggest submitting a proposal in the go repo to add a go vet analyzer that:

  1. Detects an operation that returns two values;
  2. Where the second value is either error or bool;
  3. And the first value redeclares an existing variable.

While I agree with the language designers that redeclaration in general should not be considered a problem, I've been tripped up by this specific scenario many times.

@guettli
Copy link
Author

guettli commented Feb 4, 2025

@firelizzard18 thank you for your feedback

I created an issue in the Go repo for that: golang/go#71555

@h9jiang
Copy link
Member

h9jiang commented Feb 20, 2025

Hi guettli

I find this issue very similar to your request here. I wonder if this is something you are looking for. #270

"gopls": {
        "ui.diagnostic.analyses": {
            "shadow": true
        },
},

I give it a try with function below and variable err and path have shadow warnings.

Image Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants