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: corrupted file= query with files of different cases #38602

Closed
stamblerre opened this issue Apr 22, 2020 · 5 comments
Closed

x/tools/gopls: corrupted file= query with files of different cases #38602

stamblerre opened this issue Apr 22, 2020 · 5 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@stamblerre
Copy link
Contributor

See the discussion on microsoft/vscode-go#3193.

/cc @heschik for case-insensitive file systems

@stamblerre stamblerre added this to the gopls/v0.5.0 milestone Apr 22, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Apr 22, 2020
@heschi
Copy link
Contributor

heschi commented Apr 23, 2020

I tracked this down.

panic: GetFile on  /tmp/scratch/case-insensitive import collision
golang.org/x/tools/internal/lsp/cache.(*snapshot).GetFile
golang.org/x/tools/internal/lsp/source.clearReports
golang.org/x/tools/internal/lsp/source.Diagnostics
golang.org/x/tools/internal/lsp.(*Server).diagnose.func2

clearReports probably shouldn't register a file that doesn't already exist, and we should ideally not treat case-insensitive import collision: "example.com/foo/x" and "example.com/Foo/x" as an error in the file "case-insensitive import collision".

@stamblerre
Copy link
Contributor Author

Ah, thanks for tracking this down. I was probably too ambitious when I removed "FindFile" from the snapshot. Do you have a good sense of how we could write a regtest for this? I guess we would have to check on the condition of no diagnostics for "case-insensitive import collision"?

@heschi
Copy link
Contributor

heschi commented Apr 24, 2020

Are you more worried about the gratuitous Loads or the fact that the diagnostic got swallowed? I don't know how to write a test for the former, but the latter should be easy.

@stamblerre
Copy link
Contributor Author

Really the former, but I think the latter could account for both? I'll try to figure out a good regtest for this. I can have a fix out for this soon - unless you already have one?

@gopherbot
Copy link

Change https://golang.org/cl/230315 mentions this issue: internal/lsp: make sure diagnostics only refer to exisiting files

@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v0.4.1 May 13, 2020
@golang golang locked and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants