internal/lsp: plumb fillstruct through analysis
Now that fillstruct is an analyzer, we can simplify the code that calls
it in code_action.go. We introduce a new class of analyzer --
convenience analyzers, which are closer to commands. These represent
suggestions that won't necessarily improve the quality or correctness of
your code, but they offer small helper functions for the user.
This CL also combines the refactor rewrite tests with the suggested fix
tests, since they are effectively the same.
For now, we only support convenience analyzers when a code action was
requested on the same line as the fix. I'm not sure how to otherwise
handle this without bothering the user with unnecessary diagnostics.
Change-Id: I7f0aa198b5ee9964a907d709bae6380093d4ef21
Reviewed-on: https://go-review.googlesource.com/c/tools/+/237687
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/internal/lsp/analysis/fillstruct/fillstruct.go b/internal/lsp/analysis/fillstruct/fillstruct.go
index b23a05f..6ec8a35 100644
--- a/internal/lsp/analysis/fillstruct/fillstruct.go
+++ b/internal/lsp/analysis/fillstruct/fillstruct.go
@@ -8,6 +8,7 @@
import (
"bytes"
+ "fmt"
"go/ast"
"go/format"
"go/types"
@@ -53,12 +54,10 @@
return
}
expr := n.(*ast.CompositeLit)
-
// TODO: Handle partially-filled structs as well.
if len(expr.Elts) != 0 {
return
}
-
var file *ast.File
for _, f := range pass.Files {
if f.Pos() <= expr.Pos() && expr.Pos() <= f.End() {
@@ -85,61 +84,62 @@
}
typ = typ.Underlying()
- if typ == nil {
+ obj, ok := typ.(*types.Struct)
+ if !ok {
+ return
+ }
+ fieldCount := obj.NumFields()
+ if fieldCount == 0 {
+ return
+ }
+ var fieldSourceCode strings.Builder
+ for i := 0; i < fieldCount; i++ {
+ field := obj.Field(i)
+ // Ignore fields that are not accessible in the current package.
+ if field.Pkg() != nil && field.Pkg() != pass.Pkg && !field.Exported() {
+ continue
+ }
+
+ label := field.Name()
+ value := analysisinternal.ZeroValue(pass.Fset, file, pass.Pkg, field.Type())
+ if value == nil {
+ continue
+ }
+ var valBuf bytes.Buffer
+ if err := format.Node(&valBuf, pass.Fset, value); err != nil {
+ return
+ }
+ fieldSourceCode.WriteString("\n")
+ fieldSourceCode.WriteString(label)
+ fieldSourceCode.WriteString(" : ")
+ fieldSourceCode.WriteString(valBuf.String())
+ fieldSourceCode.WriteString(",")
+ }
+
+ if fieldSourceCode.Len() == 0 {
return
}
- switch obj := typ.(type) {
- case *types.Struct:
- fieldCount := obj.NumFields()
- if fieldCount == 0 {
- return
- }
- var fieldSourceCode strings.Builder
- for i := 0; i < fieldCount; i++ {
- field := obj.Field(i)
- // Ignore fields that are not accessible in the current package.
- if field.Pkg() != nil && field.Pkg() != pass.Pkg && !field.Exported() {
- continue
- }
+ fieldSourceCode.WriteString("\n")
- label := field.Name()
- value := analysisinternal.ZeroValue(pass.Fset, file, pass.Pkg, field.Type())
- if value == nil {
- continue
- }
- var valBuf bytes.Buffer
- if err := format.Node(&valBuf, pass.Fset, value); err != nil {
- return
- }
- fieldSourceCode.WriteString("\n")
- fieldSourceCode.WriteString(label)
- fieldSourceCode.WriteString(" : ")
- fieldSourceCode.WriteString(valBuf.String())
- fieldSourceCode.WriteString(",")
- }
+ buf := []byte(fieldSourceCode.String())
- if fieldSourceCode.Len() == 0 {
- return
- }
-
- fieldSourceCode.WriteString("\n")
-
- buf := []byte(fieldSourceCode.String())
-
- pass.Report(analysis.Diagnostic{
- Pos: expr.Lbrace,
- End: expr.Rbrace,
- SuggestedFixes: []analysis.SuggestedFix{{
- Message: "Fill struct with empty values",
- TextEdits: []analysis.TextEdit{{
- Pos: expr.Lbrace + 1,
- End: expr.Rbrace,
- NewText: buf,
- }},
- }},
- })
+ msg := "Fill struct with default values"
+ if name, ok := expr.Type.(*ast.Ident); ok {
+ msg = fmt.Sprintf("Fill %s with default values", name)
}
+ pass.Report(analysis.Diagnostic{
+ Pos: expr.Lbrace,
+ End: expr.Rbrace,
+ SuggestedFixes: []analysis.SuggestedFix{{
+ Message: msg,
+ TextEdits: []analysis.TextEdit{{
+ Pos: expr.Lbrace + 1,
+ End: expr.Rbrace,
+ NewText: buf,
+ }},
+ }},
+ })
})
return nil, nil
}
diff --git a/internal/lsp/cmd/test/refactor_rewrite.go b/internal/lsp/cmd/test/refactor_rewrite.go
deleted file mode 100644
index 9d7a1d7..0000000
--- a/internal/lsp/cmd/test/refactor_rewrite.go
+++ /dev/null
@@ -1,9 +0,0 @@
-package cmdtest
-
-import (
- "testing"
-
- "golang.org/x/tools/internal/span"
-)
-
-func (r *runner) RefactorRewrite(t *testing.T, spn span.Span, title string) {}
diff --git a/internal/lsp/cmd/test/suggested_fix.go b/internal/lsp/cmd/test/suggested_fix.go
index 0419e33..f1a478a 100644
--- a/internal/lsp/cmd/test/suggested_fix.go
+++ b/internal/lsp/cmd/test/suggested_fix.go
@@ -16,12 +16,17 @@
uri := spn.URI()
filename := uri.Filename()
args := []string{"fix", "-a", fmt.Sprintf("%s", spn)}
+ for _, kind := range actionKinds {
+ if kind == "refactor.rewrite" {
+ t.Skip("refactor.rewrite is not yet supported on the command line")
+ }
+ }
args = append(args, actionKinds...)
got, _ := r.NormalizeGoplsCmd(t, args...)
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, expected:\n%v\ngot:\n%v", filename, want, got)
+ t.Errorf("suggested fixes failed for %s: %s", filename, tests.Diff(want, got))
}
}
diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go
index 178b9b3..f611fc9 100644
--- a/internal/lsp/code_action.go
+++ b/internal/lsp/code_action.go
@@ -11,10 +11,12 @@
"sort"
"strings"
+ "golang.org/x/tools/go/analysis"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/mod"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
+ "golang.org/x/tools/internal/span"
)
func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) {
@@ -111,13 +113,19 @@
})
}
}
- // Check for context cancellation before processing analysis fixes.
if ctx.Err() != nil {
return nil, ctx.Err()
}
- // Retrieve any necessary analysis fixes or edits.
+ phs, err := snapshot.PackageHandles(ctx, fh)
+ if err != nil {
+ return nil, err
+ }
+ ph, err := source.WidestPackageHandle(phs)
+ if err != nil {
+ return nil, err
+ }
if (wanted[protocol.QuickFix] || wanted[protocol.SourceFixAll]) && len(diagnostics) > 0 {
- analysisQuickFixes, highConfidenceEdits, err := analysisFixes(ctx, snapshot, fh, diagnostics)
+ analysisQuickFixes, highConfidenceEdits, err := analysisFixes(ctx, snapshot, ph, diagnostics)
if err != nil {
return nil, err
}
@@ -143,11 +151,17 @@
})
}
}
- fillActions, err := source.FillStruct(ctx, snapshot, fh, params.Range)
- if err != nil {
- return nil, err
+ if ctx.Err() != nil {
+ return nil, ctx.Err()
}
- codeActions = append(codeActions, fillActions...)
+ // Add any suggestions that do not necessarily fix any diagnostics.
+ if wanted[protocol.RefactorRewrite] {
+ fixes, err := convenienceFixes(ctx, snapshot, ph, uri, params.Range)
+ if err != nil {
+ return nil, err
+ }
+ codeActions = append(codeActions, fixes...)
+ }
default:
// Unsupported file kind for a code action.
return nil, nil
@@ -254,7 +268,7 @@
return results
}
-func analysisFixes(ctx context.Context, snapshot source.Snapshot, fh source.FileHandle, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, []protocol.TextDocumentEdit, error) {
+func analysisFixes(ctx context.Context, snapshot source.Snapshot, ph source.PackageHandle, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, []protocol.TextDocumentEdit, error) {
if len(diagnostics) == 0 {
return nil, nil, nil
}
@@ -262,16 +276,6 @@
var codeActions []protocol.CodeAction
var sourceFixAllEdits []protocol.TextDocumentEdit
- phs, err := snapshot.PackageHandles(ctx, fh)
- if err != nil {
- 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, nil, err
- }
for _, diag := range diagnostics {
srcErr, analyzer, ok := findSourceError(ctx, snapshot, ph.ID(), diag)
if !ok {
@@ -342,6 +346,45 @@
return nil, source.Analyzer{}, false
}
+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 {
+ analyzers = append(analyzers, a.Analyzer)
+ }
+ diagnostics, err := snapshot.Analyze(ctx, ph.ID(), analyzers...)
+ if err != nil {
+ return nil, err
+ }
+ var codeActions []protocol.CodeAction
+ for _, d := range diagnostics {
+ // For now, only show diagnostics for matching lines. Maybe we should
+ // alter this behavior in the future, depending on the user experience.
+ if d.URI != uri {
+ continue
+ }
+ if d.Range.Start.Line != rng.Start.Line {
+ continue
+ }
+ for _, fix := range d.SuggestedFixes {
+ action := protocol.CodeAction{
+ Title: fix.Title,
+ Kind: protocol.RefactorRewrite,
+ Edit: protocol.WorkspaceEdit{},
+ }
+ for uri, edits := range fix.Edits {
+ fh, err := snapshot.GetFile(ctx, uri)
+ if err != nil {
+ return nil, err
+ }
+ docChanges := documentChanges(fh, edits)
+ action.Edit.DocumentChanges = append(action.Edit.DocumentChanges, docChanges...)
+ }
+ codeActions = append(codeActions, action)
+ }
+ }
+ return codeActions, nil
+}
+
func documentChanges(fh source.FileHandle, edits []protocol.TextEdit) []protocol.TextDocumentEdit {
return []protocol.TextDocumentEdit{
{
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index ff31a10..cf0b4c5 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -370,48 +370,6 @@
}
}
-func (r *runner) RefactorRewrite(t *testing.T, spn span.Span, title string) {
- uri := spn.URI()
- m, err := r.data.Mapper(uri)
- if err != nil {
- t.Fatal(err)
- }
- rng, err := m.Range(spn)
- if err != nil {
- t.Fatal(err)
- }
- actions, err := r.server.CodeAction(r.ctx, &protocol.CodeActionParams{
- TextDocument: protocol.TextDocumentIdentifier{
- URI: protocol.URIFromSpanURI(uri),
- },
- Range: rng,
- })
- if err != nil {
- t.Fatal(err)
- }
- for _, action := range actions {
- // There may be more code actions available at spn (Span),
- // we only need the one specified in the title
- if action.Kind != protocol.RefactorRewrite || action.Title != title {
- continue
- }
- res, err := applyWorkspaceEdits(r, action.Edit)
- if err != nil {
- t.Fatal(err)
- }
- for u, got := range res {
- fixed := string(r.data.Golden(tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
- return []byte(got), nil
- }))
- if fixed != got {
- t.Errorf("%s failed for %s, expected:\n%#v\ngot:\n%#v", title, u.Filename(), fixed, got)
- }
- }
- return
- }
- t.Fatalf("expected code action %q, but got none", title)
-}
-
func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) {
uri := spn.URI()
view, err := r.server.session.ViewOf(uri)
@@ -435,19 +393,16 @@
r.diagnostics[key.id.URI] = diags
}
}
- var diag *source.Diagnostic
+ var diagnostics []protocol.Diagnostic
for _, d := range r.diagnostics[uri] {
// Compare the start positions rather than the entire range because
// some diagnostics have a range with the same start and end position (8:1-8:1).
// The current marker functionality prevents us from having a range of 0 length.
if protocol.ComparePosition(d.Range.Start, rng.Start) == 0 {
- diag = d
+ diagnostics = append(diagnostics, toProtocolDiagnostics([]*source.Diagnostic{d})...)
break
}
}
- if diag == nil {
- t.Fatalf("could not get any suggested fixes for %v", spn)
- }
codeActionKinds := []protocol.CodeActionKind{}
for _, k := range actionKinds {
codeActionKinds = append(codeActionKinds, protocol.CodeActionKind(k))
@@ -456,28 +411,30 @@
TextDocument: protocol.TextDocumentIdentifier{
URI: protocol.URIFromSpanURI(uri),
},
+ Range: rng,
Context: protocol.CodeActionContext{
Only: codeActionKinds,
- Diagnostics: toProtocolDiagnostics([]*source.Diagnostic{diag}),
+ Diagnostics: diagnostics,
},
})
if err != nil {
t.Fatal(err)
}
- // TODO: This test should probably be able to handle multiple code actions.
- if len(actions) == 0 {
- t.Fatal("no code actions returned")
+ // 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", len(actions))
}
res, err := applyWorkspaceEdits(r, actions[0].Edit)
if err != nil {
t.Fatal(err)
}
for u, got := range res {
- fixed := string(r.data.Golden("suggestedfix_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
+ want := string(r.data.Golden("suggestedfix_"+tests.SpanName(spn), u.Filename(), func() ([]byte, error) {
return []byte(got), nil
}))
- if fixed != got {
- t.Errorf("suggested fixes failed for %s, expected:\n%#v\ngot:\n%#v", u.Filename(), fixed, got)
+ if want != got {
+ t.Errorf("suggested fixes failed for %s: %s", u.Filename(), tests.Diff(want, got))
}
}
}
diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go
index c29f2e0..9fe25d5 100644
--- a/internal/lsp/source/diagnostics.go
+++ b/internal/lsp/source/diagnostics.go
@@ -130,10 +130,7 @@
return nil, warn, ctx.Err()
}
// If we don't have any list or parse errors, run analyses.
- analyzers := snapshot.View().Options().DefaultAnalyzers
- if hadTypeErrors {
- analyzers = snapshot.View().Options().TypeErrorAnalyzers
- }
+ analyzers := pickAnalyzers(snapshot, hadTypeErrors)
if err := analyses(ctx, snapshot, reports, ph, analyzers); err != nil {
event.Error(ctx, "analyses failed", err, tag.Snapshot.Of(snapshot.ID()), tag.Package.Of(ph.ID()))
if ctx.Err() != nil {
@@ -161,6 +158,26 @@
return false
}
+func pickAnalyzers(snapshot Snapshot, hadTypeErrors bool) map[string]Analyzer {
+ analyzers := make(map[string]Analyzer)
+
+ // Always run convenience analyzers.
+ for k, v := range snapshot.View().Options().ConvenienceAnalyzers {
+ analyzers[k] = v
+ }
+ // If we had type errors, only run type error analyzers.
+ if hadTypeErrors {
+ for k, v := range snapshot.View().Options().TypeErrorAnalyzers {
+ analyzers[k] = v
+ }
+ return analyzers
+ }
+ for k, v := range snapshot.View().Options().DefaultAnalyzers {
+ analyzers[k] = v
+ }
+ return analyzers
+}
+
func FileDiagnostics(ctx context.Context, snapshot Snapshot, uri span.URI) (FileIdentity, []*Diagnostic, error) {
fh, err := snapshot.GetFile(ctx, uri)
if err != nil {
@@ -310,6 +327,13 @@
// Report diagnostics and errors from root analyzers.
for _, e := range analysisErrors {
+ // If the diagnostic comes from a "convenience" analyzer, it is not
+ // meant to provide diagnostics, but rather only suggested fixes.
+ // Skip these types of errors in diagnostics; we will use their
+ // suggested fixes when providing code actions.
+ if isConvenienceAnalyzer(e.Category) {
+ continue
+ }
// This is a bit of a hack, but clients > 3.15 will be able to grey out unnecessary code.
// If we are deleting code as part of all of our suggested fixes, assume that this is dead code.
// TODO(golang/go#34508): Return these codes from the diagnostics themselves.
@@ -397,3 +421,12 @@
}
return false
}
+
+func isConvenienceAnalyzer(category string) bool {
+ for _, a := range DefaultOptions().ConvenienceAnalyzers {
+ if category == a.Analyzer.Name {
+ return true
+ }
+ }
+ return false
+}
diff --git a/internal/lsp/source/fill_struct.go b/internal/lsp/source/fill_struct.go
deleted file mode 100644
index f7fa001..0000000
--- a/internal/lsp/source/fill_struct.go
+++ /dev/null
@@ -1,143 +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 source
-
-import (
- "context"
- "fmt"
- "go/format"
- "go/types"
- "strings"
-
- "golang.org/x/tools/go/ast/astutil"
- "golang.org/x/tools/internal/lsp/protocol"
-)
-
-// FillStruct completes all of targeted struct's fields with their default values.
-func FillStruct(ctx context.Context, snapshot Snapshot, fh FileHandle, protoRng protocol.Range) ([]protocol.CodeAction, error) {
- pkg, pgh, err := getParsedFile(ctx, snapshot, fh, NarrowestPackageHandle)
- if err != nil {
- return nil, fmt.Errorf("getting file for struct fill code action: %v", err)
- }
- file, src, m, _, err := pgh.Cached()
- if err != nil {
- return nil, err
- }
- spn, err := m.PointSpan(protoRng.Start)
- if err != nil {
- return nil, err
- }
- spanRng, err := spn.Range(m.Converter)
- if err != nil {
- return nil, err
- }
- path, _ := astutil.PathEnclosingInterval(file, spanRng.Start, spanRng.End)
- if path == nil {
- return nil, nil
- }
-
- ecl := enclosingCompositeLiteral(path, spanRng.Start, pkg.GetTypesInfo())
- if ecl == nil || !ecl.isStruct() {
- return nil, nil
- }
-
- // If in F{ Bar<> : V} or anywhere in F{Bar : V, ...}
- // we should not fill the struct.
- if ecl.inKey || len(ecl.cl.Elts) != 0 {
- return nil, nil
- }
-
- var codeActions []protocol.CodeAction
- qfFunc := qualifier(file, pkg.GetTypes(), pkg.GetTypesInfo())
- switch obj := ecl.clType.(type) {
- case *types.Struct:
- fieldCount := obj.NumFields()
- if fieldCount == 0 {
- return nil, nil
- }
- var fieldSourceCode strings.Builder
- for i := 0; i < fieldCount; i++ {
- field := obj.Field(i)
- // Ignore fields that are not accessible in the current package.
- if field.Pkg() != nil && field.Pkg() != pkg.GetTypes() && !field.Exported() {
- continue
- }
-
- label := field.Name()
- value := formatZeroValue(field.Type(), qfFunc)
- fieldSourceCode.WriteString("\n")
- fieldSourceCode.WriteString(label)
- fieldSourceCode.WriteString(" : ")
- fieldSourceCode.WriteString(value)
- fieldSourceCode.WriteString(",")
- }
-
- if fieldSourceCode.Len() == 0 {
- return nil, nil
- }
-
- fieldSourceCode.WriteString("\n")
-
- // the range of all text between '<>', inclusive. E.g. {<> ... <}>
- mappedRange := newMappedRange(snapshot.View().Session().Cache().FileSet(), m, ecl.cl.Lbrace, ecl.cl.Rbrace+1)
- protoRange, err := mappedRange.Range()
- if err != nil {
- return nil, err
- }
- // consider formatting from the first character of the line the lbrace is on.
- // ToOffset is 1-based
- beginOffset, err := m.Converter.ToOffset(int(protoRange.Start.Line)+1, 1)
- if err != nil {
- return nil, err
- }
-
- endOffset, err := m.Converter.ToOffset(int(protoRange.Start.Line)+1, int(protoRange.Start.Character)+1)
- if err != nil {
- return nil, err
- }
-
- // An increment to make sure the lbrace is included in the slice.
- endOffset++
- // Append the edits. Then append the closing brace.
- var newSourceCode strings.Builder
- newSourceCode.Grow(endOffset - beginOffset + fieldSourceCode.Len() + 1)
- newSourceCode.WriteString(string(src[beginOffset:endOffset]))
- newSourceCode.WriteString(fieldSourceCode.String())
- newSourceCode.WriteString("}")
-
- buf, err := format.Source([]byte(newSourceCode.String()))
- if err != nil {
- return nil, err
- }
-
- // it is guaranteed that a left brace exists.
- var edit = string(buf[strings.IndexByte(string(buf), '{'):])
-
- codeActions = append(codeActions, protocol.CodeAction{
- Title: "Fill struct",
- Kind: protocol.RefactorRewrite,
- Edit: protocol.WorkspaceEdit{
- DocumentChanges: []protocol.TextDocumentEdit{
- {
- TextDocument: protocol.VersionedTextDocumentIdentifier{
- Version: fh.Version(),
- TextDocumentIdentifier: protocol.TextDocumentIdentifier{
- URI: protocol.URIFromSpanURI(fh.URI()),
- },
- },
- Edits: []protocol.TextEdit{
- {
- Range: protoRange,
- NewText: edit,
- },
- },
- },
- },
- },
- })
- }
-
- return codeActions, nil
-}
diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go
index 13aec4c..92c09e8 100644
--- a/internal/lsp/source/options.go
+++ b/internal/lsp/source/options.go
@@ -664,7 +664,7 @@
unsafeptr.Analyzer.Name: {Analyzer: unsafeptr.Analyzer, enabled: true},
unusedresult.Analyzer.Name: {Analyzer: unusedresult.Analyzer, enabled: true},
- // Non-vet analyzers
+ // Non-vet analyzers:
deepequalerrors.Analyzer.Name: {Analyzer: deepequalerrors.Analyzer, enabled: true},
sortslice.Analyzer.Name: {Analyzer: sortslice.Analyzer, enabled: true},
testinggoroutine.Analyzer.Name: {Analyzer: testinggoroutine.Analyzer, enabled: true},
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index 93b4114..84c6ab0 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -475,7 +475,6 @@
}
func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) {}
-func (r *runner) RefactorRewrite(t *testing.T, spn span.Span, title string) {}
func (r *runner) Definition(t *testing.T, spn span.Span, d tests.Definition) {
_, srcRng, err := spanToRange(r.data, d.Src)
diff --git a/internal/lsp/testdata/indirect/summary.txt.golden b/internal/lsp/testdata/indirect/summary.txt.golden
index fdd7593..5c4f74a 100644
--- a/internal/lsp/testdata/indirect/summary.txt.golden
+++ b/internal/lsp/testdata/indirect/summary.txt.golden
@@ -25,5 +25,4 @@
SignaturesCount = 0
LinksCount = 0
ImplementationsCount = 0
-RefactorRewriteCount = 0
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go
index 9bc5c45..7fd1597 100644
--- a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go
@@ -1,23 +1,23 @@
-package fillstruct
-
-type StructA struct {
- unexportedIntField int
- ExportedIntField int
- MapA map[int]string
- Array []int
- StructB
-}
-
-type StructA2 struct {
- B *StructB
-}
-
-type StructA3 struct {
- B StructB
-}
-
-func fill() {
- a := StructA{} //@refactorrewrite("}", "Fill struct")
- b := StructA2{} //@refactorrewrite("}", "Fill struct")
- c := StructA3{} //@refactorrewrite("}", "Fill struct")
-}
+package fillstruct
+
+type StructA struct {
+ unexportedIntField int
+ ExportedIntField int
+ MapA map[int]string
+ Array []int
+ StructB
+}
+
+type StructA2 struct {
+ B *StructB
+}
+
+type StructA3 struct {
+ B StructB
+}
+
+func fill() {
+ a := StructA{} //@suggestedfix("}", "refactor.rewrite")
+ b := StructA2{} //@suggestedfix("}", "refactor.rewrite")
+ c := StructA3{} //@suggestedfix("}", "refactor.rewrite")
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go.golden b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go.golden
index e29f040..13c0a0e 100644
--- a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct.go.golden
@@ -1,85 +1,85 @@
--- fill_struct_20_15 --
-package fillstruct
-
-type StructA struct {
- unexportedIntField int
- ExportedIntField int
- MapA map[int]string
- Array []int
- StructB
-}
-
-type StructA2 struct {
- B *StructB
-}
-
-type StructA3 struct {
- B StructB
-}
-
-func fill() {
+-- suggestedfix_fill_struct_20_15 --
+package fillstruct
+
+type StructA struct {
+ unexportedIntField int
+ ExportedIntField int
+ MapA map[int]string
+ Array []int
+ StructB
+}
+
+type StructA2 struct {
+ B *StructB
+}
+
+type StructA3 struct {
+ B StructB
+}
+
+func fill() {
a := StructA{
- unexportedIntField: 0,
- ExportedIntField: 0,
- MapA: nil,
- Array: nil,
- StructB: StructB{},
- } //@refactorrewrite("}", "Fill struct")
- b := StructA2{} //@refactorrewrite("}", "Fill struct")
- c := StructA3{} //@refactorrewrite("}", "Fill struct")
-}
+unexportedIntField : 0,
+ExportedIntField : 0,
+MapA : nil,
+Array : nil,
+StructB : StructB{},
+} //@suggestedfix("}", "refactor.rewrite")
+ b := StructA2{} //@suggestedfix("}", "refactor.rewrite")
+ c := StructA3{} //@suggestedfix("}", "refactor.rewrite")
+}
--- fill_struct_21_16 --
-package fillstruct
-
-type StructA struct {
- unexportedIntField int
- ExportedIntField int
- MapA map[int]string
- Array []int
- StructB
-}
-
-type StructA2 struct {
- B *StructB
-}
-
-type StructA3 struct {
- B StructB
-}
-
-func fill() {
- a := StructA{} //@refactorrewrite("}", "Fill struct")
+-- suggestedfix_fill_struct_21_16 --
+package fillstruct
+
+type StructA struct {
+ unexportedIntField int
+ ExportedIntField int
+ MapA map[int]string
+ Array []int
+ StructB
+}
+
+type StructA2 struct {
+ B *StructB
+}
+
+type StructA3 struct {
+ B StructB
+}
+
+func fill() {
+ a := StructA{} //@suggestedfix("}", "refactor.rewrite")
b := StructA2{
- B: nil,
- } //@refactorrewrite("}", "Fill struct")
- c := StructA3{} //@refactorrewrite("}", "Fill struct")
-}
+B : nil,
+} //@suggestedfix("}", "refactor.rewrite")
+ c := StructA3{} //@suggestedfix("}", "refactor.rewrite")
+}
--- fill_struct_22_16 --
-package fillstruct
-
-type StructA struct {
- unexportedIntField int
- ExportedIntField int
- MapA map[int]string
- Array []int
- StructB
-}
-
-type StructA2 struct {
- B *StructB
-}
-
-type StructA3 struct {
- B StructB
-}
-
-func fill() {
- a := StructA{} //@refactorrewrite("}", "Fill struct")
- b := StructA2{} //@refactorrewrite("}", "Fill struct")
+-- suggestedfix_fill_struct_22_16 --
+package fillstruct
+
+type StructA struct {
+ unexportedIntField int
+ ExportedIntField int
+ MapA map[int]string
+ Array []int
+ StructB
+}
+
+type StructA2 struct {
+ B *StructB
+}
+
+type StructA3 struct {
+ B StructB
+}
+
+func fill() {
+ a := StructA{} //@suggestedfix("}", "refactor.rewrite")
+ b := StructA2{} //@suggestedfix("}", "refactor.rewrite")
c := StructA3{
- B: StructB{},
- } //@refactorrewrite("}", "Fill struct")
-}
+B : StructB{},
+} //@suggestedfix("}", "refactor.rewrite")
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_nested.go b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_nested.go
index 0c4c1d8..79eb84b 100644
--- a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_nested.go
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_nested.go
@@ -1,15 +1,15 @@
-package fillstruct
-
-type StructB struct {
- StructC
-}
-
-type StructC struct {
- unexportedInt int
-}
-
-func nested() {
- c := StructB{
- StructC: StructC{}, //@refactorrewrite("}", "Fill struct")
- }
-}
+package fillstruct
+
+type StructB struct {
+ StructC
+}
+
+type StructC struct {
+ unexportedInt int
+}
+
+func nested() {
+ c := StructB{
+ StructC: StructC{}, //@suggestedfix("}", "refactor.rewrite")
+ }
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_nested.go.golden b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_nested.go.golden
index 74c1e6a..43a7b5b 100644
--- a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_nested.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_nested.go.golden
@@ -1,19 +1,19 @@
--- fill_struct_nested_13_20 --
-package fillstruct
-
-type StructB struct {
- StructC
-}
-
-type StructC struct {
- unexportedInt int
-}
-
-func nested() {
- c := StructB{
+-- suggestedfix_fill_struct_nested_13_20 --
+package fillstruct
+
+type StructB struct {
+ StructC
+}
+
+type StructC struct {
+ unexportedInt int
+}
+
+func nested() {
+ c := StructB{
StructC: StructC{
- unexportedInt: 0,
- }, //@refactorrewrite("}", "Fill struct")
- }
-}
+unexportedInt : 0,
+}, //@suggestedfix("}", "refactor.rewrite")
+ }
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_package.go b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_package.go
index e7b30e5..336a375 100644
--- a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_package.go
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_package.go
@@ -1,9 +1,9 @@
-package fillstruct
-
-import (
- data "golang.org/x/tools/internal/lsp/fillstruct/data"
-)
-
-func unexported() {
- a := data.A{} //@refactorrewrite("}", "Fill struct")
-}
+package fillstruct
+
+import (
+ "golang.org/x/tools/internal/lsp/fillstruct/data"
+)
+
+func unexported() {
+ a := data.A{} //@suggestedfix("}", "refactor.rewrite")
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_package.go.golden b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_package.go.golden
index df33786..f11a5af 100644
--- a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_package.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_package.go.golden
@@ -1,13 +1,13 @@
--- fill_struct_package_8_14 --
-package fillstruct
-
-import (
- data "golang.org/x/tools/internal/lsp/fillstruct/data"
-)
-
-func unexported() {
+-- suggestedfix_fill_struct_package_8_14 --
+package fillstruct
+
+import (
+ "golang.org/x/tools/internal/lsp/fillstruct/data"
+)
+
+func unexported() {
a := data.A{
- ExportedInt: 0,
- } //@refactorrewrite("}", "Fill struct")
-}
+ExportedInt : 0,
+} //@suggestedfix("}", "refactor.rewrite")
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_spaces.go b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_spaces.go
new file mode 100644
index 0000000..d5d1bbb
--- /dev/null
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_spaces.go
@@ -0,0 +1,9 @@
+package fillstruct
+
+type StructD struct {
+ ExportedIntField int
+}
+
+func spaces() {
+ d := StructD{} //@suggestedfix("}", "refactor.rewrite")
+}
diff --git a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_spaces.go.golden b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_spaces.go.golden
index c938b50..33643a6 100644
--- a/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_spaces.go.golden
+++ b/internal/lsp/testdata/lsp/primarymod/fillstruct/fill_struct_spaces.go.golden
@@ -1,13 +1,13 @@
--- fill_struct_spaces_10_1 --
-package fillstruct
-
-type StructB struct {
- ExportedIntField int
-}
-
-func spaces() {
- b := StructB{
- ExportedIntField: 0,
- }
-}
+-- suggestedfix_fill_struct_spaces_8_15 --
+package fillstruct
+
+type StructD struct {
+ ExportedIntField int
+}
+
+func spaces() {
+ d := StructD{
+ExportedIntField : 0,
+} //@suggestedfix("}", "refactor.rewrite")
+}
diff --git a/internal/lsp/testdata/lsp/summary.txt.golden b/internal/lsp/testdata/lsp/summary.txt.golden
index 26df609..74536e9 100644
--- a/internal/lsp/testdata/lsp/summary.txt.golden
+++ b/internal/lsp/testdata/lsp/summary.txt.golden
@@ -11,7 +11,7 @@
FoldingRangesCount = 2
FormatCount = 6
ImportCount = 8
-SuggestedFixCount = 6
+SuggestedFixCount = 12
DefinitionsCount = 53
TypeDefinitionsCount = 2
HighlightsCount = 60
@@ -25,5 +25,4 @@
SignaturesCount = 32
LinksCount = 8
ImplementationsCount = 14
-RefactorRewriteCount = 5
diff --git a/internal/lsp/testdata/missingdep/summary.txt.golden b/internal/lsp/testdata/missingdep/summary.txt.golden
index fdd7593..5c4f74a 100644
--- a/internal/lsp/testdata/missingdep/summary.txt.golden
+++ b/internal/lsp/testdata/missingdep/summary.txt.golden
@@ -25,5 +25,4 @@
SignaturesCount = 0
LinksCount = 0
ImplementationsCount = 0
-RefactorRewriteCount = 0
diff --git a/internal/lsp/testdata/missingtwodep/summary.txt.golden b/internal/lsp/testdata/missingtwodep/summary.txt.golden
index a061dc6..96ac475 100644
--- a/internal/lsp/testdata/missingtwodep/summary.txt.golden
+++ b/internal/lsp/testdata/missingtwodep/summary.txt.golden
@@ -25,5 +25,4 @@
SignaturesCount = 0
LinksCount = 0
ImplementationsCount = 0
-RefactorRewriteCount = 0
diff --git a/internal/lsp/testdata/unused/summary.txt.golden b/internal/lsp/testdata/unused/summary.txt.golden
index fdd7593..5c4f74a 100644
--- a/internal/lsp/testdata/unused/summary.txt.golden
+++ b/internal/lsp/testdata/unused/summary.txt.golden
@@ -25,5 +25,4 @@
SignaturesCount = 0
LinksCount = 0
ImplementationsCount = 0
-RefactorRewriteCount = 0
diff --git a/internal/lsp/testdata/upgradedep/summary.txt.golden b/internal/lsp/testdata/upgradedep/summary.txt.golden
index 90c3730..79042cc 100644
--- a/internal/lsp/testdata/upgradedep/summary.txt.golden
+++ b/internal/lsp/testdata/upgradedep/summary.txt.golden
@@ -25,5 +25,4 @@
SignaturesCount = 0
LinksCount = 4
ImplementationsCount = 0
-RefactorRewriteCount = 0
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 2d7eeab..850ec22 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -57,7 +57,6 @@
type Formats []span.Span
type Imports []span.Span
type SuggestedFixes map[span.Span][]string
-type RefactorRewriteActions map[span.Span]string
type Definitions map[span.Span]Definition
type Implementations map[span.Span][]span.Span
type Highlights map[span.Span][]span.Span
@@ -88,7 +87,6 @@
Formats Formats
Imports Imports
SuggestedFixes SuggestedFixes
- RefactorRewrite RefactorRewriteActions
Definitions Definitions
Implementations Implementations
Highlights Highlights
@@ -142,7 +140,6 @@
CaseSensitiveWorkspaceSymbols(*testing.T, string, []protocol.SymbolInformation, map[string]struct{})
SignatureHelp(*testing.T, span.Span, *protocol.SignatureHelp)
Link(*testing.T, span.URI, []Link)
- RefactorRewrite(*testing.T, span.Span, string)
}
type Definition struct {
@@ -219,6 +216,8 @@
source.Go: {
protocol.SourceOrganizeImports: true,
protocol.QuickFix: true,
+ protocol.RefactorRewrite: true,
+ protocol.SourceFixAll: true,
},
source.Mod: {
protocol.SourceOrganizeImports: true,
@@ -296,7 +295,6 @@
CaseSensitiveWorkspaceSymbols: make(WorkspaceSymbols),
Signatures: make(Signatures),
Links: make(Links),
- RefactorRewrite: make(RefactorRewriteActions),
t: t,
dir: folder,
@@ -421,7 +419,6 @@
"signature": datum.collectSignatures,
"link": datum.collectLinks,
"suggestedfix": datum.collectSuggestedFixes,
- "refactorrewrite": datum.collectRefactorRewrite,
}); err != nil {
t.Fatal(err)
}
@@ -599,17 +596,6 @@
}
})
- t.Run("RefactorRewrite", func(t *testing.T) {
- t.Helper()
-
- for spn, title := range data.RefactorRewrite {
- t.Run(SpanName(spn), func(t *testing.T) {
- t.Helper()
- tests.RefactorRewrite(t, spn, title)
- })
- }
- })
-
t.Run("SuggestedFix", func(t *testing.T) {
t.Helper()
for spn, actionKinds := range data.SuggestedFixes {
@@ -827,7 +813,6 @@
fmt.Fprintf(buf, "SignaturesCount = %v\n", len(data.Signatures))
fmt.Fprintf(buf, "LinksCount = %v\n", linksCount)
fmt.Fprintf(buf, "ImplementationsCount = %v\n", len(data.Implementations))
- fmt.Fprintf(buf, "RefactorRewriteCount = %v\n", len(data.RefactorRewrite))
want := string(data.Golden("summary", summaryFile, func() ([]byte, error) {
return buf.Bytes(), nil
@@ -1037,10 +1022,6 @@
data.SuggestedFixes[spn] = append(data.SuggestedFixes[spn], actionKind)
}
-func (data *Data) collectRefactorRewrite(spn span.Span, title string) {
- data.RefactorRewrite[spn] = title
-}
-
func (data *Data) collectDefinitions(src, target span.Span) {
data.Definitions[src] = Definition{
Src: src,
