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: improve UX with unimported completions and -mod=readonly #43339

Closed
stamblerre opened this issue Dec 23, 2020 · 5 comments
Closed

x/tools/gopls: improve UX with unimported completions and -mod=readonly #43339

stamblerre opened this issue Dec 23, 2020 · 5 comments
Assignees
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 23, 2020

The new -mod=readonly changes have affected the behavior of unimported completions, leading to some strange behavior. Here's an example of the problem:

  1. Create a new module. github.com/yuin/goldmark is already in your local module cache, but not in the go.mod file of the module.
  2. Create a main.go file and start typing goldmark. You will see a completion for goldmark suggested.

Screen Shot 2020-12-23 at 2 11 38 AM

  1. Accept the completion and add a . to trigger a subsequent completion.

Screen Shot 2020-12-23 at 2 12 59 AM

  1. See no results because the new package has not yet been added to the go.mod file. However, no diagnostic is shown on the import because the file is still unsaved.

  2. Typing any characters, even if they are incorrect, will cause the diagnostic to show up.

Screen Shot 2020-12-23 at 2 15 53 AM

Also, note that goimports works fine without the require in the go.mod file, but unimported completions fail in the specific case of an accepted unimported package completion followed by a . -- saving the file as-is causes the diagnostic to appear, but it does not cause unimported completions to appear.

/cc @findleyr @heschik

@gopherbot gopherbot added this to the Unreleased milestone Dec 23, 2020
@stamblerre stamblerre added this to Needs Triage in vscode-go: gopls by default via automation Dec 23, 2020
@myitcv
Copy link
Member

@myitcv myitcv commented Dec 23, 2020

This is roughly the same scenario I mentioned when discussing automatically accepting changes to go.mod in the context of govim. Because my argument is that accepting of the completion in step 3 is tantamount to automatically accepting that the requirement should be added to your go.mod (with an implied local @latest, because it's not guaranteed to be the real @latest)

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Dec 23, 2020

IMO, the fixes needed here are to (1) show the diagnostic on the unsaved file (and figure out why it's not showing up), and (2) to make sure that unimported completions work even if the module has not yet been added to the go.mod file.

@gopherbot
Copy link

@gopherbot gopherbot commented Dec 24, 2020

Change https://golang.org/cl/280095 mentions this issue: internal/lsp: fix unimported completions with -mod=readonly

@stamblerre stamblerre self-assigned this Dec 28, 2020
@stamblerre stamblerre moved this from Needs Triage to Critical in vscode-go: gopls by default Dec 28, 2020
@stamblerre stamblerre changed the title x/tools/gopls: improve UX with unimported completions and -mod=readonly x/tools/gopls: investigate why a pkgName has an Imported *types.Package with missing dependency Dec 28, 2020
@stamblerre stamblerre removed this from Critical in vscode-go: gopls by default Dec 28, 2020
@stamblerre stamblerre reopened this Dec 28, 2020
@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Dec 29, 2020

I'm not able to reproduce this simply in the playground so this may take some more investigation: https://play.golang.org/p/7NupO_XLpKX.

@stamblerre
Copy link
Contributor Author

@stamblerre stamblerre commented Dec 29, 2020

My mistake--it actually is reproducible with a simple example: https://play.golang.org/p/j5y5HtJ8Afm. Looks like this is WAI.

@stamblerre stamblerre closed this Dec 29, 2020
@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.6.2 Dec 29, 2020
@stamblerre stamblerre changed the title x/tools/gopls: investigate why a pkgName has an Imported *types.Package with missing dependency x/tools/gopls: improve UX with unimported completions and -mod=readonly Dec 29, 2020
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