Skip to content

Commit c93f28b

Browse files
suzmuesuzmue
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]>
1 parent dd7c717 commit c93f28b

File tree

7 files changed

+96
-61
lines changed

7 files changed

+96
-61
lines changed

‎internal/lsp/lsp_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,8 @@ func (r *runner) Reference(t *testing.T, data tests.References) {
510510
func (r *runner) Rename(t *testing.T, data tests.Renames) {
511511
ctx := context.Background()
512512
for spn, newText := range data {
513+
tag := fmt.Sprintf("%s-rename", newText)
514+
513515
uri := spn.URI()
514516
filename := uri.Filename()
515517
sm, err := r.mapper(uri)
@@ -529,7 +531,12 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) {
529531
NewName: newText,
530532
})
531533
if err != nil {
532-
t.Error(err)
534+
renamed := string(r.data.Golden(tag, filename, func() ([]byte, error) {
535+
return []byte(err.Error()), nil
536+
}))
537+
if err.Error() != renamed {
538+
t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v\n", newText, renamed, err)
539+
}
533540
continue
534541
}
535542

@@ -556,7 +563,6 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) {
556563

557564
got := applyEdits(string(m.Content), sedits)
558565

559-
tag := fmt.Sprintf("%s-rename", newText)
560566
gorenamed := string(r.data.Golden(tag, filename, func() ([]byte, error) {
561567
return []byte(got), nil
562568
}))

‎internal/lsp/source/rename_check.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -789,6 +789,19 @@ func (r *renamer) satisfy() map[satisfy.Constraint]bool {
789789
// Compute on demand: it's expensive.
790790
var f satisfy.Finder
791791
for _, pkg := range r.packages {
792+
// From satisfy.Finder documentation:
793+
//
794+
// The package must be free of type errors, and
795+
// info.{Defs,Uses,Selections,Types} must have been populated by the
796+
// type-checker.
797+
//
798+
// Only proceed if all packages have no errors.
799+
if errs := pkg.GetErrors(); len(errs) > 0 {
800+
r.errorf(token.NoPos, // we don't have a position for this error.
801+
"renaming %q to %q not possible because %q has errors",
802+
r.from, r.to, pkg.PkgPath())
803+
return nil
804+
}
792805
f.Find(pkg.GetTypesInfo(), pkg.GetSyntax())
793806
}
794807
r.satisfyConstraints = f.Result

‎internal/lsp/source/source_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,8 @@ func (r *runner) Reference(t *testing.T, data tests.References) {
479479
func (r *runner) Rename(t *testing.T, data tests.Renames) {
480480
ctx := context.Background()
481481
for spn, newText := range data {
482+
tag := fmt.Sprintf("%s-rename", newText)
483+
482484
f, err := r.view.GetFile(ctx, spn.URI())
483485
if err != nil {
484486
t.Fatalf("failed for %v: %v", spn, err)
@@ -492,7 +494,12 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) {
492494
}
493495
changes, err := ident.Rename(context.Background(), newText)
494496
if err != nil {
495-
t.Error(err)
497+
renamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) {
498+
return []byte(err.Error()), nil
499+
}))
500+
if err.Error() != renamed {
501+
t.Errorf("rename failed for %s, expected:\n%v\ngot:\n%v\n", newText, renamed, err)
502+
}
496503
continue
497504
}
498505

@@ -513,7 +520,6 @@ func (r *runner) Rename(t *testing.T, data tests.Renames) {
513520
}
514521

515522
got := applyEdits(string(data), edits)
516-
tag := fmt.Sprintf("%s-rename", newText)
517523
gorenamed := string(r.data.Golden(tag, spn.URI().Filename(), func() ([]byte, error) {
518524
return []byte(got), nil
519525
}))

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

Lines changed: 56 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,13 @@ func sw() {
4242
}
4343
}
4444

45-
-- myX-rename --
45+
-- fmt2-rename --
4646
package a
4747

4848
import (
4949
lg "log"
5050
"fmt"
51-
f2 "fmt"
51+
fmt2 "fmt"
5252
)
5353

5454
func Random() int {
@@ -61,11 +61,11 @@ func Random2(y int) int { //@rename("y", "z")
6161
}
6262

6363
type Pos struct {
64-
myX, y int
64+
x, y int
6565
}
6666

6767
func (p *Pos) Sum() int {
68-
return p.myX + p.y //@rename("x", "myX")
68+
return p.x + p.y //@rename("x", "myX")
6969
}
7070

7171
func _() {
@@ -82,16 +82,16 @@ func sw() {
8282
case string:
8383
lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
8484
default:
85-
f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
85+
fmt2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
8686
}
8787
}
8888

89-
-- pos-rename --
89+
-- format-rename --
9090
package a
9191

9292
import (
9393
lg "log"
94-
"fmt"
94+
format "fmt"
9595
f2 "fmt"
9696
)
9797

@@ -113,28 +113,28 @@ func (p *Pos) Sum() int {
113113
}
114114

115115
func _() {
116-
var pos Pos //@rename("p", "pos")
117-
_ = pos.Sum() //@rename("Sum", "GetSum")
116+
var p Pos //@rename("p", "pos")
117+
_ = p.Sum() //@rename("Sum", "GetSum")
118118
}
119119

120120
func sw() {
121121
var x interface{}
122122

123123
switch y := x.(type) { //@rename("y", "y0")
124124
case int:
125-
fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
125+
format.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
126126
case string:
127127
lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
128128
default:
129129
f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
130130
}
131131
}
132132

133-
-- z-rename --
133+
-- log-rename --
134134
package a
135135

136136
import (
137-
lg "log"
137+
"log"
138138
"fmt"
139139
f2 "fmt"
140140
)
@@ -144,8 +144,8 @@ func Random() int {
144144
return y
145145
}
146146

147-
func Random2(z int) int { //@rename("y", "z")
148-
return z
147+
func Random2(y int) int { //@rename("y", "z")
148+
return y
149149
}
150150

151151
type Pos struct {
@@ -168,13 +168,13 @@ func sw() {
168168
case int:
169169
fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
170170
case string:
171-
lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
171+
log.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
172172
default:
173173
f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
174174
}
175175
}
176176

177-
-- y0-rename --
177+
-- myX-rename --
178178
package a
179179

180180
import (
@@ -193,11 +193,11 @@ func Random2(y int) int { //@rename("y", "z")
193193
}
194194

195195
type Pos struct {
196-
x, y int
196+
myX, y int
197197
}
198198

199199
func (p *Pos) Sum() int {
200-
return p.x + p.y //@rename("x", "myX")
200+
return p.myX + p.y //@rename("x", "myX")
201201
}
202202

203203
func _() {
@@ -208,17 +208,17 @@ func _() {
208208
func sw() {
209209
var x interface{}
210210

211-
switch y0 := x.(type) { //@rename("y", "y0")
211+
switch y := x.(type) { //@rename("y", "y0")
212212
case int:
213-
fmt.Printf("%d", y0) //@rename("y", "y1"),rename("fmt", "format")
213+
fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
214214
case string:
215-
lg.Printf("%s", y0) //@rename("y", "y2"),rename("lg","log")
215+
lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
216216
default:
217-
f2.Printf("%v", y0) //@rename("y", "y3"),rename("f2","fmt2")
217+
f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
218218
}
219219
}
220220

221-
-- y1-rename --
221+
-- pos-rename --
222222
package a
223223

224224
import (
@@ -245,24 +245,24 @@ func (p *Pos) Sum() int {
245245
}
246246

247247
func _() {
248-
var p Pos //@rename("p", "pos")
249-
_ = p.Sum() //@rename("Sum", "GetSum")
248+
var pos Pos //@rename("p", "pos")
249+
_ = pos.Sum() //@rename("Sum", "GetSum")
250250
}
251251

252252
func sw() {
253253
var x interface{}
254254

255-
switch y1 := x.(type) { //@rename("y", "y0")
255+
switch y := x.(type) { //@rename("y", "y0")
256256
case int:
257-
fmt.Printf("%d", y1) //@rename("y", "y1"),rename("fmt", "format")
257+
fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
258258
case string:
259-
lg.Printf("%s", y1) //@rename("y", "y2"),rename("lg","log")
259+
lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
260260
default:
261-
f2.Printf("%v", y1) //@rename("y", "y3"),rename("f2","fmt2")
261+
f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
262262
}
263263
}
264264

265-
-- y2-rename --
265+
-- y0-rename --
266266
package a
267267

268268
import (
@@ -296,17 +296,17 @@ func _() {
296296
func sw() {
297297
var x interface{}
298298

299-
switch y2 := x.(type) { //@rename("y", "y0")
299+
switch y0 := x.(type) { //@rename("y", "y0")
300300
case int:
301-
fmt.Printf("%d", y2) //@rename("y", "y1"),rename("fmt", "format")
301+
fmt.Printf("%d", y0) //@rename("y", "y1"),rename("fmt", "format")
302302
case string:
303-
lg.Printf("%s", y2) //@rename("y", "y2"),rename("lg","log")
303+
lg.Printf("%s", y0) //@rename("y", "y2"),rename("lg","log")
304304
default:
305-
f2.Printf("%v", y2) //@rename("y", "y3"),rename("f2","fmt2")
305+
f2.Printf("%v", y0) //@rename("y", "y3"),rename("f2","fmt2")
306306
}
307307
}
308308

309-
-- y3-rename --
309+
-- y1-rename --
310310
package a
311311

312312
import (
@@ -340,22 +340,22 @@ func _() {
340340
func sw() {
341341
var x interface{}
342342

343-
switch y3 := x.(type) { //@rename("y", "y0")
343+
switch y1 := x.(type) { //@rename("y", "y0")
344344
case int:
345-
fmt.Printf("%d", y3) //@rename("y", "y1"),rename("fmt", "format")
345+
fmt.Printf("%d", y1) //@rename("y", "y1"),rename("fmt", "format")
346346
case string:
347-
lg.Printf("%s", y3) //@rename("y", "y2"),rename("lg","log")
347+
lg.Printf("%s", y1) //@rename("y", "y2"),rename("lg","log")
348348
default:
349-
f2.Printf("%v", y3) //@rename("y", "y3"),rename("f2","fmt2")
349+
f2.Printf("%v", y1) //@rename("y", "y3"),rename("f2","fmt2")
350350
}
351351
}
352352

353-
-- format-rename --
353+
-- y2-rename --
354354
package a
355355

356356
import (
357357
lg "log"
358-
format "fmt"
358+
"fmt"
359359
f2 "fmt"
360360
)
361361

@@ -384,21 +384,21 @@ func _() {
384384
func sw() {
385385
var x interface{}
386386

387-
switch y := x.(type) { //@rename("y", "y0")
387+
switch y2 := x.(type) { //@rename("y", "y0")
388388
case int:
389-
format.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
389+
fmt.Printf("%d", y2) //@rename("y", "y1"),rename("fmt", "format")
390390
case string:
391-
lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
391+
lg.Printf("%s", y2) //@rename("y", "y2"),rename("lg","log")
392392
default:
393-
f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
393+
f2.Printf("%v", y2) //@rename("y", "y3"),rename("f2","fmt2")
394394
}
395395
}
396396

397-
-- log-rename --
397+
-- y3-rename --
398398
package a
399399

400400
import (
401-
"log"
401+
lg "log"
402402
"fmt"
403403
f2 "fmt"
404404
)
@@ -428,32 +428,32 @@ func _() {
428428
func sw() {
429429
var x interface{}
430430

431-
switch y := x.(type) { //@rename("y", "y0")
431+
switch y3 := x.(type) { //@rename("y", "y0")
432432
case int:
433-
fmt.Printf("%d", y) //@rename("y", "y1"),rename("fmt", "format")
433+
fmt.Printf("%d", y3) //@rename("y", "y1"),rename("fmt", "format")
434434
case string:
435-
log.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
435+
lg.Printf("%s", y3) //@rename("y", "y2"),rename("lg","log")
436436
default:
437-
f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
437+
f2.Printf("%v", y3) //@rename("y", "y3"),rename("f2","fmt2")
438438
}
439439
}
440440

441-
-- fmt2-rename --
441+
-- z-rename --
442442
package a
443443

444444
import (
445445
lg "log"
446446
"fmt"
447-
fmt2 "fmt"
447+
f2 "fmt"
448448
)
449449

450450
func Random() int {
451451
y := 6 + 7
452452
return y
453453
}
454454

455-
func Random2(y int) int { //@rename("y", "z")
456-
return y
455+
func Random2(z int) int { //@rename("y", "z")
456+
return z
457457
}
458458

459459
type Pos struct {
@@ -478,7 +478,7 @@ func sw() {
478478
case string:
479479
lg.Printf("%s", y) //@rename("y", "y2"),rename("lg","log")
480480
default:
481-
fmt2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
481+
f2.Printf("%v", y) //@rename("y", "y3"),rename("f2","fmt2")
482482
}
483483
}
484484

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
-- rFunc-rename --
2+
renaming "sFunc" to "rFunc" not possible because "golang.org/x/tools/internal/lsp/rename/bad" has errors

0 commit comments

Comments
 (0)