x/tools/gopls: normalize feature sets of unimported completions and organize imports

[SteelPhase]

What version of Go are you using (go version)?

go version go1.13.4 darwin/amd64

Does this issue reproduce with the latest release?

Can't test on latest release, will update when brew makes go1.13.5 available

What operating system and processor architecture are you using (go env)?

go env Output

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/steelphase/Library/Caches/go-build"
GOENV="/Users/steelphase/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY="go.company.org"
GONOSUMDB="go.company.org"
GOOS="darwin"
GOPATH="/Users/steelphase/Code/Go"
GOPRIVATE="go.company.org"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13.4/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/0b/zk7t542s1sj_31mwpwyk3cy81gy8z8/T/go-build062639987=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Pasted in some code that referenced a dependency that needed to be imported.

What did you expect to see?

import to be autocompleted for direct dependency go.company.org/org/lib/v4/errors

What did you see instead?

import autocompleted to transitive dependency github.com/pkg/errors

Additional Details

This is the go.mod of the module I'm working in

module go.company.org/org/thing

go 1.13

require (
	go.uber.org/zap v1.13.0
	go.company.org/org/lib/v4 v4.1.0
)

output of go mod why

steelphase@macbook:lib$go mod why github.com/pkg/errors
# github.com/pkg/errors
go.company.org/org/thing/cmd/thing
go.uber.org/zap
go.uber.org/zap.test
github.com/pkg/errors

[gopherbot]

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.

[SteelPhase]

@gopherbot FeatureRequest

[heschi]

Partly a followup to https://golang.org/cl/204203 -- we should prefer direct deps more than transitive. Also, maybe look into having imports on save do the same sorting.

[gopherbot]

Change https://golang.org/cl/212021 mentions this issue: internal/imports: consider direct mod deps more relevant

[heschi]

Retitling to reflect the remaining work.

Unimported completions are missing the "sibling imports" trick. This would be a nice feature. I'm not sure whether to reuse the implementation from the imports package or rewrite it from scratch. Sharing code would be nice, but gopls already has all the parsing done. Just pass all the sibling files to GetPackageExports?

Organize Imports is missing the sorting by relevance. This should be easy to plug in to addExternalCandidates, as long as we want it to affect goimports. Sibling imports will take precedence, but that's probably appropriate.

[heschi]

...except that addExternalCandidates goes to great lengths to return the first usable candidate it finds for speed reasons. It would be very nice to prioritize the scan in the right order, rather than sorting afterward.

[muirdm]

It would be nice if gopls could scan the entire workspace for import aliasing patterns rather than only sibling files.

I also imagine it would be convenient if the aliased unimported packages were addressable by both the alias and the package name since the user may or may not anticipate the alias.

[atombender]

Any movement on this? It's quite an annoying issue.

[heschi]

Please be specific about what you're seeing and why it's annoying.

[atombender]

Sorry, I wasn't under the impression that the issue covered more than one bug. What I'm seeing is that import-organization-on-save adds imports that aren't direct dependencies (in go.mod) or even transitive deps (in go.sum).

For example, assert often pulls in gotest.tools/assert, which is in my local mod cache but nowhere else, instead of github.com/stretchr/testify/assert, which is in go.mod.

It's very annoying when writing tests, because I prefer to just type a bunch of tests and have imports be resolved on save, instead of tediously autocompleting every import.

[gopherbot]

Change https://go.dev/cl/623296 mentions this issue: internal/imports: carve out a Source interface for index integration

[linsite]

It it a reasonable temporary solution to take pkg.relevance into account when sorting []byDistanceOrImportPathShortLength{}? Or there is a more reasonable solution , please let me know. I'd like to give it a shot. The scanning order is a very complicated way to new ones like me.

...except that addExternalCandidates goes to great lengths to return the first usable candidate it finds for speed reasons. It would be very nice to prioritize the scan in the right order, rather than sorting afterward.