Skip to content

Commit d905d0b

Browse files
adonovangopherbot
authored andcommitted
gopls/internal/golang: add test for (unfixed) issue 65098
The test confirms the reported problematic behavior, and investigation shows it to be (as suspected) a dup of Also, a couple of minor cleanups from static analyzers. Updates golang/go#58461 Updates golang/go#65098 Change-Id: I316c2f3bd23005526ccd9da461a18f1198373fe4 Reviewed-on: https://go-review.googlesource.com/c/tools/+/667135 LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Alan Donovan <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent e27768f commit d905d0b

File tree

3 files changed

+60
-20
lines changed

3 files changed

+60
-20
lines changed

gopls/internal/golang/rename.go

+6-17
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ import (
5454
"path"
5555
"path/filepath"
5656
"regexp"
57+
"slices"
5758
"sort"
5859
"strconv"
5960
"strings"
@@ -169,13 +170,7 @@ func PrepareRename(ctx context.Context, snapshot *cache.Snapshot, f file.Handle,
169170

170171
func prepareRenamePackageName(ctx context.Context, snapshot *cache.Snapshot, pgf *parsego.File) (*PrepareItem, error) {
171172
// Does the client support file renaming?
172-
fileRenameSupported := false
173-
for _, op := range snapshot.Options().SupportedResourceOperations {
174-
if op == protocol.Rename {
175-
fileRenameSupported = true
176-
break
177-
}
178-
}
173+
fileRenameSupported := slices.Contains(snapshot.Options().SupportedResourceOperations, protocol.Rename)
179174
if !fileRenameSupported {
180175
return nil, errors.New("can't rename package: LSP client does not support file renaming")
181176
}
@@ -438,13 +433,7 @@ func Rename(ctx context.Context, snapshot *cache.Snapshot, f file.Handle, pp pro
438433
// become reordered) and that are either identical or
439434
// non-overlapping.
440435
diff.SortEdits(edits)
441-
filtered := edits[:0]
442-
for i, edit := range edits {
443-
if i == 0 || edit != filtered[len(filtered)-1] {
444-
filtered = append(filtered, edit)
445-
}
446-
}
447-
edits = filtered
436+
edits = slices.Compact(edits)
448437

449438
// TODO(adonovan): the logic above handles repeat edits to the
450439
// same file URI (e.g. as a member of package p and p_test) but
@@ -541,15 +530,15 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, f file.Handle
541530
//
542531
// Note that unlike Funcs, TypeNames are always canonical (they are "left"
543532
// of the type parameters, unlike methods).
544-
switch obj.(type) { // avoid "obj :=" since cases reassign the var
533+
switch obj0 := obj.(type) { // avoid "obj :=" since cases reassign the var
545534
case *types.TypeName:
546535
if _, ok := types.Unalias(obj.Type()).(*types.TypeParam); ok {
547536
// As with capitalized function parameters below, type parameters are
548537
// local.
549538
goto skipObjectPath
550539
}
551540
case *types.Func:
552-
obj = obj.(*types.Func).Origin()
541+
obj = obj0.Origin()
553542
case *types.Var:
554543
// TODO(adonovan): do vars need the origin treatment too? (issue #58462)
555544

@@ -563,7 +552,7 @@ func renameOrdinary(ctx context.Context, snapshot *cache.Snapshot, f file.Handle
563552
// objectpath, the classifies them as local vars, but as
564553
// they came from export data they lack syntax and the
565554
// correct scope tree (issue #61294).
566-
if !obj.(*types.Var).IsField() && !typesinternal.IsPackageLevel(obj) {
555+
if !obj0.IsField() && !typesinternal.IsPackageLevel(obj) {
567556
goto skipObjectPath
568557
}
569558
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
This is a test for issue 65098: a renaming in package a does not
2+
propagate to package b, even though the two packages are coupled via
3+
an assignment in c, which is renamed.
4+
5+
c
6+
/ \
7+
a b
8+
9+
The bug (a dup of #58461) is not yet fixed, so the golden file records
10+
the wrong behavior (i.e. no changes to package b).
11+
TODO(adonovan): fix.
12+
13+
-- go.mod --
14+
module example.com
15+
go 1.12
16+
17+
-- a/a.go --
18+
package a
19+
20+
type I interface {
21+
F() //@ rename("F", "FF", fToFF)
22+
}
23+
24+
-- b/b.go --
25+
package b
26+
27+
type S struct{}
28+
29+
func (s S) F() {}
30+
31+
-- c/c.go --
32+
package c
33+
34+
import (
35+
"example.com/a"
36+
"example.com/b"
37+
)
38+
39+
var _ a.I = b.S{}
40+
var _ = a.I.F
41+
42+
-- @fToFF/a/a.go --
43+
@@ -4 +4 @@
44+
- F() //@ rename("F", "FF", fToFF)
45+
+ FF() //@ rename("F", "FF", fToFF)
46+
-- @fToFF/c/c.go --
47+
@@ -9 +9 @@
48+
-var _ = a.I.F
49+
+var _ = a.I.FF

gopls/internal/test/marker/testdata/rename/methods.txt

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
This test exercises renaming of interface methods.
22

3-
The golden is currently wrong due to https://github.com/golang/go/issues/58506:
4-
the reference to B.F in package b should be renamed too.
5-
63
-- go.mod --
74
module example.com
85
go 1.12
@@ -25,6 +22,8 @@ type B interface { F() } //@rename("F", "G", BfToG)
2522
var _ B = a.A(0)
2623
var _ B = c.C(0)
2724

25+
var _ = B.F
26+
2827
-- c/c.go --
2928
package c
3029

@@ -47,6 +46,9 @@ b/b.go:6:20: (rename example.com/b.B.F if you intend to change both types)
4746
@@ -6 +6 @@
4847
-type B interface { F() } //@rename("F", "G", BfToG)
4948
+type B interface { G() } //@rename("F", "G", BfToG)
49+
@@ -11 +11 @@
50+
-var _ = B.F
51+
+var _ = B.G
5052
-- @BfToG/d/d.go --
5153
@@ -5 +5 @@
5254
-var _ = b.B.F

0 commit comments

Comments
 (0)