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

x/tools/gopls: reference shadow warning should provide more context #53052

Open
pjweinb opened this issue May 24, 2022 · 2 comments
Open

x/tools/gopls: reference shadow warning should provide more context #53052

pjweinb opened this issue May 24, 2022 · 2 comments
Labels
gopls/refactoring gopls NeedsInvestigation Tools
Milestone

Comments

@pjweinb
Copy link

@pjweinb pjweinb commented May 24, 2022

gopls version

at tip

What did you do?

I try to rename a symbol

What did you see ?

A message in a box saying
renaming this type "Limit" to "int" would shadow this reference to the type declared here

The message is unhelpful, as there is no information about where in the code the mysterious problem occurs.

@gopherbot gopherbot added Tools gopls labels May 24, 2022
@gopherbot gopherbot added this to the Unreleased milestone May 24, 2022
@mdlayher mdlayher changed the title x/tools/gopls: x/tools/gopls: reference shadow warning should provide more context May 24, 2022
@hyangah
Copy link
Contributor

@hyangah hyangah commented May 24, 2022

Indeed error messages without context are cryptic. I guess the error message was from https://github.com/golang/tools/blob/d5f48fca5379171ebce583082a3dd683c8f8ad75/internal/lsp/source/rename_check.go#L181

I see the context like position is ignored.
@suzmue Is there any reason that this position info needs to be dropped? On the other hand, if this error is surfaced as a popup like this, we need more than token.Pos

@hyangah hyangah added gopls/refactoring NeedsInvestigation labels May 24, 2022
@hyangah hyangah removed this from the Unreleased milestone May 24, 2022
@hyangah hyangah added this to the gopls/later milestone May 24, 2022
@suzmue
Copy link
Contributor

@suzmue suzmue commented May 25, 2022

I believe the positions were dropped during the migration from gorename to gopls just as a matter of making the error messages more readable. We absolutely can / probably should add the positions back in to the error messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls/refactoring gopls NeedsInvestigation Tools
Projects
None yet
Development

No branches or pull requests

4 participants