Remove PARENT_NODE from CommentsMerger (#4011)

packages/BetterPhpDocParser/Comment/CommentsMerger.php

@@ -4,18 +4,11 @@
 
 namespace Rector\BetterPhpDocParser\Comment;
 
-use PhpParser\Comment;
 use PhpParser\Node;
 use Rector\NodeTypeResolver\Node\AttributeKey;
-use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
 
 final class CommentsMerger
 {
-    public function __construct(
-        private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser
-    ) {
-    }
-
     /**
      * @param Node[] $mergedNodes
      */
@@ -36,66 +29,4 @@ public function keepComments(Node $newNode, array $mergedNodes): void
         // remove so comments "win"
         $newNode->setAttribute(AttributeKey::PHP_DOC_INFO, null);
     }
-
-    /**
-     * @api
-     */
-    public function keepParent(Node $newNode, Node $oldNode): void
-    {
-        $parentNode = $oldNode->getAttribute(AttributeKey::PARENT_NODE);
-        if (! $parentNode instanceof Node) {
-            return;
-        }
-
-        $phpDocInfo = $parentNode->getAttribute(AttributeKey::PHP_DOC_INFO);
-        $comments = $parentNode->getComments();
-
-        if ($phpDocInfo === null && $comments === []) {
-            return;
-        }
-
-        $newNode->setAttribute(AttributeKey::PHP_DOC_INFO, $phpDocInfo);
-        $newNode->setAttribute(AttributeKey::COMMENTS, $comments);
-    }
-
-    /**
-     * @api
-     */
-    public function keepChildren(Node $newNode, Node $oldNode): void
-    {
-        $childrenComments = $this->collectChildrenComments($oldNode);
-
-        if ($childrenComments === []) {
-            return;
-        }
-
-        $commentContent = '';
-        foreach ($childrenComments as $childComment) {
-            $commentContent .= $childComment->getText() . PHP_EOL;
-        }
-
-        $newNode->setAttribute(AttributeKey::COMMENTS, [new Comment($commentContent)]);
-    }
-
-    /**
-     * @return Comment[]
-     */
-    private function collectChildrenComments(Node $node): array
-    {
-        $childrenComments = [];
-
-        $this->simpleCallableNodeTraverser->traverseNodesWithCallable($node, static function (Node $node) use (
-            &$childrenComments
-        ) {
-            $comments = $node->getComments();
-
-            if ($comments !== []) {
-                $childrenComments = array_merge($childrenComments, $comments);
-            }
-
-            return null;
-        });
-
-        return $childrenComments;
-    }
 }

phpstan.neon

@@ -94,12 +94,6 @@ parameters:
             message: '#Use explicit return value over magic &reference#'
             path: rules/Php70/EregToPcreTransformer.php
 
-        # false positives checked in another method
-        -
-            message: '#If condition is always false#'
-            paths:
-                - rules/CodeQuality/Rector/Concat/JoinStringConcatRector.php
-
         - '#Method (.*?) should return array<PhpParser\\Node\\(.*?)\> but returns array<PhpParser\\Node\>#'
 
         -
@@ -158,9 +152,6 @@ parameters:
         # array_index on generic types
         - '#Method Rector\\NodeTypeResolver\\PHPStan\\Type\\TypeFactory\:\:uniquateTypes\(\) should return array<TType of PHPStan\\Type\\Type\> but returns array<int, PHPStan\\Type\\Type\>#'
 
-        # resolve later
-        - '#Method "processRenameVariable\(\)" returns bool type, so the name should start with is/has/was#'
-
         - '#Method "resolveObjectType\(\)" returns bool type, so the name should start with is/has/was#'
 
         # native filesystem calls, required for performance reasons
@@ -233,10 +224,6 @@ parameters:
         # strict - resolve later
         - '#Foreach overwrites \$(.*?) with its value variable#'
 
-        -
-            message: '#Strict comparison using \!\=\= between null and null will always evaluate to false#'
-            path: rules/Renaming/Rector/FileWithoutNamespace/PseudoNamespaceToNamespaceRector.php
-
         # stricter child type on purpose
         - '#Parameter \#1 \$nodes \(array<PhpParser\\Node\\Stmt\>\) of method Rector\\PostRector\\Rector\\(.*?)\:\:beforeTraverse\(\) should be contravariant with parameter \$nodes \(array<PhpParser\\Node\>\) of method PhpParser\\NodeVisitor\:\:beforeTraverse\(\)#'
 
@@ -342,11 +329,6 @@ parameters:
             message: '#The path "/\.\./\.\./stubs\-rector" was not found#'
             path: src/Autoloading/BootstrapFilesIncluder.php # 54
 
-        -
-            message: '#Access to an undefined property PhpParser\\Node\:\:\$expr#'
-            paths:
-                - rules/CodeQuality/Rector/FuncCall/ChangeArrayPushToArrayAssignRector.php
-
         # doc arrays cannot be trusted
         -
             message: '#Call to static method Webmozart\\Assert\\Assert\:\:allString\(\) with array<class\-string<Rector\\Core\\Contract\\Rector\\RectorInterface>> will always evaluate to true#'
@@ -741,8 +723,6 @@ parameters:
 
         - '#Parameter \#3 \$assign of method Rector\\CodeQuality\\Rector\\FunctionLike\\SimplifyUselessVariableRector\:\:processSimplifyUselessVariable\(\) expects PhpParser\\Node\\Expr\\Assign\|PhpParser\\Node\\Expr\\AssignOp, PhpParser\\Node\\Expr given#'
 
-        - '#Anonymous variable in a (.*?)->stmts\[(.*?)]\->\.\.\.\(\)` method call can lead to false dead methods\. Make sure the variable type is known#'
-
         - '#Access to an undefined property PhpParser\\Node\\Stmt\\ClassLike\|PhpParser\\Node\\Stmt\\Declare_\|Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts#'
         - '#Access to an undefined property \(PhpParser\\Node\\Stmt&Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\)\|PhpParser\\Node\\Stmt\\ClassLike\|PhpParser\\Node\\Stmt\\Declare_\:\:\$stmts#'
 
@@ -751,8 +731,10 @@ parameters:
         - '#Return type \(int\|PhpParser\\Node\\Expr\\FuncCall\|PhpParser\\Node\\Expr\\Ternary\|null\) of method Rector\\Php71\\Rector\\FuncCall\\CountOnNullRector\:\:refactorWithScope\(\) should be covariant with return type \(1\|2\|3\|4\|array<PhpParser\\Node>\|PhpParser\\Node\|null\) of method Rector\\Core\\Contract\\Rector\\ScopeAwarePhpRectorInterface\:\:refactorWithScope\(\)#'
 
         # resolve before 1.0
-        - '#Call to deprecated method resolveNextNode\(\) of class Rector\\Core\\PhpParser\\Node\\BetterNodeFinder\:#'
         - '#Parameter \$nodesToAddCollector of (.*?) has typehint with deprecated class Rector\\PostRector\\Collector\\NodesToAddCollector#'
 
         - '#Call to deprecated method addNodeToRemove\(\) of class Rector\\PostRector\\Collector\\NodesToRemoveCollector\:#'
         - '#Call to deprecated method removeNode\(\) of class Rector\\NodeRemoval\\NodeRemover#'
+        -
+            message: '#Offset 0 does not exist on array<PhpParser\\Node\\Stmt>\|null#'
+            path: rules/CodingStyle/Rector/ClassMethod/ReturnArrayClassMethodToYieldRector.php

rules-tests/CodingStyle/Rector/ClassMethod/ReturnArrayClassMethodToYieldRector/Fixture/should_not_remove_comments.php.inc

@@ -30,10 +30,10 @@ use Rector\Tests\CodingStyle\Rector\ClassMethod\ReturnArrayClassMethodToYieldRec
 
 final class ShouldNotRemoveComments extends ParentTestCase
 {
-    // Let's pretend that this comment is extremely important and meaningful.
-    // It should not be removed by Rector.
     public function provideData(): \Iterator
     {
+        // Let's pretend that this comment is extremely important and meaningful.
+        // It should not be removed by Rector.
         // This is yet another comment.
         yield ['item1'];
         // And a final one.

rules-tests/CodingStyle/Rector/ClassMethod/ReturnArrayClassMethodToYieldRector/Fixture/should_not_remove_doc_blocks.php.inc

@@ -36,13 +36,13 @@ use Rector\Tests\CodingStyle\Rector\ClassMethod\ReturnArrayClassMethodToYieldRec
 
 final class ShouldNotRemoveComment extends ParentTestCase
 {
-    /**
-     * @doto Let's pretend that this comment is extremely important and meaningful.
-     *
-     * It should not be removed by Rector.
-     */
     public function provideData(): \Iterator
     {
+        /**
+         * @doto Let's pretend that this comment is extremely important and meaningful.
+         *
+         * It should not be removed by Rector.
+         */
         /**
          * @doto Yet another important comment
          */

rules/CodingStyle/Rector/ClassMethod/ReturnArrayClassMethodToYieldRector.php

@@ -10,11 +10,11 @@
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Return_;
 use PHPStan\PhpDocParser\Ast\PhpDoc\ReturnTagValueNode;
-use Rector\BetterPhpDocParser\Comment\CommentsMerger;
 use Rector\CodingStyle\ValueObject\ReturnArrayClassMethodToYield;
 use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
 use Rector\Core\PhpParser\NodeTransformer;
 use Rector\Core\Rector\AbstractRector;
+use Rector\NodeTypeResolver\Node\AttributeKey;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 use Webmozart\Assert\Assert;
@@ -34,7 +34,6 @@ final class ReturnArrayClassMethodToYieldRector extends AbstractRector implement
 
     public function __construct(
         private readonly NodeTransformer $nodeTransformer,
-        private readonly CommentsMerger $commentsMerger
     ) {
     }
 
@@ -49,7 +48,9 @@ final class SomeTest implements TestCase
 {
     public static function provideData()
     {
-        return [['some text']];
+        return [
+            ['some text']
+        ];
     }
 }
 CODE_SAMPLE
@@ -84,6 +85,10 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
+        if ($node->stmts === null) {
+            return null;
+        }
+
         $hasChanged = false;
 
         foreach ($this->methodsToYields as $methodToYield) {
@@ -100,9 +105,18 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
+            // keep comments of 1st array item
+            $firstComment = $node->stmts[0]->getAttribute(AttributeKey::COMMENTS);
+
             $this->transformArrayToYieldsOnMethodNode($node, $arrayNode);
 
-            $this->commentsMerger->keepParent($node, $arrayNode);
+            if (is_array($firstComment)) {
+                $node->stmts[0]->setAttribute(
+                    AttributeKey::COMMENTS,
+                    array_merge($firstComment, (array) $node->stmts[0]->getAttribute(AttributeKey::COMMENTS))
+                );
+            }
+
             $hasChanged = true;
         }
 
@@ -144,7 +158,7 @@ private function collectReturnArrayNodesFromClassMethod(ClassMethod $classMethod
 
     private function transformArrayToYieldsOnMethodNode(ClassMethod $classMethod, Array_ $array): void
     {
-        $yieldNodes = $this->nodeTransformer->transformArrayToYields($array);
+        $yields = $this->nodeTransformer->transformArrayToYields($array);
 
         $this->removeReturnTag($classMethod);
 
@@ -159,7 +173,7 @@ private function transformArrayToYieldsOnMethodNode(ClassMethod $classMethod, Ar
             unset($classMethod->stmts[$key]);
         }
 
-        $classMethod->stmts = array_merge((array) $classMethod->stmts, $yieldNodes);
+        $classMethod->stmts = array_merge((array) $classMethod->stmts, $yields);
     }
 
     private function removeReturnTag(ClassMethod $classMethod): void

src/PhpParser/NodeTransformer.php

@@ -69,23 +69,25 @@ public function transformSprintfToArray(FuncCall $sprintfFuncCall): ?Array_
      */
     public function transformArrayToYields(Array_ $array): array
     {
-        $yieldNodes = [];
+        $yields = [];
 
         foreach ($array->items as $arrayItem) {
             if (! $arrayItem instanceof ArrayItem) {
                 continue;
             }
 
-            $expressionNode = new Expression(new Yield_($arrayItem->value, $arrayItem->key));
+            $yield = new Yield_($arrayItem->value, $arrayItem->key);
+            $expression = new Expression($yield);
+
             $arrayItemComments = $arrayItem->getComments();
             if ($arrayItemComments !== []) {
-                $expressionNode->setAttribute(AttributeKey::COMMENTS, $arrayItemComments);
+                $expression->setAttribute(AttributeKey::COMMENTS, $arrayItemComments);
             }
 
-            $yieldNodes[] = $expressionNode;
+            $yields[] = $expression;
         }
 
-        return $yieldNodes;
+        return $yields;
     }
 
     /**