diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index bde23b2cc5..72fd33e9ff 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -174,7 +174,7 @@ func (s *snapshot) parseLoadError(ctx context.Context, loadErr error) source.Err if err != nil { continue } - srcErr := extractGoCommandError(ctx, s, fh, loadErr) + srcErr := s.extractGoCommandError(ctx, s, fh, loadErr.Error()) if srcErr == nil { continue } diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go index c6bce318fa..87a4baefd7 100644 --- a/internal/lsp/cache/mod.go +++ b/internal/lsp/cache/mod.go @@ -12,7 +12,6 @@ import ( "os" "path/filepath" "regexp" - "strconv" "strings" "unicode" @@ -25,11 +24,11 @@ import ( "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/memoize" "golang.org/x/tools/internal/span" - errors "golang.org/x/xerrors" ) const ( - SyntaxError = "syntax" + SyntaxError = "syntax" + GoCommandError = "go command" ) type parseModHandle struct { @@ -74,7 +73,7 @@ func (s *snapshot) ParseMod(ctx context.Context, modFH source.FileHandle) (*sour // Attempt to convert the error to a standardized parse error. var parseErrors []*source.Error if err != nil { - if parseErr, extractErr := extractModParseErrors(modFH.URI(), m, err, contents); extractErr == nil { + if parseErr, extractErr := extractErrorWithPosition(ctx, err.Error(), s); extractErr == nil { parseErrors = []*source.Error{parseErr} } } @@ -123,46 +122,6 @@ func sumFilename(modURI span.URI) string { return strings.TrimSuffix(modURI.Filename(), ".mod") + ".sum" } -// extractModParseErrors processes the raw errors returned by modfile.Parse, -// extracting the filenames and line numbers that correspond to the errors. -func extractModParseErrors(uri span.URI, m *protocol.ColumnMapper, parseErr error, content []byte) (*source.Error, error) { - re := regexp.MustCompile(`.*:([\d]+): (.+)`) - matches := re.FindStringSubmatch(strings.TrimSpace(parseErr.Error())) - if len(matches) < 3 { - return nil, errors.Errorf("could not parse go.mod error message: %s", parseErr) - } - line, err := strconv.Atoi(matches[1]) - if err != nil { - return nil, err - } - lines := strings.Split(string(content), "\n") - if line > len(lines) { - return nil, errors.Errorf("could not parse go.mod error message %q, line number %v out of range", content, line) - } - // The error returned from the modfile package only returns a line number, - // so we assume that the diagnostic should be for the entire line. - endOfLine := len(lines[line-1]) - sOffset, err := m.Converter.ToOffset(line, 1) - if err != nil { - return nil, err - } - eOffset, err := m.Converter.ToOffset(line, endOfLine) - if err != nil { - return nil, err - } - spn := span.New(uri, span.NewPoint(line, 0, sOffset), span.NewPoint(line, endOfLine, eOffset)) - rng, err := m.Range(spn) - if err != nil { - return nil, err - } - return &source.Error{ - Category: SyntaxError, - Message: matches[2], - Range: rng, - URI: uri, - }, nil -} - // modKey is uniquely identifies cached data for `go mod why` or dependencies // to upgrade. type modKey struct { @@ -380,9 +339,14 @@ func containsVendor(modURI span.URI) bool { var moduleAtVersionRe = regexp.MustCompile(`^(?P.*)@(?P.*)$`) -// ExtractGoCommandError tries to parse errors that come from the go command +// 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.Error { +func (s *snapshot) extractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, goCmdError string) *source.Error { + // If the error message contains a position, use that. Don't pass a file + // handle in, as it might not be the file associated with the error. + if srcErr, err := extractErrorWithPosition(ctx, goCmdError, s); err == nil { + return srcErr + } // We try to match module versions in error messages. Some examples: // // example.com@v1.2.2: reading example.com/@v/v1.2.2.mod: no such file or directory @@ -393,7 +357,7 @@ func extractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh sou // 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 - fields := strings.FieldsFunc(loadErr.Error(), func(r rune) bool { + fields := strings.FieldsFunc(goCmdError, func(r rune) bool { return unicode.IsSpace(r) || r == ':' }) for _, s := range fields { @@ -424,7 +388,7 @@ func extractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh sou return nil } return &source.Error{ - Message: loadErr.Error(), + Message: goCmdError, Range: rng, URI: fh.URI(), } @@ -456,3 +420,55 @@ func extractGoCommandError(ctx context.Context, snapshot source.Snapshot, fh sou } return toSourceError(pm.File.Module.Syntax) } + +// errorPositionRe matches errors messages of the form ::, +// where the is optional. +var errorPositionRe = regexp.MustCompile(`(?P.*:([\d]+)(:([\d]+))?): (?P.+)`) + +// extractErrorWithPosition returns a structured error with position +// information for the given unstructured error. If a file handle is provided, +// the error position will be on that file. This is useful for parse errors, +// where we already know the file with the error. +func extractErrorWithPosition(ctx context.Context, goCmdError string, src source.FileSource) (*source.Error, error) { + matches := errorPositionRe.FindStringSubmatch(strings.TrimSpace(goCmdError)) + if len(matches) == 0 { + return nil, fmt.Errorf("error message doesn't contain a position") + } + var pos, msg string + for i, name := range errorPositionRe.SubexpNames() { + if name == "pos" { + pos = matches[i] + } + if name == "msg" { + msg = matches[i] + } + } + spn := span.Parse(pos) + fh, err := src.GetFile(ctx, spn.URI()) + if err != nil { + return nil, err + } + content, err := fh.Read() + if err != nil { + return nil, err + } + m := &protocol.ColumnMapper{ + URI: spn.URI(), + Converter: span.NewContentConverter(spn.URI().Filename(), content), + Content: content, + } + rng, err := m.Range(spn) + if err != nil { + return nil, err + } + category := GoCommandError + if fh != nil { + category = SyntaxError + } + return &source.Error{ + Category: category, + Message: msg, + Range: rng, + URI: spn.URI(), + }, nil +} diff --git a/internal/lsp/cache/view_test.go b/internal/lsp/cache/view_test.go index d911c80af8..1f66c819d9 100644 --- a/internal/lsp/cache/view_test.go +++ b/internal/lsp/cache/view_test.go @@ -8,9 +8,13 @@ import ( "io/ioutil" "os" "path/filepath" + "strings" "testing" + "golang.org/x/mod/modfile" "golang.org/x/tools/internal/lsp/fake" + "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/lsp/source" "golang.org/x/tools/internal/span" ) @@ -101,6 +105,88 @@ module fg } } +// This tests the logic used to extract positions from parse and other Go +// command errors. +func TestExtractPositionFromError(t *testing.T) { + workspace := ` +-- a/go.mod -- +modul a.com +-- b/go.mod -- +module b.com + +go 1.12.hello +-- c/go.mod -- +module c.com + +require a.com master +` + dir, err := fake.Tempdir(workspace) + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(dir) + + tests := []struct { + filename string + wantRng protocol.Range + }{ + { + filename: "a/go.mod", + wantRng: protocol.Range{}, + }, + { + filename: "b/go.mod", + wantRng: protocol.Range{ + Start: protocol.Position{Line: 2}, + End: protocol.Position{Line: 2}, + }, + }, + { + filename: "c/go.mod", + wantRng: protocol.Range{ + Start: protocol.Position{Line: 2}, + End: protocol.Position{Line: 2}, + }, + }, + } + for _, test := range tests { + ctx := context.Background() + rel := fake.RelativeTo(dir) + uri := span.URIFromPath(rel.AbsPath(test.filename)) + if source.DetectLanguage("", uri.Filename()) != source.Mod { + t.Fatalf("expected only go.mod files") + } + // Try directly parsing the given, invalid go.mod file. Then, extract a + // position from the error message. + src := osFileSource{} + modFH, err := src.GetFile(ctx, uri) + if err != nil { + t.Fatal(err) + } + content, err := modFH.Read() + if err != nil { + t.Fatal(err) + } + _, parseErr := modfile.Parse(uri.Filename(), content, nil) + if parseErr == nil { + t.Fatalf("%s: expected an unparseable go.mod file", uri.Filename()) + } + srcErr, err := extractErrorWithPosition(ctx, parseErr.Error(), src) + if err != nil { + t.Fatal(err) + } + if srcErr.URI != uri { + t.Errorf("unexpected URI: got %s, wanted %s", srcErr.URI, uri) + } + if protocol.CompareRange(test.wantRng, srcErr.Range) != 0 { + t.Errorf("unexpected range: got %s, wanted %s", srcErr.Range, test.wantRng) + } + if !strings.HasSuffix(parseErr.Error(), srcErr.Message) { + t.Errorf("unexpected message: got %s, wanted %s", srcErr.Message, parseErr) + } + } +} + func TestInVendor(t *testing.T) { for _, tt := range []struct { path string