internal/lsp: extract mod errors in load and treat them like diagnostics

This change pulls out the logic to process module-related errors from
go/packages.Load out of internal/lsp and into internal/lsp/cache. This
simplifies diagnostic handling in internal/lsp, as we can just return
a *source.ErrorList. Most of this is just a code move, not really a
logical change.

This also fixes the staticcheck error :)

Change-Id: Ic6cad3f59a7e4492dea85b8dcfa9e3f24a371803
Reviewed-on: https://go-review.googlesource.com/c/tools/+/260805
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 890b738..27cd534 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -175,7 +175,12 @@
 		event.Log(ctx, "go/packages.Load", tag.Snapshot.Of(s.ID()), tag.Directory.Of(cfg.Dir), tag.Query.Of(query), tag.PackageCount.Of(len(pkgs)))
 	}
 	if len(pkgs) == 0 {
-		if err == nil {
+		if err != nil {
+			// Try to extract the error into a diagnostic.
+			if srcErrs := s.parseLoadError(ctx, err); srcErrs != nil {
+				return srcErrs
+			}
+		} else {
 			err = fmt.Errorf("no packages returned")
 		}
 		return errors.Errorf("%v: %w", err, source.PackagesLoadError)
@@ -215,6 +220,25 @@
 	return nil
 }
 
+func (s *snapshot) parseLoadError(ctx context.Context, loadErr error) *source.ErrorList {
+	var srcErrs *source.ErrorList
+	for _, uri := range s.ModFiles() {
+		fh, err := s.GetFile(ctx, uri)
+		if err != nil {
+			continue
+		}
+		srcErr := extractGoCommandError(ctx, s, fh, loadErr)
+		if srcErr == nil {
+			continue
+		}
+		if srcErrs == nil {
+			srcErrs = &source.ErrorList{}
+		}
+		*srcErrs = append(*srcErrs, srcErr)
+	}
+	return srcErrs
+}
+
 // tempWorkspaceModule creates a temporary directory for use with
 // packages.Loads that occur from within the workspace module.
 func (s *snapshot) tempWorkspaceModule(ctx context.Context) (_ span.URI, cleanup func(), err error) {
diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index 50c94f7..099b5ce 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -14,8 +14,10 @@
 	"regexp"
 	"strconv"
 	"strings"
+	"unicode"
 
 	"golang.org/x/mod/modfile"
+	"golang.org/x/mod/module"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/protocol"
@@ -375,3 +377,82 @@
 	}
 	return f.IsDir()
 }
+
+var moduleAtVersionRe = regexp.MustCompile(`^(?P<module>.*)@(?P<version>.*)$`)
+
+// 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 {
+	// 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
+	//  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 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
+	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 {
+			continue
+		}
+		path, version := match[1], match[2]
+		// Any module versions that come from the workspace module should not
+		// be shown to the user.
+		if source.IsWorkspaceModuleVersion(version) {
+			continue
+		}
+		if err := module.Check(path, version); err != nil {
+			continue
+		}
+		v.Path, v.Version = path, version
+		break
+	}
+	pm, err := snapshot.ParseMod(ctx, fh)
+	if err != nil {
+		return nil
+	}
+	toSourceError := func(line *modfile.Line) *source.Error {
+		rng, err := rangeFromPositions(pm.Mapper, line.Start, line.End)
+		if err != nil {
+			return nil
+		}
+		return &source.Error{
+			Message: loadErr.Error(),
+			Range:   rng,
+			URI:     fh.URI(),
+		}
+	}
+	// Check if there are any require, exclude, or replace statements that
+	// match this module version.
+	for _, req := range pm.File.Require {
+		if req.Mod != v {
+			continue
+		}
+		return toSourceError(req.Syntax)
+	}
+	for _, ex := range pm.File.Exclude {
+		if ex.Mod != v {
+			continue
+		}
+		return toSourceError(ex.Syntax)
+	}
+	for _, rep := range pm.File.Replace {
+		if rep.New != v && rep.Old != v {
+			continue
+		}
+		return toSourceError(rep.Syntax)
+	}
+	// No match for the module path was found in the go.mod file.
+	// Show the error on the module declaration, if one exists.
+	if pm.File.Module == nil {
+		return nil
+	}
+	return toSourceError(pm.File.Module.Syntax)
+}
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 5360612..f3e59c6 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -513,29 +513,5 @@
 		}
 		return true
 	}
-	// If there is a go.mod-related error, as well as a workspace load error,
-	// there is likely an issue with the go.mod file. Try to parse the error
-	// message and create a diagnostic.
-	if modErr == nil {
-		return false
-	}
-	if errors.Is(loadErr, source.PackagesLoadError) {
-		// TODO(rstambler): Construct the diagnostics in internal/lsp/cache
-		// so that we can avoid this here.
-		reports := new(reportSet)
-		for _, uri := range snapshot.ModFiles() {
-			fh, err := snapshot.GetFile(ctx, uri)
-			if err != nil {
-				return false
-			}
-			diag, err := mod.ExtractGoCommandError(ctx, snapshot, fh, loadErr)
-			if err != nil {
-				return false
-			}
-			reports.add(fh.VersionedFileIdentity(), false, diag)
-			s.publishReports(ctx, snapshot, reports)
-			return true
-		}
-	}
 	return false
 }
diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go
index b834c7e..e8c4533 100644
--- a/internal/lsp/mod/diagnostics.go
+++ b/internal/lsp/mod/diagnostics.go
@@ -8,18 +8,11 @@
 
 import (
 	"context"
-	"fmt"
-	"regexp"
-	"strings"
-	"unicode"
 
-	"golang.org/x/mod/modfile"
-	"golang.org/x/mod/module"
 	"golang.org/x/tools/internal/event"
 	"golang.org/x/tools/internal/lsp/debug/tag"
 	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
-	"golang.org/x/tools/internal/span"
 )
 
 func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.VersionedFileIdentity][]*source.Diagnostic, error) {
@@ -60,101 +53,3 @@
 	}
 	return reports, nil
 }
-
-var moduleAtVersionRe = regexp.MustCompile(`^(?P<module>.*)@(?P<version>.*)$`)
-
-// 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:
-	//
-	//  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 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
-	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 {
-			continue
-		}
-		path, version := match[1], match[2]
-		// Any module versions that come from the workspace module should not
-		// be shown to the user.
-		if source.IsWorkspaceModuleVersion(version) {
-			continue
-		}
-		if err := module.Check(path, version); err != nil {
-			continue
-		}
-		v.Path, v.Version = path, version
-		break
-	}
-	pm, err := snapshot.ParseMod(ctx, fh)
-	if err != nil {
-		return nil, err
-	}
-	toDiagnostic := func(line *modfile.Line) (*source.Diagnostic, error) {
-		rng, err := rangeFromPositions(fh.URI(), pm.Mapper, line.Start, line.End)
-		if err != nil {
-			return nil, err
-		}
-		return &source.Diagnostic{
-			Message:  loadErr.Error(),
-			Range:    rng,
-			Severity: protocol.SeverityError,
-		}, nil
-	}
-	// Check if there are any require, exclude, or replace statements that
-	// match this module version.
-	for _, req := range pm.File.Require {
-		if req.Mod != v {
-			continue
-		}
-		return toDiagnostic(req.Syntax)
-	}
-	for _, ex := range pm.File.Exclude {
-		if ex.Mod != v {
-			continue
-		}
-		return toDiagnostic(ex.Syntax)
-	}
-	for _, rep := range pm.File.Replace {
-		if rep.New != v && rep.Old != v {
-			continue
-		}
-		return toDiagnostic(rep.Syntax)
-	}
-	// No match for the module path was found in the go.mod file.
-	// Show the error on the module declaration, if one exists.
-	if pm.File.Module == nil {
-		return nil, fmt.Errorf("no module declaration in %s", fh.URI())
-	}
-	return toDiagnostic(pm.File.Module.Syntax)
-}
-
-func rangeFromPositions(uri span.URI, m *protocol.ColumnMapper, s, e modfile.Position) (protocol.Range, error) {
-	toPoint := func(offset int) (span.Point, error) {
-		l, c, err := m.Converter.ToPosition(offset)
-		if err != nil {
-			return span.Point{}, err
-		}
-		return span.NewPoint(l, c, offset), nil
-	}
-	start, err := toPoint(s.Byte)
-	if err != nil {
-		return protocol.Range{}, err
-	}
-	end, err := toPoint(e.Byte)
-	if err != nil {
-		return protocol.Range{}, err
-	}
-	return m.Range(span.New(uri, start, end))
-}