[DX] Remove parent node from AbstractRector (#4465)

* take on removing parent * get chain nested call limit from simple parameter provider * ref BC for parent node * rollback removed fixtures * fix isset check * Fix isset * remove #[RunTestsInSeparateProcesses()] * remove #[RunTestsInSeparateProcesses()] * Fix isset * fix * fix * fix * fix * fix * fix * fix --------- Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com>
rectorphp · Jul 9, 2023 · 868612a · 868612a
1 parent a771c56
commit 868612a
Show file tree

Hide file tree

Showing 16 changed files with 50 additions and 61 deletions.
diff --git a/config/config.php b/config/config.php
@@ -12,7 +12,6 @@
 use PhpParser\Lexer;
 use PhpParser\NodeFinder;
 use PhpParser\NodeVisitor\CloningVisitor;
-use PhpParser\NodeVisitor\ParentConnectingVisitor;
 use PHPStan\Analyser\NodeScopeResolver;
 use PHPStan\Analyser\ScopeFactory;
 use PHPStan\Dependency\DependencyResolver;
@@ -192,7 +191,6 @@
 
     $services->set(BuilderFactory::class);
     $services->set(CloningVisitor::class);
-    $services->set(ParentConnectingVisitor::class);
     $services->set(NodeFinder::class);
 
     $services->set(RectorConsoleOutputStyle::class)

diff --git a/packages/NodeTypeResolver/Node/AttributeKey.php b/packages/NodeTypeResolver/Node/AttributeKey.php
@@ -71,6 +71,11 @@ final class AttributeKey
      * @internal of php-parser, do not change
      * @see https://github.com/nikic/PHP-Parser/pull/681/files
      * @var string
+     *
+     * @api for BC layer
+     *
+     * The parent node can be still enabled by using custom PHPStan configuration,
+     * @see https://github.com/rectorphp/rector-src/pull/4458#discussion_r1257478146
      */
     public const PARENT_NODE = 'parent';
 

diff --git a/packages/NodeTypeResolver/NodeScopeAndMetadataDecorator.php b/packages/NodeTypeResolver/NodeScopeAndMetadataDecorator.php
@@ -7,7 +7,6 @@
 use PhpParser\Node\Stmt;
 use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitor\CloningVisitor;
-use PhpParser\NodeVisitor\ParentConnectingVisitor;
 use Rector\Core\PhpParser\NodeTraverser\FileWithoutNamespaceNodeTraverser;
 use Rector\Core\PHPStan\NodeVisitor\UnreachableStatementNodeVisitor;
 use Rector\Core\ValueObject\Application\File;
@@ -22,19 +21,14 @@ final class NodeScopeAndMetadataDecorator
     public function __construct(
         CloningVisitor $cloningVisitor,
         private readonly PHPStanNodeScopeResolver $phpStanNodeScopeResolver,
-        ParentConnectingVisitor $parentConnectingVisitor,
         FunctionLikeParamArgPositionNodeVisitor $functionLikeParamArgPositionNodeVisitor,
         private readonly ScopeFactory $scopeFactory,
         private readonly FileWithoutNamespaceNodeTraverser $fileWithoutNamespaceNodeTraverser
     ) {
         $this->nodeTraverser = new NodeTraverser();
 
-        // needed also for format preserving printing
+        // needed for format preserving printing
         $this->nodeTraverser->addVisitor($cloningVisitor);
-
-        // this one has to be run again to re-connect parent nodes with new attributes
-        $this->nodeTraverser->addVisitor($parentConnectingVisitor);
-
         $this->nodeTraverser->addVisitor($functionLikeParamArgPositionNodeVisitor);
     }
 

diff --git a/packages/NodeTypeResolver/PHPStan/Scope/NodeVisitor/RemoveDeepChainMethodCallNodeVisitor.php b/packages/NodeTypeResolver/PHPStan/Scope/NodeVisitor/RemoveDeepChainMethodCallNodeVisitor.php
@@ -11,7 +11,7 @@
 use PhpParser\NodeTraverser;
 use PhpParser\NodeVisitorAbstract;
 use Rector\Core\Configuration\Option;
-use Rector\Core\Configuration\Parameter\ParameterProvider;
+use Rector\Core\Configuration\Parameter\SimpleParameterProvider;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\NodeTypeResolver\PHPStan\Scope\Contract\NodeVisitor\ScopeResolverNodeVisitorInterface;
 
@@ -26,9 +26,8 @@ final class RemoveDeepChainMethodCallNodeVisitor extends NodeVisitorAbstract imp
 
     public function __construct(
         private readonly BetterNodeFinder $betterNodeFinder,
-        ParameterProvider $parameterProvider
     ) {
-        $this->nestedChainMethodCallLimit = (int) $parameterProvider->provideParameter(
+        $this->nestedChainMethodCallLimit = SimpleParameterProvider::provideIntParameter(
             Option::NESTED_CHAIN_METHOD_CALL_LIMIT
         );
     }

diff --git a/packages/PostRector/Rector/ClassRenamingPostRector.php b/packages/PostRector/Rector/ClassRenamingPostRector.php
@@ -5,8 +5,11 @@
 namespace Rector\PostRector\Rector;
 
 use PhpParser\Node;
+use PhpParser\Node\Arg;
+use PhpParser\Node\Expr;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Namespace_;
+use PhpParser\Node\Stmt\PropertyProperty;
 use PHPStan\Analyser\Scope;
 use Rector\CodingStyle\Application\UseImportsRemover;
 use Rector\Core\Configuration\Option;
@@ -62,6 +65,11 @@ public function getRectorDependencies(): array
 
     public function enterNode(Node $node): ?Node
     {
+        // cannot be renamed
+        if ($node instanceof Expr || $node instanceof Arg || $node instanceof PropertyProperty) {
+            return null;
+        }
+
         $oldToNewClasses = $this->renamedClassesDataCollector->getOldToNewClasses();
         if ($oldToNewClasses === []) {
             return null;

diff --git a/packages/PostRector/Rector/UnusedImportRemovingPostRector.php b/packages/PostRector/Rector/UnusedImportRemovingPostRector.php
@@ -57,7 +57,6 @@ public function enterNode(Node $node): ?Node
             }
 
             $useUse = $namespaceStmt->uses[0];
-
             if ($this->isUseImportUsed($useUse, $names)) {
                 continue;
             }

diff --git a/...ture/return_static_doc_clone_this.php.inc → ...meClassRector/Fixture/cloned_this.php.inc b/...ture/return_static_doc_clone_this.php.inc → ...meClassRector/Fixture/cloned_this.php.inc
@@ -2,12 +2,12 @@
 
 namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Fixture;
 
-class ReturnStaticDocCloneThis extends \DateTime
+class CloneThis extends \DateTime
 {
     /**
      * @return static
      */
-    public function run(\DateTime $dateTime)
+    public function executeCloneThis(\DateTime $dateTime)
     {
         $obj = clone $this;
         return $obj;
@@ -20,12 +20,12 @@ class ReturnStaticDocCloneThis extends \DateTime
 
 namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Fixture;
 
-class ReturnStaticDocCloneThis extends \DateTime
+class CloneThis extends \DateTime
 {
     /**
      * @return static
      */
-    public function run(\DateTimeInterface $dateTime)
+    public function executeCloneThis(\DateTimeInterface $dateTime)
     {
         $obj = clone $this;
         return $obj;

diff --git a/...or/Fixture/do_not_implement_twice.php.inc → .../do_not_implement_interface_twice.php.inc b/...or/Fixture/do_not_implement_twice.php.inc → .../do_not_implement_interface_twice.php.inc
@@ -5,7 +5,7 @@ namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Fixture;
 use Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source\Contract\FirstInterface;
 use Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source\Contract\SecondInterface;
 
-final class DoNotImplementTwice implements FirstInterface, SecondInterface
+final class DoNotImplementInterfaceTwice implements FirstInterface, SecondInterface
 {
 }
 
@@ -15,7 +15,7 @@ final class DoNotImplementTwice implements FirstInterface, SecondInterface
 
 namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Fixture;
 
-final class DoNotImplementTwice implements \Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source\Contract\ThirdInterface
+final class DoNotImplementInterfaceTwice implements \Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source\Contract\ThirdInterface
 {
 }
 

diff --git a/rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/keep_return_tag.php.inc b/rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/keep_return_tag.php.inc
@@ -11,5 +11,6 @@ final class KeepReturnTag
      */
     public function keep()
     {
+        return new TwigFilter();
     }
 }
diff --git a/rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/name_insensitive.php.inc b/rules-tests/Renaming/Rector/Name/RenameClassRector/Fixture/name_insensitive.php.inc
@@ -9,6 +9,8 @@ class NameInsensitive extends OldClassWithTypO
     public function run(): OLDClassWithTYPO
     {
         $oldClassWithTypo = new OldClassWithTYPO;
+
+        return $oldClassWithTypo;
     }
 }
 
@@ -23,6 +25,8 @@ class NameInsensitive extends \Rector\Tests\Renaming\Rector\Name\RenameClassRect
     public function run(): \Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source\NewClassWithoutTypo
     {
         $oldClassWithTypo = new \Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source\NewClassWithoutTypo;
+
+        return $oldClassWithTypo;
     }
 }
 

diff --git a/...block_rename_different_namespace4.php.inc → ...cblock_imported_by_last_group_use.php.inc b/...block_rename_different_namespace4.php.inc → ...cblock_imported_by_last_group_use.php.inc
@@ -6,7 +6,7 @@
 
 use \Company2\{ Bar };
 
-final class SkipDocblockRenameDifferentNamespace4
+final class SkipRenameDocblockImportedViaSameNamespace
 {
     /**
      * @param Bar $foo

diff --git a/...block_rename_different_namespace3.php.inc → ...espace_by_last_group_in_namespace.php.inc b/...block_rename_different_namespace3.php.inc → ...espace_by_last_group_in_namespace.php.inc
@@ -4,7 +4,7 @@ namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Fixture;
 
 use \Company2\{ Bar };
 
-final class SkipDocblockRenameDifferentNamespace3
+final class SkipRenameDocblockViaSameNamespaceInNamespace
 {
     /**
      * @param Bar $foo

diff --git a/...r/Fixture/rename_before_attribute.php.inc → ...ture/some_rename_before_attribute.php.inc b/...r/Fixture/rename_before_attribute.php.inc → ...ture/some_rename_before_attribute.php.inc
@@ -3,8 +3,9 @@
 namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Fixture;
 
 use Doctrine\DBAL\DBALException;
+use Symfony\Component\Routing\Annotation\Route;
 
-class RenameBeforeAttribute
+class SomeRenameBeforeAttribute
 {
     /**
      * @throws DBALException
@@ -22,8 +23,9 @@ class RenameBeforeAttribute
 namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\Fixture;
 
 use Doctrine\DBAL\DBALException;
+use Symfony\Component\Routing\Annotation\Route;
 
-class RenameBeforeAttribute
+class SomeRenameBeforeAttribute
 {
     /**
      * @throws \Doctrine\DBAL\Exception

diff --git a/...Rector/FixtureAutoImportNames/rename_class_with_same_name_but_different_namespace.php.inc b/...Rector/FixtureAutoImportNames/rename_class_with_same_name_but_different_namespace.php.inc
@@ -4,7 +4,13 @@ namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\FixtureAutoImportN
 
 use Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source\FirstNamespace\SomeServiceClass;
 
-$someService = new SomeServiceClass();
+final class RenameClassWithSameNameButDifferentNamespace
+{
+    public function run(): void
+    {
+        $someService = new SomeServiceClass();
+    }
+}
 
 ?>
 -----
@@ -14,6 +20,12 @@ namespace Rector\Tests\Renaming\Rector\Name\RenameClassRector\FixtureAutoImportN
 
 use Rector\Tests\Renaming\Rector\Name\RenameClassRector\Source\SecondNamespace\SomeServiceClass;
 
-$someService = new SomeServiceClass();
+final class RenameClassWithSameNameButDifferentNamespace
+{
+    public function run(): void
+    {
+        $someService = new SomeServiceClass();
+    }
+}
 
 ?>
diff --git a/src/Rector/AbstractRector.php b/src/Rector/AbstractRector.php
@@ -227,7 +227,7 @@ final public function enterNode(Node $node)
             throw new ShouldNotHappenException($errorMessage);
         }
 
-        return $this->postRefactorProcess($originalNode, $node, $refactoredNode);
+        return $this->postRefactorProcess($originalNode, $refactoredNode);
     }
 
     /**
@@ -321,16 +321,14 @@ protected function appendArgs(array $currentArgs, array $appendingArgs): array
     /**
      * @param Node|Node[] $refactoredNode
      */
-    private function postRefactorProcess(Node $originalNode, Node $node, Node|array|int $refactoredNode): Node
+    private function postRefactorProcess(Node $originalNode, Node|array|int $refactoredNode): Node
     {
         /** @var non-empty-array<Node>|Node $refactoredNode */
         $this->createdByRuleDecorator->decorate($refactoredNode, $originalNode, static::class);
 
         $rectorWithLineChange = new RectorWithLineChange(static::class, $originalNode->getLine());
         $this->file->addRectorClassWithLine($rectorWithLineChange);
 
-        $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
-
         /** @var MutatingScope|null $currentScope */
         $currentScope = $originalNode->getAttribute(AttributeKey::SCOPE);
         $filePath = $this->file->getFilePath();
@@ -342,7 +340,6 @@ private function postRefactorProcess(Node $originalNode, Node $node, Node|array|
             $firstNode = current($refactoredNode);
             $this->mirrorComments($firstNode, $originalNode);
 
-            $this->updateParentNodes($refactoredNode, $parentNode);
             $this->refreshScopeNodes($refactoredNode, $filePath, $currentScope);
 
             $this->nodesToReturn[$originalNodeHash] = $refactoredNode;
@@ -351,7 +348,6 @@ private function postRefactorProcess(Node $originalNode, Node $node, Node|array|
             return $originalNode;
         }
 
-        $this->updateParentNodes($refactoredNode, $parentNode);
         $this->refreshScopeNodes($refactoredNode, $filePath, $currentScope);
 
         $this->nodesToReturn[$originalNodeHash] = $refactoredNode;
@@ -394,23 +390,6 @@ private function shouldSkipCurrentNode(Node $node): bool
         return $this->rectifiedAnalyzer->hasRectified(static::class, $node);
     }
 
-    /**
-     * @param Node[]|Node $node
-     */
-    private function updateParentNodes(array | Node $node, ?Node $parentNode): void
-    {
-        if (! $parentNode instanceof Node) {
-            return;
-        }
-
-        $nodes = $node instanceof Node ? [$node] : $node;
-
-        foreach ($nodes as $node) {
-            // update parents relations
-            $node->setAttribute(AttributeKey::PARENT_NODE, $parentNode);
-        }
-    }
-
     private function printCurrentFileAndRule(): void
     {
         $relativeFilePath = $this->filePathHelper->relativePath($this->file->getFilePath());

diff --git a/src/Rector/AbstractScopeAwareRector.php b/src/Rector/AbstractScopeAwareRector.php
@@ -41,21 +41,9 @@ public function refactor(Node $node)
         }
 
         if (! $currentScope instanceof Scope) {
-            /**
-             * @var Node $parentNode
-             *
-             * $parentNode is always a Node when $mutatingScope is null, as checked in previous
-             *
-             *      $this->scopeAnalyzer->resolveScope()
-             *
-             *  which verify if no parent and no scope, it resolve Scope from File
-             */
-            $parentNode = $node->getAttribute(AttributeKey::PARENT_NODE);
-
             $errorMessage = sprintf(
-                'Scope not available on "%s" node with parent node of "%s", but is required by a refactorWithScope() method of "%s" rule. Fix scope refresh on changed nodes first',
+                'Scope not available on "%s" node, but is required by a refactorWithScope() method of "%s" rule. Fix scope refresh on changed nodes first',
                 $node::class,
-                $parentNode::class,
                 static::class,
             );