internal/lsp: rename identifiers in test packages
Support renaming of identifiers in test packages. The packages for
all of the references must be checked and the changes need to be
deduped, since both a package and its test package contain some of the
same files.
Fixes golang/go#32974
Change-Id: Ie51e19716faae77ce7e5254eeb3956faa42c2a09
Reviewed-on: https://go-review.googlesource.com/c/tools/+/185277
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 05329ee..6c358ac 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -10,6 +10,7 @@
"fmt"
"go/token"
"os/exec"
+ "path/filepath"
"sort"
"strings"
"testing"
@@ -540,35 +541,41 @@
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 5cbb442..8fd0df3 100644
--- a/internal/lsp/source/references.go
+++ b/internal/lsp/source/references.go
@@ -20,6 +20,7 @@
Range span.Range
ident *ast.Ident
obj types.Object
+ pkg Package
isDeclaration bool
}
@@ -34,17 +35,6 @@
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 @@
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 @@
Range: span.NewRange(i.File.FileSet(), ident.Pos(), ident.End()),
ident: ident,
obj: obj,
+ pkg: pkg,
isDeclaration: true,
}}, references...)
}
@@ -76,6 +80,7 @@
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 0b72a1d..eb5f8ba 100644
--- a/internal/lsp/source/rename.go
+++ b/internal/lsp/source/rename.go
@@ -73,7 +73,9 @@
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 @@
// 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 @@
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 4eb95de..78d2e90 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -9,6 +9,7 @@
"context"
"fmt"
"os/exec"
+ "path/filepath"
"sort"
"strings"
"testing"
@@ -503,29 +504,40 @@
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 0ad12fe..53bbc29 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 @@
}
-- fmt2-rename --
+random.go:
package a
import (
@@ -87,6 +89,7 @@
}
-- format-rename --
+random.go:
package a
import (
@@ -131,6 +134,7 @@
}
-- log-rename --
+random.go:
package a
import (
@@ -175,6 +179,7 @@
}
-- myX-rename --
+random.go:
package a
import (
@@ -219,6 +224,7 @@
}
-- pos-rename --
+random.go:
package a
import (
@@ -263,6 +269,7 @@
}
-- y0-rename --
+random.go:
package a
import (
@@ -307,6 +314,7 @@
}
-- y1-rename --
+random.go:
package a
import (
@@ -351,6 +359,7 @@
}
-- y2-rename --
+random.go:
package a
import (
@@ -395,6 +404,7 @@
}
-- y3-rename --
+random.go:
package a
import (
@@ -439,6 +449,7 @@
}
-- 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 0000000..e587625
--- /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 0000000..955761b
--- /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 0000000..3d86e84
--- /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 0000000..2bad81e
--- /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 030b2bd..a1589d8 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -34,7 +34,7 @@
ExpectedTypeDefinitionsCount = 2
ExpectedHighlightsCount = 2
ExpectedReferencesCount = 5
- ExpectedRenamesCount = 13
+ ExpectedRenamesCount = 16
ExpectedSymbolsCount = 1
ExpectedSignaturesCount = 21
ExpectedLinksCount = 2
