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: bad diagnostics for indirect dependency #39784

Closed
stamblerre opened this issue Jun 23, 2020 · 3 comments
Closed

x/tools/gopls: bad diagnostics for indirect dependency #39784

stamblerre opened this issue Jun 23, 2020 · 3 comments
Labels
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Jun 23, 2020

Reported by @hyangah on chat:

while trying to repro a user's issue report, I encountered this repo and think how the current presentation about missing indirect dependency can be confusing.

In this code https://github.com/kstenerud/go-concise-encoding/blob/master/builder_array.go#L29, gopls says "github.com/cockroachdb/apd is not in your go.mod file."

It is because "github.com/cockroachdb/apd/v2" depends on "github.com/cockroachdb/apd". And the current "go.mod" has only v2 module. Actual fix is to add the missing indirect dependency to go.mod.

But I think it can be somewhat confusing to users who are new to modules. I got 52 error messages (one for each .go file that imports v2), but zero error for the actual go.mod. Why isn't gopls sending this diagnostics msg for the go.mod file, but each individual files that have indirect dependencies?

@gopherbot gopherbot added this to the Unreleased milestone Jun 23, 2020
@stamblerre stamblerre modified the milestones: Unreleased, gopls/v0.5.0 Jun 23, 2020
@heschik
Copy link
Contributor

@heschik heschik commented Jun 23, 2020

Just remembered that this bug is probably very specific to this situation. The missing dep code uses naive prefix checking to decide which module to complain about, and it probably isn't realizing that /v2 provides the package despite go mod tidy wanting to add v1.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 15, 2020

Change https://golang.org/cl/242579 mentions this issue: internal/lsp: handle diagnostics from bad module versions

gopherbot pushed a commit to golang/tools that referenced this issue Jul 16, 2020
Currently, diagnostics for modules that are missing from the go.mod
appear in the Go files in which those modules are imported. As a result,
the diagnostics were previously calculated as part of the Go file
diagnostic calculations. This is convoluted and required passing around
an extra map.

This CL puts that logic in the ModTidyHandle where it belongs.
The diagnostics for the Go files are combined from the multiple sources.

Also, added a skipped test for golang/go#39784, since this CL was
originally intended to be a fix for that issue...

Change-Id: Ic0f9aa235dcd56ea131a2339de9801346f715415
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242579
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 17, 2020

Change https://golang.org/cl/243218 mentions this issue: internal/lsp: show errors for missing modules that don't map to an import

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