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>
diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go
index 17d845c..05329ee 100644
--- a/internal/lsp/lsp_test.go
+++ b/internal/lsp/lsp_test.go
@@ -510,6 +510,8 @@
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 @@
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 @@
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
}))
diff --git a/internal/lsp/source/rename_check.go b/internal/lsp/source/rename_check.go
index 0b602f4..8d2ac92 100644
--- a/internal/lsp/source/rename_check.go
+++ b/internal/lsp/source/rename_check.go
@@ -789,6 +789,19 @@
// 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
diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go
index d310250..4eb95de 100644
--- a/internal/lsp/source/source_test.go
+++ b/internal/lsp/source/source_test.go
@@ -479,6 +479,8 @@
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 @@
}
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 @@
}
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
}))
diff --git a/internal/lsp/testdata/rename/a/random.go.golden b/internal/lsp/testdata/rename/a/random.go.golden
index 427b7d2..0ad12fe 100644
--- a/internal/lsp/testdata/rename/a/random.go.golden
+++ b/internal/lsp/testdata/rename/a/random.go.golden
@@ -42,6 +42,138 @@
}
}
+-- 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 @@
}
}
--- 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 @@
}
}
--- 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 @@
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 @@
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")
}
}
diff --git a/internal/lsp/testdata/rename/bad/bad.go.golden b/internal/lsp/testdata/rename/bad/bad.go.golden
new file mode 100644
index 0000000..7f45813
--- /dev/null
+++ b/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
diff --git a/internal/lsp/testdata/rename/bad/bad.go.in b/internal/lsp/testdata/rename/bad/bad.go.in
new file mode 100644
index 0000000..56dbee7
--- /dev/null
+++ b/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
+}
diff --git a/internal/lsp/tests/tests.go b/internal/lsp/tests/tests.go
index 65a29f3..aa5fd60 100644
--- a/internal/lsp/tests/tests.go
+++ b/internal/lsp/tests/tests.go
@@ -34,7 +34,7 @@
ExpectedTypeDefinitionsCount = 2
ExpectedHighlightsCount = 2
ExpectedReferencesCount = 4
- ExpectedRenamesCount = 11
+ ExpectedRenamesCount = 12
ExpectedSymbolsCount = 1
ExpectedSignaturesCount = 21
ExpectedLinksCount = 2
