diff --git a/gopls/internal/regtest/expectation.go b/gopls/internal/regtest/expectation.go index 02e00dd3f3..f86bcb6dff 100644 --- a/gopls/internal/regtest/expectation.go +++ b/gopls/internal/regtest/expectation.go @@ -582,8 +582,9 @@ func NoDiagnosticWithMessage(name, msg string) DiagnosticExpectation { return DiagnosticExpectation{path: name, message: msg, present: false} } -// GoSum asserts that a "go.sum is out of sync" diagnostic for the given module -// (as formatted in a go.mod file, e.g. "example.com v1.0.0") is present. +// GoSumDiagnostic asserts that a "go.sum is out of sync" diagnostic for the +// given module (as formatted in a go.mod file, e.g. "example.com v1.0.0") is +// present. func (e *Env) GoSumDiagnostic(name, module string) Expectation { e.T.Helper() // In 1.16, go.sum diagnostics should appear on the relevant module. Earlier diff --git a/gopls/internal/regtest/misc/fix_test.go b/gopls/internal/regtest/misc/fix_test.go index 395bfd8096..9225a83295 100644 --- a/gopls/internal/regtest/misc/fix_test.go +++ b/gopls/internal/regtest/misc/fix_test.go @@ -81,6 +81,25 @@ func Foo() error { env.DiagnosticAtRegexpWithMessage("main.go", `return`, "wrong number of return values"), ReadDiagnostics("main.go", &d), )) + codeActions := env.CodeAction("main.go", d.Diagnostics) + if len(codeActions) != 2 { + t.Fatalf("expected 2 code actions, got %v", len(codeActions)) + } + var foundQuickFix, foundFixAll bool + for _, a := range codeActions { + if a.Kind == protocol.QuickFix { + foundQuickFix = true + } + if a.Kind == protocol.SourceFixAll { + foundFixAll = true + } + } + if !foundQuickFix { + t.Fatalf("expected quickfix code action, got none") + } + if !foundFixAll { + t.Fatalf("expected fixall code action, got none") + } env.ApplyQuickFixes("main.go", d.Diagnostics) env.Await(EmptyDiagnostics("main.go")) }) diff --git a/gopls/internal/regtest/misc/imports_test.go b/gopls/internal/regtest/misc/imports_test.go index 9a95208cb5..2a666c4e61 100644 --- a/gopls/internal/regtest/misc/imports_test.go +++ b/gopls/internal/regtest/misc/imports_test.go @@ -74,7 +74,7 @@ func main() { Run(t, "", func(t *testing.T, env *Env) { env.CreateBuffer("main.go", vim1) env.OrganizeImports("main.go") - actions := env.CodeAction("main.go") + actions := env.CodeAction("main.go", nil) if len(actions) > 0 { got := env.Editor.BufferText("main.go") t.Errorf("unexpected actions %#v", actions) @@ -107,7 +107,7 @@ func main() { Run(t, "", func(t *testing.T, env *Env) { env.CreateBuffer("main.go", vim2) env.OrganizeImports("main.go") - actions := env.CodeAction("main.go") + actions := env.CodeAction("main.go", nil) if len(actions) > 0 { t.Errorf("unexpected actions %#v", actions) } diff --git a/gopls/internal/regtest/wrappers.go b/gopls/internal/regtest/wrappers.go index 4aafdf20e9..8281dab1e1 100644 --- a/gopls/internal/regtest/wrappers.go +++ b/gopls/internal/regtest/wrappers.go @@ -388,9 +388,9 @@ func (e *Env) AcceptCompletion(path string, pos fake.Pos, item protocol.Completi // CodeAction calls testDocument/codeAction for the given path, and calls // t.Fatal if there are errors. -func (e *Env) CodeAction(path string) []protocol.CodeAction { +func (e *Env) CodeAction(path string, diagnostics []protocol.Diagnostic) []protocol.CodeAction { e.T.Helper() - actions, err := e.Editor.CodeAction(e.Ctx, path, nil) + actions, err := e.Editor.CodeAction(e.Ctx, path, nil, diagnostics) if err != nil { e.T.Fatal(err) } diff --git a/internal/lsp/cache/analysis.go b/internal/lsp/cache/analysis.go index 19675af11a..702513e789 100644 --- a/internal/lsp/cache/analysis.go +++ b/internal/lsp/cache/analysis.go @@ -423,6 +423,7 @@ func (s *snapshot) DiagnosePackage(ctx context.Context, spkg source.Package) (ma clone := *diag clone.SuggestedFixes = eaDiag.SuggestedFixes clone.Tags = eaDiag.Tags + clone.Analyzer = eaDiag.Analyzer diag = &clone } } diff --git a/internal/lsp/cache/errors.go b/internal/lsp/cache/errors.go index f18d5edb1e..ace6947fa9 100644 --- a/internal/lsp/cache/errors.go +++ b/internal/lsp/cache/errors.go @@ -163,7 +163,7 @@ func goGetQuickFixes(snapshot *snapshot, uri span.URI, pkg string) ([]source.Sug if err != nil { return nil, err } - return []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)}, nil + return []source.SuggestedFix{source.SuggestedFixFromCommand(cmd, protocol.QuickFix)}, nil } func analysisDiagnosticDiagnostics(ctx context.Context, snapshot *snapshot, pkg *pkg, a *analysis.Analyzer, e *analysis.Diagnostic) ([]*source.Diagnostic, error) { @@ -184,7 +184,11 @@ func analysisDiagnosticDiagnostics(ctx context.Context, snapshot *snapshot, pkg if err != nil { return nil, err } - fixes, err := suggestedAnalysisFixes(snapshot, pkg, e) + kinds := srcAnalyzer.ActionKind + if len(srcAnalyzer.ActionKind) == 0 { + kinds = append(kinds, protocol.QuickFix) + } + fixes, err := suggestedAnalysisFixes(snapshot, pkg, e, kinds) if err != nil { return nil, err } @@ -197,7 +201,9 @@ func analysisDiagnosticDiagnostics(ctx context.Context, snapshot *snapshot, pkg if err != nil { return nil, err } - fixes = append(fixes, source.SuggestedFixFromCommand(cmd)) + for _, kind := range kinds { + fixes = append(fixes, source.SuggestedFixFromCommand(cmd, kind)) + } } related, err := relatedInformation(snapshot, pkg, e) if err != nil { @@ -242,7 +248,7 @@ func typesCodeHref(snapshot *snapshot, code typesinternal.ErrorCode) string { return fmt.Sprintf("https://%s/golang.org/x/tools/internal/typesinternal#%s", target, code.String()) } -func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnostic) ([]source.SuggestedFix, error) { +func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnostic, kinds []protocol.CodeActionKind) ([]source.SuggestedFix, error) { var fixes []source.SuggestedFix for _, fix := range diag.SuggestedFixes { edits := make(map[span.URI][]protocol.TextEdit) @@ -260,10 +266,14 @@ func suggestedAnalysisFixes(snapshot *snapshot, pkg *pkg, diag *analysis.Diagnos NewText: string(e.NewText), }) } - fixes = append(fixes, source.SuggestedFix{ - Title: fix.Message, - Edits: edits, - }) + for _, kind := range kinds { + fixes = append(fixes, source.SuggestedFix{ + Title: fix.Message, + Edits: edits, + ActionKind: kind, + }) + } + } return fixes, nil } diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go index 2e1917af08..a915d052c0 100644 --- a/internal/lsp/cache/mod.go +++ b/internal/lsp/cache/mod.go @@ -358,7 +358,7 @@ func (s *snapshot) goCommandDiagnostic(pm *source.ParsedModule, spn span.Span, g Source: source.ListError, Message: `Inconsistent vendoring detected. Please re-run "go mod vendor". See https://github.com/golang/go/issues/39164 for more detail on this issue.`, - SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)}, + SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd, protocol.QuickFix)}, }, nil case strings.Contains(goCmdError, "updates to go.sum needed"), strings.Contains(goCmdError, "missing go.sum entry"): @@ -385,8 +385,8 @@ See https://github.com/golang/go/issues/39164 for more detail on this issue.`, Source: source.ListError, Message: msg, SuggestedFixes: []source.SuggestedFix{ - source.SuggestedFixFromCommand(tidyCmd), - source.SuggestedFixFromCommand(updateCmd), + source.SuggestedFixFromCommand(tidyCmd, protocol.QuickFix), + source.SuggestedFixFromCommand(updateCmd, protocol.QuickFix), }, }, nil case strings.Contains(goCmdError, "disabled by GOPROXY=off") && innermost != nil: @@ -405,7 +405,7 @@ See https://github.com/golang/go/issues/39164 for more detail on this issue.`, Severity: protocol.SeverityError, Message: fmt.Sprintf("%v@%v has not been downloaded", innermost.Path, innermost.Version), Source: source.ListError, - SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)}, + SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd, protocol.QuickFix)}, }, nil default: return &source.Diagnostic{ diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go index 6891a39314..0fb9ff671a 100644 --- a/internal/lsp/cache/mod_tidy.go +++ b/internal/lsp/cache/mod_tidy.go @@ -341,7 +341,7 @@ func unusedDiagnostic(m *protocol.ColumnMapper, req *modfile.Require, onlyDiagno Severity: protocol.SeverityWarning, Source: source.ModTidyError, Message: fmt.Sprintf("%s is not used in this module", req.Mod.Path), - SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)}, + SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd, protocol.QuickFix)}, }, nil } @@ -383,6 +383,7 @@ func directnessDiagnostic(m *protocol.ColumnMapper, req *modfile.Require, comput Edits: map[span.URI][]protocol.TextEdit{ m.URI: edits, }, + ActionKind: protocol.QuickFix, }}, }, nil } @@ -413,7 +414,7 @@ func missingModuleDiagnostic(snapshot source.Snapshot, pm *source.ParsedModule, Severity: protocol.SeverityError, Source: source.ModTidyError, Message: fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path), - SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)}, + SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd, protocol.QuickFix)}, }, nil } diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index da6cf97200..7a389b5878 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -147,11 +147,14 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara if len(d.SuggestedFixes) == 0 { continue } - kind := protocol.QuickFix - if d.Analyzer != nil && d.Analyzer.ActionKind != "" { - kind = d.Analyzer.ActionKind + var isFix bool + for _, fix := range d.SuggestedFixes { + if fix.ActionKind == protocol.QuickFix || fix.ActionKind == protocol.SourceFixAll { + isFix = true + break + } } - if kind == protocol.QuickFix || kind == protocol.SourceFixAll { + if isFix { fixDiags = append(fixDiags, d) } else { nonFixDiags = append(nonFixDiags, d) @@ -356,25 +359,13 @@ func codeActionsMatchingDiagnostics(ctx context.Context, snapshot source.Snapsho func codeActionsForDiagnostic(ctx context.Context, snapshot source.Snapshot, sd *source.Diagnostic, pd *protocol.Diagnostic) ([]protocol.CodeAction, error) { var actions []protocol.CodeAction for _, fix := range sd.SuggestedFixes { - action := protocol.CodeAction{ - Title: fix.Title, - Kind: protocol.QuickFix, - Edit: protocol.WorkspaceEdit{}, - Command: fix.Command, - } - if pd != nil { - action.Diagnostics = []protocol.Diagnostic{*pd} - } - if sd.Analyzer != nil && sd.Analyzer.ActionKind != "" { - action.Kind = sd.Analyzer.ActionKind - } - + var changes []protocol.TextDocumentEdit for uri, edits := range fix.Edits { fh, err := snapshot.GetVersionedFile(ctx, uri) if err != nil { return nil, err } - action.Edit.DocumentChanges = append(action.Edit.DocumentChanges, protocol.TextDocumentEdit{ + changes = append(changes, protocol.TextDocumentEdit{ TextDocument: protocol.OptionalVersionedTextDocumentIdentifier{ Version: fh.Version(), TextDocumentIdentifier: protocol.TextDocumentIdentifier{ @@ -384,6 +375,17 @@ func codeActionsForDiagnostic(ctx context.Context, snapshot source.Snapshot, sd Edits: edits, }) } + action := protocol.CodeAction{ + Title: fix.Title, + Kind: fix.ActionKind, + Edit: protocol.WorkspaceEdit{ + DocumentChanges: changes, + }, + Command: fix.Command, + } + if pd != nil { + action.Diagnostics = []protocol.Diagnostic{*pd} + } actions = append(actions, action) } return actions, nil diff --git a/internal/lsp/fake/editor.go b/internal/lsp/fake/editor.go index 725c5961a8..9678fb1f18 100644 --- a/internal/lsp/fake/editor.go +++ b/internal/lsp/fake/editor.go @@ -1039,7 +1039,7 @@ func (e *Editor) References(ctx context.Context, path string, pos Pos) ([]protoc } // CodeAction executes a codeAction request on the server. -func (e *Editor) CodeAction(ctx context.Context, path string, rng *protocol.Range) ([]protocol.CodeAction, error) { +func (e *Editor) CodeAction(ctx context.Context, path string, rng *protocol.Range, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { if e.Server == nil { return nil, nil } @@ -1051,6 +1051,9 @@ func (e *Editor) CodeAction(ctx context.Context, path string, rng *protocol.Rang } params := &protocol.CodeActionParams{ TextDocument: e.textDocumentIdentifier(path), + Context: protocol.CodeActionContext{ + Diagnostics: diagnostics, + }, } if rng != nil { params.Range = *rng diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index 0aa8cafa51..6495aeb9b8 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -81,7 +81,7 @@ func DiagnosticsForMod(ctx context.Context, snapshot source.Snapshot, fh source. Severity: protocol.SeverityInformation, Source: source.UpgradeNotification, Message: fmt.Sprintf("%v can be upgraded", req.Mod.Path), - SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd)}, + SuggestedFixes: []source.SuggestedFix{source.SuggestedFixFromCommand(cmd, protocol.QuickFix)}, }) } diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index 1948381d5a..58154ca1c8 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -12,9 +12,10 @@ import ( ) type SuggestedFix struct { - Title string - Edits map[span.URI][]protocol.TextEdit - Command *protocol.Command + Title string + Edits map[span.URI][]protocol.TextEdit + Command *protocol.Command + ActionKind protocol.CodeActionKind } type RelatedInformation struct { diff --git a/internal/lsp/source/fix.go b/internal/lsp/source/fix.go index 391835593d..6a012396cc 100644 --- a/internal/lsp/source/fix.go +++ b/internal/lsp/source/fix.go @@ -42,10 +42,11 @@ var suggestedFixes = map[string]SuggestedFixFunc{ ExtractFunction: extractFunction, } -func SuggestedFixFromCommand(cmd protocol.Command) SuggestedFix { +func SuggestedFixFromCommand(cmd protocol.Command, kind protocol.CodeActionKind) SuggestedFix { return SuggestedFix{ - Title: cmd.Title, - Command: &cmd, + Title: cmd.Title, + Command: &cmd, + ActionKind: kind, } } diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 0f8dda1e40..41557f4508 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -1080,7 +1080,7 @@ func typeErrorAnalyzers() map[string]*Analyzer { return map[string]*Analyzer{ fillreturns.Analyzer.Name: { Analyzer: fillreturns.Analyzer, - ActionKind: protocol.SourceFixAll, + ActionKind: []protocol.CodeActionKind{protocol.SourceFixAll, protocol.QuickFix}, Enabled: true, }, nonewvars.Analyzer.Name: { @@ -1105,7 +1105,7 @@ func convenienceAnalyzers() map[string]*Analyzer { Analyzer: fillstruct.Analyzer, Fix: FillStruct, Enabled: true, - ActionKind: protocol.RefactorRewrite, + ActionKind: []protocol.CodeActionKind{protocol.RefactorRewrite}, }, } } @@ -1150,9 +1150,21 @@ func defaultAnalyzers() map[string]*Analyzer { unusedwrite.Analyzer.Name: {Analyzer: unusedwrite.Analyzer, Enabled: false}, // gofmt -s suite: - simplifycompositelit.Analyzer.Name: {Analyzer: simplifycompositelit.Analyzer, Enabled: true, ActionKind: protocol.SourceFixAll}, - simplifyrange.Analyzer.Name: {Analyzer: simplifyrange.Analyzer, Enabled: true, ActionKind: protocol.SourceFixAll}, - simplifyslice.Analyzer.Name: {Analyzer: simplifyslice.Analyzer, Enabled: true, ActionKind: protocol.SourceFixAll}, + simplifycompositelit.Analyzer.Name: { + Analyzer: simplifycompositelit.Analyzer, + Enabled: true, + ActionKind: []protocol.CodeActionKind{protocol.SourceFixAll, protocol.QuickFix}, + }, + simplifyrange.Analyzer.Name: { + Analyzer: simplifyrange.Analyzer, + Enabled: true, + ActionKind: []protocol.CodeActionKind{protocol.SourceFixAll, protocol.QuickFix}, + }, + simplifyslice.Analyzer.Name: { + Analyzer: simplifyslice.Analyzer, + Enabled: true, + ActionKind: []protocol.CodeActionKind{protocol.SourceFixAll, protocol.QuickFix}, + }, } } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 9292a84ec4..412866c0dc 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -531,7 +531,7 @@ type Analyzer struct { // ActionKind is the kind of code action this analyzer produces. If // unspecified the type defaults to quickfix. - ActionKind protocol.CodeActionKind + ActionKind []protocol.CodeActionKind } func (a Analyzer) IsEnabled(view View) bool {