From 7e8bd1230bedece2da067002c202af7a9524589c Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Mon, 3 Nov 2025 10:52:45 +0100 Subject: [PATCH] add multi callbacks support --- .../Fixture/include_no_args.php.inc | 51 +++++ .../Fixture/multi_callbacks.php.inc | 51 +++++ .../Fixture/skip_no_args.php.inc | 22 -- .../NodeAnalyser/ClosureUsesResolver.php | 74 +++++++ .../FromBinaryAndAssertExpressionsFactory.php | 12 +- ...backIdenticalToStandaloneAssertsRector.php | 202 ++++++++---------- 6 files changed, 275 insertions(+), 137 deletions(-) create mode 100644 rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/include_no_args.php.inc create mode 100644 rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/multi_callbacks.php.inc delete mode 100644 rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/skip_no_args.php.inc create mode 100644 rules/CodeQuality/NodeAnalyser/ClosureUsesResolver.php diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/include_no_args.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/include_no_args.php.inc new file mode 100644 index 00000000..513ad4e0 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/include_no_args.php.inc @@ -0,0 +1,51 @@ +getMockBuilder('AnyType')->getMock(); + + $someMock->expects($this->any()) + ->method('trans') + ->with($this->callback(function (): bool { + $args= [1, 2, 3]; + return count($args) === 5 && $args[0] === 'some_value'; + })); + } +} + +?> +----- +getMockBuilder('AnyType')->getMock(); + + $someMock->expects($this->any()) + ->method('trans') + ->with($this->callback(function (): bool { + $args= [1, 2, 3]; + $this->assertCount(5, $args); + $this->assertSame('some_value', $args[0]); + return true; + })); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/multi_callbacks.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/multi_callbacks.php.inc new file mode 100644 index 00000000..604bac95 --- /dev/null +++ b/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/multi_callbacks.php.inc @@ -0,0 +1,51 @@ +getMockBuilder('AnyType')->getMock(); + + $someMock->expects($this->any()) + ->method('trans') + ->with( + $this->callback(fn (array $args) => array_key_exists(5, $args)), + $this->callback(fn (array $args) => array_key_exists(50, $args)), + ); + } +} + +?> +----- +getMockBuilder('AnyType')->getMock(); + + $someMock->expects($this->any()) + ->method('trans') + ->with( + $this->callback(function (array $args): bool { + $this->assertArrayHasKey(5, $args); + return true; + }), + $this->callback(function (array $args): bool { + $this->assertArrayHasKey(50, $args); + return true; + }), + ); + } +} + +?> diff --git a/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/skip_no_args.php.inc b/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/skip_no_args.php.inc deleted file mode 100644 index db74a91b..00000000 --- a/rules-tests/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector/Fixture/skip_no_args.php.inc +++ /dev/null @@ -1,22 +0,0 @@ -getMockBuilder('AnyType')->getMock(); - - $someMock->expects($this->any()) - ->method('trans') - ->with($this->callback(function (): bool { - $args= [1, 2, 3]; - return count($args) === 5 && $args[0] === 'some_value'; - })); - } -} diff --git a/rules/CodeQuality/NodeAnalyser/ClosureUsesResolver.php b/rules/CodeQuality/NodeAnalyser/ClosureUsesResolver.php new file mode 100644 index 00000000..2f32eb63 --- /dev/null +++ b/rules/CodeQuality/NodeAnalyser/ClosureUsesResolver.php @@ -0,0 +1,74 @@ +betterNodeFinder->findInstancesOfScoped( + $arrowFunction->getStmts(), + Variable::class + ); + + $paramNames = $this->resolveParamNames($arrowFunction); + + $externalVariableNames = []; + + foreach ($arrowFunctionVariables as $arrowFunctionVariable) { + // skip those defined in params + if ($this->nodeNameResolver->isNames($arrowFunctionVariable, $paramNames)) { + continue; + } + + $variableName = $this->nodeNameResolver->getName($arrowFunctionVariable); + if (! is_string($variableName)) { + continue; + } + + $externalVariableNames[] = $variableName; + } + + $externalVariableNames = array_unique($externalVariableNames); + $externalVariableNames = array_diff($externalVariableNames, ['this']); + + $closureUses = []; + foreach ($externalVariableNames as $externalVariableName) { + $closureUses[] = new ClosureUse(new Variable($externalVariableName)); + } + + return $closureUses; + } + + /** + * @return string[] + */ + private function resolveParamNames(ArrowFunction $arrowFunction): array + { + $paramNames = []; + foreach ($arrowFunction->getParams() as $param) { + $paramNames[] = $this->nodeNameResolver->getName($param); + } + + return $paramNames; + } +} diff --git a/rules/CodeQuality/NodeFactory/FromBinaryAndAssertExpressionsFactory.php b/rules/CodeQuality/NodeFactory/FromBinaryAndAssertExpressionsFactory.php index c4bc4327..e8b880f8 100644 --- a/rules/CodeQuality/NodeFactory/FromBinaryAndAssertExpressionsFactory.php +++ b/rules/CodeQuality/NodeFactory/FromBinaryAndAssertExpressionsFactory.php @@ -30,9 +30,9 @@ public function __construct( /** * @param Expr[] $exprs - * @return Stmt[]|null + * @return Stmt[] */ - public function create(array $exprs): ?array + public function create(array $exprs): array { $assertMethodCalls = []; @@ -62,7 +62,7 @@ public function create(array $exprs): ?array ); } else { // not supported yet - return null; + return []; } } @@ -102,7 +102,7 @@ public function create(array $exprs): ?array } // unclear, fallback to no change - return null; + return []; } // create assertSame() @@ -113,12 +113,12 @@ public function create(array $exprs): ?array ); } else { // not supported expr - return null; + return []; } } if ($assertMethodCalls === []) { - return null; + return []; } // to keep order from binary diff --git a/rules/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector.php b/rules/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector.php index eaa6fd08..dbab7cea 100644 --- a/rules/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector.php +++ b/rules/CodeQuality/Rector/MethodCall/WithCallbackIdenticalToStandaloneAssertsRector.php @@ -5,7 +5,7 @@ namespace Rector\PHPUnit\CodeQuality\Rector\MethodCall; use PhpParser\Node; -use PhpParser\Node\ClosureUse; +use PhpParser\Node\Arg; use PhpParser\Node\Expr; use PhpParser\Node\Expr\ArrowFunction; use PhpParser\Node\Expr\BinaryOp\BooleanAnd; @@ -17,10 +17,10 @@ use PhpParser\Node\Expr\Instanceof_; use PhpParser\Node\Expr\Isset_; use PhpParser\Node\Expr\MethodCall; -use PhpParser\Node\Expr\Variable; use PhpParser\Node\Identifier; +use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Return_; -use Rector\PhpParser\Node\BetterNodeFinder; +use Rector\PHPUnit\CodeQuality\NodeAnalyser\ClosureUsesResolver; use Rector\PHPUnit\CodeQuality\NodeFactory\FromBinaryAndAssertExpressionsFactory; use Rector\PHPUnit\CodeQuality\ValueObject\ArgAndFunctionLike; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; @@ -34,9 +34,9 @@ final class WithCallbackIdenticalToStandaloneAssertsRector extends AbstractRector { public function __construct( + private readonly ClosureUsesResolver $closureUsesResolver, private readonly TestsNodeAnalyzer $testsNodeAnalyzer, private readonly FromBinaryAndAssertExpressionsFactory $fromBinaryAndAssertExpressionsFactory, - private readonly BetterNodeFinder $betterNodeFinder, ) { } @@ -108,82 +108,68 @@ public function refactor(Node $node): MethodCall|null return null; } - $argAndFunctionLike = $this->matchWithCallbackInnerClosure($node); - if (! $argAndFunctionLike instanceof ArgAndFunctionLike) { - return null; - } + $hasChanged = false; - if (! $argAndFunctionLike->hasParams()) { + if (! $this->isName($node->name, 'with')) { return null; } - $innerSoleExpr = $this->matchInnerSoleExpr($argAndFunctionLike->getFunctionLike()); - if ($innerSoleExpr instanceof BooleanAnd) { - $joinedExprs = $this->extractJoinedExprs($innerSoleExpr); - } elseif ($innerSoleExpr instanceof Identical || $innerSoleExpr instanceof Instanceof_ || $innerSoleExpr instanceof Isset_ || ($innerSoleExpr instanceof FuncCall && $this->isName( - $innerSoleExpr->name, - 'array_key_exists' - )) || $innerSoleExpr instanceof Equal) { - $joinedExprs = [$innerSoleExpr]; - } else { - return null; - } + foreach ($node->getArgs() as $arg) { + $argAndFunctionLike = $this->matchCallbackArgAndFunctionLike($arg); + if (! $argAndFunctionLike instanceof ArgAndFunctionLike) { + continue; + } - if ($joinedExprs === null || $joinedExprs === []) { - return null; - } + $joinedExprs = $this->resolveInnerReturnJoinedExprs($argAndFunctionLike); + if ($joinedExprs === []) { + continue; + } - $assertExpressions = $this->fromBinaryAndAssertExpressionsFactory->create($joinedExprs); - if ($assertExpressions === null) { - return null; - } + $assertExpressions = $this->fromBinaryAndAssertExpressionsFactory->create($joinedExprs); + if ($assertExpressions === []) { + continue; + } - // all stmts but last - $functionLike = $argAndFunctionLike->getFunctionLike(); + $nonReturnCallbackStmts = $this->resolveNonReturnCallbackStmts($argAndFunctionLike); - if ($functionLike instanceof Closure) { - $functionStmts = $functionLike->stmts; + // last si return true; + $assertExpressions[] = new Return_($this->nodeFactory->createTrue()); - if (count($functionStmts) >= 2) { - unset($functionStmts[array_key_last($functionStmts)]); - } else { - $functionStmts = []; - } - } else { - $functionStmts = []; - } + $innerFunctionLike = $argAndFunctionLike->getFunctionLike(); - // last si return true; - $assertExpressions[] = new Return_($this->nodeFactory->createTrue()); + if ($innerFunctionLike instanceof Closure) { + $innerFunctionLike->stmts = array_merge($nonReturnCallbackStmts, $assertExpressions); + } else { + // arrow function -> flip to closure + $functionLikeInArg = $argAndFunctionLike->getArg(); - $innerFunctionLike = $argAndFunctionLike->getFunctionLike(); + $externalVariables = $this->closureUsesResolver->resolveFromArrowFunction($innerFunctionLike); - if ($innerFunctionLike instanceof Closure) { - $innerFunctionLike->stmts = array_merge($functionStmts, $assertExpressions); - } else { - // arrow function -> flip to closure - $functionLikeInArg = $argAndFunctionLike->getArg(); + $closure = new Closure([ + 'params' => $argAndFunctionLike->getFunctionLike() + ->params, + 'stmts' => $assertExpressions, + 'returnType' => new Identifier('bool'), + 'uses' => $externalVariables, + ]); - $externalVariables = $this->resolveExternalClosureUses($innerFunctionLike); + $functionLikeInArg->value = $closure; + } - $closure = new Closure([ - 'params' => $argAndFunctionLike->getFunctionLike() - ->params, - 'stmts' => $assertExpressions, - 'returnType' => new Identifier('bool'), - 'uses' => $externalVariables, - ]); + $hasChanged = true; + } - $functionLikeInArg->value = $closure; + if (! $hasChanged) { + return null; } return $node; } /** - * @return Expr[]|null + * @return Expr[] */ - private function extractJoinedExprs(BooleanAnd $booleanAnd): ?array + private function extractJoinedExprs(BooleanAnd $booleanAnd): array { // must be full queue of BooleanAnds $joinedExprs = []; @@ -193,7 +179,7 @@ private function extractJoinedExprs(BooleanAnd $booleanAnd): ?array do { // is binary op, but not "&&" if ($currentNode->right instanceof BooleanOr) { - return null; + return []; } $joinedExprs[] = $currentNode->right; @@ -206,87 +192,85 @@ private function extractJoinedExprs(BooleanAnd $booleanAnd): ?array return $joinedExprs; } - private function matchWithCallbackInnerClosure(MethodCall $methodCall): null|ArgAndFunctionLike + private function matchInnerSoleExpr(Closure|ArrowFunction $functionLike): ?Expr { - if (! $this->isName($methodCall->name, 'with')) { + if ($functionLike instanceof Closure) { + foreach ($functionLike->getStmts() as $stmt) { + if (! $stmt instanceof Return_) { + continue; + } + + return $stmt->expr; + } + return null; } - $firstArg = $methodCall->getArgs()[0]; - if (! $firstArg->value instanceof MethodCall) { + return $functionLike->expr; + } + + private function matchCallbackArgAndFunctionLike(Arg $arg): ?ArgAndFunctionLike + { + if (! $arg->value instanceof MethodCall) { return null; } - if (! $this->isName($firstArg->value->name, 'callback')) { + if (! $this->isName($arg->value->name, 'callback')) { return null; } - $callbackMethodCall = $firstArg->value; + $callbackMethodCall = $arg->value; $innerFirstArg = $callbackMethodCall->getArgs()[0]; - if ($innerFirstArg->value instanceof Closure || $innerFirstArg->value instanceof ArrowFunction) { - return new ArgAndFunctionLike($innerFirstArg, $innerFirstArg->value); + if (! $innerFirstArg->value instanceof Closure && ! $innerFirstArg->value instanceof ArrowFunction) { + return null; } - return null; + return new ArgAndFunctionLike($innerFirstArg, $innerFirstArg->value); } - private function matchInnerSoleExpr(Closure|ArrowFunction $functionLike): ?Expr + /** + * @return Expr[] + */ + private function resolveInnerReturnJoinedExprs(ArgAndFunctionLike $argAndFunctionLike): array { - if ($functionLike instanceof Closure) { - foreach ($functionLike->getStmts() as $stmt) { - if (! $stmt instanceof Return_) { - continue; - } - - return $stmt->expr; - } + $innerSoleExpr = $this->matchInnerSoleExpr($argAndFunctionLike->getFunctionLike()); + if ($innerSoleExpr instanceof BooleanAnd) { + return $this->extractJoinedExprs($innerSoleExpr); + } - return null; + if ($innerSoleExpr instanceof Identical || $innerSoleExpr instanceof Instanceof_ || $innerSoleExpr instanceof Isset_ || ($innerSoleExpr instanceof FuncCall && $this->isName( + $innerSoleExpr->name, + 'array_key_exists' + )) || $innerSoleExpr instanceof Equal) { + return [$innerSoleExpr]; } - return $functionLike->expr; + return []; } /** - * @return ClosureUse[] + * @return Stmt[] */ - private function resolveExternalClosureUses(ArrowFunction $arrowFunction): array + private function resolveNonReturnCallbackStmts(ArgAndFunctionLike $argAndFunctionLike): array { - // fill needed uses from arrow function to closure - $arrowFunctionVariables = $this->betterNodeFinder->findInstancesOfScoped( - $arrowFunction->getStmts(), - Variable::class - ); - $paramNames = []; - foreach ($arrowFunction->getParams() as $param) { - $paramNames[] = $this->getName($param); - } + // all stmts but last + $functionLike = $argAndFunctionLike->getFunctionLike(); - $externalVariableNames = []; + if ($functionLike instanceof Closure) { + $functionStmts = $functionLike->stmts; - foreach ($arrowFunctionVariables as $arrowFunctionVariable) { - // skip those defined in params - if ($this->isNames($arrowFunctionVariable, $paramNames)) { - continue; - } + foreach ($functionStmts as $key => $value) { + if (! $value instanceof Return_) { + continue; + } - $variableName = $this->getName($arrowFunctionVariable); - if (! is_string($variableName)) { - continue; + unset($functionStmts[$key]); } - $externalVariableNames[] = $variableName; - } - - $externalVariableNames = array_unique($externalVariableNames); - $externalVariableNames = array_diff($externalVariableNames, ['this']); - - $closureUses = []; - foreach ($externalVariableNames as $externalVariableName) { - $closureUses[] = new ClosureUse(new Variable($externalVariableName)); + return $functionStmts; } - return $closureUses; + return []; } }