internal/lsp: add support for sourceFixAll analyzers

This change adds support for analyzers that have suggested fixes of the kind Source.FixAll. This will allow these fixes to be applied on save if the user desires.

To auto apply these fixes on save, make sure your settings.json looks like:

"[go]": {
	"editor.codeActionsOnSave": {
		...
		"source.fixAll": true,
		...
	},
	...
}

Update golang/go#37221

Change-Id: I534e4f6c8c51ec2848cf2899aab68f587ba68423
Reviewed-on: https://go-review.googlesource.com/c/tools/+/223658
Run-TryBot: Rohan Challa <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rebecca Stambler <[email protected]>

Author: ridersofrohan

internal/lsp/cache/pkg.go

@@ -125,22 +125,23 @@ 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, error) {
+func (s *snapshot) FindAnalysisError(ctx context.Context, pkgID, analyzerName, msg string, rng protocol.Range) (*source.Error, *source.Analyzer, error) {
+	var analyzer source.Analyzer
 	analyzer, ok := s.View().Options().Analyzers[analyzerName]
 	if !ok {
-		return nil, errors.Errorf("unexpected analyzer: %s", analyzerName)
+		return nil, nil, errors.Errorf("unexpected analyzer: %s", analyzerName)
 	}
 	if !analyzer.Enabled {
-		return nil, errors.Errorf("disabled analyzer: %s", analyzerName)
+		return nil, nil, errors.Errorf("disabled analyzer: %s", analyzerName)
 	}
 
 	act, err := s.actionHandle(ctx, packageID(pkgID), analyzer.Analyzer)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	errs, _, err := act.analyze(ctx)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	for _, err := range errs {
 		if err.Category != analyzerName {
@@ -152,7 +153,7 @@ func (s *snapshot) FindAnalysisError(ctx context.Context, pkgID, analyzerName, m
 		if protocol.CompareRange(err.Range, rng) != 0 {
 			continue
 		}
-		return err, nil
+		return err, &analyzer, nil
 	}
-	return nil, errors.Errorf("no matching diagnostic for %s:%v", pkgID, analyzerName)
+	return nil, nil, errors.Errorf("no matching diagnostic for %s:%v", pkgID, analyzerName)
 }

internal/lsp/code_action.go

@@ -65,23 +65,37 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 			})
 		}
 	case source.Go:
-		edits, editsPerFix, err := source.AllImportsFixes(ctx, snapshot, fh)
-		if err != nil {
-			return nil, err
+		diagnostics := params.Context.Diagnostics
+
+		var importEdits []protocol.TextEdit
+		var importEditsPerFix []*source.ImportFix
+		var analysisQuickFixes []protocol.CodeAction
+		var highConfidenceEdits []protocol.TextDocumentEdit
+
+		// Retrieve any necessary import edits or fixes.
+		if wanted[protocol.QuickFix] && len(diagnostics) > 0 || wanted[protocol.SourceOrganizeImports] {
+			importEdits, importEditsPerFix, err = source.AllImportsFixes(ctx, snapshot, fh)
+			if err != nil {
+				return nil, err
+			}
 		}
-		if diagnostics := params.Context.Diagnostics; wanted[protocol.QuickFix] && len(diagnostics) > 0 {
-			// First, add the quick fixes reported by go/analysis.
-			qf, err := quickFixes(ctx, snapshot, fh, diagnostics)
+		// Retrieve any necessary analysis fixes or edits.
+		if (wanted[protocol.QuickFix] || wanted[protocol.SourceFixAll]) && len(diagnostics) > 0 {
+			analysisQuickFixes, highConfidenceEdits, err = analysisFixes(ctx, snapshot, fh, diagnostics)
 			if err != nil {
-				event.Error(ctx, "quick fixes failed", err, tag.URI.Of(uri))
+				event.Error(ctx, "analysis fixes failed", err, tag.URI.Of(uri))
 			}
-			codeActions = append(codeActions, qf...)
+		}
+
+		if wanted[protocol.QuickFix] && len(diagnostics) > 0 {
+			// First, add the quick fixes reported by go/analysis.
+			codeActions = append(codeActions, analysisQuickFixes...)
 
 			// If we also have diagnostics for missing imports, we can associate them with quick fixes.
 			if findImportErrors(diagnostics) {
 				// Separate this into a set of codeActions per diagnostic, where
 				// each action is the addition, removal, or renaming of one import.
-				for _, importFix := range editsPerFix {
+				for _, importFix := range importEditsPerFix {
 					// Get the diagnostics this fix would affect.
 					if fixDiagnostics := importDiagnostics(importFix.Fix, diagnostics); len(fixDiagnostics) > 0 {
 						codeActions = append(codeActions, protocol.CodeAction{
@@ -95,6 +109,8 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 					}
 				}
 			}
+
+			// Get any actions that might be attributed to missing modules in the go.mod file.
 			actions, err := mod.SuggestedGoFixes(ctx, snapshot, fh, diagnostics)
 			if err != nil {
 				event.Error(ctx, "quick fixes failed", err, tag.URI.Of(uri))
@@ -103,12 +119,21 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 				codeActions = append(codeActions, actions...)
 			}
 		}
-		if wanted[protocol.SourceOrganizeImports] && len(edits) > 0 {
+		if wanted[protocol.SourceOrganizeImports] && len(importEdits) > 0 {
 			codeActions = append(codeActions, protocol.CodeAction{
 				Title: "Organize Imports",
 				Kind:  protocol.SourceOrganizeImports,
 				Edit: protocol.WorkspaceEdit{
-					DocumentChanges: documentChanges(fh, edits),
+					DocumentChanges: documentChanges(fh, importEdits),
+				},
+			})
+		}
+		if wanted[protocol.SourceFixAll] && len(highConfidenceEdits) > 0 {
+			codeActions = append(codeActions, protocol.CodeAction{
+				Title: "Simplifications",
+				Kind:  protocol.SourceFixAll,
+				Edit: protocol.WorkspaceEdit{
+					DocumentChanges: highConfidenceEdits,
 				},
 			})
 		}
@@ -199,23 +224,28 @@ func importDiagnostics(fix *imports.ImportFix, diagnostics []protocol.Diagnostic
 	return results
 }
 
-func quickFixes(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) {
+func analysisFixes(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, []protocol.TextDocumentEdit, error) {
+	if len(diagnostics) == 0 {
+		return nil, nil, nil
+	}
+
 	var codeActions []protocol.CodeAction
+	var sourceFixAllEdits []protocol.TextDocumentEdit
 
 	phs, err := snapshot.PackageHandles(ctx, fh)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	// We get the package that source.Diagnostics would've used. This is hack.
 	// TODO(golang/go#32443): The correct solution will be to cache diagnostics per-file per-snapshot.
 	ph, err := source.WidestPackageHandle(phs)
 	if err != nil {
-		return nil, err
+		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, err := snapshot.FindAnalysisError(ctx, ph.ID(), diag.Source, diag.Message, diag.Range)
+		srcErr, analyzer, err := snapshot.FindAnalysisError(ctx, ph.ID(), diag.Source, diag.Message, diag.Range)
 		if err != nil {
 			continue
 		}
@@ -232,12 +262,16 @@ func quickFixes(ctx context.Context, snapshot source.Snapshot, fh source.FileHan
 					event.Error(ctx, "no file", err, tag.URI.Of(uri))
 					continue
 				}
-				action.Edit.DocumentChanges = append(action.Edit.DocumentChanges, documentChanges(fh, edits)...)
+				docChanges := documentChanges(fh, edits)
+				if analyzer.HighConfidence {
+					sourceFixAllEdits = append(sourceFixAllEdits, docChanges...)
+				}
+				action.Edit.DocumentChanges = append(action.Edit.DocumentChanges, docChanges...)
 			}
 			codeActions = append(codeActions, action)
 		}
 	}
-	return codeActions, nil
+	return codeActions, sourceFixAllEdits, nil
 }
 
 func documentChanges(fh source.FileHandle, edits []protocol.TextEdit) []protocol.TextDocumentEdit {

internal/lsp/source/options.go

@@ -57,6 +57,7 @@ func DefaultOptions() Options {
 		ServerOptions: ServerOptions{
 			SupportedCodeActions: map[FileKind]map[protocol.CodeActionKind]bool{
 				Go: {
+					protocol.SourceFixAll:          true,
 					protocol.SourceOrganizeImports: true,
 					protocol.QuickFix:              true,
 				},
@@ -508,5 +509,7 @@ func defaultAnalyzers() map[string]Analyzer {
 		deepequalerrors.Analyzer.Name:  {Analyzer: deepequalerrors.Analyzer, Enabled: true},
 		sortslice.Analyzer.Name:        {Analyzer: sortslice.Analyzer, Enabled: true},
 		testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, Enabled: true},
+
+		// gofmt -s suite:
 	}
 }

internal/lsp/source/view.go

@@ -46,7 +46,7 @@ type Snapshot interface {
 
 	// 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, error)
+	FindAnalysisError(ctx context.Context, pkgID, analyzerName, msg string, rng protocol.Range) (*Error, *Analyzer, 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.
@@ -368,6 +368,10 @@ const (
 type Analyzer struct {
 	Analyzer *analysis.Analyzer
 	Enabled  bool
+
+	// If this is true, then we can apply the suggested fixes
+	// as part of a source.FixAll codeaction.
+	HighConfidence bool
 }
 
 // Package represents a Go package that has been type-checked. It maintains