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 <rohan@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/cache/pkg.go b/internal/lsp/cache/pkg.go
index b408b0b..6627bd1 100644
--- a/internal/lsp/cache/pkg.go
+++ b/internal/lsp/cache/pkg.go
@@ -125,22 +125,23 @@
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 @@
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)
}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index 0d0b02c..f3dd2c2 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -65,23 +65,37 @@
})
}
case source.Go:
- edits, editsPerFix, 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)
+ 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 {
- event.Error(ctx, "quick fixes failed", err, tag.URI.Of(uri))
+ return nil, err
}
- codeActions = append(codeActions, qf...)
+ }
+ // 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, "analysis fixes failed", err, tag.URI.Of(uri))
+ }
+ }
+
+ 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 @@
}
}
}
+
+ // 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 @@
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 @@
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 @@
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 {
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index febc96a..b70b3ba 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -57,6 +57,7 @@
ServerOptions: ServerOptions{
SupportedCodeActions: map[FileKind]map[protocol.CodeActionKind]bool{
Go: {
+ protocol.SourceFixAll: true,
protocol.SourceOrganizeImports: true,
protocol.QuickFix: true,
},
@@ -508,5 +509,7 @@
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:
}
}
diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go
index 15bc75d..ab9e666 100644
--- a/internal/lsp/source/view.go
+++ b/internal/lsp/source/view.go
@@ -46,7 +46,7 @@
// 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 @@
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
