diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 05329ee6..6c358ac0 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -10,6 +10,7 @@ import ( "fmt" "go/token" "os/exec" + "path/filepath" "sort" "strings" "testing" @@ -540,35 +541,41 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) { continue } - _, m, err := getSourceFile(ctx, r.server.session.ViewOf(uri), uri) - if err != nil { - t.Error(err) + var res []string + for uri, edits := range *workspaceEdits.Changes { + spnURI := span.URI(uri) + _, m, err := getSourceFile(ctx, r.server.session.ViewOf(span.URI(spnURI)), spnURI) + if err != nil { + t.Error(err) + } + + sedits, err := FromProtocolEdits(m, edits) + if err != nil { + t.Error(err) + } + + filename := filepath.Base(m.URI.Filename()) + contents := applyEdits(string(m.Content), sedits) + res = append(res, fmt.Sprintf("%s:\n%s", filename, contents)) } - changes := *workspaceEdits.Changes - if len(changes) != 1 { // Renames must only affect a single file in these tests. - t.Errorf("rename failed for %s, edited %d files, wanted 1 file", newText, len(*workspaceEdits.Changes)) - continue + // Sort on filename + sort.Strings(res) + + var got string + for i, val := range res { + if i != 0 { + got += "\n" + } + got += val } - edits := changes[string(uri)] - if edits == nil { - t.Errorf("rename failed for %s, did not edit %s", newText, filename) - continue - } - sedits, err := FromProtocolEdits(m, edits) - if err != nil { - t.Error(err) - } - - got := applyEdits(string(m.Content), sedits) - - gorenamed := string(r.data.Golden(tag, filename, func() ([]byte, error) { + renamed := string(r.data.Golden(tag, filename, func() ([]byte, error) { return []byte(got), nil })) - if gorenamed != got { - t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, gorenamed, got) + if renamed != got { + t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, renamed, got) } } } diff --git a/internal/lsp/source/references.go b/internal/lsp/source/references.go index 5cbb4429..8fd0df36 100644 --- a/internal/lsp/source/references.go +++ b/internal/lsp/source/references.go @@ -20,6 +20,7 @@ type ReferenceInfo struct { Range span.Range ident *ast.Ident obj types.Object + pkg Package isDeclaration bool } @@ -34,17 +35,6 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro if i.decl.obj == nil { return nil, fmt.Errorf("no references for an import spec") } - if i.decl.wasImplicit { - // The definition is implicit, so we must add it separately. - // This occurs when the variable is declared in a type switch statement - // or is an implicit package name. Both implicits are local to a file. - references = append(references, &ReferenceInfo{ - Name: i.decl.obj.Name(), - Range: i.decl.rng, - obj: i.decl.obj, - isDeclaration: true, - }) - } pkgs := i.File.GetPackages(ctx) for _, pkg := range pkgs { @@ -55,6 +45,19 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro if info == nil { return nil, fmt.Errorf("package %s has no types info", pkg.PkgPath()) } + + if i.decl.wasImplicit { + // The definition is implicit, so we must add it separately. + // This occurs when the variable is declared in a type switch statement + // or is an implicit package name. Both implicits are local to a file. + references = append(references, &ReferenceInfo{ + Name: i.decl.obj.Name(), + Range: i.decl.rng, + obj: i.decl.obj, + pkg: pkg, + isDeclaration: true, + }) + } for ident, obj := range info.Defs { if obj == nil || !sameObj(obj, i.decl.obj) { continue @@ -65,6 +68,7 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro Range: span.NewRange(i.File.FileSet(), ident.Pos(), ident.End()), ident: ident, obj: obj, + pkg: pkg, isDeclaration: true, }}, references...) } @@ -76,6 +80,7 @@ func (i *IdentifierInfo) References(ctx context.Context) ([]*ReferenceInfo, erro Name: ident.Name, Range: span.NewRange(i.File.FileSet(), ident.Pos(), ident.End()), ident: ident, + pkg: pkg, obj: obj, }) } diff --git a/internal/lsp/source/rename.go b/internal/lsp/source/rename.go index 0b72a1d3..eb5f8ba8 100644 --- a/internal/lsp/source/rename.go +++ b/internal/lsp/source/rename.go @@ -73,7 +73,9 @@ func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.U to: newName, packages: make(map[*types.Package]Package), } - r.packages[i.pkg.GetTypes()] = i.pkg + for _, from := range refs { + r.packages[from.pkg.GetTypes()] = from.pkg + } // Check that the renaming of the identifier is ok. for _, from := range refs { @@ -89,6 +91,7 @@ func (i *IdentifierInfo) Rename(ctx context.Context, newName string) (map[span.U // Rename all references to the identifier. func (r *renamer) update() (map[span.URI][]TextEdit, error) { result := make(map[span.URI][]TextEdit) + seen := make(map[span.Span]bool) docRegexp, err := regexp.Compile(`\b` + r.from + `\b`) if err != nil { @@ -99,6 +102,10 @@ func (r *renamer) update() (map[span.URI][]TextEdit, error) { if err != nil { return nil, err } + if seen[refSpan] { + continue + } + seen[refSpan] = true // Renaming a types.PkgName may result in the addition or removal of an identifier, // so we deal with this separately. diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index 4eb95de9..78d2e90f 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "os/exec" + "path/filepath" "sort" "strings" "testing" @@ -503,29 +504,40 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) { continue } - if len(changes) != 1 { // Renames must only affect a single file in these tests. - t.Errorf("rename failed for %s, edited %d files, wanted 1 file", newText, len(changes)) - continue + var res []string + for editSpn, edits := range changes { + f, err := r.view.GetFile(ctx, editSpn) + if err != nil { + t.Fatalf("failed for %v: %v", spn, err) + } + + data, _, err := f.Handle(ctx).Read(ctx) + if err != nil { + t.Error(err) + continue + } + filename := filepath.Base(editSpn.Filename()) + contents := applyEdits(string(data), edits) + res = append(res, fmt.Sprintf("%s:\n%s", filename, contents)) } - edits := changes[spn.URI()] - if edits == nil { - t.Errorf("rename failed for %s, did not edit %s", newText, spn.URI()) - continue - } - data, _, err := f.Handle(ctx).Read(ctx) - if err != nil { - t.Error(err) - continue + // Sort on filename + sort.Strings(res) + + var got string + for i, val := range res { + if i != 0 { + got += "\n" + } + got += val } - got := applyEdits(string(data), edits) - gorenamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) { + renamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) { return []byte(got), nil })) - if gorenamed != got { - t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, gorenamed, got) + if renamed != got { + t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, renamed, got) } } } diff --git a/internal/lsp/testdata/rename/a/random.go.golden b/internal/lsp/testdata/rename/a/random.go.golden index 0ad12fe7..53bbc29d 100644 --- a/internal/lsp/testdata/rename/a/random.go.golden +++ b/internal/lsp/testdata/rename/a/random.go.golden @@ -1,4 +1,5 @@ -- GetSum-rename -- +random.go: package a import ( @@ -43,6 +44,7 @@ func sw() { } -- fmt2-rename -- +random.go: package a import ( @@ -87,6 +89,7 @@ func sw() { } -- format-rename -- +random.go: package a import ( @@ -131,6 +134,7 @@ func sw() { } -- log-rename -- +random.go: package a import ( @@ -175,6 +179,7 @@ func sw() { } -- myX-rename -- +random.go: package a import ( @@ -219,6 +224,7 @@ func sw() { } -- pos-rename -- +random.go: package a import ( @@ -263,6 +269,7 @@ func sw() { } -- y0-rename -- +random.go: package a import ( @@ -307,6 +314,7 @@ func sw() { } -- y1-rename -- +random.go: package a import ( @@ -351,6 +359,7 @@ func sw() { } -- y2-rename -- +random.go: package a import ( @@ -395,6 +404,7 @@ func sw() { } -- y3-rename -- +random.go: package a import ( @@ -439,6 +449,7 @@ func sw() { } -- z-rename -- +random.go: package a import ( diff --git a/internal/lsp/testdata/rename/testy/testy.go b/internal/lsp/testdata/rename/testy/testy.go new file mode 100644 index 00000000..e5876259 --- /dev/null +++ b/internal/lsp/testdata/rename/testy/testy.go @@ -0,0 +1,6 @@ +package testy + +type tt int //@rename("tt", "testyType") + +func a() { +} diff --git a/internal/lsp/testdata/rename/testy/testy.go.golden b/internal/lsp/testdata/rename/testy/testy.go.golden new file mode 100644 index 00000000..955761b7 --- /dev/null +++ b/internal/lsp/testdata/rename/testy/testy.go.golden @@ -0,0 +1,9 @@ +-- testyType-rename -- +testy.go: +package testy + +type testyType int //@rename("tt", "testyType") + +func a() { +} + diff --git a/internal/lsp/testdata/rename/testy/testy_test.go b/internal/lsp/testdata/rename/testy/testy_test.go new file mode 100644 index 00000000..3d86e845 --- /dev/null +++ b/internal/lsp/testdata/rename/testy/testy_test.go @@ -0,0 +1,8 @@ +package testy + +import "testing" + +func TestSomething(t *testing.T) { + var x int //@rename("x", "testyX") + a() //@rename("a", "b") +} diff --git a/internal/lsp/testdata/rename/testy/testy_test.go.golden b/internal/lsp/testdata/rename/testy/testy_test.go.golden new file mode 100644 index 00000000..2bad81e7 --- /dev/null +++ b/internal/lsp/testdata/rename/testy/testy_test.go.golden @@ -0,0 +1,30 @@ +-- b-rename -- +testy.go: +package testy + +type tt int //@rename("tt", "testyType") + +func b() { +} + +testy_test.go: +package testy + +import "testing" + +func TestSomething(t *testing.T) { + var x int //@rename("x", "testyX") + b() //@rename("a", "b") +} + +-- testyX-rename -- +testy_test.go: +package testy + +import "testing" + +func TestSomething(t *testing.T) { + var testyX int //@rename("x", "testyX") + a() //@rename("a", "b") +} + diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go index 030b2bde..a1589d82 100644 --- a/internal/lsp/tests/tests.go +++ b/internal/lsp/tests/tests.go @@ -34,7 +34,7 @@ const ( ExpectedTypeDefinitionsCount = 2 ExpectedHighlightsCount = 2 ExpectedReferencesCount = 5 - ExpectedRenamesCount = 13 + ExpectedRenamesCount = 16 ExpectedSymbolsCount = 1 ExpectedSignaturesCount = 21 ExpectedLinksCount = 2