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: unimported completions offer circular imports #41869

Open
stamblerre opened this issue Oct 8, 2020 · 5 comments
Open

x/tools/gopls: unimported completions offer circular imports #41869

stamblerre opened this issue Oct 8, 2020 · 5 comments

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Oct 8, 2020

To reproduce:

Type cache.New inside of golang.org/x/tools/internal/lsp/cache and accept the completion.

Related: #36077.

/cc @heschik

@gopherbot gopherbot added this to the Unreleased milestone Oct 8, 2020
@stamblerre stamblerre removed this from the Unreleased milestone Oct 8, 2020
@stamblerre stamblerre added the NeedsFix label Oct 8, 2020
@heschik
Copy link
Contributor

@heschik heschik commented Oct 8, 2020

Can't reproduce, with or without experimentalWorkspaceModule. Either way I don't think it's related to #36077. A log might help, but given how opaque the imports code is, I'm not sure :-/

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Oct 8, 2020

Weird, can't repro in cache anymore because a honnef.co cache package is now outranking it.
Here's a log with source: https://gist.github.com/stamblerre/38f1589eec5c8a5758301835b125f7e3.
Sorry it's really unedited, so it may not be super useful.

@heschik
Copy link
Contributor

@heschik heschik commented Oct 9, 2020

The log doesn't include a completion request, so I'm not sure what I'm supposed to be looking at?

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Oct 9, 2020

I must've messed it up when I was trying to truncate it. Just updated it.
The easiest thing will probably be to try reproducing yourself. Open up golang.org/x/tools/internal/lsp/source/rename_check.go, and in the first function, type source., and accept whatever the first completion is. This will add an import for golang.org/x/tools/internal/lsp/source.

@heschik
Copy link
Contributor

@heschik heschik commented Oct 9, 2020

I still can't reproduce the problem, but the logs were enough. These are typed completions from gopls, not the imports package, and it looks like I completely forgot to add a circular import check to those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.