From e33469676fb452337fef005f66b8d1a03d8c2d6c Mon Sep 17 00:00:00 2001 From: Rob Findley Date: Tue, 3 Dec 2024 02:06:15 +0000 Subject: [PATCH] gopls/internal/golang: ignore effects for change signature refactoring While using the change signature refactoring, I never want it to literalize changed signatures based on its (very coarse) effects analysis. Disable this inlining analysis for the purpose of change signature refactoring. For golang/go#70599 Change-Id: I979c4f5b6be520b67a3441fa6e0c55afe1fe9196 Reviewed-on: https://go-review.googlesource.com/c/tools/+/633255 Reviewed-by: Alan Donovan LUCI-TryBot-Result: Go LUCI --- gopls/internal/golang/change_signature.go | 6 +++++- gopls/internal/golang/inline_all.go | 8 ++++++-- .../test/marker/testdata/codeaction/moveparam.txt | 5 +---- .../test/marker/testdata/codeaction/removeparam.txt | 10 ++-------- .../marker/testdata/codeaction/removeparam_imports.txt | 10 ---------- .../marker/testdata/codeaction/removeparam_method.txt | 7 +------ .../marker/testdata/codeaction/removeparam_resolve.txt | 10 ++-------- 7 files changed, 17 insertions(+), 39 deletions(-) diff --git a/gopls/internal/golang/change_signature.go b/gopls/internal/golang/change_signature.go index fbbb62f29c3..8157c6d03fb 100644 --- a/gopls/internal/golang/change_signature.go +++ b/gopls/internal/golang/change_signature.go @@ -641,7 +641,11 @@ func rewriteCalls(ctx context.Context, rw signatureRewrite) (map[protocol.Docume } post := func(got []byte) []byte { return bytes.ReplaceAll(got, []byte(tag), nil) } - return inlineAllCalls(ctx, logf, rw.snapshot, rw.pkg, rw.pgf, rw.origDecl, calleeInfo, post) + opts := &inline.Options{ + Logf: logf, + IgnoreEffects: true, + } + return inlineAllCalls(ctx, rw.snapshot, rw.pkg, rw.pgf, rw.origDecl, calleeInfo, post, opts) } // reTypeCheck re-type checks orig with new file contents defined by fileMask. diff --git a/gopls/internal/golang/inline_all.go b/gopls/internal/golang/inline_all.go index fe39dba0235..ec9a458d61a 100644 --- a/gopls/internal/golang/inline_all.go +++ b/gopls/internal/golang/inline_all.go @@ -44,7 +44,7 @@ import ( // // The code below notes where are assumptions are made that only hold true in // the case of parameter removal (annotated with 'Assumption:') -func inlineAllCalls(ctx context.Context, logf func(string, ...any), snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, origDecl *ast.FuncDecl, callee *inline.Callee, post func([]byte) []byte) (map[protocol.DocumentURI][]byte, error) { +func inlineAllCalls(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, origDecl *ast.FuncDecl, callee *inline.Callee, post func([]byte) []byte, opts *inline.Options) (map[protocol.DocumentURI][]byte, error) { // Collect references. var refs []protocol.Location { @@ -215,7 +215,7 @@ func inlineAllCalls(ctx context.Context, logf func(string, ...any), snapshot *ca Call: calls[currentCall], Content: content, } - res, err := inline.Inline(caller, callee, &inline.Options{Logf: logf}) + res, err := inline.Inline(caller, callee, opts) if err != nil { return nil, fmt.Errorf("inlining failed: %v", err) } @@ -252,6 +252,10 @@ func inlineAllCalls(ctx context.Context, logf func(string, ...any), snapshot *ca // anything in the surrounding scope. // // TODO(rfindley): improve this. + logf := func(string, ...any) {} + if opts != nil { + logf = opts.Logf + } tpkg, tinfo, err = reTypeCheck(logf, callInfo.pkg, map[protocol.DocumentURI]*ast.File{uri: file}, true) if err != nil { return nil, bug.Errorf("type checking after inlining failed: %v", err) diff --git a/gopls/internal/test/marker/testdata/codeaction/moveparam.txt b/gopls/internal/test/marker/testdata/codeaction/moveparam.txt index 09a12f24262..2cc0cd8244f 100644 --- a/gopls/internal/test/marker/testdata/codeaction/moveparam.txt +++ b/gopls/internal/test/marker/testdata/codeaction/moveparam.txt @@ -70,10 +70,7 @@ func b() int { return 2 } var _ = basic.Foo(2, 1) // Check that we can refactor a call with effects in a toplevel var decl. -var _ = func() int { - var a int = a() - return basic.Foo(b(), a) -}() +var _ = basic.Foo(b(), a()) func _() { // check various refactorings in a function body, and comment handling. diff --git a/gopls/internal/test/marker/testdata/codeaction/removeparam.txt b/gopls/internal/test/marker/testdata/codeaction/removeparam.txt index 4062e77f84c..c8fddb0fff7 100644 --- a/gopls/internal/test/marker/testdata/codeaction/removeparam.txt +++ b/gopls/internal/test/marker/testdata/codeaction/removeparam.txt @@ -146,12 +146,10 @@ func _() { Ellipsis() Ellipsis() Ellipsis() - var _ []any = []any{1, f(), g()} Ellipsis() func(_ ...any) { Ellipsis() }(h()) - var _ []any = i() Ellipsis() } @@ -225,12 +223,8 @@ func f() int func g() int func _() { - var x, _ int = f(), g() - effects(x) - { - var x, _ int = f(), g() - effects(x) - } + effects(f()) + effects(f()) } -- recursive/recursive.go -- package recursive diff --git a/gopls/internal/test/marker/testdata/codeaction/removeparam_imports.txt b/gopls/internal/test/marker/testdata/codeaction/removeparam_imports.txt index e23fe6e2aa1..d9f4f22dc7e 100644 --- a/gopls/internal/test/marker/testdata/codeaction/removeparam_imports.txt +++ b/gopls/internal/test/marker/testdata/codeaction/removeparam_imports.txt @@ -67,16 +67,13 @@ package a import ( "mod.test/b" - "mod.test/c" ) func _() { - var _ c.C = <-b.Chan b.B(<-b.Chan) } func _() { - var _ c.C = <-b.Chan b.B(<-b.Chan) } -- @b/a/a2.go -- @@ -84,13 +81,10 @@ package a import ( "mod.test/b" - "mod.test/c" ) func _() { - var _ c.C = <-b.Chan b.B(<-b.Chan) - var _ c.C = <-b.Chan b.B(<-b.Chan) } -- @b/a/a1.go -- @@ -98,11 +92,9 @@ package a import ( "mod.test/b" - "mod.test/c" ) func _() { - var _ c.C = <-b.Chan b.B(<-b.Chan) } -- @b/a/a4.go -- @@ -113,11 +105,9 @@ package a import ( "mod.test/b" . "mod.test/b" - "mod.test/c" ) func _() { - var _ c.C = <-Chan b.B(<-Chan) } -- @b/b/b.go -- diff --git a/gopls/internal/test/marker/testdata/codeaction/removeparam_method.txt b/gopls/internal/test/marker/testdata/codeaction/removeparam_method.txt index c949186a614..9b01edd5ae8 100644 --- a/gopls/internal/test/marker/testdata/codeaction/removeparam_method.txt +++ b/gopls/internal/test/marker/testdata/codeaction/removeparam_method.txt @@ -68,11 +68,7 @@ import "example.com/rm" func _() { x := new(rm.Basic) - var ( - t rm.Basic = *x - _ int = sideEffects() - ) - t.Foo() + x.Foo() rm.Basic(1).Foo() } @@ -137,7 +133,6 @@ import "example.com/rm" func _() { x := rm.Missing{} - var _ int = sideEffects() x.M(1, 3, 4) } diff --git a/gopls/internal/test/marker/testdata/codeaction/removeparam_resolve.txt b/gopls/internal/test/marker/testdata/codeaction/removeparam_resolve.txt index c3484d34d9c..b51dd6fb8cf 100644 --- a/gopls/internal/test/marker/testdata/codeaction/removeparam_resolve.txt +++ b/gopls/internal/test/marker/testdata/codeaction/removeparam_resolve.txt @@ -157,12 +157,10 @@ func _() { Ellipsis() Ellipsis() Ellipsis() - var _ []any = []any{1, f(), g()} Ellipsis() func(_ ...any) { Ellipsis() }(h()) - var _ []any = i() Ellipsis() } @@ -236,12 +234,8 @@ func f() int func g() int func _() { - var x, _ int = f(), g() - effects(x) - { - var x, _ int = f(), g() - effects(x) - } + effects(f()) + effects(f()) } -- recursive/recursive.go -- package recursive