Remove NEXT_NODE from SimplifyIfReturnBoolRector (#3915)

rules/CodeQuality/Rector/If_/SimplifyIfReturnBoolRector.php

@@ -14,9 +14,9 @@
 use PhpParser\Node\Stmt\Return_;
 use Rector\BetterPhpDocParser\Comment\CommentsMerger;
 use Rector\CodeQuality\NodeManipulator\ExprBoolCaster;
+use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\Contract\PhpParser\NodePrinterInterface;
 use Rector\Core\Rector\AbstractRector;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -46,7 +46,9 @@ public function getRuleDefinition(): RuleDefinition
 return false;
 CODE_SAMPLE
                     ,
-                    'return strpos($docToken->getContent(), "\n") === false;'
+                    <<<'CODE_SAMPLE'
+return strpos($docToken->getContent(), "\n") === false;
+CODE_SAMPLE
                 ),
             ]
         );
@@ -57,55 +59,61 @@ public function getRuleDefinition(): RuleDefinition
      */
     public function getNodeTypes(): array
     {
-        return [If_::class];
+        return [StmtsAwareInterface::class];
     }
 
     /**
-     * @param If_ $node
+     * @param StmtsAwareInterface $node
      */
     public function refactor(Node $node): ?Node
     {
-        if ($this->shouldSkip($node)) {
+        if ($node->stmts === null) {
             return null;
         }
 
-        /** @var Return_ $ifInnerNode */
-        $ifInnerNode = $node->stmts[0];
+        foreach ($node->stmts as $key => $stmt) {
+            if (! $stmt instanceof Return_) {
+                continue;
+            }
 
-        /** @var Return_ $nextNode */
-        $nextNode = $node->getAttribute(AttributeKey::NEXT_NODE);
+            $previousStmt = $node->stmts[$key - 1] ?? null;
+            if (! $previousStmt instanceof If_) {
+                continue;
+            }
 
-        $innerIfInnerNode = $ifInnerNode->expr;
-        if (! $innerIfInnerNode instanceof Expr) {
-            return null;
-        }
+            $if = $previousStmt;
+            if ($this->shouldSkipIfAndReturn($previousStmt, $stmt)) {
+                continue;
+            }
 
-        if ($this->valueResolver->isTrue($innerIfInnerNode)) {
-            $newReturnNode = $this->processReturnTrue($node, $nextNode);
-        } elseif ($this->valueResolver->isFalse($innerIfInnerNode)) {
-            /** @var Expr $expr */
-            $expr = $nextNode->expr;
-            if ($node->cond instanceof NotIdentical && $this->valueResolver->isTrue($expr)) {
-                $node->cond = new Identical($node->cond->left, $node->cond->right);
-                $newReturnNode = $this->processReturnTrue($node, $nextNode);
-            } else {
-                $newReturnNode = $this->processReturnFalse($node, $nextNode);
+            $return = $stmt;
+
+            /** @var Return_ $ifInnerNode */
+            $ifInnerNode = $if->stmts[0];
+
+            $innerIfInnerNode = $ifInnerNode->expr;
+            if (! $innerIfInnerNode instanceof Expr) {
+                continue;
             }
-        } else {
-            return null;
-        }
 
-        if (! $newReturnNode instanceof Return_) {
-            return null;
-        }
+            $newReturn = $this->resolveReturn($innerIfInnerNode, $if, $return);
+            if (! $newReturn instanceof Return_) {
+                continue;
+            }
 
-        $this->commentsMerger->keepComments($newReturnNode, [$node, $ifInnerNode, $nextNode, $newReturnNode]);
-        $this->removeNode($nextNode);
+            $this->commentsMerger->keepComments($newReturn, [$if, $return, $ifInnerNode]);
 
-        return $newReturnNode;
+            // remove previous IF
+            unset($node->stmts[$key - 1]);
+            $node->stmts[$key] = $newReturn;
+
+            return $node;
+        }
+
+        return null;
     }
 
-    private function shouldSkip(If_ $if): bool
+    private function shouldSkipIfAndReturn(If_ $if, Return_ $return): bool
     {
         if ($if->elseifs !== []) {
             return true;
@@ -129,23 +137,18 @@ private function shouldSkip(If_ $if): bool
             return true;
         }
 
-        $nextNode = $if->getAttribute(AttributeKey::NEXT_NODE);
-        if (! $nextNode instanceof Return_) {
-            return true;
-        }
-
-        if (! $nextNode->expr instanceof Expr) {
+        if (! $return->expr instanceof Expr) {
             return true;
         }
 
         // negate + negate → skip for now
         if (! $this->valueResolver->isFalse($returnedExpr)) {
-            return ! $this->valueResolver->isTrueOrFalse($nextNode->expr);
+            return ! $this->valueResolver->isTrueOrFalse($return->expr);
         }
 
         $condString = $this->nodePrinter->print($if->cond);
         if (! \str_contains($condString, '!=')) {
-            return ! $this->valueResolver->isTrueOrFalse($nextNode->expr);
+            return ! $this->valueResolver->isTrueOrFalse($return->expr);
         }
 
         return ! $if->cond instanceof NotIdentical;
@@ -221,4 +224,24 @@ private function isIfWithSingleReturnExpr(If_ $if): bool
         // return must have value
         return $ifInnerNode->expr instanceof Expr;
     }
+
+    private function resolveReturn(Expr $innerExpr, If_ $if, Return_ $return): ?Return_
+    {
+        if ($this->valueResolver->isTrue($innerExpr)) {
+            return $this->processReturnTrue($if, $return);
+        }
+
+        if ($this->valueResolver->isFalse($innerExpr)) {
+            /** @var Expr $expr */
+            $expr = $return->expr;
+            if ($if->cond instanceof NotIdentical && $this->valueResolver->isTrue($expr)) {
+                $if->cond = new Identical($if->cond->left, $if->cond->right);
+                return $this->processReturnTrue($if, $return);
+            }
+
+            return $this->processReturnFalse($if, $return);
+        }
+
+        return null;
+    }
 }