Use more readable ifs on will return in WithConsecutiveRector (#380)

Author: TomasVotruba

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

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

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

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;

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
     {