internal/lsp: check no errs for assignability rename

The satisfy package has a precondition for Finder.Find that requires
that the package has no type errors. If this is a check that we would
perform, give an error and do not rename.

Fixes golang/go#32882

Change-Id: Id44b451bf86ff883fd78a6306f2b2565ad3bdeb9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184857
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

@@ -510,6 +510,8 @@ func (r *runner) Reference(t *testing.T, data tests.References) {
 func (r *runner) Rename(t *testing.T, data tests.Renames) {
 	ctx := context.Background()
 	for spn, newText := range data {
+		tag := fmt.Sprintf("%s-rename", newText)
+
 		uri := spn.URI()
 		filename := uri.Filename()
 		sm, err := r.mapper(uri)
@@ -529,7 +531,12 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) {
 			NewName:  newText,
 		})
 		if err != nil {
-			t.Error(err)
+			renamed := string(r.data.Golden(tag, filename, func() ([]byte, error) {
+				return []byte(err.Error()), nil
+			}))
+			if err.Error() != renamed {
+				t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v\n", newText, renamed, err)
+			}
 			continue
 		}
 
@@ -556,7 +563,6 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) {
 
 		got := applyEdits(string(m.Content), sedits)
 
-		tag := fmt.Sprintf("%s-rename", newText)
 		gorenamed := string(r.data.Golden(tag, filename, func() ([]byte, error) {
 			return []byte(got), nil
 		}))

internal/lsp/source/rename_check.go

@@ -789,6 +789,19 @@ func (r *renamer) satisfy() map[satisfy.Constraint]bool {
 		// Compute on demand: it's expensive.
 		var f satisfy.Finder
 		for _, pkg := range r.packages {
+			// From satisfy.Finder documentation:
+			//
+			// The package must be free of type errors, and
+			// info.{Defs,Uses,Selections,Types} must have been populated by the
+			// type-checker.
+			//
+			// Only proceed if all packages have no errors.
+			if errs := pkg.GetErrors(); len(errs) > 0 {
+				r.errorf(token.NoPos, // we don't have a position for this error.
+					"renaming %q to %q not possible because %q has errors",
+					r.from, r.to, pkg.PkgPath())
+				return nil
+			}
 			f.Find(pkg.GetTypesInfo(), pkg.GetSyntax())
 		}
 		r.satisfyConstraints = f.Result

internal/lsp/source/source_test.go

@@ -479,6 +479,8 @@ func (r *runner) Reference(t *testing.T, data tests.References) {
 func (r *runner) Rename(t *testing.T, data tests.Renames) {
 	ctx := context.Background()
 	for spn, newText := range data {
+		tag := fmt.Sprintf("%s-rename", newText)
+
 		f, err := r.view.GetFile(ctx, spn.URI())
 		if err != nil {
 			t.Fatalf("failed for %v: %v", spn, err)
@@ -492,7 +494,12 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) {
 		}
 		changes, err := ident.Rename(context.Background(), newText)
 		if err != nil {
-			t.Error(err)
+			renamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) {
+				return []byte(err.Error()), nil
+			}))
+			if err.Error() != renamed {
+				t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v\n", newText, renamed, err)
+			}
 			continue
 		}
 
@@ -513,7 +520,6 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) {
 		}
 
 		got := applyEdits(string(data), edits)
-		tag := fmt.Sprintf("%s-rename", newText)
 		gorenamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) {
 			return []byte(got), nil
 		}))

internal/lsp/testdata/rename/a/random.go.golden

@@ -42,6 +42,138 @@ func sw() {
 	}
 }
 
+-- fmt2-rename --
+package a
+
+import (
+	lg "log"
+	"fmt"
+	fmt2 "fmt"
+)
+
+func Random() int {
+	y := 6 + 7
+	return y
+}
+
+func Random2(y int) int { //@rename("y", "z")
+	return y
+}
+
+type Pos struct {
+	x, y int
+}
+
+func (p *Pos) Sum() int {
+	return p.x + p.y //@rename("x", "myX")
+}
+
+func _() {
+	var p Pos   //@rename("p", "pos")
+	_ = p.Sum() //@rename("Sum", "GetSum")
+}
+
+func sw() {
+	var x interface{}
+
+	switch y := x.(type) { //@rename("y", "y0")
+	case int:
+		fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
+	case string:
+		lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
+	default:
+		fmt2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
+	}
+}
+
+-- format-rename --
+package a
+
+import (
+	lg "log"
+	format "fmt"
+	f2 "fmt"
+)
+
+func Random() int {
+	y := 6 + 7
+	return y
+}
+
+func Random2(y int) int { //@rename("y", "z")
+	return y
+}
+
+type Pos struct {
+	x, y int
+}
+
+func (p *Pos) Sum() int {
+	return p.x + p.y //@rename("x", "myX")
+}
+
+func _() {
+	var p Pos   //@rename("p", "pos")
+	_ = p.Sum() //@rename("Sum", "GetSum")
+}
+
+func sw() {
+	var x interface{}
+
+	switch y := x.(type) { //@rename("y", "y0")
+	case int:
+		format.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
+	case string:
+		lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
+	default:
+		f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
+	}
+}
+
+-- log-rename --
+package a
+
+import (
+	"log"
+	"fmt"
+	f2 "fmt"
+)
+
+func Random() int {
+	y := 6 + 7
+	return y
+}
+
+func Random2(y int) int { //@rename("y", "z")
+	return y
+}
+
+type Pos struct {
+	x, y int
+}
+
+func (p *Pos) Sum() int {
+	return p.x + p.y //@rename("x", "myX")
+}
+
+func _() {
+	var p Pos   //@rename("p", "pos")
+	_ = p.Sum() //@rename("Sum", "GetSum")
+}
+
+func sw() {
+	var x interface{}
+
+	switch y := x.(type) { //@rename("y", "y0")
+	case int:
+		fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
+	case string:
+		log.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
+	default:
+		f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
+	}
+}
+
 -- myX-rename --
 package a
 
@@ -130,50 +262,6 @@ func sw() {
 	}
 }
 
--- z-rename --
-package a
-
-import (
-	lg "log"
-	"fmt"
-	f2 "fmt"
-)
-
-func Random() int {
-	y := 6 + 7
-	return y
-}
-
-func Random2(z int) int { //@rename("y", "z")
-	return z
-}
-
-type Pos struct {
-	x, y int
-}
-
-func (p *Pos) Sum() int {
-	return p.x + p.y //@rename("x", "myX")
-}
-
-func _() {
-	var p Pos   //@rename("p", "pos")
-	_ = p.Sum() //@rename("Sum", "GetSum")
-}
-
-func sw() {
-	var x interface{}
-
-	switch y := x.(type) { //@rename("y", "y0")
-	case int:
-		fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
-	case string:
-		lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
-	default:
-		f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
-	}
-}
-
 -- y0-rename --
 package a
 
@@ -350,55 +438,11 @@ func sw() {
 	}
 }
 
--- format-rename --
+-- z-rename --
 package a
 
 import (
 	lg "log"
-	format "fmt"
-	f2 "fmt"
-)
-
-func Random() int {
-	y := 6 + 7
-	return y
-}
-
-func Random2(y int) int { //@rename("y", "z")
-	return y
-}
-
-type Pos struct {
-	x, y int
-}
-
-func (p *Pos) Sum() int {
-	return p.x + p.y //@rename("x", "myX")
-}
-
-func _() {
-	var p Pos   //@rename("p", "pos")
-	_ = p.Sum() //@rename("Sum", "GetSum")
-}
-
-func sw() {
-	var x interface{}
-
-	switch y := x.(type) { //@rename("y", "y0")
-	case int:
-		format.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
-	case string:
-		lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
-	default:
-		f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
-	}
-}
-
--- log-rename --
-package a
-
-import (
-	"log"
 	"fmt"
 	f2 "fmt"
 )
@@ -408,52 +452,8 @@ func Random() int {
 	return y
 }
 
-func Random2(y int) int { //@rename("y", "z")
-	return y
-}
-
-type Pos struct {
-	x, y int
-}
-
-func (p *Pos) Sum() int {
-	return p.x + p.y //@rename("x", "myX")
-}
-
-func _() {
-	var p Pos   //@rename("p", "pos")
-	_ = p.Sum() //@rename("Sum", "GetSum")
-}
-
-func sw() {
-	var x interface{}
-
-	switch y := x.(type) { //@rename("y", "y0")
-	case int:
-		fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
-	case string:
-		log.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
-	default:
-		f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
-	}
-}
-
--- fmt2-rename --
-package a
-
-import (
-	lg "log"
-	"fmt"
-	fmt2 "fmt"
-)
-
-func Random() int {
-	y := 6 + 7
-	return y
-}
-
-func Random2(y int) int { //@rename("y", "z")
-	return y
+func Random2(z int) int { //@rename("y", "z")
+	return z
 }
 
 type Pos struct {
@@ -478,7 +478,7 @@ func sw() {
 	case string:
 		lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
 	default:
-		fmt2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
+		f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
 	}
 }
 

internal/lsp/testdata/rename/bad/bad.go.golden

@@ -0,0 +1,2 @@
+-- rFunc-rename --
+renaming "sFunc" to "rFunc" not possible because "golang.org/x/tools/internal/lsp/rename/bad" has errors

internal/lsp/testdata/rename/bad/bad.go.in

@@ -0,0 +1,8 @@
+package bad
+
+type myStruct struct {
+}
+
+func (s *myStruct) sFunc() bool { //@rename("sFunc", "rFunc")
+	return s.Bad
+}

internal/lsp/tests/tests.go

@@ -34,7 +34,7 @@ const (
 	ExpectedTypeDefinitionsCount   = 2
 	ExpectedHighlightsCount        = 2
 	ExpectedReferencesCount        = 4
-	ExpectedRenamesCount           = 11
+	ExpectedRenamesCount           = 12
 	ExpectedSymbolsCount           = 1
 	ExpectedSignaturesCount        = 21
 	ExpectedLinksCount             = 2