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 <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: findleyr

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.

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)

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.

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

gopls/internal/test/marker/testdata/codeaction/removeparam_imports.txt

@@ -67,42 +67,34 @@ 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 --
 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 --
 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 --

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)
 }
 

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