From bd462a6a05e2efb6fc752676a7061d5509ed298c Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 6 Oct 2024 09:17:40 +0200 Subject: [PATCH] Use more readable ifs on will return in WithConsecutiveRector (#380) --- ...ith_on_consecutive_without_expects.php.inc | 16 ++++---- ...h_will_return_on_consecutive_calls.php.inc | 16 ++++---- .../WithConsecutiveRector.php | 29 ++++++-------- src/NodeFactory/ConsecutiveIfsFactory.php | 39 ++++++++++++++++++- .../WithConsecutiveMatchFactory.php | 20 ++++++---- 5 files changed, 79 insertions(+), 41 deletions(-) diff --git a/rules-tests/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector/Fixture/combine_with_on_consecutive_without_expects.php.inc b/rules-tests/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector/Fixture/combine_with_on_consecutive_without_expects.php.inc index 61deb3cc..82b0a243 100644 --- a/rules-tests/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector/Fixture/combine_with_on_consecutive_without_expects.php.inc +++ b/rules-tests/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector/Fixture/combine_with_on_consecutive_without_expects.php.inc @@ -31,14 +31,14 @@ final class CombineWithOnConsecutiveWithoutExcepts extends TestCase { $matcher = $this->exactly(2); $this->userServiceMock->expects($matcher)->method('prepare')->willReturnCallback(function (...$parameters) use ($matcher) { - match ($matcher->numberOfInvocations()) { - 1 => self::assertEquals([1, 2], $parameters), - 2 => self::assertEquals([3, 4], $parameters), - }; - return match ($matcher->numberOfInvocations()) { - 1 => 5, - 2 => 6, - }; + if ($matcher->numberOfInvocations() === 1) { + $this->assertSame([1, 2], $parameters); + return 5; + } + if ($matcher->numberOfInvocations() === 2) { + $this->assertSame([3, 4], $parameters); + return 6; + } }); } } diff --git a/rules-tests/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector/Fixture/combine_with_will_return_on_consecutive_calls.php.inc b/rules-tests/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector/Fixture/combine_with_will_return_on_consecutive_calls.php.inc index 4b957467..f8206429 100644 --- a/rules-tests/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector/Fixture/combine_with_will_return_on_consecutive_calls.php.inc +++ b/rules-tests/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector/Fixture/combine_with_will_return_on_consecutive_calls.php.inc @@ -33,14 +33,14 @@ final class CombineWithWillReturnOnConsecutiveCalls extends TestCase $matcher = self::exactly(2); $this->userServiceMock->expects($matcher) ->method('prepare')->willReturnCallback(function (...$parameters) use ($matcher) { - match ($matcher->numberOfInvocations()) { - 1 => self::assertEquals([1, 2], $parameters), - 2 => self::assertEquals([3, 4], $parameters), - }; - return match ($matcher->numberOfInvocations()) { - 1 => 5, - 2 => 6, - }; + if ($matcher->numberOfInvocations() === 1) { + $this->assertSame([1, 2], $parameters); + return 5; + } + if ($matcher->numberOfInvocations() === 2) { + $this->assertSame([3, 4], $parameters); + return 6; + } }); } } diff --git a/rules/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector.php b/rules/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector.php index a2395ff7..f4d48045 100644 --- a/rules/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector.php +++ b/rules/PHPUnit100/Rector/StmtsAwareInterface/WithConsecutiveRector.php @@ -10,12 +10,10 @@ use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\Closure; -use PhpParser\Node\Expr\Match_; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\StaticCall; use PhpParser\Node\Expr\Variable; use PhpParser\Node\Identifier; -use PhpParser\Node\MatchArm; use PhpParser\Node\Param; use PhpParser\Node\Scalar\LNumber; use PhpParser\Node\Stmt; @@ -27,7 +25,7 @@ use Rector\PhpParser\Node\BetterNodeFinder; use Rector\PHPUnit\Enum\ConsecutiveVariable; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; -use Rector\PHPUnit\NodeFactory\MatcherInvocationCountMethodCallNodeFactory; +use Rector\PHPUnit\NodeFactory\ConsecutiveIfsFactory; use Rector\PHPUnit\NodeFactory\WithConsecutiveMatchFactory; use Rector\Rector\AbstractRector; use Rector\ValueObject\PhpVersion; @@ -49,7 +47,7 @@ public function __construct( private readonly TestsNodeAnalyzer $testsNodeAnalyzer, private readonly BetterNodeFinder $betterNodeFinder, private readonly WithConsecutiveMatchFactory $withConsecutiveMatchFactory, - private readonly MatcherInvocationCountMethodCallNodeFactory $matcherInvocationCountMethodCallNodeFactory, + private readonly ConsecutiveIfsFactory $consecutiveIfsFactory, ) { } @@ -142,9 +140,15 @@ public function refactor(Node $node) $returnStmts[] = $this->createWillReturnArgument($willReturnArgument); } + $areIfsPreferred = false; + $willReturnOnConsecutiveCallsArgument = $this->findMethodCall($node, 'willReturnOnConsecutiveCalls'); if ($willReturnOnConsecutiveCallsArgument instanceof MethodCall) { - $returnStmts[] = $this->createReturnMatch($willReturnOnConsecutiveCallsArgument); + $returnStmts = $this->consecutiveIfsFactory->createCombinedIfs( + $withConsecutiveMethodCall, + $willReturnOnConsecutiveCallsArgument + ); + $areIfsPreferred = true; } $willThrowException = $this->findMethodCall($node, 'willThrowException'); @@ -195,6 +199,7 @@ public function refactor(Node $node) $referenceVariable, $expectsCall, $node, + $areIfsPreferred ); } @@ -304,11 +309,13 @@ private function refactorToWillReturnCallback( Expr|Variable|null $referenceVariable, StaticCall|MethodCall $expectsCall, Expression $expression, + bool $areIfsPreferred ): array { $closure = $this->withConsecutiveMatchFactory->createClosure( $withConsecutiveMethodCall, $returnStmts, $referenceVariable, + $areIfsPreferred ); $withConsecutiveMethodCall->name = new Identifier('willReturnCallback'); @@ -424,16 +431,4 @@ private function createWillReturnArgument(MethodCall $willReturnArgumentMethodCa return new Return_(new ArrayDimFetch($parametersVariable, $firstArgs->value)); } - - private function createReturnMatch(MethodCall $willReturnOnConsecutiveCallsMethodCall): Return_ - { - $numberOfInvocationsMethodCall = $this->matcherInvocationCountMethodCallNodeFactory->create(); - - $matchArms = []; - foreach ($willReturnOnConsecutiveCallsMethodCall->getArgs() as $key => $arg) { - $matchArms[] = new MatchArm([new LNumber($key + 1)], $arg->value); - } - - return new Return_(new Match_($numberOfInvocationsMethodCall, $matchArms)); - } } diff --git a/src/NodeFactory/ConsecutiveIfsFactory.php b/src/NodeFactory/ConsecutiveIfsFactory.php index 267c46d8..8c8cbcc2 100644 --- a/src/NodeFactory/ConsecutiveIfsFactory.php +++ b/src/NodeFactory/ConsecutiveIfsFactory.php @@ -17,6 +17,7 @@ use PhpParser\Node\Scalar\LNumber; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\If_; +use PhpParser\Node\Stmt\Return_; use Rector\Exception\NotImplementedYetException; use Rector\Exception\ShouldNotHappenException; use Rector\NodeNameResolver\NodeNameResolver; @@ -30,6 +31,40 @@ public function __construct( ) { } + /** + * @return If_[] + */ + public function createCombinedIfs( + MethodCall $withConsecutiveMethodCall, + MethodCall $willReturnOnConsecutiveCallsArgument + ): array { + $ifs = []; + $matcherMethodCall = $this->matcherInvocationCountMethodCallNodeFactory->create(); + + $willReturnArgs = $willReturnOnConsecutiveCallsArgument->getArgs(); + + foreach ($withConsecutiveMethodCall->getArgs() as $key => $withConsecutiveArg) { + if (! $withConsecutiveArg->value instanceof Array_) { + throw new ShouldNotHappenException(); + } + + $ifStmts = []; + $args = [new Arg($withConsecutiveArg->value), new Arg(new Variable('parameters'))]; + $ifStmts[] = new Expression(new MethodCall(new Variable('this'), 'assertSame', $args)); + + $willReturnArg = $willReturnArgs[$key] ?? null; + if ($willReturnArg instanceof Arg) { + $ifStmts[] = new Return_($willReturnArg->value); + } + + $ifs[] = new If_(new Identical($matcherMethodCall, new LNumber($key + 1)), [ + 'stmts' => $ifStmts, + ]); + } + + return $ifs; + } + /** * @return If_[] */ @@ -52,7 +87,9 @@ public function createIfs(MethodCall $withConsecutiveMethodCall): array } if (! $assertArrayItem->value instanceof MethodCall) { - throw new NotImplementedYetException(); + $args = [new Arg($assertArrayItem), new Arg(new Variable('parameters'))]; + $ifStmts[] = new Expression(new MethodCall(new Variable('this'), 'assertSame', $args)); + continue; } $assertMethodCall = $assertArrayItem->value; diff --git a/src/NodeFactory/WithConsecutiveMatchFactory.php b/src/NodeFactory/WithConsecutiveMatchFactory.php index 947d44df..f2276035 100644 --- a/src/NodeFactory/WithConsecutiveMatchFactory.php +++ b/src/NodeFactory/WithConsecutiveMatchFactory.php @@ -21,6 +21,7 @@ use PhpParser\Node\Scalar\LNumber; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Expression; +use PhpParser\Node\Stmt\If_; use PhpParser\NodeFinder; use Rector\NodeNameResolver\NodeNameResolver; use Rector\PHPUnit\Enum\ConsecutiveVariable; @@ -43,17 +44,22 @@ public function __construct( public function createClosure( MethodCall $withConsecutiveMethodCall, array $returnStmts, - Variable|Expr|null $referenceVariable + Variable|Expr|null $referenceVariable, + bool $areIfsPreferred ): Closure { $matcherVariable = new Variable(ConsecutiveVariable::MATCHER); $usedVariables = $this->usedVariablesResolver->resolveUsedVariables($withConsecutiveMethodCall, $returnStmts); - $matchOrIfs = $this->createParametersMatch($withConsecutiveMethodCall); - - if (is_array($matchOrIfs)) { - $closureStmts = array_merge($matchOrIfs, $returnStmts); + if ($areIfsPreferred) { + $closureStmts = $returnStmts; } else { - $closureStmts = [new Expression($matchOrIfs), ...$returnStmts]; + $matchOrIfs = $this->createParametersMatch($withConsecutiveMethodCall); + + if (is_array($matchOrIfs)) { + $closureStmts = array_merge($matchOrIfs, $returnStmts); + } else { + $closureStmts = [new Expression($matchOrIfs), ...$returnStmts]; + } } $parametersParam = new Param(new Variable(ConsecutiveVariable::PARAMETERS)); @@ -68,7 +74,7 @@ public function createClosure( } /** - * @return Match_|MethodCall|Stmt\If_[] + * @return Match_|MethodCall|If_[] */ public function createParametersMatch(MethodCall $withConsecutiveMethodCall): Match_|MethodCall|array {