internal/lsp: add type error fixes to existing diagnostics

This change is the first step in handling golang/go#38136. Instead of
creating multiple diagnostic reports for type error analyzers, we add
suggested fixes to the existing reports. To match the analyzers for
FindAnalysisError, we add an ErrorMatch function to source.Analyzer.

This is not an ideal solution, but it was the best one I could come up
with without modifying the go/analysis API. analysisinternal could be
used for this purpose, but it seemed to complicated to be worth it, and
this is fairly simple. I think that go/analysis itself might need to be
extended for type error analyzers, but these temporary measures will
help us understand the kinds of features we need for type error
analyzers.

A follow-up CL might be to not add reports for type error analyzers
until the end of source.Diagnostic, which would remove the need for the
look-up.

Fixes golang/go#38136

Change-Id: I25bc6396b09d49facecd918bf5591d2d5bdf1b3a
Reviewed-on: https://go-review.googlesource.com/c/tools/+/226777
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>

22 files changed, 179 insertions, 150 deletions

diff --git a/internal/lsp/analysis/fillreturns/fillreturns.go b/internal/lsp/analysis/fillreturns/fillreturns.go
index a75f64513..7ed965375 100644
--- a/internal/lsp/analysis/fillreturns/fillreturns.go
+++ b/internal/lsp/analysis/fillreturns/fillreturns.go
@@ -50,23 +50,9 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	errors := analysisinternal.GetTypeErrors(pass)
 	// Filter out the errors that are not relevant to this analyzer.
 	for _, typeErr := range errors {
-		matches := wrongReturnNumRegex.FindStringSubmatch(strings.TrimSpace(typeErr.Msg))
-		if len(matches) < 3 {
+		if !FixesError(typeErr.Msg) {
 			continue
 		}
-		wantNum, err := strconv.Atoi(matches[1])
-		if err != nil {
-			continue
-		}
-		gotNum, err := strconv.Atoi(matches[2])
-		if err != nil {
-			continue
-		}
-		// Logic for handling more return values than expected is hard.
-		if wantNum < gotNum {
-			continue
-		}
-
 		var file *ast.File
 		for _, f := range pass.Files {
 			if f.Pos() <= typeErr.Pos && typeErr.Pos <= f.End() {
@@ -203,3 +189,20 @@ func equalTypes(t1, t2 types.Type) bool {
 	// TODO: Figure out if we want to check for types.AssignableTo(t1, t2) || types.ConvertibleTo(t1, t2)
 	return false
 }
+
+func FixesError(msg string) bool {
+	matches := wrongReturnNumRegex.FindStringSubmatch(strings.TrimSpace(msg))
+	if len(matches) < 3 {
+		return false
+	}
+	wantNum, err := strconv.Atoi(matches[1])
+	if err != nil {
+		return false
+	}
+	gotNum, err := strconv.Atoi(matches[2])
+	if err != nil {
+		return false
+	}
+	// Logic for handling more return values than expected is hard.
+	return wantNum >= gotNum
+}
diff --git a/internal/lsp/analysis/nonewvars/nonewvars.go b/internal/lsp/analysis/nonewvars/nonewvars.go
index 31dcd25b9..e7fa430cc 100644
--- a/internal/lsp/analysis/nonewvars/nonewvars.go
+++ b/internal/lsp/analysis/nonewvars/nonewvars.go
@@ -37,8 +37,6 @@ var Analyzer = &analysis.Analyzer{
 	RunDespiteErrors: true,
 }
 
-const noNewVarsMsg = "no new variables on left side of :="
-
 func run(pass *analysis.Pass) (interface{}, error) {
 	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 	errors := analysisinternal.GetTypeErrors(pass)
@@ -63,7 +61,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		}
 
 		for _, err := range errors {
-			if err.Msg != noNewVarsMsg {
+			if !FixesError(err.Msg) {
 				continue
 			}
 			if assignStmt.Pos() > err.Pos || err.Pos >= assignStmt.End() {
@@ -89,3 +87,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	})
 	return nil, nil
 }
+
+func FixesError(msg string) bool {
+	return msg == "no new variables on left side of :="
+}
diff --git a/internal/lsp/analysis/noresultvalues/noresultvalues.go b/internal/lsp/analysis/noresultvalues/noresultvalues.go
index 0259691ed..0e6b26f8b 100644
--- a/internal/lsp/analysis/noresultvalues/noresultvalues.go
+++ b/internal/lsp/analysis/noresultvalues/noresultvalues.go
@@ -34,8 +34,6 @@ var Analyzer = &analysis.Analyzer{
 	RunDespiteErrors: true,
 }
 
-const noResultValuesMsg = "no result values expected"
-
 func run(pass *analysis.Pass) (interface{}, error) {
 	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
 	errors := analysisinternal.GetTypeErrors(pass)
@@ -56,7 +54,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		}
 
 		for _, err := range errors {
-			if err.Msg != noResultValuesMsg {
+			if !FixesError(err.Msg) {
 				continue
 			}
 			if retStmt.Pos() >= err.Pos || err.Pos >= retStmt.End() {
@@ -83,3 +81,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	})
 	return nil, nil
 }
+
+func FixesError(msg string) bool {
+	return msg == "no result values expected"
+}
diff --git a/internal/lsp/analysis/undeclaredname/undeclared.go b/internal/lsp/analysis/undeclaredname/undeclared.go
index 21059fa52..cf7c770a9 100644
--- a/internal/lsp/analysis/undeclaredname/undeclared.go
+++ b/internal/lsp/analysis/undeclaredname/undeclared.go
@@ -36,7 +36,7 @@ const undeclaredNamePrefix = "undeclared name: "
 
 func run(pass *analysis.Pass) (interface{}, error) {
 	for _, err := range analysisinternal.GetTypeErrors(pass) {
-		if !strings.HasPrefix(err.Msg, undeclaredNamePrefix) {
+		if !FixesError(err.Msg) {
 			continue
 		}
 		name := strings.TrimPrefix(err.Msg, undeclaredNamePrefix)
@@ -184,3 +184,7 @@ func baseIfStmt(path []ast.Node, index int) ast.Stmt {
 	}
 	return stmt.(ast.Stmt)
 }
+
+func FixesError(msg string) bool {
+	return strings.HasPrefix(msg, undeclaredNamePrefix)
+}
diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go
index ecedc31a9..563d3b53b 100644
--- a/internal/lsp/cache/analysis.go
+++ b/internal/lsp/cache/analysis.go
@@ -23,7 +23,7 @@ import (
 	errors "golang.org/x/xerrors"
 )
 
-func (s *snapshot) Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*source.Error, error) {
+func (s *snapshot) Analyze(ctx context.Context, id string, analyzers ...*analysis.Analyzer) ([]*source.Error, error) {
 	var roots []*actionHandle
 
 	for _, a := range analyzers {
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index 9fe7a02e9..bfda93a7e 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -5,11 +5,9 @@
 package cache
 
 import (
-	"context"
 	"go/ast"
 	"go/types"
 
-	"golang.org/x/tools/internal/lsp/protocol"
 	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/packagesinternal"
 	"golang.org/x/tools/internal/span"
@@ -125,46 +123,3 @@ func (p *pkg) Imports() []source.Package {
 func (p *pkg) Module() *packagesinternal.Module {
 	return p.module
 }
-
-func (s *snapshot) FindAnalysisError(ctx context.Context, pkgID, analyzerName, msg string, rng protocol.Range) (*source.Error, *source.Analyzer, error) {
-	analyzer := findAnalyzer(s, analyzerName)
-	if analyzer.Analyzer == nil {
-		return nil, nil, errors.Errorf("unexpected analyzer: %s", analyzerName)
-	}
-	if !analyzer.Enabled(s) {
-		return nil, nil, errors.Errorf("disabled analyzer: %s", analyzerName)
-	}
-	act, err := s.actionHandle(ctx, packageID(pkgID), analyzer.Analyzer)
-	if err != nil {
-		return nil, nil, err
-	}
-	errs, _, err := act.analyze(ctx)
-	if err != nil {
-		return nil, nil, err
-	}
-	for _, err := range errs {
-		if err.Category != analyzer.Analyzer.Name {
-			continue
-		}
-		if err.Message != msg {
-			continue
-		}
-		if protocol.CompareRange(err.Range, rng) != 0 {
-			continue
-		}
-		return err, &analyzer, nil
-	}
-	return nil, nil, errors.Errorf("no matching diagnostic for %s:%v", pkgID, analyzerName)
-}
-
-func findAnalyzer(s *snapshot, analyzerName string) source.Analyzer {
-	checked := s.View().Options().DefaultAnalyzers
-	if a, ok := checked[analyzerName]; ok {
-		return a
-	}
-	checked = s.View().Options().TypeErrorAnalyzers
-	if a, ok := checked[analyzerName]; ok {
-		return a
-	}
-	return source.Analyzer{}
-}
diff --git a/internal/lsp/cmd/test/check.go b/internal/lsp/cmd/test/check.go
index f21194f8b..24e916f00 100644
--- a/internal/lsp/cmd/test/check.go
+++ b/internal/lsp/cmd/test/check.go
@@ -14,7 +14,7 @@ import (
 	"golang.org/x/tools/internal/span"
 )
 
-func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) {
+func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnostic) {
 	if len(want) == 1 && want[0].Message == "" {
 		return
 	}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index e12537556..4757a1ee1 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -259,10 +259,8 @@ func analysisFixes(ctx context.Context, snapshot source.Snapshot, fh source.File
 		return nil, nil, err
 	}
 	for _, diag := range diagnostics {
-		// This code assumes that the analyzer name is the Source of the diagnostic.
-		// If this ever changes, this will need to be addressed.
-		srcErr, analyzer, err := snapshot.FindAnalysisError(ctx, ph.ID(), diag.Source, diag.Message, diag.Range)
-		if err != nil {
+		srcErr, analyzer, ok := findSourceError(ctx, snapshot, ph.ID(), diag)
+		if !ok {
 			continue
 		}
 		for _, fix := range srcErr.SuggestedFixes {
@@ -289,6 +287,47 @@ func analysisFixes(ctx context.Context, snapshot source.Snapshot, fh source.File
 	return codeActions, sourceFixAllEdits, nil
 }
 
+func findSourceError(ctx context.Context, snapshot source.Snapshot, pkgID string, diag protocol.Diagnostic) (*source.Error, source.Analyzer, bool) {
+	var analyzer *source.Analyzer
+
+	// If the source is "compiler", we expect a type error analyzer.
+	if diag.Source == "compiler" {
+		for _, a := range snapshot.View().Options().TypeErrorAnalyzers {
+			if a.FixesError(diag.Message) {
+				analyzer = &a
+				break
+			}
+		}
+	} else {
+		// This code assumes that the analyzer name is the Source of the diagnostic.
+		// If this ever changes, this will need to be addressed.
+		if a, ok := snapshot.View().Options().DefaultAnalyzers[diag.Source]; ok {
+			analyzer = &a
+		}
+	}
+	if analyzer == nil || !analyzer.Enabled(snapshot) {
+		return nil, source.Analyzer{}, false
+	}
+	analysisErrors, err := snapshot.Analyze(ctx, pkgID, analyzer.Analyzer)
+	if err != nil {
+		return nil, source.Analyzer{}, false
+	}
+	for _, err := range analysisErrors {
+		if err.Message != diag.Message {
+			continue
+		}
+		if protocol.CompareRange(err.Range, diag.Range) != 0 {
+			continue
+		}
+		if err.Category != analyzer.Analyzer.Name {
+			continue
+		}
+		// The error matches.
+		return err, *analyzer, true
+	}
+	return nil, source.Analyzer{}, false
+}
+
 func documentChanges(fh source.FileHandle, edits []protocol.TextEdit) []protocol.TextDocumentEdit {
 	return []protocol.TextDocumentEdit{
 		{
diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go
index 1519f322b..94592be3c 100644
--- a/internal/lsp/diagnostics.go
+++ b/internal/lsp/diagnostics.go
@@ -39,7 +39,7 @@ func (s *Server) diagnoseSnapshot(snapshot source.Snapshot) {
 
 // diagnose is a helper function for running diagnostics with a given context.
 // Do not call it directly.
-func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysAnalyze bool) map[diagnosticKey][]source.Diagnostic {
+func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysAnalyze bool) map[diagnosticKey][]*source.Diagnostic {
 	ctx, done := event.StartSpan(ctx, "lsp:background-worker")
 	defer done()
 
@@ -51,7 +51,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
 	}
 	defer func() { <-s.diagnosticsSema }()
 
-	allReports := make(map[diagnosticKey][]source.Diagnostic)
+	allReports := make(map[diagnosticKey][]*source.Diagnostic)
 	var reportsMu sync.Mutex
 	var wg sync.WaitGroup
 
@@ -123,7 +123,7 @@ func (s *Server) diagnose(ctx context.Context, snapshot source.Snapshot, alwaysA
 	return allReports
 }
 
-func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, reports map[diagnosticKey][]source.Diagnostic) {
+func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, reports map[diagnosticKey][]*source.Diagnostic) {
 	// Check for context cancellation before publishing diagnostics.
 	if ctx.Err() != nil {
 		return
@@ -189,7 +189,7 @@ func (s *Server) publishReports(ctx context.Context, snapshot source.Snapshot, r
 
 // equalDiagnostics returns true if the 2 lists of diagnostics are equal.
 // It assumes that both a and b are already sorted.
-func equalDiagnostics(a, b []source.Diagnostic) bool {
+func equalDiagnostics(a, b []*source.Diagnostic) bool {
 	if len(a) != len(b) {
 		return false
 	}
@@ -201,7 +201,7 @@ func equalDiagnostics(a, b []source.Diagnostic) bool {
 	return true
 }
 
-func toProtocolDiagnostics(diagnostics []source.Diagnostic) []protocol.Diagnostic {
+func toProtocolDiagnostics(diagnostics []*source.Diagnostic) []protocol.Diagnostic {
 	reports := []protocol.Diagnostic{}
 	for _, diag := range diagnostics {
 		related := make([]protocol.DiagnosticRelatedInformation, 0, len(diag.Related))
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 0b38d1da1..6e9d4b12f 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -38,7 +38,7 @@ func TestLSP(t *testing.T) {
 type runner struct {
 	server      *Server
 	data        *tests.Data
-	diagnostics map[span.URI][]source.Diagnostic
+	diagnostics map[span.URI][]*source.Diagnostic
 	ctx         context.Context
 }
 
@@ -119,10 +119,10 @@ func (r *runner) CodeLens(t *testing.T, uri span.URI, want []protocol.CodeLens)
 	}
 }
 
-func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) {
+func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnostic) {
 	// Get the diagnostics for this view if we have not done it before.
 	if r.diagnostics == nil {
-		r.diagnostics = make(map[span.URI][]source.Diagnostic)
+		r.diagnostics = make(map[span.URI][]*source.Diagnostic)
 		v := r.server.session.View(r.data.Config.Dir)
 		// Always run diagnostics with analysis.
 		reports := r.server.diagnose(r.ctx, v.Snapshot(), true)
@@ -383,7 +383,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string)
 	}
 	// Get the diagnostics for this view if we have not done it before.
 	if r.diagnostics == nil {
-		r.diagnostics = make(map[span.URI][]source.Diagnostic)
+		r.diagnostics = make(map[span.URI][]*source.Diagnostic)
 		// Always run diagnostics with analysis.
 		reports := r.server.diagnose(r.ctx, view.Snapshot(), true)
 		for key, diags := range reports {
@@ -396,7 +396,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string)
 		// some diagnostics have a range with the same start and end position (8:1-8:1).
 		// The current marker functionality prevents us from having a range of 0 length.
 		if protocol.ComparePosition(d.Range.Start, rng.Start) == 0 {
-			diag = &d
+			diag = d
 			break
 		}
 	}
@@ -413,7 +413,7 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string)
 		},
 		Context: protocol.CodeActionContext{
 			Only:        codeActionKinds,
-			Diagnostics: toProtocolDiagnostics([]source.Diagnostic{*diag}),
+			Diagnostics: toProtocolDiagnostics([]*source.Diagnostic{diag}),
 		},
 	})
 	if err != nil {
diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go
index e36db7397..ba7730d4e 100644
--- a/internal/lsp/mod/diagnostics.go
+++ b/internal/lsp/mod/diagnostics.go
@@ -16,7 +16,7 @@ import (
 	"golang.org/x/tools/internal/telemetry/event"
 )
 
-func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]source.Diagnostic, map[string]*modfile.Require, error) {
+func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.FileIdentity][]*source.Diagnostic, map[string]*modfile.Require, error) {
 	// TODO: We will want to support diagnostics for go.mod files even when the -modfile flag is turned off.
 	realURI, tempURI := snapshot.View().ModFiles()
 
@@ -39,11 +39,11 @@ func Diagnostics(ctx context.Context, snapshot source.Snapshot) (map[source.File
 	if err != nil {
 		return nil, nil, err
 	}
-	reports := map[source.FileIdentity][]source.Diagnostic{
+	reports := map[source.FileIdentity][]*source.Diagnostic{
 		realfh.Identity(): {},
 	}
 	for _, e := range parseErrors {
-		diag := source.Diagnostic{
+		diag := &source.Diagnostic{
 			Message:        e.Message,
 			Range:          e.Range,
 			SuggestedFixes: e.SuggestedFixes,
diff --git a/internal/lsp/server.go b/internal/lsp/server.go
index d8122a06a..44e441219 100644
--- a/internal/lsp/server.go
+++ b/internal/lsp/server.go
@@ -88,7 +88,7 @@ type Server struct {
 type sentDiagnostics struct {
 	version      float64
 	identifier   string
-	sorted       []source.Diagnostic
+	sorted       []*source.Diagnostic
 	withAnalysis bool
 	snapshotID   uint64
 }
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index 2c97cba12..ec3e9d397 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -42,7 +42,7 @@ type RelatedInformation struct {
 	Message string
 }
 
-func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missingModules map[string]*modfile.Require, withAnalysis bool) (map[FileIdentity][]Diagnostic, bool, error) {
+func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missingModules map[string]*modfile.Require, withAnalysis bool) (map[FileIdentity][]*Diagnostic, bool, error) {
 	// If we are missing dependencies, it may because the user's workspace is
 	// not correctly configured. Report errors, if possible.
 	var warn bool
@@ -83,7 +83,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missi
 	}
 
 	// Prepare the reports we will send for the files in this package.
-	reports := make(map[FileIdentity][]Diagnostic)
+	reports := make(map[FileIdentity][]*Diagnostic)
 	for _, fh := range pkg.CompiledGoFiles() {
 		if err := clearReports(snapshot, reports, fh.File().Identity().URI); err != nil {
 			return nil, warn, err
@@ -139,7 +139,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missi
 	return reports, warn, nil
 }
 
-func FileDiagnostics(ctx context.Context, snapshot Snapshot, uri span.URI) (FileIdentity, []Diagnostic, error) {
+func FileDiagnostics(ctx context.Context, snapshot Snapshot, uri span.URI) (FileIdentity, []*Diagnostic, error) {
 	fh, err := snapshot.GetFile(uri)
 	if err != nil {
 		return FileIdentity{}, nil, err
@@ -167,7 +167,7 @@ type diagnosticSet struct {
 	listErrors, parseErrors, typeErrors []*Diagnostic
 }
 
-func diagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, pkg Package, hasMissingDeps bool) (bool, bool, error) {
+func diagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, pkg Package, hasMissingDeps bool) (bool, bool, error) {
 	ctx, done := event.StartSpan(ctx, "source.diagnostics", tag.Package.Of(pkg.ID()))
 	_ = ctx // circumvent SA4006
 	defer done()
@@ -217,7 +217,7 @@ func diagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentit
 	return nonEmptyDiagnostics, hasTypeErrors, nil
 }
 
-func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, missingModules map[string]*modfile.Require, uri span.URI) error {
+func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, missingModules map[string]*modfile.Require, uri span.URI) error {
 	if snapshot.View().Ignore(uri) || len(missingModules) == 0 {
 		return nil
 	}
@@ -244,7 +244,7 @@ func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports m
 		return nil
 	}
 	if reports[fh.Identity()] == nil {
-		reports[fh.Identity()] = []Diagnostic{}
+		reports[fh.Identity()] = []*Diagnostic{}
 	}
 	for mod, req := range missingModules {
 		if req.Syntax == nil {
@@ -262,7 +262,7 @@ func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports m
 		if err != nil {
 			return err
 		}
-		reports[fh.Identity()] = append(reports[fh.Identity()], Diagnostic{
+		reports[fh.Identity()] = append(reports[fh.Identity()], &Diagnostic{
 			Message:  fmt.Sprintf("%s is not in your go.mod file.", req.Mod.Path),
 			Range:    rng,
 			Source:   "go mod tidy",
@@ -272,7 +272,7 @@ func missingModulesDiagnostics(ctx context.Context, snapshot Snapshot, reports m
 	return nil
 }
 
-func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]Diagnostic, ph PackageHandle, analyses map[string]Analyzer) error {
+func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, ph PackageHandle, analyses map[string]Analyzer) error {
 	var analyzers []*analysis.Analyzer
 	for _, a := range analyses {
 		if !a.Enabled(snapshot) {
@@ -280,8 +280,7 @@ func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][
 		}
 		analyzers = append(analyzers, a.Analyzer)
 	}
-
-	analysisErrors, err := snapshot.Analyze(ctx, ph.ID(), analyzers)
+	analysisErrors, err := snapshot.Analyze(ctx, ph.ID(), analyzers...)
 	if err != nil {
 		return err
 	}
@@ -290,7 +289,7 @@ func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][
 	for _, e := range analysisErrors {
 		// This is a bit of a hack, but clients > 3.15 will be able to grey out unnecessary code.
 		// If we are deleting code as part of all of our suggested fixes, assume that this is dead code.
-		// TODO(golang/go/#34508): Return these codes from the diagnostics themselves.
+		// TODO(golang/go#34508): Return these codes from the diagnostics themselves.
 		var tags []protocol.DiagnosticTag
 		if onlyDeletions(e.SuggestedFixes) {
 			tags = append(tags, protocol.Unnecessary)
@@ -310,7 +309,7 @@ func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][
 	return nil
 }
 
-func clearReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, uri span.URI) error {
+func clearReports(snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, uri span.URI) error {
 	if snapshot.View().Ignore(uri) {
 		return nil
 	}
@@ -318,11 +317,11 @@ func clearReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, uri
 	if err != nil {
 		return err
 	}
-	reports[fh.Identity()] = []Diagnostic{}
+	reports[fh.Identity()] = []*Diagnostic{}
 	return nil
 }
 
-func addReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, uri span.URI, diagnostics ...*Diagnostic) error {
+func addReports(snapshot Snapshot, reports map[FileIdentity][]*Diagnostic, uri span.URI, diagnostics ...*Diagnostic) error {
 	if snapshot.View().Ignore(uri) {
 		return nil
 	}
@@ -330,21 +329,29 @@ func addReports(snapshot Snapshot, reports map[FileIdentity][]Diagnostic, uri sp
 	if err != nil {
 		return err
 	}
-	if _, ok := reports[fh.Identity()]; !ok {
+	identity := fh.Identity()
+	existingDiagnostics, ok := reports[identity]
+	if !ok {
 		return errors.Errorf("diagnostics for unexpected file %s", uri)
 	}
-Outer:
-	for _, diag := range diagnostics {
-		if diag == nil {
-			continue
-		}
-		for i, d := range reports[fh.Identity()] {
-			if diag.Message == d.Message && protocol.CompareRange(d.Range, diag.Range) == 0 {
-				reports[fh.Identity()][i].Source = diag.Source
-				continue Outer
+	if len(diagnostics) == 1 {
+		d1 := diagnostics[0]
+		if _, ok := snapshot.View().Options().TypeErrorAnalyzers[d1.Source]; ok {
+			for i, d2 := range existingDiagnostics {
+				if r := protocol.CompareRange(d1.Range, d2.Range); r != 0 {
+					continue
+				}
+				if d1.Message != d2.Message {
+					continue
+				}
+				reports[identity][i].SuggestedFixes = append(reports[identity][i].SuggestedFixes, d1.SuggestedFixes...)
+				reports[identity][i].Tags = append(reports[identity][i].Tags, d1.Tags...)
 			}
+			return nil
 		}
-		reports[fh.Identity()] = append(reports[fh.Identity()], *diag)
+	}
+	for _, diag := range diagnostics {
+		reports[fh.Identity()] = append(reports[fh.Identity()], diag)
 	}
 	return nil
 }
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 6b33d63b1..06b56b74d 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -497,10 +497,26 @@ func (r *OptionResult) setBool(b *bool) {
 
 func typeErrorAnalyzers() map[string]Analyzer {
 	return map[string]Analyzer{
-		fillreturns.Analyzer.Name:    {Analyzer: fillreturns.Analyzer, enabled: false},
-		nonewvars.Analyzer.Name:      {Analyzer: nonewvars.Analyzer, enabled: false},
-		noresultvalues.Analyzer.Name: {Analyzer: noresultvalues.Analyzer, enabled: false},
-		undeclaredname.Analyzer.Name: {Analyzer: undeclaredname.Analyzer, enabled: false},
+		fillreturns.Analyzer.Name: {
+			Analyzer:   fillreturns.Analyzer,
+			enabled:    false,
+			FixesError: fillreturns.FixesError,
+		},
+		nonewvars.Analyzer.Name: {
+			Analyzer:   nonewvars.Analyzer,
+			enabled:    false,
+			FixesError: nonewvars.FixesError,
+		},
+		noresultvalues.Analyzer.Name: {
+			Analyzer:   noresultvalues.Analyzer,
+			enabled:    false,
+			FixesError: noresultvalues.FixesError,
+		},
+		undeclaredname.Analyzer.Name: {
+			Analyzer:   undeclaredname.Analyzer,
+			enabled:    false,
+			FixesError: undeclaredname.FixesError,
+		},
 	}
 }
 
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index b9da1a0c6..619999ecb 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -88,7 +88,7 @@ func testSource(t *testing.T, exporter packagestest.Exporter) {
 	}
 }
 
-func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []source.Diagnostic) {
+func (r *runner) Diagnostics(t *testing.T, uri span.URI, want []*source.Diagnostic) {
 	snapshot := r.view.Snapshot()
 
 	fileID, got, err := source.FileDiagnostics(r.ctx, snapshot, uri)
diff --git a/internal/lsp/source/util.go b/internal/lsp/source/util.go
index 627d7f482..5d275bddc 100644
--- a/internal/lsp/source/util.go
+++ b/internal/lsp/source/util.go
@@ -621,13 +621,13 @@ func formatFunction(params []string, results []string, writeResultParens bool) s
 	return detail.String()
 }
 
-func SortDiagnostics(d []Diagnostic) {
+func SortDiagnostics(d []*Diagnostic) {
 	sort.Slice(d, func(i int, j int) bool {
 		return CompareDiagnostic(d[i], d[j]) < 0
 	})
 }
 
-func CompareDiagnostic(a, b Diagnostic) int {
+func CompareDiagnostic(a, b *Diagnostic) int {
 	if r := protocol.CompareRange(a.Range, b.Range); r != 0 {
 		return r
 	}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index eba54e609..cf6bc489e 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -42,11 +42,7 @@ type Snapshot interface {
 	IsSaved(uri span.URI) bool
 
 	// Analyze runs the analyses for the given package at this snapshot.
-	Analyze(ctx context.Context, id string, analyzers []*analysis.Analyzer) ([]*Error, error)
-
-	// FindAnalysisError returns the analysis error represented by the diagnostic.
-	// This is used to get the SuggestedFixes associated with that error.
-	FindAnalysisError(ctx context.Context, pkgID, analyzerName, msg string, rng protocol.Range) (*Error, *Analyzer, error)
+	Analyze(ctx context.Context, pkgID string, analyzers ...*analysis.Analyzer) ([]*Error, error)
 
 	// ModTidyHandle returns a ModTidyHandle for the given go.mod file handle.
 	// This function can have no data or error if there is no modfile detected.
@@ -372,6 +368,11 @@ type Analyzer struct {
 	// If this is true, then we can apply the suggested fixes
 	// as part of a source.FixAll codeaction.
 	HighConfidence bool
+
+	// FixesError is only set for type-error analyzers.
+	// It reports true if the message provided indicates an error that could be
+	// fixed by the analyzer.
+	FixesError func(msg string) bool
 }
 
 func (a Analyzer) Enabled(snapshot Snapshot) bool {
diff --git a/internal/lsp/testdata/lsp/primarymod/bad/bad1.go b/internal/lsp/testdata/lsp/primarymod/bad/bad1.go
index 00451bff1..eb547f793 100644
--- a/internal/lsp/testdata/lsp/primarymod/bad/bad1.go
+++ b/internal/lsp/testdata/lsp/primarymod/bad/bad1.go
@@ -14,8 +14,8 @@ func random() int { //@item(random, "random", "func() int", "func")
 
 func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func"),item(bad_y_param, "y", "int", "var")
 	x := 6       //@item(x, "x", "int", "var"),diag("x", "compiler", "x declared but not used", "error")
-	var q blah   //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used", "error"),diag("blah", "undeclaredname", "undeclared name: blah", "error")
-	var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared but not used", "error"),diag("blob", "undeclaredname", "undeclared name: blob", "error")
+	var q blah   //@item(q, "q", "blah", "var"),diag("q", "compiler", "q declared but not used", "error"),diag("blah", "compiler", "undeclared name: blah", "error")
+	var t **blob //@item(t, "t", "**blob", "var"),diag("t", "compiler", "t declared but not used", "error"),diag("blob", "compiler", "undeclared name: blob", "error")
 	//@complete("", q, t, x, bad_y_param, global_a, bob, stateFunc, random, random2, random3, stuff)
 
 	return y
@@ -24,10 +24,10 @@ func random2(y int) int { //@item(random2, "random2", "func(y int) int", "func")
 func random3(y ...int) { //@item(random3, "random3", "func(y ...int)", "func"),item(y_variadic_param, "y", "[]int", "var")
 	//@complete("", y_variadic_param, global_a, bob, stateFunc, random, random2, random3, stuff)
 
-	var ch chan (favType1)   //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared but not used", "error"),diag("favType1", "undeclaredname", "undeclared name: favType1", "error")
-	var m map[keyType]int    //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared but not used", "error"),diag("keyType", "undeclaredname", "undeclared name: keyType", "error")
-	var arr []favType2       //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared but not used", "error"),diag("favType2", "undeclaredname", "undeclared name: favType2", "error")
-	var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared but not used", "error"),diag("badResult", "undeclaredname", "undeclared name: badResult", "error")
-	var fn2 func(badParam)   //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared but not used", "error"),diag("badParam", "undeclaredname", "undeclared name: badParam", "error")
+	var ch chan (favType1)   //@item(ch, "ch", "chan (favType1)", "var"),diag("ch", "compiler", "ch declared but not used", "error"),diag("favType1", "compiler", "undeclared name: favType1", "error")
+	var m map[keyType]int    //@item(m, "m", "map[keyType]int", "var"),diag("m", "compiler", "m declared but not used", "error"),diag("keyType", "compiler", "undeclared name: keyType", "error")
+	var arr []favType2       //@item(arr, "arr", "[]favType2", "var"),diag("arr", "compiler", "arr declared but not used", "error"),diag("favType2", "compiler", "undeclared name: favType2", "error")
+	var fn1 func() badResult //@item(fn1, "fn1", "func() badResult", "var"),diag("fn1", "compiler", "fn1 declared but not used", "error"),diag("badResult", "compiler", "undeclared name: badResult", "error")
+	var fn2 func(badParam)   //@item(fn2, "fn2", "func(badParam)", "var"),diag("fn2", "compiler", "fn2 declared but not used", "error"),diag("badParam", "compiler", "undeclared name: badParam", "error")
 	//@complete("", arr, ch, fn1, fn2, m, y_variadic_param, global_a, bob, stateFunc, random, random2, random3, stuff)
 }
diff --git a/internal/lsp/testdata/lsp/primarymod/undeclared/var.go b/internal/lsp/testdata/lsp/primarymod/undeclared/var.go
index 1a43292cf..b5f9287d4 100644
--- a/internal/lsp/testdata/lsp/primarymod/undeclared/var.go
+++ b/internal/lsp/testdata/lsp/primarymod/undeclared/var.go
@@ -1,13 +1,13 @@
 package undeclared
 
 func m() int {
-	z, _ := 1+y, 11 //@diag("y", "undeclaredname", "undeclared name: y", "error"),suggestedfix("y", "quickfix")
+	z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix")
 	if 100 < 90 {
 		z = 1
-	} else if 100 > n+2 { //@diag("n", "undeclaredname", "undeclared name: n", "error"),suggestedfix("n", "quickfix")
+	} else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix")
 		z = 4
 	}
-	for i < 200 { //@diag("i", "undeclaredname", "undeclared name: i", "error"),suggestedfix("i", "quickfix")
+	for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix")
 	}
 	r() //@diag("r", "compiler", "undeclared name: r", "error")
 	return z
diff --git a/internal/lsp/testdata/lsp/primarymod/undeclared/var.go.golden b/internal/lsp/testdata/lsp/primarymod/undeclared/var.go.golden
index 63cf58ad6..68e900fd1 100644
--- a/internal/lsp/testdata/lsp/primarymod/undeclared/var.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/undeclared/var.go.golden
@@ -3,13 +3,13 @@ package undeclared
 
 func m() int {
 	y := 
-	z, _ := 1+y, 11 //@diag("y", "undeclaredname", "undeclared name: y", "error"),suggestedfix("y", "quickfix")
+	z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix")
 	if 100 < 90 {
 		z = 1
-	} else if 100 > n+2 { //@diag("n", "undeclaredname", "undeclared name: n", "error"),suggestedfix("n", "quickfix")
+	} else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix")
 		z = 4
 	}
-	for i < 200 { //@diag("i", "undeclaredname", "undeclared name: i", "error"),suggestedfix("i", "quickfix")
+	for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix")
 	}
 	r() //@diag("r", "compiler", "undeclared name: r", "error")
 	return z
@@ -19,14 +19,14 @@ func m() int {
 package undeclared
 
 func m() int {
-	z, _ := 1+y, 11 //@diag("y", "undeclaredname", "undeclared name: y", "error"),suggestedfix("y", "quickfix")
+	z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix")
 	n := 
 	if 100 < 90 {
 		z = 1
-	} else if 100 > n+2 { //@diag("n", "undeclaredname", "undeclared name: n", "error"),suggestedfix("n", "quickfix")
+	} else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix")
 		z = 4
 	}
-	for i < 200 { //@diag("i", "undeclaredname", "undeclared name: i", "error"),suggestedfix("i", "quickfix")
+	for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix")
 	}
 	r() //@diag("r", "compiler", "undeclared name: r", "error")
 	return z
@@ -36,14 +36,14 @@ func m() int {
 package undeclared
 
 func m() int {
-	z, _ := 1+y, 11 //@diag("y", "undeclaredname", "undeclared name: y", "error"),suggestedfix("y", "quickfix")
+	z, _ := 1+y, 11 //@diag("y", "compiler", "undeclared name: y", "error"),suggestedfix("y", "quickfix")
 	if 100 < 90 {
 		z = 1
-	} else if 100 > n+2 { //@diag("n", "undeclaredname", "undeclared name: n", "error"),suggestedfix("n", "quickfix")
+	} else if 100 > n+2 { //@diag("n", "compiler", "undeclared name: n", "error"),suggestedfix("n", "quickfix")
 		z = 4
 	}
 	i := 
-	for i < 200 { //@diag("i", "undeclaredname", "undeclared name: i", "error"),suggestedfix("i", "quickfix")
+	for i < 200 { //@diag("i", "compiler", "undeclared name: i", "error"),suggestedfix("i", "quickfix")
 	}
 	r() //@diag("r", "compiler", "undeclared name: r", "error")
 	return z
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index c4c483444..f843a5fac 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -44,7 +44,7 @@ const (
 var UpdateGolden = flag.Bool("golden", false, "Update golden files")
 
 type CodeLens map[span.URI][]protocol.CodeLens
-type Diagnostics map[span.URI][]source.Diagnostic
+type Diagnostics map[span.URI][]*source.Diagnostic
 type CompletionItems map[token.Pos]*source.CompletionItem
 type Completions map[span.Span][]Completion
 type CompletionSnippets map[span.Span][]CompletionSnippet
@@ -116,7 +116,7 @@ type Data struct {
 
 type Tests interface {
 	CodeLens(*testing.T, span.URI, []protocol.CodeLens)
-	Diagnostics(*testing.T, span.URI, []source.Diagnostic)
+	Diagnostics(*testing.T, span.URI, []*source.Diagnostic)
 	Completion(*testing.T, span.Span, Completion, CompletionItems)
 	CompletionSnippet(*testing.T, span.Span, CompletionSnippet, bool, CompletionItems)
 	UnimportedCompletion(*testing.T, span.Span, Completion, CompletionItems)
@@ -907,7 +907,7 @@ func (data *Data) collectCodeLens(spn span.Span, title, cmd string) {
 
 func (data *Data) collectDiagnostics(spn span.Span, msgSource, msg, msgSeverity string) {
 	if _, ok := data.Diagnostics[spn.URI()]; !ok {
-		data.Diagnostics[spn.URI()] = []source.Diagnostic{}
+		data.Diagnostics[spn.URI()] = []*source.Diagnostic{}
 	}
 	m, err := data.Mapper(spn.URI())
 	if err != nil {
@@ -929,7 +929,7 @@ func (data *Data) collectDiagnostics(spn span.Span, msgSource, msg, msgSeverity
 		severity = protocol.SeverityInformation
 	}
 	// This is not the correct way to do this, but it seems excessive to do the full conversion here.
-	want := source.Diagnostic{
+	want := &source.Diagnostic{
 		Range:    rng,
 		Severity: severity,
 		Source:   msgSource,
diff --git a/internal/lsp/tests/util.go b/internal/lsp/tests/util.go
index e6de57f88..472b93c94 100644
--- a/internal/lsp/tests/util.go
+++ b/internal/lsp/tests/util.go
@@ -163,7 +163,7 @@ func summarizeWorkspaceSymbols(i int, want, got []protocol.SymbolInformation, re
 
 // DiffDiagnostics prints the diff between expected and actual diagnostics test
 // results.
-func DiffDiagnostics(uri span.URI, want, got []source.Diagnostic) string {
+func DiffDiagnostics(uri span.URI, want, got []*source.Diagnostic) string {
 	source.SortDiagnostics(want)
 	source.SortDiagnostics(got)
 
@@ -197,7 +197,7 @@ func DiffDiagnostics(uri span.URI, want, got []source.Diagnostic) string {
 	return ""
 }
 
-func summarizeDiagnostics(i int, uri span.URI, want, got []source.Diagnostic, reason string, args ...interface{}) string {
+func summarizeDiagnostics(i int, uri span.URI, want, got []*source.Diagnostic, reason string, args ...interface{}) string {
 	msg := &bytes.Buffer{}
 	fmt.Fprint(msg, "diagnostics failed")
 	if i >= 0 {