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>

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)
+		var res []string
-		if err != nil {
+		for uri, edits := range *workspaceEdits.Changes {
-			t.Error(err)
+			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
+		// Sort on filename
-		if len(changes) != 1 { // Renames must only affect a single file in these tests.
+		sort.Strings(res)
-			t.Errorf("rename failed for %s, edited %d files, wanted 1 file", newText, len(*workspaceEdits.Changes))
+
-			continue
+		var got string
+		for i, val := range res {
+			if i != 0 {
+				got += "\n"
+			}
+			got += val
 		}
 
-		edits := changes[string(uri)]
+		renamed := string(r.data.Golden(tag, filename, func() ([]byte, error) {
-		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) {
 			return []byte(got), nil
 		}))
 
-		if gorenamed != got {
+		if renamed != got {
-			t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, gorenamed, got)
+			t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, renamed, got)
 		}
 	}
 }

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,
 			})
 		}

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.

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.
+		var res []string
-			t.Errorf("rename failed for %s, edited %d files, wanted 1 file", newText, len(changes))
+		for editSpn, edits := range changes {
-			continue
+			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()]
+		// Sort on filename
-		if edits == nil {
+		sort.Strings(res)
-			t.Errorf("rename failed for %s, did not edit %s", newText, spn.URI())
+
-			continue
+		var got string
-		}
+		for i, val := range res {
-		data, _, err := f.Handle(ctx).Read(ctx)
+			if i != 0 {
-		if err != nil {
+				got += "\n"
-			t.Error(err)
+			}
-			continue
+			got += val
 		}
 
-		got := applyEdits(string(data), edits)
+		renamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) {
-		gorenamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) {
 			return []byte(got), nil
 		}))
 
-		if gorenamed != got {
+		if renamed != got {
-			t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, gorenamed, got)
+			t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v", newText, renamed, got)
 		}
 	}
 }

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 (

internal/lsp/testdata/rename/testy/testy.go

@@ -0,0 +1,6 @@
+package testy
+
+type tt int //@rename("tt", "testyType")
+
+func a() {
+}

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() {
+}
+

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")
+}

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")
+}
+

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