x/tools/gopls: should suppress diagnostics when GOMODCACHE is under the repo root

[pjweinb]

Although running go 1.18, gopls shows error messages like:
2022/03/25 17:19:07 falling back to safe trimming due to type errors: [/usr/local/google/home/pjw/go/pkg/mod/github.com/google/go-cmp@v0.5.7/cmp/options.go:469:24: invalid operation: signed shift count iota (untyped int constant 0) requires go1.13 or later

These are misleading (go1.13?) and unhelpful

[peterldowns]

+1 to this, we upgraded to go1.18 and gopls 0.8.1 recently and I've noticed hundreds of these issues being reported from installed packages (installed in GOMODCACHE).

I would much prefer to not have to see these errors since I don't think they are real (we can build our binary just fine).

[hyangah]

@pjweinb @peterldowns
Thanks for the report. I think "invalid operation: signed shift count iota" error message is a different issue.

Signed shift count is indeed a language change introduced in go1.13 but go-cmp's go.mod still uses go version 1.11. which should be updated to go1.13. (related: #30791)

On the other hand, according to the original proposal, this is a backwards compatible change and go build happily builds without error reporting. So, I don't understand the purpose of this diagnostic. Not exactly the same situation, but I see error messages caused by language restriction were suppressed from compiler (e.g. https://go-review.googlesource.com/c/go/+/205977/). Should we apply the same rule and drop this diagnostic? @findleyr

@peterldowns Were you browsing the source code or directory of go-cmp when you saw the diagnostics on the packages in GOMODCACHE? AFAIK gopls does not surface diagnostics on packages outside the workspace (e.g. dependencies stored in module cache) unless the source file is open. If you close the files, the diagnostics should disappear. If not, please file an issue.

[findleyr]

@hyangah I believe this is actually a bug in go/types, when used with -lang < 1.13.

[peterldowns]

@hyangah thank you for the quick response, it's kind of you to help me triage this.

Were you browsing the source code or directory of go-cmp when you saw the diagnostics on the packages in GOMODCACHE? AFAIK gopls does not surface diagnostics on packages outside the workspace (e.g. dependencies stored in module cache) unless the source file is open. If you close the files, the diagnostics should disappear. If not, please file an issue.

I was not browsing the source code or directory of go-cmp when I saw the diagnostics on the packages in GOMODCACHE. I'm sorry for not supplying a more easily reproducible version of the problem, I'm in a monorepo with a slightly complex setup. The root of the repository has a go.work file like this:

go 1.18

use ./backend

and the backend directory contains all of our go code, including the go.mod and go.sum files. We're doing this to allow opening the root of our repository in VSCode, which seems to only play nice with the go1.18 workspaces stuff. So code . from inside the repo root works perfectly, no problems reported to VSCode. When I look at the gopls server output I see:

[Info  - 9:25:26 AM] 2022/03/30 09:25:26 go env for /Users/pld/code/pipe
(root /Users/pld/code/pipe)
(go version go version go1.18 darwin/arm64)
(valid build configuration = true)
(build flags: [])
GOCACHE=/Users/pld/Library/Caches/go-build
GOPATH=/Users/pld/code/pipe/.go
GOPROXY=https://proxy.golang.org,direct
GOROOT=/nix/store/adwrscl1v5qxbqv0l5f3wkljb0ha77mx-go-1.18/share/go
GONOPROXY=
GOINSECURE=
GONOSUMDB=
GOMODCACHE=/Users/pld/code/pipe/.go/pkg/mod
GOPRIVATE=
GOSUMDB=sum.golang.org
GO111MODULE=
GOFLAGS=
GOWORK=/Users/pld/code/pipe/go.work
GOMOD=/dev/null

and I see that the iota types are being correctly ignored, with log output like:

[Info  - 9:25:27 AM] 2022/03/30 09:25:27 falling back to safe trimming due to type errors: [/Users/pld/code/pipe/.go/pkg/mod/google.golang.org/protobuf@v1.27.1/internal/encoding/json/decode_token.go:17:23: invalid operation: signed shift count iota (untyped int constant 0) requires go1.13 or later /Users/pld/code/pipe/.go/pkg/mod/google.golang.org/protobuf@v1.27.1/internal/encoding/json/decode_token.go:18:2: invalid operation: signed shift count iota (untyped int constant 1) requires go1.13 or later /

but no entries in the Problems tab.

The issue is that if I open just the backend directory in VSCode, with code backend. Then I see all the problems that I screenshot above, even with no code open (not browsing the go modules code). The gopls server output looks like this:

[Info  - 9:27:33 AM] 2022/03/30 09:27:33 go env for /Users/pld/code/pipe/backend
(root /Users/pld/code/pipe)
(go version go version go1.18 darwin/arm64)
(valid build configuration = true)
(build flags: [])
GOINSECURE=
GO111MODULE=
GOSUMDB=sum.golang.org
GOWORK=/Users/pld/code/pipe/go.work
GOMODCACHE=/Users/pld/code/pipe/.go/pkg/mod
GOPATH=/Users/pld/code/pipe/.go
GONOSUMDB=
GONOPROXY=
GOROOT=/nix/store/adwrscl1v5qxbqv0l5f3wkljb0ha77mx-go-1.18/share/go
GOFLAGS=
GOPRIVATE=
GOCACHE=/Users/pld/Library/Caches/go-build
GOMOD=/Users/pld/code/pipe/backend/go.mod
GOPROXY=https://proxy.golang.org,direct

but no messages about falling back to safe trimming for the packages that are reporting errors in the Problem view.

If I add a backend/go.work file with the following contents:

go 1.18

use ./

then code backend works correctly, and does not report any issues in the Problems tab. The gopls server output looks like this

[Info  - 9:29:50 AM] 2022/03/30 09:29:50 go env for /Users/pld/code/pipe/backend
(root /Users/pld/code/pipe/backend)
(go version go version go1.18 darwin/arm64)
(valid build configuration = true)
(build flags: [])
GOPATH=/Users/pld/code/pipe/.go
GOROOT=/nix/store/adwrscl1v5qxbqv0l5f3wkljb0ha77mx-go-1.18/share/go
GO111MODULE=
GOCACHE=/Users/pld/Library/Caches/go-build
GOSUMDB=sum.golang.org
GOFLAGS=
GOMODCACHE=/Users/pld/code/pipe/.go/pkg/mod
GOPRIVATE=
GONOSUMDB=
GONOPROXY=
GOWORK=/Users/pld/code/pipe/backend/go.work
GOMOD=/Users/pld/code/pipe/backend/go.mod
GOPROXY=https://proxy.golang.org,direct
GOINSECURE=

and I see messages about falling back to safe trimming

[Info  - 9:29:51 AM] 2022/03/30 09:29:51 falling back to safe trimming due to type errors: [/Users/pld/code/pipe/.go/pkg/mod/github.com/google/go-cmp@v0.5.6/cmp/options.go:469:24: invalid operation: signed shift count iota (untyped int constant 0) requires go1.13 or later /Users/pld/code/pipe/.go/pkg/mod/github.com/google/go-cmp@v0.5.6/cmp/options.go:471:2: invalid operation: signed shift count iota (untyped int constant 1) requires go1.13 or later /Users/pld/code/pipe/.go/pkg/mod/github.com/google/go-cmp@v0.5.6/cmp/options.go:472:2: invalid operation: signed shift count iota (untyped int constant 2) requires go1.13 or later /Users/pld/code/pipe/.go/pkg/mod/github.com/google/go-cmp@v0.5.6/cmp/options.go:473:2: invalid operation: signed shift count iota (untyped int constant 3) requires go1.13 or later /Users/pld/code/pipe/.go/pkg/mod/github.com/google/go-cmp@v0.5.6/cmp/options.go:474:2: invalid operation: signed shift count iota (untyped int constant 4) requires go1.13 or later /Users/pld/code/pipe/.go/pkg/mod/github.com/google/go-cmp@v0.5.6/cmp/options.go:475:2: invalid operation: signed shift count iota (untyped int constant 5) requires go1.13 or later /Users/pld/code/pipe/.go/pkg/mod/github.com/google/go-cmp@v0.5.6/cmp/options.go:476:2: invalid operation: signed shift count iota (untyped int constant 6) requires go1.13 or later] or still-missing identifiers: map[]
	package="github.com/google/go-cmp/cmp"

[findleyr]

@hyangah these diagnostics are reported for packages outside of the workspace because they are type-checker errors on package-level declarations, which must type-check.

The fundamental problem here is not gopls, but that there is a bug in go/types that was only recently exposed when gopls started setting types.Config.GoVersion based on go.mod. This is probably a rarely encountered bug, by virtue of relating to constant shifts with GoVersion < 1.13. Nevertheless, it should be fixed for Go 1.18.1. I filed #52031, and have a fix in progress.

Sorry for the breakage, and thank you for the diligent report. It may have been some time before we noticed this bug.

[hyangah]

Thanks for the quick fix @findleyr and thanks @peterldowns for sharing the details.
I didn't check the code. Indeed, they are untyped const!

these diagnostics are reported for packages outside of the workspace

@findleyr that is different from what I observe - you can quickly check it from x/tools/gopls/internal/regtest/misc/import_test.go that depends on go-cmp.

BTW the title of the issue and the issue we've discussed so far don't match.

[findleyr]

@findleyr that is different from what I observe - you can quickly check it from x/tools/gopls/internal/regtest/misc/import_test.go that depends on go-cmp.

Indeed you are right. @peterldowns where is your GOMODCACHE located?

BTW the title of the issue and the issue we've discussed so far don't match.

Yes, the issue @pjweinb reports is related to noise in the logs, not noise in the diagnostics. The root cause of the noise he reported is the same, however.

[peterldowns]

@findleyr

Indeed you are right. @peterldowns where is your GOMODCACHE located?

From the terminal output above, GOMODCACHE is located at the root of our repo $repo/.go/pkg/mod/, and this is consistent between the working and buggy gopls states:

GOMODCACHE=/Users/pld/code/pipe/.go/pkg/mod

We do not set this environment variable explicitly, we just set GOPATH=$repo/.go. It seems that the go tooling infers GOMODCACHE from GOPATH.

[peterldowns]

One fact that I'll highlight is that adding the backend/go.work file fixed the issue I was encountering. Not sure if that was clear from the long initial comment I left. I'm not sure how this would have affected gopls and/or the issue with the types config. And I'm using:

# go version
go version go1.18 darwin/arm64
# gopls version
golang.org/x/tools/gopls v0.8.1
    golang.org/x/tools/gopls@v0.8.1 h1:q5nDpRopYrnF4DN/1o8ZQ7Oar4Yd4I5OtGMx5RyV2/8=

Again, sorry for not providing a more minimal repro. If you need any new information, please let me know, and I will try to find time to come up with a minimal repro if it helps.

[findleyr]

@peterldowns thanks (and sorry, I missed that your go env was in the logs).

Gopls is not suppressing diagnostics because GOMODCACHE is under the repo root, even though it is not included in the workspace. We should fix this.

[peterldowns]

@findleyr

Gopls is not suppressing diagnostics because GOMODCACHE is under the repo root, even though it is not included in the workspace. We should fix this.

Strangely, it is suppressing the diagnostics when vscode is launched in the repo root, just not when vscode is launched in the backend subdirectory. I realize the paths and the language I'm using here are confusing, so here is my best attempt at a minimal-case reproduction of the issue I've been encountering, including instructions on how to attempt the repro, a nix-shell config for getting the correct versions of go and gopls, and screenshots of the problems.

https://github.com/peterldowns/gopls-repro-example

I am happy to file a bug report separately from this thread if my issue is unrelated. Lastly, THANK YOU for helping triage this issue and for your patience with my imprecise language. I am not very familiar with the go1.18 workspace concepts, gopls, or how vscode plays into all this. I rely every day on the work you and your fellow maintainers do to make Go great and extremely usable. Thank you very much 🙇

[gopherbot]

Change https://go.dev/cl/431836 mentions this issue: gopls/internal/lsp/cache: remove distracting "safe trimming" log

[adonovan]

There were three bugs here:

1. a spurious and confusing "falling back to safe trimming" log message, which was removed by CL 431836. Trimming was not the cause of the error.
2. a bug in go/types related to signed shift counts, reported as go/types: spurious diagnostics for untyped shift operands with GoVersion < go1.13 #52031 and since fixed.
3. "Gopls is not suppressing diagnostics because GOMODCACHE is under the repo root, even though it is not included in the workspace. We should fix this."

I'm going to repurpose this issue for the third item, and assign it to Rob since it's a metadata issue, and he's the expert on that.