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

Jump to bottom

x/tools/gopls: redundant import name added #35397

Closed
muirdm opened this issue Nov 6, 2019 · 6 comments
Closed

x/tools/gopls: redundant import name added #35397

muirdm opened this issue Nov 6, 2019 · 6 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
Unreleased

Comments

@muirdm
Copy link

muirdm commented Nov 6, 2019

Steps to reproduce:

Starting with:

package main

import "os/exec"

func main() {
	exec.CommandContext()
}
  1. Complete at CommandContext(<>) and select "context.Background"
  2. I expect to get an import for "context", but I get context "context" (redundant alias).

alias

Also note that it says from "context" in the candidate description although it is not required in this case.

/cc @stamblerre @heschik
@gopherbot gopherbot added this to the Unreleased milestone Nov 6, 2019
@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 Nov 6, 2019
@gopherbot
Copy link

Thank you for filing a gopls issue! Please take a look at the Troubleshooting guide, and make sure that you have provided all of the relevant information here.

@gopherbot
Copy link

Change https://golang.org/cl/205657 mentions this issue: internal/lsp/source: don't unconditionally name imports

@heschi
Copy link
Contributor

heschi commented Nov 6, 2019

Silly mistake, thanks for the report. Why do you say that from "context" isn't necessary? It's adding a new import and I think the user should know what it is. They could have multiple context packages, or be missing the one they actually want.

@muirdm
Copy link
Author

muirdm commented Nov 6, 2019 •
edited

Why do you say that from "context" isn't necessary?

In this case I happen to know that I'm getting completions from the package of the function parameter (i.e. "context"), so there is only one possibility. But you bring up a good point that there is no existing import so the user normally can't/won't know where the candidates are coming from.

@heschi
Copy link
Contributor

heschi commented Nov 6, 2019

That would be true in some cases, but not this one. context.Context is an interface, so I was also getting completions from x/net/context, for example. I think it's probably healthy to show it unconditionally, FWIW.

This was referenced Nov 6, 2019
Closed
Closed
@gopherbot
Copy link

Change https://golang.org/cl/205738 mentions this issue: internal/lsp/source: don't unconditionally name imports

gopherbot pushed a commit to golang/tools that referenced this issue Nov 7, 2019
@heschi @stamblerre
internal/lsp/source: don't unconditionally name imports
2b2ab33 
In CL 205501 I thoughtlessly set import name to package name, but really
we only want to name imports when goimports would do it. For now, it's
better to not name them and let the usual imports code add a name if
necessary.

Fixes golang/go#35397.

Change-Id: Id0df866f95e5e86ed72b25fbd1a7224c79ee8084
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205657
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
(cherry picked from commit c2ac6c2)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205738
Reviewed-by: Heschi Kreinick <heschi@google.com>
@golang golang locked and limited conversation to collaborators Nov 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
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
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@gopherbot @muirdm @heschi