Skip to content

Commit 8a71c39

Browse files
findleyrgopherbot
authored andcommitted
gopls/internal/lsp/source: abort change signature on overlapping calls
The heuristic used to identify remaining uninlined calls by counting fails when the inlining operation changes the order of "original" calls, as may be the case with overlapping calls, following CL 535456. I spent around an hour trying to fix this properly (by tracking calls that were "just" inlined), but it's too hard because we must also track calls that had previously been inlined. It needs more theory and thought. For now, remove support for overlapping calls, as in all other cases the top-to-bottom counting heuristic should still work. For golang/go#63534 Change-Id: I59fe4aa5d20e56b0eb9dcbc01d29dd2ece082c17 Reviewed-on: https://go-review.googlesource.com/c/tools/+/535795 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Alan Donovan <[email protected]> Auto-Submit: Robert Findley <[email protected]>
1 parent f744e4b commit 8a71c39

File tree

2 files changed

+13
-13
lines changed

2 files changed

+13
-13
lines changed

gopls/internal/lsp/source/inline_all.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"go/ast"
1111
"go/parser"
12+
"go/types"
1213

1314
"golang.org/x/tools/go/ast/astutil"
1415
"golang.org/x/tools/go/types/typeutil"
@@ -170,6 +171,17 @@ func inlineAllCalls(ctx context.Context, logf func(string, ...any), snapshot Sna
170171
file = callInfo.pgf.File
171172
content = callInfo.pgf.Src
172173
)
174+
175+
// Check for overlapping calls (such as Foo(Foo())). We can't handle these
176+
// because inlining may change the source order of the inner call with
177+
// respect to the inlined outer call, and so the heuristic we use to find
178+
// the next call (counting from top-to-bottom) does not work.
179+
for i := range calls {
180+
if i > 0 && calls[i-1].End() > calls[i].Pos() {
181+
return nil, fmt.Errorf("%s: can't inline overlapping call %s", uri, types.ExprString(calls[i-1]))
182+
}
183+
}
184+
173185
currentCall := 0
174186
for currentCall < len(calls) {
175187
caller := &inline.Caller{

gopls/internal/regtest/marker/testdata/codeaction/removeparam.txt

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -194,27 +194,15 @@ func h() (int, int)
194194
-- overlapping/overlapping.go --
195195
package overlapping
196196

197-
func Overlapping(i int) int { //@codeaction("refactor.rewrite", re"(i) int", re"(i) int", overlapping)
197+
func Overlapping(i int) int { //@codeactionerr("refactor.rewrite", re"(i) int", re"(i) int", re"overlapping")
198198
return 0
199199
}
200200

201201
func _() {
202202
x := Overlapping(Overlapping(0))
203203
_ = x
204204
}
205-
-- @overlapping/overlapping/overlapping.go --
206-
package overlapping
207-
208-
func Overlapping() int { //@codeaction("refactor.rewrite", re"(i) int", re"(i) int", overlapping)
209-
return 0
210-
}
211205

212-
func _() {
213-
x := func(_ int) int {
214-
return Overlapping()
215-
}(Overlapping())
216-
_ = x
217-
}
218206
-- effects/effects.go --
219207
package effects
220208

0 commit comments

Comments
 (0)