internal/lsp: fix support for SourceFixAll code actions

Some of the refactoring changed the way that we label code action kinds,
and we need to add quickfix and fixall kinds for each diagnostic type.
Support a per-kind suggested fix, and fix a small issue in setting the
analyzer for a fixall code action.

Fixes golang/go#45111

Change-Id: I6bb32c9aa7427b690f42910672d3139579e84478
Reviewed-on: https://go-review.googlesource.com/c/tools/+/303209
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2021-03-18 12:12:50 -10:00
parent 877f9c48b6
commit 09a00c1ab1
15 changed files with 103 additions and 52 deletions

View File

@ -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

View File

@ -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"))
})

View File

@ -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)
}

View File

@ -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)
}

View File

@ -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
}
}

View File

@ -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
}

View File

@ -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{

View File

@ -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
}

View File

@ -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

View File

@ -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

View File

@ -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)},
})
}

View File

@ -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 {

View File

@ -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,
}
}

View File

@ -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},
},
}
}

View File

@ -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 {