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: when working in vendor mode autoimport adds packages from vendor dir #56291

Open
inliquid opened this issue Oct 18, 2022 · 7 comments
Assignees
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@inliquid
Copy link

inliquid commented Oct 18, 2022

gopls version

>gopls -v version
Build info
----------
golang.org/x/tools/gopls master
    golang.org/x/tools/gopls@(devel)
    github.com/BurntSushi/toml@v1.2.0 h1:Rt8g24XnyGTyglgET/PRUNlrUeu9F5L+7FilkXfZgs0=
    github.com/google/go-cmp@v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg=
    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
    golang.org/x/exp@v0.0.0-20220722155223-a9213eeb770e h1:+WEEuIdZHnUeJJmEUjyYC2gfUMj69yZXw17EnHg/otA=
    golang.org/x/exp/typeparams@v0.0.0-20220722155223-a9213eeb770e h1:7Xs2YCOpMlNqSQSmrrnhlzBXIE/bpMecZplbLePTJvE=
    golang.org/x/mod@v0.6.0-dev.0.20220419223038-86c51ed26bb4 h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
    golang.org/x/sync@v0.0.0-20220722155255-886fb9371eb4 h1:uVc8UZUe6tr40fFVnUP5Oj+veunVezqYl9z7DYw9xzw=
    golang.org/x/sys@v0.0.0-20220808155132-1c4a2a72c664 h1:v1W7bwXHsnLLloWYTVEdvGvA7BHMeBYsPcF0GLDxIRs=
    golang.org/x/text@v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk=
    golang.org/x/tools@v0.1.13-0.20220928184430-f80e98464e27 => ../
    golang.org/x/vuln@v0.0.0-20221010193109-563322be2ea9 h1:KaYZQUtEEaV8aVADIHAuYBTjo77aUcCvC7KTGKM3J1I=
    honnef.co/go/tools@v0.3.3 h1:oDx7VAwstgpYpb3wv0oxiZlxY+foCpRAwY7Vk6XpAgA=
    mvdan.cc/gofumpt@v0.3.1 h1:avhhrOmv0IuvQVK7fvwV91oFSGAk5/6Po8GXTzICeu8=
    mvdan.cc/xurls/v2@v2.4.0 h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc=
go: go1.19.2

go env

>go env
set GO111MODULE=on
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\***\AppData\Local\go-build
set GOENV=C:\Users\***\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\***\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\***\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.19.2
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=C:\Users\***\AppData\Local\Temp\go-build1332515651=/tmp/go-build -gno-record-gcc-switches

What did you do?

When working in vendor module mode, and file does not contain imports yet, press Ctrl-S to force format and auto import actions.

What did you expect to see?

Imports added with correct package names, for example: google.golang.org/grpc

What did you see instead?

Packages added from vendor dir, for example: "path/to/my/module/vendor/google.golang.org/grpc"

@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 Oct 18, 2022
@gopherbot gopherbot added this to the Unreleased milestone Oct 18, 2022
@findleyr
Copy link
Contributor

findleyr commented Oct 19, 2022

Thank you for the report.

CC @adonovan, as this coincidentally seems very similar to https://go.dev/cl/443636: clearly a package path and import path are being confused here (though I don't necessarily think this will be fixed by that CL -- goimports is mostly disconnected from the rest of gopls).

@inliquid are you by any chance working on an open-source project? If not, we can try to reproduce from scratch.

@findleyr findleyr modified the milestones: Unreleased, gopls/v0.10.1 Oct 19, 2022
@inliquid
Copy link
Author

inliquid commented Oct 19, 2022

@findleyr, unfortunately this is a private repo.

@adonovan adonovan self-assigned this Oct 20, 2022
@inliquid
Copy link
Author

inliquid commented Oct 29, 2022

I also found that when you use "References" or "Implementations" features, VS Code also displays paths from vendor:
изображение

@findleyr findleyr modified the milestones: gopls/v0.10.1, gopls/v0.10.2 Nov 1, 2022
@jstroem
Copy link

jstroem commented Nov 9, 2022

Working on a private repo I've noticed the same.

@gopherbot
Copy link

gopherbot commented Nov 11, 2022

Change https://go.dev/cl/448377 mentions this issue: gopls/internal/lsp/cache: use Package.FileSet where possible

gopherbot pushed a commit to golang/tools that referenced this issue Nov 11, 2022
This change adds a FileSet field to Package, and uses it in
preference to Snapshot.FileSet wherever that is appropriate:
all but a handful of places.

For now, they must continue to refer to the same instance,
but once we do away with the Snapshot's cache of parsed files,
there will be no need for a global FileSet and each Package
will be able to create its own. (Some care will be required
to make sure it is always clear which package owns each
each token.Pos/ast.Node/types.Object when there are several
in play.)

Updates golang/go#56291

Change-Id: I017eb794ffb737550da6ae88462d23a8f5c1e1e7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448377
Run-TryBot: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
@adonovan
Copy link
Member

adonovan commented Nov 17, 2022

Hi @inliquid, I've tried to reproduce this bug a number of ways but with no success. (For example, I vendored a number of packages, created a file with missing imports of packages among the vendored set, and ran 'gopls imports' on them, and each time it got the right answer. I tried various versions of gopls before recent changes in relevant logic. And I tried it without and without a go.work file. I also looked at the various places in our code that we've flagged as suspicious in the way in which they convert between import paths and package paths, and none of them looks like an obvious candidate to explain it.)

I understand this is private code you can't share, but next time you encounter the problem, would you be willing to spend a few minutes trying to create a reproducible test case in terms of packages you can share? As soon as I can reproduce this I can work on a fix. Thanks.

@adonovan adonovan added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 17, 2022
@inliquid
Copy link
Author

inliquid commented Nov 17, 2022

@adonovan I work with gopls@master and update it regularly, does it say anything?

Just tried once again by adding glob.Glob in one of working files and pressing Ctrl-S:
изображение

If you have any idea what additional information I can provide please let me know. You are correct this is private repo so I can't share its code unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants