diff --git a/gopls/internal/test/marker/testdata/codeaction/removeparam_issue65217.txt b/gopls/internal/test/marker/testdata/codeaction/removeparam_issue65217.txt index d4f0ed4c6b3..93729577444 100644 --- a/gopls/internal/test/marker/testdata/codeaction/removeparam_issue65217.txt +++ b/gopls/internal/test/marker/testdata/codeaction/removeparam_issue65217.txt @@ -48,7 +48,6 @@ func _() { func _() { var s S - var _ S = s i := f(s.Int()) _ = i } diff --git a/internal/refactor/inline/inline.go b/internal/refactor/inline/inline.go index 0fda1c579f9..6a7fc129763 100644 --- a/internal/refactor/inline/inline.go +++ b/internal/refactor/inline/inline.go @@ -763,11 +763,15 @@ func (st *state) inlineCall() (*inlineCallResult, error) { n := len(params) - 1 ordinary, extra := args[:n], args[n:] var elts []ast.Expr + freevars := make(map[string]bool) pure, effects := true, false for _, arg := range extra { elts = append(elts, arg.expr) pure = pure && arg.pure effects = effects || arg.effects + for k, v := range arg.freevars { + freevars[k] = v + } } args = append(ordinary, &argument{ expr: &ast.CompositeLit{ @@ -779,7 +783,7 @@ func (st *state) inlineCall() (*inlineCallResult, error) { pure: pure, effects: effects, duplicable: false, - freevars: nil, // not needed + freevars: freevars, }) } } @@ -1453,9 +1457,26 @@ next: // references among other arguments which have non-zero references // within the callee. if v, ok := caller.lookup(free).(*types.Var); ok && within(v.Pos(), caller.enclosingFunc.Body) && !isUsedOutsideCall(caller, v) { - logf("keeping param %q: arg contains perhaps the last reference to caller local %v @ %v", - param.info.Name, v, caller.Fset.PositionFor(v.Pos(), false)) - continue next + + // Check to see if the substituted var is used within other args + // whose corresponding params ARE used in the callee + usedElsewhere := func() bool { + for i, param := range params { + if i < len(args) && len(param.info.Refs) > 0 { // excludes original param + for name := range args[i].freevars { + if caller.lookup(name) == v { + return true + } + } + } + } + return false + } + if !usedElsewhere() { + logf("keeping param %q: arg contains perhaps the last reference to caller local %v @ %v", + param.info.Name, v, caller.Fset.PositionFor(v.Pos(), false)) + continue next + } } } } diff --git a/internal/refactor/inline/inline_test.go b/internal/refactor/inline/inline_test.go index 8da5fa98cd3..f2233462b37 100644 --- a/internal/refactor/inline/inline_test.go +++ b/internal/refactor/inline/inline_test.go @@ -731,13 +731,25 @@ func TestSubstitution(t *testing.T) { `func _() { var local int; _ = local }`, }, { - "Arguments that are used are detected", + "Arguments that are used by other arguments are detected", `func f(x, y int) { print(x) }`, `func _() { var z int; f(z, z) }`, + `func _() { var z int; print(z) }`, + }, + { + "Arguments that are used by other variadic arguments are detected", + `func f(x int, ys ...int) { print(ys) }`, + `func _() { var z int; f(z, 1, 2, 3, z) }`, + `func _() { var z int; print([]int{1, 2, 3, z}) }`, + }, + { + "Arguments that are used by other variadic arguments are detected, 2", + `func f(x int, ys ...int) { print(ys) }`, + `func _() { var z int; f(z) }`, `func _() { var z int var _ int = z - print(z) + print([]int{}) }`, }, {