[DX] Use direct return of multi nodes instead of 2 addNode before/after node (#2232)

Co-authored-by: GitHub Action <action@github.com>

packages/PostRector/Collector/NodesToAddCollector.php

@@ -40,6 +40,9 @@ final class NodesToAddCollector implements NodeCollectorInterface
         return $this->nodesToAddAfter !== [] || $this->nodesToAddBefore !== [];
     }
 
+    /**
+     * @internal Try to avoid using this magic added. Better return directly array of nodes in the Rector rule.
+     */
     public function addNodeBeforeNode(Node $addedNode, Node $positionNode): void
     {
         if ($positionNode->getAttributes() === []) {

phpstan.neon

@@ -416,6 +416,7 @@ parameters:
 
         # specific generics
         - '#Parameter \#1 \$node (.*?) of method Rector\\(.*?)Rector\:\:refactor\(\) should be contravariant with parameter \$node \(PhpParser\\Node\) of method Rector\\Core\\Contract\\Rector\\PhpRectorInterface\:\:refactor\(\)#'
+        - '#Return type (.*?) of method Rector\\(.*?)\\Rector\\(.*?)\\(.*?)\:\:refactor\(\) should be covariant with return type \(array<PhpParser\\Node>\|PhpParser\\Node\|null\) of method Rector\\Core\\Contract\\Rector\\PhpRectorInterface\:\:refactor\(\)#'
 
         # only for empty ctor
         - '#Rector\\Core\\PhpParser\\NodeTraverser\\RectorNodeTraverser\:\:__construct\(\) does not call parent constructor from PhpParser\\NodeTraverser#'
@@ -590,3 +591,6 @@ parameters:
         - '#Cognitive complexity for "Rector\\Removing\\NodeManipulator\\ComplexNodeRemover\:\:removeConstructorDependency\(\)" is \d+, keep it under 10#'
         - '#Class "Rector\\DeadCode\\Rector\\Property\\RemoveUnusedPrivatePropertyRector" has invalid namespace category "Property"\. Pick one of\: "Class_"#'
         - '#Cognitive complexity for "Rector\\ReadWrite\\NodeAnalyzer\\ReadWritePropertyAnalyzer\:\:isRead\(\)" is 14, keep it under 10#'
+
+        # avoid moving rule around, while re-targeting to stmt
+        - '#Class "Rector\\(.*?)\\Rector\\(.*?) has invalid namespace category "(.*?)"\. Pick one of\: "Expression"#'

rules-tests/CodingStyle/Rector/Assign/SplitDoubleAssignRector/Fixture/use_prev_assign_var_call_new.php.inc

@@ -7,9 +7,7 @@ class UsePrevAssignVarCallNew
     public function run()
     {
         $one = $two = $this->execute();
-        $one = $two = self::execute();
-        $one = $two = execute();
-        $one = $two = new \stdClass;
+        $anotherOne = $anotherTwo = new \stdClass;
     }
 }
 
@@ -25,12 +23,8 @@ class UsePrevAssignVarCallNew
     {
         $one = $this->execute();
         $two = $one;
-        $one = self::execute();
-        $two = $one;
-        $one = execute();
-        $two = $one;
-        $one = new \stdClass;
-        $two = $one;
+        $anotherOne = new \stdClass;
+        $anotherTwo = $anotherOne;
     }
 }
 

rules/CodeQuality/Rector/LogicalAnd/AndAssignsToSeparateLinesRector.php

@@ -9,7 +9,6 @@ use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\BinaryOp\LogicalAnd;
 use PhpParser\Node\Stmt\Expression;
 use Rector\Core\Rector\AbstractRector;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -54,29 +53,31 @@ CODE_SAMPLE
      */
     public function getNodeTypes(): array
     {
-        return [LogicalAnd::class];
+        return [Expression::class];
     }
 
     /**
-     * @param LogicalAnd $node
+     * @param Expression $node
+     * @return Expression[]|null
      */
-    public function refactor(Node $node): ?Node
+    public function refactor(Node $node): ?array
     {
-        if (! $node->left instanceof Assign) {
+        if (! $node->expr instanceof LogicalAnd) {
             return null;
         }
 
-        if (! $node->right instanceof Assign) {
+        $logicalAnd = $node->expr;
+        if (! $logicalAnd->left instanceof Assign) {
             return null;
         }
 
-        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
-        if (! $parentNode instanceof Expression) {
+        if (! $logicalAnd->right instanceof Assign) {
             return null;
         }
 
-        $this->nodesToAddCollector->addNodeAfterNode($node->right, $node);
+        $leftAssignExpression = new Expression($logicalAnd->left);
+        $rightAssignExpression = new Expression($logicalAnd->right);
 
-        return $node->left;
+        return [$leftAssignExpression, $rightAssignExpression];
     }
 }

rules/CodingStyle/Rector/Assign/SplitDoubleAssignRector.php

@@ -9,7 +9,6 @@ use PhpParser\Node\Expr\Assign;
 use PhpParser\Node\Expr\CallLike;
 use PhpParser\Node\Stmt\Expression;
 use Rector\Core\Rector\AbstractRector;
-use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -54,33 +53,38 @@ CODE_SAMPLE
      */
     public function getNodeTypes(): array
     {
-        return [Assign::class];
+        return [Expression::class];
     }
 
     /**
-     * @param Assign $node
+     * @param Expression $node
+     * @return Expression[]|null
      */
-    public function refactor(Node $node): ?Node
+    public function refactor(Node $node): ?array
     {
-        $parent = $node->getAttribute(AttributeKey::PARENT_NODE);
-        if (! $parent instanceof Expression) {
-            return null;
-        }
-
         if (! $node->expr instanceof Assign) {
             return null;
         }
 
-        $newAssign = new Assign($node->var, $node->expr->expr);
-
-        if (! $node->expr->expr instanceof CallLike) {
-            $this->nodesToAddCollector->addNodeAfterNode($node->expr, $node);
-            return $newAssign;
+        $firstAssign = $node->expr;
+        if (! $firstAssign->expr instanceof Assign) {
+            return null;
         }
 
-        $varAssign = new Assign($node->expr->var, $node->var);
-        $this->nodesToAddCollector->addNodeBeforeNode(new Expression($newAssign), $node);
+        $nestedAssign = $firstAssign->expr;
 
-        return $varAssign;
+        $newAssign = new Assign($firstAssign->var, $nestedAssign->expr);
+        $newAssignExpression = new Expression($newAssign);
+
+        // avoid calling the same method/funtion/new twice
+        if (! $nestedAssign->expr instanceof CallLike) {
+            $varAssign = new Assign($nestedAssign->var, $nestedAssign->expr);
+
+            return [$newAssignExpression, new Expression($varAssign)];
+        }
+
+        $varAssign = new Assign($nestedAssign->var, $firstAssign->var);
+
+        return [$newAssignExpression, new Expression($varAssign)];
     }
 }

rules/CodingStyle/Rector/Stmt/NewlineAfterStatementRector.php

@@ -145,6 +145,7 @@ CODE_SAMPLE
             }
         }
 
+        // @todo refactor to direct return of array
         $this->nodesToAddCollector->addNodeAfterNode(new Nop(), $node);
 
         return $node;

tests/Issues/SplitMultiAssignRemovePrivate/Fixture/multi_assign_extract.php.inc

@@ -32,7 +32,6 @@ final class MultiAssignExtract
     {
         $this->content_type = "Something";
         $content_type = "Something";
-
         return $content_type;
     }
 }