[DX] Refactor ChangeNestedIfsToEarlyReturnRector to direct return (#2250)

packages/PostRector/Collector/NodesToAddCollector.php

@@ -41,7 +41,7 @@ final class NodesToAddCollector implements NodeCollectorInterface
     }
 
     /**
-     * @internal Try to avoid using this magic added. Better return directly array of nodes in the Rector rule.
+     * @deprecated Return created nodes right in refactor() method to keep context instead.
      */
     public function addNodeBeforeNode(Node $addedNode, Node $positionNode): void
     {
@@ -58,6 +58,7 @@ final class NodesToAddCollector implements NodeCollectorInterface
 
     /**
      * @param Node[] $addedNodes
+     * @deprecated Return created nodes right in refactor() method to keep context instead.
      */
     public function addNodesAfterNode(array $addedNodes, Node $positionNode): void
     {
@@ -114,6 +115,7 @@ final class NodesToAddCollector implements NodeCollectorInterface
     }
 
     /**
+     * @deprecated Return created nodes right in refactor() method to keep context instead.
      * @param Node[] $newNodes
      */
     public function addNodesBeforeNode(array $newNodes, Node $positionNode): void

phpstan.neon

@@ -111,9 +111,6 @@ parameters:
         -
             message: '#Use explicit return value over magic &reference#'
             path: rules/Php70/EregToPcreTransformer.php
-        -
-            message: '#Use value object over return of values#'
-            path: rules/Php70/EregToPcreTransformer.php
 
         # symplify 9
         - '#Use another value object over array with string\-keys and objects, array<string, ValueObject\>#'
@@ -217,11 +214,6 @@ parameters:
             paths:
                 - packages/BetterPhpDocParser/PhpDocParser/StaticDoctrineAnnotationParser/ArrayParser.php
 
-        -
-            message: '#Use value object over return of values#'
-            paths:
-                - packages/BetterPhpDocParser/PhpDocParser/StaticDoctrineAnnotationParser/ArrayParser.php
-
         -
             message: '#\$this as argument is not allowed\. Refactor method to service composition#'
             paths:
@@ -398,11 +390,6 @@ parameters:
             message: '#Instead of abstract class, use specific service with composition#'
             path: src/Console/Command/AbstractProcessCommand.php
 
-        # json for paralell serialization
-        -
-            message: '#Use value object over return of values#'
-            path: packages/Parallel/Application/ParallelFileProcessor.php
-
         # known value object, nullable due to typed property
         -
             message: '#Cannot call method (.*?)\(\) on (.*?)\\ProcessPool\|null#'
@@ -501,7 +488,12 @@ parameters:
 
         -
             message: '#Use value object over return of values#'
-            path: src/Application/ApplicationFileProcessor.php
+            paths:
+                - src/Application/ApplicationFileProcessor.php
+                - packages/Parallel/Application/ParallelFileProcessor.php
+                - packages/BetterPhpDocParser/PhpDocParser/StaticDoctrineAnnotationParser/ArrayParser.php
+                - rules/EarlyReturn/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php
+                - rules/Php70/EregToPcreTransformer.php
 
         - '#Method Rector\\Core\\Application\\ApplicationFileProcessor\:\:runParallel\(\) should return array\{system_errors\: array<Rector\\Core\\ValueObject\\Error\\SystemError\>, file_diffs\: array<Rector\\Core\\ValueObject\\Reporting\\FileDiff\>\} but returns array#'
 
@@ -592,7 +584,7 @@ parameters:
         - '#Cognitive complexity for "Rector\\Removing\\NodeManipulator\\ComplexNodeRemover\:\:removePropertyAndUsages\(\)" is \d+, keep it under 10#'
 
         # avoid moving rule around, while re-targeting to stmt
-        - '#Class "Rector\\(.*?)\\Rector\\(.*?) has invalid namespace category "(.*?)"\. Pick one of\: "(Expression|Class_)"#'
+        - '#Class "Rector\\(.*?)\\Rector\\(.*?) has invalid namespace category "(.*?)"\. Pick one of\: "(Expression|Class_|NodeTypeGroup)"#'
 
         - '#Cognitive complexity for "Rector\\CodingStyle\\Rector\\ClassMethod\\MakeInheritedMethodVisibilitySameAsParentRector\:\:refactorWithScope\(\)" is 11, keep it under 10#'
         -

rules/EarlyReturn/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php

@@ -7,12 +7,15 @@ namespace Rector\EarlyReturn\Rector\If_;
 use PhpParser\Node;
 use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
 use PhpParser\Node\Expr\BooleanNot;
+use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Function_;
 use PhpParser\Node\Stmt\If_;
 use PhpParser\Node\Stmt\Return_;
+use Rector\CodeQuality\NodeTypeGroup;
 use Rector\Core\NodeManipulator\IfManipulator;
 use Rector\Core\Rector\AbstractRector;
 use Rector\EarlyReturn\NodeTransformer\ConditionInverter;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -73,71 +76,90 @@ CODE_SAMPLE
      */
     public function getNodeTypes(): array
     {
-        return [If_::class];
+        return NodeTypeGroup::STMTS_AWARE;
     }
 
     /**
-     * @param If_ $node
+     * @param Function_|ClassMethod $node
      */
     public function refactor(Node $node): ?Node
     {
-        // A. next node is return
-        $nextNode = $node->getAttribute(AttributeKey::NEXT_NODE);
-        if (! $nextNode instanceof Return_) {
+        $stmts = $node->stmts;
+        if ($stmts === null) {
             return null;
         }
 
-        $nestedIfsWithOnlyReturn = $this->ifManipulator->collectNestedIfsWithOnlyReturn($node);
-        if ($nestedIfsWithOnlyReturn === []) {
-            return null;
-        }
+        foreach ($stmts as $key => $stmt) {
+            $nextStmt = $stmts[$key + 1] ?? null;
+            if (! $nextStmt instanceof Return_) {
+                continue;
+            }
 
-        $this->processNestedIfsWithOnlyReturn($node, $nestedIfsWithOnlyReturn, $nextNode);
-        $this->removeNode($node);
+            if (! $stmt instanceof If_) {
+                continue;
+            }
+
+            $nestedIfsWithOnlyReturn = $this->ifManipulator->collectNestedIfsWithOnlyReturn($stmt);
+            if ($nestedIfsWithOnlyReturn === []) {
+                continue;
+            }
+
+            $node->stmts = $this->processNestedIfsWithOnlyReturn($nestedIfsWithOnlyReturn, $nextStmt);
+
+            return $node;
+        }
 
         return null;
     }
 
     /**
      * @param If_[] $nestedIfsWithOnlyReturn
+     * @return Stmt[]
      */
-    private function processNestedIfsWithOnlyReturn(If_ $if, array $nestedIfsWithOnlyReturn, Return_ $nextReturn): void
+    private function processNestedIfsWithOnlyReturn(array $nestedIfsWithOnlyReturn, Return_ $nextReturn): array
     {
         // add nested if openly after this
         $nestedIfsWithOnlyReturnCount = count($nestedIfsWithOnlyReturn);
 
+        $newStmts = [];
+
         /** @var int $key */
         foreach ($nestedIfsWithOnlyReturn as $key => $nestedIfWithOnlyReturn) {
             // last item → the return node
             if ($nestedIfsWithOnlyReturnCount === $key + 1) {
-                $this->nodesToAddCollector->addNodeAfterNode($nestedIfWithOnlyReturn, $if);
+                $newStmts[] = $nestedIfWithOnlyReturn;
             } else {
-                $this->addStandaloneIfsWithReturn($nestedIfWithOnlyReturn, $if, $nextReturn);
+                $standaloneIfs = $this->createStandaloneIfsWithReturn($nestedIfWithOnlyReturn, $nextReturn);
+                $newStmts = array_merge($newStmts, $standaloneIfs);
             }
         }
+
+        $newStmts[] = $nextReturn;
+
+        return $newStmts;
     }
 
-    private function addStandaloneIfsWithReturn(If_ $nestedIfWithOnlyReturn, If_ $if, Return_ $return): void
+    /**
+     * @return Stmt[]
+     */
+    private function createStandaloneIfsWithReturn(If_ $nestedIfWithOnlyReturn, Return_ $return): array
     {
-        $return = clone $return;
-
         $invertedCondition = $this->conditionInverter->createInvertedCondition($nestedIfWithOnlyReturn->cond);
 
         // special case
         if ($invertedCondition instanceof BooleanNot && $invertedCondition->expr instanceof BooleanAnd) {
             $booleanNotPartIf = new If_(new BooleanNot($invertedCondition->expr->left));
             $booleanNotPartIf->stmts = [clone $return];
-            $this->nodesToAddCollector->addNodeAfterNode($booleanNotPartIf, $if);
 
             $secondBooleanNotPartIf = new If_(new BooleanNot($invertedCondition->expr->right));
             $secondBooleanNotPartIf->stmts = [clone $return];
-            $this->nodesToAddCollector->addNodeAfterNode($secondBooleanNotPartIf, $if);
-            return;
+
+            return [$booleanNotPartIf, $secondBooleanNotPartIf];
         }
 
         $nestedIfWithOnlyReturn->cond = $invertedCondition;
-        $nestedIfWithOnlyReturn->stmts = [clone $return];
+        $nestedIfWithOnlyReturn->stmts = [$return];
 
-        $this->nodesToAddCollector->addNodeAfterNode($nestedIfWithOnlyReturn, $if);
+        return [$nestedIfWithOnlyReturn];
     }
 }