internal/lsp: use pointers to source.Error, and not to ErrorList

We should be consistent about whether we pass around Error or *Error.
*Error was somewhat more common, and its Error method has a pointer
receiver, so I picked *Error.

ErrorList, on the other hand, is usually dereferenced when used, and
slice headers are small enough to pass around casually. So switch that
over to a non-pointer and change its Error method to a value receiver.

Change-Id: I19e70201ed1d6ef36d217db21c9101bb810bcf0c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/272092
Trust: Heschi Kreinick <heschi@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index ec40b22..bde23b2 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -167,8 +167,8 @@
 	return nil
 }
 
-func (s *snapshot) parseLoadError(ctx context.Context, loadErr error) *source.ErrorList {
-	var srcErrs *source.ErrorList
+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 {
@@ -179,9 +179,9 @@
 			continue
 		}
 		if srcErrs == nil {
-			srcErrs = &source.ErrorList{}
+			srcErrs = source.ErrorList{}
 		}
-		*srcErrs = append(*srcErrs, srcErr)
+		srcErrs = append(srcErrs, srcErr)
 	}
 	return srcErrs
 }
diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index ef01864..c6bce31 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -72,10 +72,10 @@
 		file, err := modfile.Parse(modFH.URI().Filename(), contents, nil)
 
 		// Attempt to convert the error to a standardized parse error.
-		var parseErrors []source.Error
+		var parseErrors []*source.Error
 		if err != nil {
 			if parseErr, extractErr := extractModParseErrors(modFH.URI(), m, err, contents); extractErr == nil {
-				parseErrors = []source.Error{*parseErr}
+				parseErrors = []*source.Error{parseErr}
 			}
 		}
 		return &parseModData{
diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go
index 4d94090..f33d9c3 100644
--- a/internal/lsp/cache/mod_tidy.go
+++ b/internal/lsp/cache/mod_tidy.go
@@ -173,7 +173,7 @@
 			return nil, false
 		}
 		return &source.TidiedModule{
-			Errors: []source.Error{{
+			Errors: []*source.Error{{
 				URI:   fh.URI(),
 				Range: rng,
 				Kind:  source.ListError,
@@ -212,7 +212,7 @@
 // modTidyErrors computes the differences between the original and tidied
 // go.mod files to produce diagnostic and suggested fixes. Some diagnostics
 // may appear on the Go files that import packages from missing modules.
-func modTidyErrors(ctx context.Context, snapshot source.Snapshot, pm *source.ParsedModule, ideal *modfile.File, workspacePkgs []source.Package) (errors []source.Error, err error) {
+func modTidyErrors(ctx context.Context, snapshot source.Snapshot, pm *source.ParsedModule, ideal *modfile.File, workspacePkgs []source.Package) (errors []*source.Error, err error) {
 	// First, determine which modules are unused and which are missing from the
 	// original go.mod file.
 	var (
@@ -333,16 +333,16 @@
 }
 
 // unusedError returns a source.Error for an unused require.
-func unusedError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) (source.Error, error) {
+func unusedError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) (*source.Error, error) {
 	rng, err := rangeFromPositions(m, req.Syntax.Start, req.Syntax.End)
 	if err != nil {
-		return source.Error{}, err
+		return nil, err
 	}
 	args, err := source.MarshalArgs(m.URI, false, []string{req.Mod.Path + "@none"})
 	if err != nil {
-		return source.Error{}, err
+		return nil, err
 	}
-	return source.Error{
+	return &source.Error{
 		Category: source.GoModTidy,
 		Message:  fmt.Sprintf("%s is not used in this module", req.Mod.Path),
 		Range:    rng,
@@ -360,10 +360,10 @@
 
 // directnessError extracts errors when a dependency is labeled indirect when
 // it should be direct and vice versa.
-func directnessError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) (source.Error, error) {
+func directnessError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) (*source.Error, error) {
 	rng, err := rangeFromPositions(m, req.Syntax.Start, req.Syntax.End)
 	if err != nil {
-		return source.Error{}, err
+		return nil, err
 	}
 	direction := "indirect"
 	if req.Indirect {
@@ -376,16 +376,16 @@
 			end.Byte += len([]byte(comments.Suffix[0].Token))
 			rng, err = rangeFromPositions(m, comments.Suffix[0].Start, end)
 			if err != nil {
-				return source.Error{}, err
+				return nil, err
 			}
 		}
 	}
 	// If the dependency should be indirect, add the // indirect.
 	edits, err := switchDirectness(req, m, computeEdits)
 	if err != nil {
-		return source.Error{}, err
+		return nil, err
 	}
-	return source.Error{
+	return &source.Error{
 		Message:  fmt.Sprintf("%s should be %s", req.Mod.Path, direction),
 		Range:    rng,
 		URI:      m.URI,
@@ -399,7 +399,7 @@
 	}, nil
 }
 
-func missingModuleError(snapshot source.Snapshot, pm *source.ParsedModule, req *modfile.Require) (source.Error, error) {
+func missingModuleError(snapshot source.Snapshot, pm *source.ParsedModule, req *modfile.Require) (*source.Error, error) {
 	var rng protocol.Range
 	// Default to the start of the file if there is no module declaration.
 	if pm.File != nil && pm.File.Module != nil && pm.File.Module.Syntax != nil {
@@ -407,14 +407,14 @@
 		var err error
 		rng, err = rangeFromPositions(pm.Mapper, start, end)
 		if err != nil {
-			return source.Error{}, err
+			return nil, err
 		}
 	}
 	args, err := source.MarshalArgs(pm.Mapper.URI, !req.Indirect, []string{req.Mod.Path + "@" + req.Mod.Version})
 	if err != nil {
-		return source.Error{}, err
+		return nil, err
 	}
-	return source.Error{
+	return &source.Error{
 		URI:      pm.Mapper.URI,
 		Range:    rng,
 		Message:  fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path),
@@ -466,19 +466,19 @@
 
 // missingModuleForImport creates an error for a given import path that comes
 // from a missing module.
-func missingModuleForImport(snapshot source.Snapshot, m *protocol.ColumnMapper, imp *ast.ImportSpec, req *modfile.Require, fixes []source.SuggestedFix) (source.Error, error) {
+func missingModuleForImport(snapshot source.Snapshot, m *protocol.ColumnMapper, imp *ast.ImportSpec, req *modfile.Require, fixes []source.SuggestedFix) (*source.Error, error) {
 	if req.Syntax == nil {
-		return source.Error{}, fmt.Errorf("no syntax for %v", req)
+		return nil, fmt.Errorf("no syntax for %v", req)
 	}
 	spn, err := span.NewRange(snapshot.FileSet(), imp.Path.Pos(), imp.Path.End()).Span()
 	if err != nil {
-		return source.Error{}, err
+		return nil, err
 	}
 	rng, err := m.Range(spn)
 	if err != nil {
-		return source.Error{}, err
+		return nil, err
 	}
-	return source.Error{
+	return &source.Error{
 		Category:       source.GoModTidy,
 		URI:            m.URI,
 		Range:          rng,
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index 9b4de73..8c682bf 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -515,12 +515,9 @@
 
 		// If we have multiple modules, we need to load them by paths.
 		var scopes []interface{}
-		var modErrors *source.ErrorList
+		var modErrors source.ErrorList
 		addError := func(uri span.URI, err error) {
-			if modErrors == nil {
-				modErrors = &source.ErrorList{}
-			}
-			*modErrors = append(*modErrors, &source.Error{
+			modErrors = append(modErrors, &source.Error{
 				URI:      uri,
 				Category: "compiler",
 				Kind:     source.ListError,
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index b3a3ce3..35fccea 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -528,7 +528,7 @@
 	return quickFixes, nil
 }
 
-func sameDiagnostic(d protocol.Diagnostic, e source.Error) bool {
+func sameDiagnostic(d protocol.Diagnostic, e *source.Error) bool {
 	return d.Message == e.Message && protocol.CompareRange(d.Range, e.Range) == 0 && d.Source == e.Category
 }
 
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index e8e2868..49298bc 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -193,8 +193,8 @@
 
 	if err != nil {
 		// Some error messages can also be displayed as diagnostics.
-		if errList := (*source.ErrorList)(nil); errors.As(err, &errList) {
-			_ = errorsToDiagnostic(ctx, snapshot, *errList, reports)
+		if errList := (source.ErrorList)(nil); errors.As(err, &errList) {
+			_ = errorsToDiagnostic(ctx, snapshot, errList, reports)
 		}
 		event.Error(ctx, "errors diagnosing workspace", err, tag.Snapshot.Of(snapshot.ID()), tag.Directory.Of(snapshot.View().Folder()))
 		return reports, nil
diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go
index 20fd606..23f0059 100644
--- a/internal/lsp/mod/diagnostics.go
+++ b/internal/lsp/mod/diagnostics.go
@@ -51,7 +51,7 @@
 	return reports, nil
 }
 
-func ErrorsForMod(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) ([]source.Error, error) {
+func ErrorsForMod(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle) ([]*source.Error, error) {
 	pm, err := snapshot.ParseMod(ctx, fh)
 	if err != nil {
 		if pm == nil || len(pm.ParseErrors) == 0 {
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 89f4c6a..ff4a4ca 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -252,13 +252,13 @@
 	URI         span.URI
 	File        *modfile.File
 	Mapper      *protocol.ColumnMapper
-	ParseErrors []Error
+	ParseErrors []*Error
 }
 
 // A TidiedModule contains the results of running `go mod tidy` on a module.
 type TidiedModule struct {
 	// Diagnostics representing changes made by `go mod tidy`.
-	Errors []Error
+	Errors []*Error
 	// The bytes of the go.mod file after it was tidied.
 	TidiedContent []byte
 }
@@ -532,10 +532,10 @@
 
 type ErrorList []*Error
 
-func (err *ErrorList) Error() string {
+func (err ErrorList) Error() string {
 	var b strings.Builder
 	b.WriteString("source error list:")
-	for _, e := range *err {
+	for _, e := range err {
 		b.WriteString(fmt.Sprintf("\n\t%s", e))
 	}
 	return b.String()