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: gopls completes a function from an unimported module in the vendor path but uses an incorrect import path with "vendor/" #71474

Open
xckomorebi opened this issue Jan 29, 2025 · 7 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. 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

@xckomorebi
Copy link

gopls version

Build info

golang.org/x/tools/gopls (devel)
golang.org/x/tools/gopls@(devel)
github.com/BurntSushi/toml@v1.4.1-0.20240526193622-a339e1f7089c h1:pxW6RcqyfI9/kWtOwnv/G+AzdKuy2ZrqINhenH4HyNs=
github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
golang.org/x/exp/typeparams@v0.0.0-20241210194714-1829a127f884 h1:1xaZTydL5Gsg78QharTwKfA9FY9CZ1VQj6D/AZEvHR0=
golang.org/x/mod@v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
golang.org/x/sync@v0.10.0 h1:3NQrjDixjgGwUOCaF8w2+VYHv0Ve/vGYSbdkTa98gmQ=
golang.org/x/telemetry@v0.0.0-20241220003058-cc96b6e0d3d9 h1:L2k9GUV2TpQKVRGMjN94qfUMgUwOFimSQ6gipyJIjKw=
golang.org/x/text@v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo=
golang.org/x/tools@v0.28.0 => ../
golang.org/x/vuln@v1.1.3 h1:NPGnvPOTgnjBc9HTaUx+nj+EaUYxl5SJOWqaDYGaFYw=
honnef.co/go/tools@v0.5.1 h1:4bH5o3b5ZULQ4UrBmP+63W9r7qIkqJClEA9ko5YKx+I=
mvdan.cc/gofumpt@v0.7.0 h1:bg91ttqXmi9y2xawvkuMXyvAA/1ZGJqYAEGjXuP0JXU=
mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8=
go: go1.23.5

golang.org/x/tools commit hash: 8171d94fe98a51fb8471b40726983dd098a3fde6

go env

GO111MODULE='off'
GOARCH='arm64'
GOBIN='/Users/<myname>/go/bin'
GOCACHE='/Users/<myname>/Library/Caches/go-build'
GOENV='/Users/<myname>/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/<myname>/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/<myname>/go'
GOPRIVATE=''
GOPROXY='https://goproxy.cn'
GOROOT='/opt/homebrew/Cellar/go/1.23.5/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.23.5/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.5'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/<myname>/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD=''
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/cs/34k5362x5b3dl83fzc5h3vm80000gn/T/go-build39960039=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

working in a repo with the following structure.

$GOPATH/src/github.com/foo
├── foo.go
└── vendor
    └── github.com
        └── bar
            └── bar.go
Image

What did you see happen?

If I accept the completion, it will import "bar" from "github.com/foo/vendor/github.com/bar" instead of "github.com/bar"

Image

What did you expect to see?

gopls should treat modules from the vendor path as normal modules.

I added a test case that reproduces the bug and noticed it only occurs when the cache is used.

golang/tools@master...xckomorebi:tools:master

Editor and settings

No response

Logs

No response

@xckomorebi xckomorebi added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Jan 29, 2025
@gopherbot gopherbot added this to the Unreleased milestone Jan 29, 2025
@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Jan 29, 2025
@ansaba
Copy link

ansaba commented Jan 29, 2025

@xckomorebi, the issue is not reproducible. Are you working on an open source repo on which we can check?

@xckomorebi
Copy link
Author

@xckomorebi, the issue is not reproducible. Are you working on an open source repo on which we can check?

Please check this commit

xckomorebi/tools@324bdd3

@findleyr
Copy link
Member

findleyr commented Feb 6, 2025

@xckomorebi I see you are using GO111MODULE='off' (i.e. GOPATH mode). That is almost certainly related.

GOPATH mode does not have good support, and is unlikely to get fixes. I suggest using Go modules.

@findleyr findleyr modified the milestones: Unreleased, gopls/backlog Feb 6, 2025
@xckomorebi
Copy link
Author

@xckomorebi I see you are using GO111MODULE='off' (i.e. GOPATH mode). That is almost certainly related.

GOPATH mode does not have good support, and is unlikely to get fixes. I suggest using Go modules.

Thank you for your clarification. However I went ahead and implemented a simple fix for this issue in my own fork, and it has been working well for me so far

Would you be open to reviewing a pull request for this? Thank you for your time.

golang/tools@5d04c69

@findleyr
Copy link
Member

findleyr commented Feb 7, 2025

@xckomorebi I would be willing to review a fix, provided:

  • It implements correct semantics of GOPATH vendoring (as described at https://go.dev/design/25719-go15vendor). The patch you linked just assumes that any vendored package can be imported.
  • It ONLY applies to GOPATH vendoring (need to check Snapshot.View().Type()).
  • It includes tests.

Sorry for being so demanding, but since we are trying not to devote resources to GOPATH, we can only accept CLs that come in narrowly scoped, tested, and correct.

@findleyr
Copy link
Member

findleyr commented Feb 7, 2025

I should add though: it does look like you've found the relevant place where PkgPath is being misinterpreted. The subtlety here is that a PkgPath (=linker path) is not the same as an Import path, and you have found exactly where the two are conflated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. 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

5 participants