internal/lsp: extract filenames from go command errors

Update the logic that extracts positions from error messages to handle
optional column numbers, and then use that function when extracting
go command errors. This will be useful in parsing go list errors.

Change-Id: Ie68de1439f002c30f785c0c121c5cec4f2fea727
Reviewed-on: https://go-review.googlesource.com/c/tools/+/272095
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index bde23b2..72fd33e 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -174,7 +174,7 @@
 		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 c6bce31..87a4bae 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -12,7 +12,6 @@
 	"os"
 	"path/filepath"
 	"regexp"
-	"strconv"
 	"strings"
 	"unicode"
 
@@ -25,11 +24,11 @@
 	"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 @@
 		// 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 @@
 	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 @@
 
 var moduleAtVersionRe = regexp.MustCompile(`^(?P<module>.*)@(?P<version>.*)$`)
 
-// 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 @@
 	// 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 @@
 			return nil
 		}
 		return &source.Error{
-			Message: loadErr.Error(),
+			Message: goCmdError,
 			Range:   rng,
 			URI:     fh.URI(),
 		}
@@ -456,3 +420,55 @@
 	}
 	return toSourceError(pm.File.Module.Syntax)
 }
+
+// errorPositionRe matches errors messages of the form <filename>:<line>:<col>,
+// where the <col> is optional.
+var errorPositionRe = regexp.MustCompile(`(?P<pos>.*:([\d]+)(:([\d]+))?): (?P<msg>.+)`)
+
+// 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 d911c80..1f66c81 100644
--- a/internal/lsp/cache/view_test.go
+++ b/internal/lsp/cache/view_test.go
@@ -8,9 +8,13 @@
 	"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 @@
 	}
 }
 
+// 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