diff --git a/internal/lsp/analysis/fillstruct/fillstruct.go b/internal/lsp/analysis/fillstruct/fillstruct.go index b23a05f9cf..6ec8a35a49 100644 --- a/internal/lsp/analysis/fillstruct/fillstruct.go +++ b/internal/lsp/analysis/fillstruct/fillstruct.go @@ -8,6 +8,7 @@ package fillstruct import ( "bytes" + "fmt" "go/ast" "go/format" "go/types" @@ -53,12 +54,10 @@ func run(pass *analysis.Pass) (interface{}, error) { 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 @@ func run(pass *analysis.Pass) (interface{}, error) { } 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 9d7a1d734d..0000000000 --- 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 0419e33acd..f1a478a30c 100644 --- a/internal/lsp/cmd/test/suggested_fix.go +++ b/internal/lsp/cmd/test/suggested_fix.go @@ -16,12 +16,17 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) 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 178b9b39fc..f611fc9022 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -11,10 +11,12 @@ import ( "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 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara }) } } - // 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 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara }) } } - fillActions, err := source.FillStruct(ctx, snapshot, fh, params.Range) - if err != nil { - return nil, err + if ctx.Err() != nil { + return nil, ctx.Err() + } + // 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...) } - codeActions = append(codeActions, fillActions...) default: // Unsupported file kind for a code action. return nil, nil @@ -254,7 +268,7 @@ func importDiagnostics(fix *imports.ImportFix, diagnostics []protocol.Diagnostic 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 @@ func analysisFixes(ctx context.Context, snapshot source.Snapshot, fh source.File 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 @@ func findSourceError(ctx context.Context, snapshot source.Snapshot, pkgID string 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 ff31a10ec8..cf0b4c5242 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -370,48 +370,6 @@ func (r *runner) Import(t *testing.T, spn span.Span) { } } -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 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) 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 @@ func (r *runner) SuggestedFix(t *testing.T, spn span.Span, actionKinds []string) 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 c29f2e07d3..9fe25d5eca 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -130,10 +130,7 @@ func Diagnostics(ctx context.Context, snapshot Snapshot, ph PackageHandle, missi 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 @@ func ignoreFile(path string) bool { 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 @@ func analyses(ctx context.Context, snapshot Snapshot, reports map[FileIdentity][ // 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 @@ func hasUndeclaredErrors(pkg Package) bool { } 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 f7fa00108f..0000000000 --- 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 13aec4ccd2..92c09e8343 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -664,7 +664,7 @@ func defaultAnalyzers() map[string]Analyzer { 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 93b4114ef4..84c6ab0403 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -475,7 +475,6 @@ func (r *runner) Import(t *testing.T, spn span.Span) { } 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 fdd759352e..5c4f74a660 100644 --- a/internal/lsp/testdata/indirect/summary.txt.golden +++ b/internal/lsp/testdata/indirect/summary.txt.golden @@ -25,5 +25,4 @@ CaseSensitiveWorkspaceSymbolsCount = 0 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 9bc5c4588e..7fd1597c90 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 e29f04053a..13c0a0e84a 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 0c4c1d88c2..79eb84b747 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 74c1e6a624..43a7b5b19c 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{ - StructC: StructC{ - unexportedInt: 0, - }, //@refactorrewrite("}", "Fill struct") - } -} +-- 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, +}, //@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 e7b30e53aa..336a375180 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 df33786e6c..f11a5afe52 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() { - a := data.A{ - ExportedInt: 0, - } //@refactorrewrite("}", "Fill struct") -} +-- suggestedfix_fill_struct_package_8_14 -- +package fillstruct + +import ( + "golang.org/x/tools/internal/lsp/fillstruct/data" +) + +func unexported() { + a := data.A{ +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 0000000000..d5d1bbba5c --- /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 c938b501ae..33643a6f43 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 26df609c26..74536e9276 100644 --- a/internal/lsp/testdata/lsp/summary.txt.golden +++ b/internal/lsp/testdata/lsp/summary.txt.golden @@ -11,7 +11,7 @@ DiagnosticsCount = 44 FoldingRangesCount = 2 FormatCount = 6 ImportCount = 8 -SuggestedFixCount = 6 +SuggestedFixCount = 12 DefinitionsCount = 53 TypeDefinitionsCount = 2 HighlightsCount = 60 @@ -25,5 +25,4 @@ CaseSensitiveWorkspaceSymbolsCount = 2 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 fdd759352e..5c4f74a660 100644 --- a/internal/lsp/testdata/missingdep/summary.txt.golden +++ b/internal/lsp/testdata/missingdep/summary.txt.golden @@ -25,5 +25,4 @@ CaseSensitiveWorkspaceSymbolsCount = 0 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 a061dc661a..96ac4750a8 100644 --- a/internal/lsp/testdata/missingtwodep/summary.txt.golden +++ b/internal/lsp/testdata/missingtwodep/summary.txt.golden @@ -25,5 +25,4 @@ CaseSensitiveWorkspaceSymbolsCount = 0 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 fdd759352e..5c4f74a660 100644 --- a/internal/lsp/testdata/unused/summary.txt.golden +++ b/internal/lsp/testdata/unused/summary.txt.golden @@ -25,5 +25,4 @@ CaseSensitiveWorkspaceSymbolsCount = 0 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 90c373043f..79042cc6c8 100644 --- a/internal/lsp/testdata/upgradedep/summary.txt.golden +++ b/internal/lsp/testdata/upgradedep/summary.txt.golden @@ -25,5 +25,4 @@ CaseSensitiveWorkspaceSymbolsCount = 0 SignaturesCount = 0 LinksCount = 4 ImplementationsCount = 0 -RefactorRewriteCount = 0 diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 2d7eeab458..850ec229b3 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -57,7 +57,6 @@ type FoldingRanges []span.Span 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 @@ type Data struct { Formats Formats Imports Imports SuggestedFixes SuggestedFixes - RefactorRewrite RefactorRewriteActions Definitions Definitions Implementations Implementations Highlights Highlights @@ -142,7 +140,6 @@ type Tests interface { 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 @@ func DefaultOptions() source.Options { source.Go: { protocol.SourceOrganizeImports: true, protocol.QuickFix: true, + protocol.RefactorRewrite: true, + protocol.SourceFixAll: true, }, source.Mod: { protocol.SourceOrganizeImports: true, @@ -296,7 +295,6 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) []*Data { CaseSensitiveWorkspaceSymbols: make(WorkspaceSymbols), Signatures: make(Signatures), Links: make(Links), - RefactorRewrite: make(RefactorRewriteActions), t: t, dir: folder, @@ -421,7 +419,6 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) []*Data { "signature": datum.collectSignatures, "link": datum.collectLinks, "suggestedfix": datum.collectSuggestedFixes, - "refactorrewrite": datum.collectRefactorRewrite, }); err != nil { t.Fatal(err) } @@ -599,17 +596,6 @@ func Run(t *testing.T, tests Tests, data *Data) { } }) - 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 @@ func checkData(t *testing.T, data *Data) { 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 @@ func (data *Data) collectSuggestedFixes(spn span.Span, actionKind string) { 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,