[PHP 8.0] Move more logic to MatchFactory (#2802)

Co-authored-by: GitHub Action <action@github.com>
rectorphp · Aug 19, 2022 · 4bc378b · 4bc378b
1 parent 6712529
commit 4bc378b
Show file tree

Hide file tree

Showing 3 changed files with 125 additions and 70 deletions.
diff --git a/rules/Php80/NodeFactory/MatchFactory.php b/rules/Php80/NodeFactory/MatchFactory.php
@@ -5,22 +5,111 @@
 namespace Rector\Php80\NodeFactory;
 
 use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\ArrayDimFetch;
+use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Match_;
+use PhpParser\Node\Expr\Throw_;
+use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\Return_;
+use PhpParser\Node\Stmt\Throw_ as ThrowsStmt;
+use Rector\Php80\Enum\MatchKind;
+use Rector\Php80\NodeAnalyzer\MatchSwitchAnalyzer;
 use Rector\Php80\ValueObject\CondAndExpr;
+use Rector\Php80\ValueObject\MatchResult;
 
 final class MatchFactory
 {
     public function __construct(
-        private readonly MatchArmsFactory $matchArmsFactory
+        private readonly MatchArmsFactory $matchArmsFactory,
+        private readonly MatchSwitchAnalyzer $matchSwitchAnalyzer
     ) {
     }
 
     /**
      * @param CondAndExpr[] $condAndExprs
      */
-    public function createFromCondAndExprs(Expr $condExpr, array $condAndExprs): Match_
+    public function createFromCondAndExprs(Expr $condExpr, array $condAndExprs, ?Stmt $nextStmt): ?MatchResult
     {
         $matchArms = $this->matchArmsFactory->createFromCondAndExprs($condAndExprs);
-        return new Match_($condExpr, $matchArms);
+        $match = new Match_($condExpr, $matchArms);
+
+        // implicit return default after switch
+        if ($nextStmt instanceof Return_ && $nextStmt->expr instanceof Expr) {
+            return $this->processImplicitReturnAfterSwitch($match, $condAndExprs, $nextStmt->expr);
+        }
+
+        if ($nextStmt instanceof ThrowsStmt) {
+            return $this->processImplicitThrowsAfterSwitch($match, $condAndExprs, $nextStmt->expr);
+        }
+
+        return new MatchResult($match, false);
+    }
+
+    /**
+     * @param CondAndExpr[] $condAndExprs
+     */
+    private function processImplicitReturnAfterSwitch(
+        Match_ $match,
+        array $condAndExprs,
+        Expr $returnExpr
+    ): ?MatchResult {
+        if ($this->matchSwitchAnalyzer->hasDefaultValue($match)) {
+            return new MatchResult($match, false);
+        }
+
+        $expr = $this->resolveAssignVar($condAndExprs);
+        if ($expr instanceof ArrayDimFetch) {
+            return null;
+        }
+
+        $shouldRemoveNextStmt = false;
+        if (! $expr instanceof Expr) {
+            $shouldRemoveNextStmt = true;
+        }
+
+        $condAndExprs[] = new CondAndExpr([], $returnExpr, MatchKind::RETURN);
+
+        $matchArms = $this->matchArmsFactory->createFromCondAndExprs($condAndExprs);
+        $wrappedMatch = new Match_($match->cond, $matchArms);
+
+        return new MatchResult($wrappedMatch, $shouldRemoveNextStmt);
+    }
+
+    /**
+     * @param CondAndExpr[] $condAndExprs
+     */
+    private function resolveAssignVar(array $condAndExprs): ?Expr
+    {
+        foreach ($condAndExprs as $condAndExpr) {
+            $expr = $condAndExpr->getExpr();
+            if (! $expr instanceof Assign) {
+                continue;
+            }
+
+            return $expr->var;
+        }
+
+        return null;
+    }
+
+    /**
+     * @param CondAndExpr[] $condAndExprs
+     */
+    private function processImplicitThrowsAfterSwitch(
+        Match_ $match,
+        array $condAndExprs,
+        Expr $throwExpr
+    ): MatchResult {
+        if ($this->matchSwitchAnalyzer->hasDefaultValue($match)) {
+            return new MatchResult($match, false);
+        }
+
+        $throw = new Throw_($throwExpr);
+        $condAndExprs[] = new CondAndExpr([], $throw, MatchKind::RETURN);
+
+        $matchArms = $this->matchArmsFactory->createFromCondAndExprs($condAndExprs);
+        $wrappedMatch = new Match_($match->cond, $matchArms);
+
+        return new MatchResult($wrappedMatch, true);
     }
 }
diff --git a/rules/Php80/Rector/Switch_/ChangeSwitchToMatchRector.php b/rules/Php80/Rector/Switch_/ChangeSwitchToMatchRector.php
@@ -6,7 +6,6 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr;
-use PhpParser\Node\Expr\ArrayDimFetch;
 use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\Match_;
 use PhpParser\Node\Expr\Throw_;
@@ -15,7 +14,6 @@
 use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Return_;
 use PhpParser\Node\Stmt\Switch_;
-use PhpParser\Node\Stmt\Throw_ as ThrowsStmt;
 use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\ValueObject\PhpVersionFeature;
@@ -25,6 +23,7 @@
 use Rector\Php80\NodeFactory\MatchFactory;
 use Rector\Php80\NodeResolver\SwitchExprsResolver;
 use Rector\Php80\ValueObject\CondAndExpr;
+use Rector\Php80\ValueObject\MatchResult;
 use Rector\VersionBonding\Contract\MinPhpVersionInterface;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -125,15 +124,15 @@ public function refactor(Node $node): ?Node
                 }
             }
 
-            $match = $this->matchFactory->createFromCondAndExprs($stmt->cond, $condAndExprs);
-
-            // implicit return default after switch
-            $match = $this->processImplicitReturnAfterSwitch($match, $condAndExprs, $nextStmt);
-            if (! $match instanceof Match_) {
+            $matchResult = $this->matchFactory->createFromCondAndExprs($stmt->cond, $condAndExprs, $nextStmt);
+            if (! $matchResult instanceof MatchResult) {
                 continue;
             }
 
-            $match = $this->processImplicitThrowsAfterSwitch($stmt, $match, $condAndExprs, $nextStmt);
+            $match = $matchResult->getMatch();
+            if ($matchResult->shouldRemoveNextStmt()) {
+                unset($node->stmts[$key + 1]);
+            }
 
             $assignVar = $this->resolveAssignVar($condAndExprs);
             $hasDefaultValue = $this->matchSwitchAnalyzer->hasDefaultValue($match);
@@ -238,65 +237,6 @@ private function resolveAssignVar(array $condAndExprs): ?Expr
         return null;
     }
 
-    /**
-     * @param CondAndExpr[] $condAndExprs
-     */
-    private function processImplicitReturnAfterSwitch(
-        Match_ $match,
-        array $condAndExprs,
-        ?Stmt $nextStmt
-    ): ?Match_ {
-        if (! $nextStmt instanceof Return_) {
-            return $match;
-        }
-
-        $returnedExpr = $nextStmt->expr;
-        if (! $returnedExpr instanceof Expr) {
-            return $match;
-        }
-
-        if ($this->matchSwitchAnalyzer->hasDefaultValue($match)) {
-            return $match;
-        }
-
-        $assignVar = $this->resolveAssignVar($condAndExprs);
-        if ($assignVar instanceof ArrayDimFetch) {
-            return null;
-        }
-
-        if (! $assignVar instanceof Expr) {
-            $this->removeNode($nextStmt);
-        }
-
-        $condAndExprs[] = new CondAndExpr([], $returnedExpr, MatchKind::RETURN);
-        return $this->matchFactory->createFromCondAndExprs($match->cond, $condAndExprs);
-    }
-
-    /**
-     * @param CondAndExpr[] $condAndExprs
-     */
-    private function processImplicitThrowsAfterSwitch(
-        Switch_ $switch,
-        Match_ $match,
-        array $condAndExprs,
-        ?Stmt $nextStmt
-    ): Match_ {
-        if (! $nextStmt instanceof ThrowsStmt) {
-            return $match;
-        }
-
-        if ($this->matchSwitchAnalyzer->hasDefaultValue($match)) {
-            return $match;
-        }
-
-        $this->removeNode($nextStmt);
-
-        $throw = new Throw_($nextStmt->expr);
-
-        $condAndExprs[] = new CondAndExpr([], $throw, MatchKind::RETURN);
-        return $this->matchFactory->createFromCondAndExprs($switch->cond, $condAndExprs);
-    }
-
     private function isFollowedByReturnWithExprUsage(Stmt|null $nextStmt, Expr $expr): bool
     {
         if (! $nextStmt instanceof Return_) {

diff --git a/rules/Php80/ValueObject/MatchResult.php b/rules/Php80/ValueObject/MatchResult.php
@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Php80\ValueObject;
+
+use PhpParser\Node\Expr\Match_;
+
+final class MatchResult
+{
+    public function __construct(
+        private readonly Match_ $match,
+        private readonly bool $shouldRemoveNextStmt
+    ) {
+    }
+
+    public function getMatch(): Match_
+    {
+        return $this->match;
+    }
+
+    public function shouldRemoveNextStmt(): bool
+    {
+        return $this->shouldRemoveNextStmt;
+    }
+}