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: missing direct requirements also listed in go.mod diagnostics #40470

Closed
myitcv opened this issue Jul 29, 2020 · 5 comments
Closed

x/tools/gopls: missing direct requirements also listed in go.mod diagnostics #40470

myitcv opened this issue Jul 29, 2020 · 5 comments

Comments

@myitcv
Copy link
Member

@myitcv myitcv commented Jul 29, 2020

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

$ go version
go version devel +71218dbc40 Wed Jul 22 21:31:19 2020 +0000 linux/amd64
$ go list -m golang.org/x/tools
golang.org/x/tools v0.0.0-20200729041821-df70183b1872
$ go list -m golang.org/x/tools/gopls
golang.org/x/tools/gopls v0.0.0-20200729041821-df70183b1872

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/myitcv/.cache/go-build"
GOENV="/home/myitcv/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/myitcv/gostuff/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/myitcv/gostuff"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/myitcv/gos"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/myitcv/gos/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/myitcv/gostuff/src/github.com/myitcv/govim/go.mod"
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 -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build466676480=/tmp/go-build -gno-record-gcc-switches"

What did you do?

CL 243218 added diagnostics to go.mod file where indirect requirements are missing. Quoting from the commit message:

Some modules may need to be added to the go.mod without being associated
with an import statement. In such cases, we show the missing module
diagnostic on the whole go.mod file. This isn't ideal, but mapping to
the full require statement isn't that simple, and this is an easy enough
starting point. The code in mod_tidy.go is becoming more unwieldy - I
think I will clean it up in a follow-up.

However we are seeing non-indirect requirements being reported as go.mod diagnostics in the scenario_tempmodfile/modfile_changes govim test.

--- FAIL: TestScripts (1.54s)
    main_test.go:77: using gopls at "/tmp/gobin-gopls-installdir340246205/gopls"
    --- FAIL: TestScripts/scenario_tempmodfile (0.04s)
        --- FAIL: TestScripts/scenario_tempmodfile/modfile_changes (10.62s)
            testscript.go:382:
                # A simple test that verifies we get a diagnostic for imports
                # that are missing from our go.mod file (0.000s)
                # Open main.go and save to ensure we are actually doing something via gopls (0.339s)
                # Sleep because we don't have an event we are waiting for (5.000s)
                # Verify that there have been no changes to go.mod (0.000s)
                # Verify that we diagnostics for the import in main.go that can't
                # be satisfied by the go.mod on disk (4.939s)
                > vimexprwait errors.golden GOVIMTest_getqflist()
                [stderr]
                 [
                   {
                -    "bufname": "go.mod",
                -    "col": 1,
                -    "lnum": 1,
                -    "module": "",
                -    "nr": 0,
                -    "pattern": "",
                -    "text": "example.com/blah is not in your go.mod file",
                -    "type": "",
                -    "valid": 1,
                -    "vcol": 0
                -  },
                -  {
                     "bufname": "main.go",
                     "col": 8,
                     "lnum": 3,
                     "module": "",
                     "nr": 0,
                     "pattern": "",
                     "text": "example.com/blah is not in your go.mod file",
                     "type": "",
                     "valid": 1,
                     "vcol": 0
                   }
                 ]
                [exit status 1]
                FAIL: testdata/scenario_tempmodfile/modfile_changes.txt:20: unexpected command failure
FAIL

What did you expect to see?

Only missing indirect requirements to be listed as go.mod diagnostics.

What did you see instead?

Missing direct requirements being listed twice: once in the file where they are imported, and secondly in the go.mod file.


cc @stamblerre @findleyr

FYI @leitzler

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jul 29, 2020

Hm, this is actually my intention, but it wasn't supposed to happen in this CL. https://golang.org/cl/244610 is meant to add this behavior. I'll look into the bug.

@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.4.4 Jul 29, 2020
@myitcv
Copy link
Member Author

@myitcv myitcv commented Jul 29, 2020

Ah if this is ultimately intended, perhaps not worth worrying about?

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 29, 2020

Change https://golang.org/cl/245537 mentions this issue: internal/lsp: don't show diagnostic in go.mod for direct dependencies

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jul 29, 2020

It's going into the release so we should fix it before it does. But yes, in general, the plan is to show those diagnostics in the future.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 29, 2020

Change https://golang.org/cl/245541 mentions this issue: [gopls-release-branch.0.4] internal/lsp: don't show diagnostic in go.mod for direct dependencies

gopherbot pushed a commit to golang/tools that referenced this issue Jul 29, 2020
…mod for direct dependencies

Fixes golang/go#40470.

Change-Id: I4831b837e6db5cc344c20480abde1e688ddab0dc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245537
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
(cherry picked from commit 8afe7e0)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/245541
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.