From d2c1b4b27078a4fcaa2256fe01d516f36c899606 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Wed, 22 Jul 2020 20:29:48 -0400 Subject: [PATCH] internal/lsp: show errors in indirect dependencies as diagnostics This change is a follow-up from CL 242477. We now also surface errors in indirect dependencies as diagnostics in the go.mod file. Any errors we cannot match are surfaced as diagnostics on the entire go.mod file. This isn't the most user-friendly way, but it seems simplest. Change-Id: Ife92c68c09b74a0b53de29d95b6c4c2a9c78d3b9 Reviewed-on: https://go-review.googlesource.com/c/tools/+/244438 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/mod/diagnostics.go | 24 +++++++++----- internal/lsp/regtest/modfile_test.go | 49 ++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index 5a85b7caef..b70b64a4c1 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -8,9 +8,9 @@ package mod import ( "context" - "fmt" "regexp" "strings" + "unicode" "golang.org/x/mod/modfile" "golang.org/x/mod/module" @@ -68,21 +68,25 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File return reports, nil } -var moduleAtVersionRe = regexp.MustCompile(`(?P.*)@(?P.*)`) +var moduleAtVersionRe = regexp.MustCompile(`^(?P.*)@(?P.*)$`) // ExtractGoCommandError tries to parse errors that come from the go command // and shape them into go.mod diagnostics. func ExtractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, loadErr error) (*source.Diagnostic, error) { // We try to match module versions in error messages. Some examples: // - // err: exit status 1: stderr: go: example.com@v1.2.2: reading example.com/@v/v1.2.2.mod: no such file or directory - // exit status 1: go: github.com/cockroachdb/apd/v2@v2.0.72: reading github.com/cockroachdb/apd/go.mod at revision v2.0.72: unknown revision v2.0.72 + // example.com@v1.2.2: reading example.com/@v/v1.2.2.mod: no such file or directory + // go: github.com/cockroachdb/apd/v2@v2.0.72: reading github.com/cockroachdb/apd/go.mod at revision v2.0.72: unknown revision v2.0.72 + // go: example.com@v1.2.3 requires\n\trandom.org@v1.2.3: parsing go.mod:\n\tmodule declares its path as: bob.org\n\tbut was required as: random.org // - // We split on colons and attempt to match on something that matches - // module@version. If we're able to find a match, we try to find anything - // that matches it in the go.mod file. + // We split on colons and whitespace, and attempt to match on something + // that matches module@version. If we're able to find a match, we try to + // find anything that matches it in the go.mod file. var v module.Version - for _, s := range strings.Split(loadErr.Error(), ":") { + fields := strings.FieldsFunc(loadErr.Error(), func(r rune) bool { + return unicode.IsSpace(r) || r == ':' + }) + for _, s := range fields { s = strings.TrimSpace(s) match := moduleAtVersionRe.FindStringSubmatch(s) if match == nil || len(match) < 3 { @@ -133,7 +137,9 @@ func ExtractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh sou } return toDiagnostic(rep.Syntax) } - return nil, fmt.Errorf("no diagnostics for %v", loadErr) + // No match for the module path was found in the go.mod file. + // Show the error on the module declaration. + return toDiagnostic(parsed.Module.Syntax) } func rangeFromPositions(uri span.URI, m *protocol.ColumnMapper, s, e modfile.Position) (protocol.Range, error) { diff --git a/internal/lsp/regtest/modfile_test.go b/internal/lsp/regtest/modfile_test.go index b5dd4ebb29..ab3e235b02 100644 --- a/internal/lsp/regtest/modfile_test.go +++ b/internal/lsp/regtest/modfile_test.go @@ -445,3 +445,52 @@ require example.com v1.2.3 } }) } + +// Confirm that an error in an indirect dependency of a requirement is surfaced +// as a diagnostic in the go.mod file. +func TestErrorInIndirectDependency(t *testing.T) { + testenv.NeedsGo1Point(t, 14) + + const badProxy = ` +-- example.com@v1.2.3/go.mod -- +module example.com + +go 1.12 + +require random.org v1.2.3 // indirect +-- example.com@v1.2.3/blah/blah.go -- +package blah + +const Name = "Blah" +-- random.org@v1.2.3/go.mod -- +module bob.org + +go 1.12 +-- random.org@v1.2.3/blah/blah.go -- +package hello + +const Name = "Hello" +` + const module = ` +-- go.mod -- +module mod.com + +go 1.14 + +require example.com v1.2.3 +-- main.go -- +package main + +import "example.com/blah" + +func main() { + println(blah.Name) +} +` + withOptions(WithProxy(badProxy)).run(t, module, func(t *testing.T, env *Env) { + env.OpenFile("go.mod") + env.Await( + env.DiagnosticAtRegexp("go.mod", "require example.com v1.2.3"), + ) + }) +}