Skip to content

Commit 1c2c083

Browse files
committed
[CodeQuality] Fix MergeWithCallableAndWillReturnRector to add closure use() for variables nested in willReturn expr
Previously only a bare Variable willReturn arg got a closure use(). A variable nested inside an array (e.g. willReturn([$initialStatements, []])) was moved into the closure without a use(), producing a syntax error. Now all variables referenced in the returned expression are collected (skipping closure params and $this) and added as closure uses. Fixes #604
1 parent 6d8a000 commit 1c2c083

2 files changed

Lines changed: 98 additions & 2 deletions

File tree

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MergeWithCallableAndWillReturnRector\Fixture;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class ReturnArrayWithVariableUse extends TestCase
8+
{
9+
public function test()
10+
{
11+
$initialStatements = [1, 2];
12+
13+
$this->createMock('SomeClass')
14+
->method('someMethod')
15+
->with($this->callback(function ($arg): bool {
16+
return true;
17+
}))
18+
->willReturn([$initialStatements, []]);
19+
}
20+
}
21+
22+
?>
23+
-----
24+
<?php
25+
26+
namespace Rector\PHPUnit\Tests\CodeQuality\Rector\MethodCall\MergeWithCallableAndWillReturnRector\Fixture;
27+
28+
use PHPUnit\Framework\TestCase;
29+
30+
final class ReturnArrayWithVariableUse extends TestCase
31+
{
32+
public function test()
33+
{
34+
$initialStatements = [1, 2];
35+
36+
$this->createMock('SomeClass')
37+
->method('someMethod')
38+
->willReturnCallback(function ($arg) use ($initialStatements): array {
39+
return [$initialStatements, []];
40+
});
41+
}
42+
}
43+
44+
?>

‎rules/CodeQuality/Rector/MethodCall/MergeWithCallableAndWillReturnRector.php‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@
77
use PhpParser\Node;
88
use PhpParser\Node\Arg;
99
use PhpParser\Node\ClosureUse;
10+
use PhpParser\Node\Expr;
1011
use PhpParser\Node\Expr\Closure;
1112
use PhpParser\Node\Expr\MethodCall;
1213
use PhpParser\Node\Expr\Variable;
1314
use PhpParser\Node\Identifier;
1415
use PhpParser\Node\Stmt\Return_;
16+
use Rector\PhpParser\Node\BetterNodeFinder;
1517
use Rector\PhpParser\Node\Value\ValueResolver;
1618
use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
1719
use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
@@ -29,6 +31,7 @@ public function __construct(
2931
private readonly TestsNodeAnalyzer $testsNodeAnalyzer,
3032
private readonly ValueResolver $valueResolver,
3133
private readonly StaticTypeMapper $staticTypeMapper,
34+
private readonly BetterNodeFinder $betterNodeFinder,
3235
) {
3336
}
3437

@@ -143,8 +146,8 @@ public function refactor(Node $node): MethodCall|null
143146
$parentCaller->name = new Identifier('willReturnCallback');
144147
$parentCaller->args = [new Arg($innerClosure)];
145148

146-
if ($returnedExpr instanceof Variable) {
147-
$innerClosure->uses[] = new ClosureUse($returnedExpr);
149+
foreach ($this->resolveExternalUses($innerClosure, $returnedExpr) as $closureUse) {
150+
$innerClosure->uses[] = $closureUse;
148151
}
149152

150153
$returnedExprType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($returnedExpr);
@@ -171,6 +174,55 @@ private function matchFirstArgCallbackMethodCall(MethodCall $withMethodCall): ?M
171174
return $firstArgValue;
172175
}
173176

177+
/**
178+
* @return ClosureUse[]
179+
*/
180+
private function resolveExternalUses(Closure $innerClosure, Expr $returnedExpr): array
181+
{
182+
$paramNames = [];
183+
foreach ($innerClosure->getParams() as $param) {
184+
$paramNames[] = $this->getName($param);
185+
}
186+
187+
$existingUseNames = [];
188+
foreach ($innerClosure->uses as $existingUse) {
189+
$existingUseNames[] = $this->getName($existingUse->var);
190+
}
191+
192+
$closureUses = [];
193+
$seenNames = [];
194+
195+
/** @var Variable[] $variables */
196+
$variables = $this->betterNodeFinder->findInstancesOf($returnedExpr, [Variable::class]);
197+
foreach ($variables as $variable) {
198+
$variableName = $this->getName($variable);
199+
if (! is_string($variableName)) {
200+
continue;
201+
}
202+
203+
if ($variableName === 'this') {
204+
continue;
205+
}
206+
207+
if (in_array($variableName, $paramNames, true)) {
208+
continue;
209+
}
210+
211+
if (in_array($variableName, $existingUseNames, true)) {
212+
continue;
213+
}
214+
215+
if (in_array($variableName, $seenNames, true)) {
216+
continue;
217+
}
218+
219+
$seenNames[] = $variableName;
220+
$closureUses[] = new ClosureUse(new Variable($variableName));
221+
}
222+
223+
return $closureUses;
224+
}
225+
174226
private function isLastStmtReturnTrue(Closure $closure): bool
175227
{
176228
$lastStmt = $closure->stmts[count($closure->stmts) - 1];

0 commit comments

Comments
 (0)