diff --git a/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go.golden b/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go.golden deleted file mode 100644 index 290f3f5bea..0000000000 --- a/internal/lsp/analysis/undeclaredname/testdata/src/a/a.go.golden +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright 2020 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package undeclared - -func x() int { - var z int - y := - z = y // want "undeclared name: y" - - m := - if z == m { // want "undeclared name: m" - z = 1 - } - - n := - if z == 1 { - z = 1 - } else if z == n+1 { // want "undeclared name: n" - z = 1 - } - - a := - switch z { - case 10: - z = 1 - case a: // want "undeclared name: a" - z = 1 - } - return z -} diff --git a/internal/lsp/analysis/undeclaredname/undeclared.go b/internal/lsp/analysis/undeclaredname/undeclared.go index fa239bee1c..89de82f6fd 100644 --- a/internal/lsp/analysis/undeclaredname/undeclared.go +++ b/internal/lsp/analysis/undeclaredname/undeclared.go @@ -10,7 +10,8 @@ import ( "bytes" "fmt" "go/ast" - "go/printer" + "go/token" + "go/types" "strings" "golang.org/x/tools/go/analysis" @@ -65,51 +66,61 @@ func run(pass *analysis.Pass) (interface{}, error) { if _, ok := path[1].(*ast.SelectorExpr); ok { continue } - // TODO(golang.org/issue/34644): in a follow up handle call expressions - // with suggested fix to create function + // TODO(golang.org/issue/34644): Handle call expressions with suggested + // fixes to create a function. if _, ok := path[1].(*ast.CallExpr); ok { continue } - - // Get the place to insert the new statement. - insertBeforeStmt := analysisinternal.StmtToInsertVarBefore(path) - if insertBeforeStmt == nil { + tok := pass.Fset.File(file.Pos()) + if tok == nil { continue } - - var buf bytes.Buffer - if err := printer.Fprint(&buf, pass.Fset, file); err != nil { - continue - } - old := buf.Bytes() - insertBefore := pass.Fset.Position(insertBeforeStmt.Pos()).Offset - - // Get the indent to add on the line after the new statement. - // Since this will have a parse error, we can not use format.Source(). - contentBeforeStmt, indent := old[:insertBefore], "\n" - if nl := bytes.LastIndex(contentBeforeStmt, []byte("\n")); nl != -1 { - indent = string(contentBeforeStmt[nl:]) - } - // Create the new local variable statement. - newStmt := fmt.Sprintf("%s := %s", ident.Name, indent) - + offset := pass.Fset.Position(err.Pos).Offset + end := tok.Pos(offset + len(name)) pass.Report(analysis.Diagnostic{ Pos: err.Pos, - End: analysisinternal.TypeErrorEndPos(pass.Fset, old, err.Pos), + End: end, Message: err.Msg, - SuggestedFixes: []analysis.SuggestedFix{{ - Message: fmt.Sprintf("Create variable \"%s\"", ident.Name), - TextEdits: []analysis.TextEdit{{ - Pos: insertBeforeStmt.Pos(), - End: insertBeforeStmt.Pos(), - NewText: []byte(newStmt), - }}, - }}, }) } return nil, nil } +func SuggestedFix(fset *token.FileSet, pos token.Pos, content []byte, file *ast.File, _ *types.Package, _ *types.Info) (*analysis.SuggestedFix, error) { + path, _ := astutil.PathEnclosingInterval(file, pos, pos) + if len(path) < 2 { + return nil, fmt.Errorf("") + } + ident, ok := path[0].(*ast.Ident) + if !ok { + return nil, fmt.Errorf("") + } + // Get the place to insert the new statement. + insertBeforeStmt := analysisinternal.StmtToInsertVarBefore(path) + if insertBeforeStmt == nil { + return nil, fmt.Errorf("") + } + + insertBefore := fset.Position(insertBeforeStmt.Pos()).Offset + + // Get the indent to add on the line after the new statement. + // Since this will have a parse error, we can not use format.Source(). + contentBeforeStmt, indent := content[:insertBefore], "\n" + if nl := bytes.LastIndex(contentBeforeStmt, []byte("\n")); nl != -1 { + indent = string(contentBeforeStmt[nl:]) + } + // Create the new local variable statement. + newStmt := fmt.Sprintf("%s := %s", ident.Name, indent) + return &analysis.SuggestedFix{ + Message: fmt.Sprintf("Create variable \"%s\"", ident.Name), + TextEdits: []analysis.TextEdit{{ + Pos: insertBeforeStmt.Pos(), + End: insertBeforeStmt.Pos(), + NewText: []byte(newStmt), + }}, + }, nil +} + func FixesError(msg string) bool { return strings.HasPrefix(msg, undeclaredNamePrefix) } diff --git a/internal/lsp/analysis/undeclaredname/undeclared_test.go b/internal/lsp/analysis/undeclaredname/undeclared_test.go index eb9e494b3d..b715439374 100644 --- a/internal/lsp/analysis/undeclaredname/undeclared_test.go +++ b/internal/lsp/analysis/undeclaredname/undeclared_test.go @@ -13,5 +13,5 @@ import ( func Test(t *testing.T) { testdata := analysistest.TestData() - analysistest.RunWithSuggestedFixes(t, testdata, undeclaredname.Analyzer, "a") + analysistest.Run(t, testdata, undeclaredname.Analyzer, "a") } diff --git a/internal/lsp/analysis/unusedparams/unusedparams.go b/internal/lsp/analysis/unusedparams/unusedparams.go index 3b3a605905..f79d25aed0 100644 --- a/internal/lsp/analysis/unusedparams/unusedparams.go +++ b/internal/lsp/analysis/unusedparams/unusedparams.go @@ -120,21 +120,20 @@ func run(pass *analysis.Pass) (interface{}, error) { if nObj := pass.TypesInfo.ObjectOf(n); nObj != param.typObj { return false } - if _, ok := unused[param]; ok { - delete(unused, param) - } + delete(unused, param) return false }) // Create the reports for the unused parameters. - for u, _ := range unused { + for u := range unused { start, end := u.field.Pos(), u.field.End() if len(u.field.Names) > 1 { start, end = u.ident.Pos(), u.ident.End() } - // TODO(golang/go#36602): add suggested fixes to automatically remove the unused parameter, - // to start, just remove it from the function declaration, - // later, remove from every use of this function + // TODO(golang/go#36602): Add suggested fixes to automatically + // remove the unused parameter. To start, just remove it from the + // function declaration. Later, remove it from every use of this + // function. pass.Report(analysis.Diagnostic{ Pos: start, End: end, diff --git a/internal/lsp/cmd/cmd.go b/internal/lsp/cmd/cmd.go index e49bc6beaa..1b142ec60c 100644 --- a/internal/lsp/cmd/cmd.go +++ b/internal/lsp/cmd/cmd.go @@ -74,8 +74,8 @@ type Application struct { PrepareOptions func(*source.Options) } -func (a *Application) verbose() bool { - return a.Verbose || a.VeryVerbose +func (app *Application) verbose() bool { + return app.Verbose || app.VeryVerbose } // New returns a new Application ready to run. @@ -188,7 +188,7 @@ func (app *Application) featureCommands() []tool.Application { &references{app: app}, &rename{app: app}, &signature{app: app}, - &suggestedfix{app: app}, + &suggestedFix{app: app}, &symbols{app: app}, &workspaceSymbol{app: app}, } diff --git a/internal/lsp/cmd/suggested_fix.go b/internal/lsp/cmd/suggested_fix.go index 5738747b58..51ab4db909 100644 --- a/internal/lsp/cmd/suggested_fix.go +++ b/internal/lsp/cmd/suggested_fix.go @@ -18,8 +18,8 @@ import ( errors "golang.org/x/xerrors" ) -// suggestedfix implements the fix verb for gopls. -type suggestedfix struct { +// suggestedFix implements the fix verb for gopls. +type suggestedFix struct { Diff bool `flag:"d" help:"display diffs instead of rewriting files"` Write bool `flag:"w" help:"write result to (source) file instead of stdout"` All bool `flag:"a" help:"apply all fixes, not just preferred fixes"` @@ -27,10 +27,10 @@ type suggestedfix struct { app *Application } -func (s *suggestedfix) Name() string { return "fix" } -func (s *suggestedfix) Usage() string { return "" } -func (s *suggestedfix) ShortHelp() string { return "apply suggested fixes" } -func (s *suggestedfix) DetailedHelp(f *flag.FlagSet) { +func (s *suggestedFix) Name() string { return "fix" } +func (s *suggestedFix) Usage() string { return "" } +func (s *suggestedFix) ShortHelp() string { return "apply suggested fixes" } +func (s *suggestedFix) DetailedHelp(f *flag.FlagSet) { fmt.Fprintf(f.Output(), ` Example: apply suggested fixes for this file: @@ -45,7 +45,7 @@ gopls fix flags are: // - if -w is specified, updates the file in place; // - if -d is specified, prints out unified diffs of the changes; or // - otherwise, prints the new versions to stdout. -func (s *suggestedfix) Run(ctx context.Context, args ...string) error { +func (s *suggestedFix) Run(ctx context.Context, args ...string) error { if len(args) < 1 { return tool.CommandLineErrorf("fix expects at least 1 argument") } @@ -96,6 +96,9 @@ func (s *suggestedfix) Run(ctx context.Context, args ...string) error { } var edits []protocol.TextEdit for _, a := range actions { + if a.Command != nil { + return fmt.Errorf("ExecuteCommand is not yet supported on the command line") + } if !a.IsPreferred && !s.All { continue } diff --git a/internal/lsp/cmd/test/suggested_fix.go b/internal/lsp/cmd/test/suggested_fix.go index f1a478a30c..965edd4aab 100644 --- a/internal/lsp/cmd/test/suggested_fix.go +++ b/internal/lsp/cmd/test/suggested_fix.go @@ -22,11 +22,14 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) } } args = append(args, actionKinds...) - got, _ := r.NormalizeGoplsCmd(t, args...) + got, stderr := r.NormalizeGoplsCmd(t, args...) + if stderr == "ExecuteCommand is not yet supported on the command line" { + t.Skipf(stderr) + } want := string(r.data.Golden("suggestedfix_"+tests.SpanName(spn), filename, func() ([]byte, error) { return []byte(got), nil })) if want != got { - t.Errorf("suggested fixes failed for %s: %s", filename, tests.Diff(want, got)) + t.Errorf("suggested fixes failed for %s:\n%s", filename, tests.Diff(want, got)) } } diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 48782b75ec..b7fd2847c5 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -13,7 +13,6 @@ import ( "golang.org/x/tools/go/analysis" "golang.org/x/tools/internal/event" "golang.org/x/tools/internal/imports" - "golang.org/x/tools/internal/lsp/analysis/fillstruct" "golang.org/x/tools/internal/lsp/debug/tag" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" @@ -242,15 +241,25 @@ func analysisFixes(ctx context.Context, snapshot source.Snapshot, ph source.Pack if len(diagnostics) == 0 { return nil, nil, nil } - - var codeActions []protocol.CodeAction - var sourceFixAllEdits []protocol.TextDocumentEdit - + var ( + codeActions []protocol.CodeAction + sourceFixAllEdits []protocol.TextDocumentEdit + ) for _, diag := range diagnostics { srcErr, analyzer, ok := findSourceError(ctx, snapshot, ph.ID(), diag) if !ok { continue } + // If the suggested fix for the diagnostic is expected to be separate, + // see if there are any supported commands available. + if analyzer.SuggestedFix != nil { + action, err := diagnosticToCommandCodeAction(ctx, snapshot, srcErr, &diag, protocol.QuickFix) + if err != nil { + return nil, nil, err + } + codeActions = append(codeActions, *action) + continue + } for _, fix := range srcErr.SuggestedFixes { action := protocol.CodeAction{ Title: fix.Title, @@ -276,24 +285,8 @@ func analysisFixes(ctx context.Context, snapshot source.Snapshot, ph source.Pack } 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) { + analyzer := diagnosticToAnalyzer(snapshot, diag.Source, diag.Message) + if analyzer == nil { return nil, source.Analyzer{}, false } analysisErrors, err := snapshot.Analyze(ctx, pkgID, analyzer.Analyzer) @@ -316,12 +309,48 @@ func findSourceError(ctx context.Context, snapshot source.Snapshot, pkgID string return nil, source.Analyzer{}, false } +// diagnosticToAnalyzer return the analyzer associated with a given diagnostic. +// It assumes that the diagnostic's source will be the name of the analyzer. +// If this changes, this approach will need to be reworked. +func diagnosticToAnalyzer(snapshot source.Snapshot, src, msg string) (analyzer *source.Analyzer) { + // Make sure that the analyzer we found is enabled. + defer func() { + if analyzer != nil && !analyzer.Enabled(snapshot) { + analyzer = nil + } + }() + if a, ok := snapshot.View().Options().DefaultAnalyzers[src]; ok { + return &a + } + if a, ok := snapshot.View().Options().ConvenienceAnalyzers[src]; ok { + return &a + } + // Hack: We publish diagnostics with the source "compiler" for type errors, + // but these analyzers have different names. Try both possibilities. + if a, ok := snapshot.View().Options().TypeErrorAnalyzers[src]; ok { + return &a + } + if src != "compiler" { + return nil + } + for _, a := range snapshot.View().Options().TypeErrorAnalyzers { + if a.FixesError(msg) { + return &a + } + } + return nil +} + func convenienceFixes(ctx context.Context, snapshot source.Snapshot, ph source.PackageHandle, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) { var analyzers []*analysis.Analyzer for _, a := range snapshot.View().Options().ConvenienceAnalyzers { if !a.Enabled(snapshot) { continue } + if a.SuggestedFix == nil { + event.Error(ctx, "convenienceFixes", fmt.Errorf("no suggested fixes for convenience analyzer %s", a.Analyzer.Name)) + continue + } analyzers = append(analyzers, a.Analyzer) } diagnostics, err := snapshot.Analyze(ctx, ph.ID(), analyzers...) @@ -338,29 +367,45 @@ func convenienceFixes(ctx context.Context, snapshot source.Snapshot, ph source.P if d.Range.Start.Line != rng.Start.Line { continue } - // The fix depends on the category of the analyzer. - switch d.Category { - case fillstruct.Analyzer.Name: - jsonArgs, err := source.EncodeArgs(d.URI, d.Range) - if err != nil { - return nil, err - } - action := protocol.CodeAction{ - Title: d.Message, - Kind: protocol.RefactorRewrite, - Command: &protocol.Command{ - Command: source.CommandFillStruct, - Title: d.Message, - Arguments: jsonArgs, - }, - } - codeActions = append(codeActions, action) + action, err := diagnosticToCommandCodeAction(ctx, snapshot, d, nil, protocol.RefactorRewrite) + if err != nil { + return nil, err } - + codeActions = append(codeActions, *action) } return codeActions, nil } +func diagnosticToCommandCodeAction(ctx context.Context, snapshot source.Snapshot, e *source.Error, d *protocol.Diagnostic, kind protocol.CodeActionKind) (*protocol.CodeAction, error) { + // The fix depends on the category of the analyzer. The diagnostic may be + // nil, so use the error's category. + analyzer := diagnosticToAnalyzer(snapshot, e.Category, e.Message) + if analyzer == nil { + return nil, fmt.Errorf("no convenience analyzer for category %s", e.Category) + } + if analyzer.Command == "" { + return nil, fmt.Errorf("no command for convenience analyzer %s", analyzer.Analyzer.Name) + } + jsonArgs, err := source.EncodeArgs(e.URI, e.Range) + if err != nil { + return nil, err + } + var diagnostics []protocol.Diagnostic + if d != nil { + diagnostics = append(diagnostics, *d) + } + return &protocol.CodeAction{ + Title: e.Message, + Kind: kind, + Diagnostics: diagnostics, + Command: &protocol.Command{ + Command: analyzer.Command, + Title: e.Message, + Arguments: jsonArgs, + }, + }, nil +} + func extractionFixes(ctx context.Context, snapshot source.Snapshot, ph source.PackageHandle, uri span.URI, rng protocol.Range) ([]protocol.CodeAction, error) { fh, err := snapshot.GetFile(ctx, uri) if err != nil { diff --git a/internal/lsp/command.go b/internal/lsp/command.go index cd1d545cac..476dc67ca5 100644 --- a/internal/lsp/command.go +++ b/internal/lsp/command.go @@ -100,7 +100,7 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom } err := s.directGoModCommand(ctx, uri, "get", deps...) return nil, err - case source.CommandFillStruct: + case source.CommandFillStruct, source.CommandUndeclaredName: var uri protocol.DocumentURI var rng protocol.Range if err := source.DecodeArgs(params.Arguments, &uri, &rng); err != nil { @@ -110,7 +110,7 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom if !ok { return nil, err } - edits, err := source.FillStruct(ctx, snapshot, fh, rng) + edits, err := commandToEdits(ctx, snapshot, fh, rng, params.Command) if err != nil { return nil, err } @@ -125,7 +125,7 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom if !r.Applied { return nil, s.client.ShowMessage(ctx, &protocol.ShowMessageParams{ Type: protocol.Error, - Message: fmt.Sprintf("fillstruct failed: %v", r.FailureReason), + Message: fmt.Sprintf("%s failed: %v", params.Command, r.FailureReason), }) } default: @@ -134,6 +134,23 @@ func (s *Server) executeCommand(ctx context.Context, params *protocol.ExecuteCom return nil, nil } +func commandToEdits(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, rng protocol.Range, cmd string) ([]protocol.TextDocumentEdit, error) { + var analyzer *source.Analyzer + for _, a := range source.EnabledAnalyzers(snapshot) { + if cmd == a.Command { + analyzer = &a + break + } + } + if analyzer == nil { + return nil, fmt.Errorf("no known analyzer for %s", cmd) + } + if analyzer.SuggestedFix == nil { + return nil, fmt.Errorf("no fix function for %s", cmd) + } + return source.CommandSuggestedFixes(ctx, snapshot, fh, rng, analyzer.SuggestedFix) +} + func (s *Server) directGoModCommand(ctx context.Context, uri protocol.DocumentURI, verb string, args ...string) error { view, err := s.session.ViewOf(uri.SpanURI()) if err != nil { diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 1525f7ac5e..e445e494cb 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -352,7 +352,7 @@ func (r *runner) Import(t *testing.T, spn span.Span) { } got := string(m.Content) if len(actions) > 0 { - res, err := applyWorkspaceEdits(r, actions[0].Edit) + res, err := applyTextDocumentEdits(r, actions[0].Edit.DocumentChanges) if err != nil { t.Fatal(err) } @@ -373,6 +373,11 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) if err != nil { t.Fatal(err) } + snapshot := view.Snapshot() + fh, err := snapshot.GetFile(r.ctx, uri) + if err != nil { + t.Fatal(err) + } m, err := r.data.Mapper(uri) if err != nil { t.Fatal(err) @@ -417,17 +422,37 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) if err != nil { t.Fatalf("CodeAction %s failed: %v", spn, err) } - // Hack: We assume that we only get one code action per range. - // TODO(rstambler): Support multiple code actions per test. - if len(actions) == 0 || len(actions) > 1 { - t.Fatalf("unexpected number of code actions, want 1, got %v (%v)", len(actions), actions) + if len(actions) != 1 { + // Hack: We assume that we only get one code action per range. + // TODO(rstambler): Support multiple code actions per test. + t.Fatalf("unexpected number of code actions, want 1, got %v", len(actions)) } - if actions[0].Command != nil { - t.Skip("no tests for code action commands") + action := actions[0] + var match bool + for _, k := range codeActionKinds { + if action.Kind == k { + match = true + break + } } - res, err := applyWorkspaceEdits(r, actions[0].Edit) - if err != nil { - t.Fatal(err) + if !match { + t.Fatalf("unexpected kind for code action %s, expected one of %v, got %v", action.Title, codeActionKinds, action.Kind) + } + var res map[span.URI]string + if cmd := action.Command; cmd != nil { + edits, err := commandToEdits(r.ctx, snapshot, fh, rng, action.Command.Command) + if err != nil { + t.Fatal(err) + } + res, err = applyTextDocumentEdits(r, edits) + if err != nil { + t.Fatal(err) + } + } else { + res, err = applyTextDocumentEdits(r, action.Edit.DocumentChanges) + if err != nil { + t.Fatal(err) + } } for u, got := range res { want := string(r.data.Golden("suggestedfix_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) { @@ -471,7 +496,7 @@ func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span if len(actions) == 0 || len(actions) > 1 { t.Fatalf("unexpected number of code actions, want 1, got %v", len(actions)) } - res, err := applyWorkspaceEdits(r, actions[0].Edit) + res, err := applyTextDocumentEdits(r, actions[0].Edit.DocumentChanges) if err != nil { t.Fatal(err) } @@ -731,7 +756,7 @@ func (r *runner) Rename(t *testing.T, spn span.Span, newText string) { } return } - res, err := applyWorkspaceEdits(r, *wedit) + res, err := applyTextDocumentEdits(r, wedit.DocumentChanges) if err != nil { t.Fatal(err) } @@ -802,9 +827,9 @@ func (r *runner) PrepareRename(t *testing.T, src span.Span, want *source.Prepare } } -func applyWorkspaceEdits(r *runner, wedit protocol.WorkspaceEdit) (map[span.URI]string, error) { +func applyTextDocumentEdits(r *runner, edits []protocol.TextDocumentEdit) (map[span.URI]string, error) { res := map[span.URI]string{} - for _, docEdits := range wedit.DocumentChanges { + for _, docEdits := range edits { uri := docEdits.TextDocument.URI.SpanURI() m, err := r.data.Mapper(uri) if err != nil { diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index e38fa9116e..056c10979b 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -76,6 +76,10 @@ const ( // CommandFillStruct is a gopls command to fill a struct with default // values. CommandFillStruct = "fill_struct" + + // CommandUndeclaredName is a gopls command to add a variable declaration + // for an undeclared name. + CommandUndeclaredName = "undeclared_name" ) // DefaultOptions is the options that are used for Gopls execution independent @@ -112,6 +116,7 @@ func DefaultOptions() Options { CommandRegenerateCgo, CommandTest, CommandTidy, + CommandUndeclaredName, CommandUpgradeDependency, CommandVendor, }, @@ -683,28 +688,51 @@ func (r *OptionResult) setString(s *string) { } } +// EnabledAnalyzers returns all of the analyzers enabled for the given +// snapshot. +func EnabledAnalyzers(snapshot Snapshot) (analyzers []Analyzer) { + for _, a := range snapshot.View().Options().DefaultAnalyzers { + if a.Enabled(snapshot) { + analyzers = append(analyzers, a) + } + } + for _, a := range snapshot.View().Options().TypeErrorAnalyzers { + if a.Enabled(snapshot) { + analyzers = append(analyzers, a) + } + } + for _, a := range snapshot.View().Options().ConvenienceAnalyzers { + if a.Enabled(snapshot) { + analyzers = append(analyzers, a) + } + } + return analyzers +} + func typeErrorAnalyzers() map[string]Analyzer { return map[string]Analyzer{ fillreturns.Analyzer.Name: { Analyzer: fillreturns.Analyzer, - enabled: true, FixesError: fillreturns.FixesError, HighConfidence: true, + enabled: true, }, nonewvars.Analyzer.Name: { Analyzer: nonewvars.Analyzer, - enabled: true, FixesError: nonewvars.FixesError, + enabled: true, }, noresultvalues.Analyzer.Name: { Analyzer: noresultvalues.Analyzer, - enabled: true, FixesError: noresultvalues.FixesError, + enabled: true, }, undeclaredname.Analyzer.Name: { - Analyzer: undeclaredname.Analyzer, - enabled: true, - FixesError: undeclaredname.FixesError, + Analyzer: undeclaredname.Analyzer, + FixesError: undeclaredname.FixesError, + SuggestedFix: undeclaredname.SuggestedFix, + Command: CommandUndeclaredName, + enabled: true, }, } } @@ -712,8 +740,10 @@ func typeErrorAnalyzers() map[string]Analyzer { func convenienceAnalyzers() map[string]Analyzer { return map[string]Analyzer{ fillstruct.Analyzer.Name: { - Analyzer: fillstruct.Analyzer, - enabled: true, + Analyzer: fillstruct.Analyzer, + SuggestedFix: fillstruct.SuggestedFix, + Command: CommandFillStruct, + enabled: true, }, } } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 4841526cfd..b1bc10ebfe 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -474,63 +474,6 @@ func (r *runner) Import(t *testing.T, spn span.Span) { } } -func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) { - var refactorRewrite bool - for _, actionKind := range actionKinds { - if actionKind == "refactor.rewrite" { - refactorRewrite = true - break - } - } - // Only test refactor rewrite code actions. - if !refactorRewrite { - return - } - snapshot := r.view.Snapshot() - fh, err := snapshot.GetFile(r.ctx, spn.URI()) - if err != nil { - t.Fatal(err) - } - edits, err := source.FillStruct(r.ctx, snapshot, fh, protocol.Range{ - Start: protocol.Position{ - Line: float64(spn.Start().Line() - 1), - Character: float64(spn.Start().Column() - 1), - }, - End: protocol.Position{ - Line: float64(spn.End().Line() - 1), - Character: float64(spn.End().Column() - 1), - }, - }) - if err != nil { - t.Fatal(err) - } - m, err := r.data.Mapper(fh.URI()) - if err != nil { - t.Fatal(err) - } - if len(edits) == 0 || len(edits) > 1 { - t.Fatalf("expected 1 edit, got %v", len(edits)) - } - diffEdits, err := source.FromProtocolEdits(m, edits[0].Edits) - if err != nil { - t.Error(err) - } - data, err := fh.Read() - if err != nil { - t.Fatal(err) - } - got := diff.ApplyEdits(string(data), diffEdits) - want := string(r.data.Golden("suggestedfix_"+tests.SpanName(spn), fh.URI().Filename(), func() ([]byte, error) { - return []byte(got), nil - })) - if want != got { - d := myers.ComputeEdits(spn.URI(), want, got) - t.Errorf("import failed for %s: %s", spn.URI().Filename(), diff.ToUnified("want", "got", want, d)) - } -} - -func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span) {} - func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) { _, srcRng, err := spanToRange(r.data, d.Src) if err != nil { @@ -918,9 +861,10 @@ func (r *runner) SignatureHelp(t *testing.T, spn span.Span, want *protocol.Signa } } -func (r *runner) Link(t *testing.T, uri span.URI, wantLinks []tests.Link) { - // This is a pure LSP feature, no source level functionality to be tested. -} +// These are pure LSP features, no source level functionality to be tested. +func (r *runner) Link(t *testing.T, uri span.URI, wantLinks []tests.Link) {} +func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) {} +func (r *runner) FunctionExtraction(t *testing.T, start span.Span, end span.Span) {} func (r *runner) CodeLens(t *testing.T, uri span.URI, want []protocol.CodeLens) { fh, err := r.view.Snapshot().GetFile(r.ctx, uri) diff --git a/internal/lsp/source/fill_struct.go b/internal/lsp/source/suggested_fix.go similarity index 59% rename from internal/lsp/source/fill_struct.go rename to internal/lsp/source/suggested_fix.go index 5fe339416d..6b9f9204cd 100644 --- a/internal/lsp/source/fill_struct.go +++ b/internal/lsp/source/suggested_fix.go @@ -7,13 +7,26 @@ package source import ( "context" "fmt" + "go/ast" + "go/token" + "go/types" - "golang.org/x/tools/internal/lsp/analysis/fillstruct" + "golang.org/x/tools/go/analysis" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/span" ) -func FillStruct(ctx context.Context, snapshot Snapshot, fh FileHandle, pRng protocol.Range) ([]protocol.TextDocumentEdit, error) { +// SuggestedFixFunc is a function used to get the suggested fixes for a given +// go/analysis.Analyzer. Some of the analyzers in internal/lsp/analysis are not +// efficient enough to include suggested fixes with their diagnostics, so we +// have to compute them separately. Such analyzers should provide a function +// with a signature of SuggestedFixFunc. +type SuggestedFixFunc func(*token.FileSet, token.Pos, []byte, *ast.File, *types.Package, *types.Info) (*analysis.SuggestedFix, error) + +// CommandSuggestedFixes returns the text edits for a given file and +// SuggestedFixFunc. It can be used to execute any command that provides its +// edits through a SuggestedFixFunc. +func CommandSuggestedFixes(ctx context.Context, snapshot Snapshot, fh FileHandle, pRng protocol.Range, fn SuggestedFixFunc) ([]protocol.TextDocumentEdit, error) { pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestPackageHandle) if err != nil { return nil, fmt.Errorf("getting file for Identifier: %w", err) @@ -35,7 +48,7 @@ func FillStruct(ctx context.Context, snapshot Snapshot, fh FileHandle, pRng prot return nil, err } fset := snapshot.View().Session().Cache().FileSet() - fix, err := fillstruct.SuggestedFix(fset, rng.Start, content, file, pkg.GetTypes(), pkg.GetTypesInfo()) + fix, err := fn(fset, rng.Start, content, file, pkg.GetTypes(), pkg.GetTypesInfo()) if err != nil { return nil, err } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index 47beaa2520..50a1355bba 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -460,6 +460,15 @@ type Analyzer struct { Analyzer *analysis.Analyzer enabled bool + // SuggestedFix is non-nil if we expect this analyzer to provide its fix + // separately from its diagnostics. That is, we should apply the analyzer's + // suggested fixes through a Command, not a TextEdit. + SuggestedFix SuggestedFixFunc + + // Command is the name of the command used to invoke the suggested fixes + // for the analyzer. + Command string + // If this is true, then we can apply the suggested fixes // as part of a source.FixAll codeaction. HighConfidence bool