Infinity loop check #2 (#54)

config/services.php

@@ -72,6 +72,7 @@ return static function (ContainerConfigurator $containerConfigurator): void {
             __DIR__ . '/../src/PhpParser/Node/CustomNode',
             __DIR__ . '/../src/functions',
             __DIR__ . '/../src/constants.php',
+            __DIR__ . '/../src/PhpParser/NodeVisitor/CreatedByRuleNodeVisitor.php',
         ]);
 
     $services->alias(SymfonyApplication::class, ConsoleApplication::class);

phpstan.neon

@@ -137,7 +137,6 @@ parameters:
         - '#Parameter \#1 \$node of method Rector\\DeadCode\\Rector\\Plus\\RemoveDeadZeroAndOneOperationRector\:\:refactor\(\) expects PhpParser\\Node\\Expr\\AssignOp\\Div\|PhpParser\\Node\\Expr\\AssignOp\\Minus\|PhpParser\\Node\\Expr\\AssignOp\\Mul\|PhpParser\\Node\\Expr\\AssignOp\\Plus\|PhpParser\\Node\\Expr\\BinaryOp\\Div\|PhpParser\\Node\\Expr\\BinaryOp\\Minus\|PhpParser\\Node\\Expr\\BinaryOp\\Mul\|PhpParser\\Node\\Expr\\BinaryOp\\Plus, PhpParser\\Node\\Expr\\AssignOp\|PhpParser\\Node\\Expr\\BinaryOp given#'
 
         # symplify 9
-        - '#Use decoupled factory service to create "(.*?)" object#'
         - '#Use another value object over array with string\-keys and objects, array<string, ValueObject\>#'
         - '#Do not use factory/method call in constructor\. Put factory in config and get service with dependency injection#'
 
@@ -171,8 +170,6 @@ parameters:
             message: '#Do not use setter on a service#'
             paths:
                 - src/Configuration/Configuration.php
-                # prevent circular dependnecy
-                - packages/StaticTypeMapper/Naming/NameScopeFactory.php
 
         -
             message: '#Cannot cast array<string\>\|bool\|string\|null to string#'
@@ -253,13 +250,8 @@ parameters:
         -
             message: '#Function "class_exists\(\)" cannot be used/left in the code#'
             paths:
-                - bin/rector.php
                 - src/Bootstrap/ExtensionConfigResolver.php
 
-        -
-            message: '#Do not use static property#'
-            path: 'bin/rector.php'
-
         # @todo fix later
         -
             message: '#There should be no empty class#'
@@ -282,10 +274,6 @@ parameters:
             message: '#\$this as argument is not allowed\. Refactor method to service composition#'
             paths:
                 - src/Rector/AbstractRector.php
-                # setter to avoid circular dependency in nested collector
-                - packages/StaticTypeMapper/StaticTypeMapper.php
-                - packages/PHPStanStaticTypeMapper/PHPStanStaticTypeMapper.php
-                # refactor later
 
         -
             message: '#Use defined constant Symplify\\ComposerJsonManipulator\\ValueObject\\ComposerJsonSection\:\:REQUIRE over string require#'
@@ -379,11 +367,6 @@ parameters:
             paths:
                 - packages/BetterPhpDocParser/ValueObject/Parser/BetterTokenIterator.php
 
-        -
-            message: '#Array with keys is not allowed\. Use value object to pass data instead#'
-            paths:
-                - rules/NetteToSymfony/Rector/ClassMethod/RouterListToControllerAnnotationsRector.php
-
         - '#Cognitive complexity for "Rector\\BetterPhpDocParser\\PhpDocParser\\DoctrineAnnotationDecorator\:\:mergeNestedDoctrineAnnotations\(\)" is \d+, keep it under 9#'
 
         - '#Cognitive complexity for "Rector\\BetterPhpDocParser\\Printer\\PhpDocInfoPrinter\:\:printDocChildNode\(\)" is \d+, keep it under 9#'
@@ -445,10 +428,6 @@ parameters:
         - '#Method Rector\\Core\\Tests\\DependencyInjection\\ConfigurableRectorImportConfigCallsMergeTest\:\:provideData\(\) return type has no value type specified in iterable type Iterator#'
         - '#Class with base "PhpVersion" name is already used in "PHPStan\\Php\\PhpVersion", "Rector\\Core\\ValueObject\\PhpVersion"\. Use unique name to make classes easy to recognize#'
 
-        -
-            message: '#Use dependency injection instead of dependency juggling#'
-            path: src/HttpKernel/RectorKernel.php
-
         -
             message: '#Use separate function calls with readable variable names#'
             path: src/FileSystem/FilesFinder.php
@@ -457,34 +436,12 @@ parameters:
         -
             message: '#Use dependency injection instead of dependency juggling#'
             paths:
-                 - src/Application/FileProcessor/PhpFileProcessor.php
-                 - rules/NetteToSymfony/Rector/ClassMethod/RouterListToControllerAnnotationsRector.php
-                 - packages/FamilyTree/Reflection/FamilyRelationsAnalyzer.php
-                 - packages/Caching/FileSystem/DependencyResolver.php
-                 - packages/BetterPhpDocParser/PhpDocNodeMapper.php
-                 - packages/NodeTypeResolver/NodeVisitor/FileNodeVisitor.php
-                 # hacking value object
-                 - packages/BetterPhpDocParser/PhpDocInfo/PhpDocInfo.php
                  # better reflection static design
                  - packages/NodeTypeResolver/Reflection/BetterReflection/SourceLocatorProvider/DynamicSourceLocatorProvider.php
 
-        -
-            message: '#Use separate function calls with readable variable names#'
-            path: src/Application/ActiveRectorsProvider.php
-
-        # skip for define()
-        -
-           message: '#Do not compare call directly, use a variable assign#'
-           path:  src/constants.php
-
-        -
-            message: '#Method call on new expression is not allowed#'
-            path: utils/compiler/src/Command/DowngradePathsCommand.php
-
         -
             message: '#Use value object over return of values#'
             paths:
-                 - rules/EarlyReturn/Rector/If_/ChangeNestedIfsToEarlyReturnRector.php
                  - rules/EarlyReturn/Rector/If_/ChangeAndIfToEarlyReturnRector.php
 
         - '#Class with base "PhpVersionFactory" name is already used in "PHPStan\\Php\\PhpVersionFactory", "Rector\\Core\\Util\\PhpVersionFactory"\. Use unique name to make classes easy to recognize#'
@@ -493,17 +450,6 @@ parameters:
             message: '#Function "function_exists\(\)" cannot be used/left in the code#'
             path: src/functions/node_helper.php
 
-        # temporary out of order after rector-src extract
-        - '#Class like namespace "Rector\\(.*?)" does not follow PSR\-4 configuration in composer\.json#'
-
-        # temporary out of order after rector-src extract
-        - '#Class like namespace "Rector\\(.*?)" does not follow PSR\-4 configuration in composer\.json#'
-
-        # bug in PHP 8, fix in dev-main
-        -
-            message: '#Do not call parent method if parent method is empty#'
-            path: packages/BetterPhpDocParser/PhpDoc/DoctrineAnnotationTagValueNode.php
-
         # upgrade to PHP 7.4 wip
         - '#This property type might be inlined to PHP\. Do you have confidence it is correct\? Put it here#'
 
@@ -515,7 +461,6 @@ parameters:
         - '#Use required typed property over of nullable property#'
         - '#Method Rector\\BetterPhpDocParser\\PhpDocParser\\BetterPhpDocParser\:\:parseChildAndStoreItsPositions\(\) should return PHPStan\\PhpDocParser\\Ast\\PhpDoc\\PhpDocTagNode\|PHPStan\\PhpDocParser\\Ast\\PhpDoc\\PhpDocTextNode but returns PHPStan\\PhpDocParser\\Ast\\PhpDoc\\PhpDocChildNode#'
 
-        - '#Parameter \#1 \$type of method Rector\\BetterPhpDocParser\\PhpDocInfo\\PhpDocInfo<PHPStan\\PhpDocParser\\Ast\\Node\>\:\:removeByType\(\) expects class\-string<PHPStan\\PhpDocParser\\Ast\\PhpDoc\\PhpDocTagValueNode\>, class\-string<PHPStan\\PhpDocParser\\Ast\\PhpDoc\\PhpDocTagValueNode\>\|PHPStan\\PhpDocParser\\Ast\\PhpDoc\\PhpDocTagValueNode given#'
 
         - '#Method Rector\\Core\\Application\\VersionResolver\:\:resolvePackageData\(\) return type has no value type specified in iterable type array#'
         - '#Cognitive complexity for "Rector\\Core\\Rector\\AbstractRector\:\:enterNode\(\)" is \d+, keep it under 9#'
@@ -525,19 +470,5 @@ parameters:
             path: src/Rector/AbstractRector.php
 
 
-        -
-            message: '#Method call argument on position 0 must use constant \(e\.g\. "Option\:\:NAME"\) over value#'
-            path: src/Rector/AbstractRector.php
-
-        -
-            message: '#Namespace "Rector\\Core\\Rector" is only reserved for "Rector"\. Move the class somewhere else#'
-            path: src/Rector/AbstractRector.php
-
-        -
-            message: '#Anonymous class is not allowed#'
-            path: src/Rector/AbstractRector.php
-
-        -
-            message: '#Class cognitive complexity is 38, keep it under 30#'
-            path: src/Rector/AbstractRector.php
-
+        # fixed in dev-main
+        - '#Class like namespace "Rector\\(.*?)" does not follow PSR\-4 configuration in composer\.json#'

src/PhpParser/NodeVisitor/CreatedByRuleNodeVisitor.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\PhpParser\NodeVisitor;
+
+use PhpParser\Node;
+use PhpParser\NodeVisitorAbstract;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+
+final class CreatedByRuleNodeVisitor extends NodeVisitorAbstract
+{
+    public function __construct(
+        private string $rectorClass
+    ) {
+    }
+
+    public function enterNode(Node $node)
+    {
+        $node->setAttribute(AttributeKey::CREATED_BY_RULE, $this->rectorClass);
+        return $node;
+    }
+}

src/Rector/AbstractRector.php

@@ -25,7 +25,6 @@ use Rector\Core\Application\FileSystem\RemovedAndAddedFilesCollector;
 use Rector\Core\Configuration\CurrentNodeProvider;
 use Rector\Core\Configuration\Option;
 use Rector\Core\Contract\Rector\PhpRectorInterface;
-use Rector\Core\Exception\NodeTraverser\InfiniteLoopTraversingException;
 use Rector\Core\Exception\ShouldNotHappenException;
 use Rector\Core\Exclusion\ExclusionManager;
 use Rector\Core\Logging\CurrentRectorProvider;
@@ -37,9 +36,9 @@ use Rector\Core\PhpParser\Node\NodeFactory;
 use Rector\Core\PhpParser\Node\Value\ValueResolver;
 use Rector\Core\PhpParser\Printer\BetterStandardPrinter;
 use Rector\Core\Provider\CurrentFileProvider;
+use Rector\Core\Validation\InfiniteLoopValidator;
 use Rector\Core\ValueObject\Application\File;
 use Rector\Core\ValueObject\ProjectType;
-use Rector\DowngradePhp80\Rector\NullsafeMethodCall\DowngradeNullsafeToTernaryOperatorRector;
 use Rector\NodeCollector\NodeCollector\NodeRepository;
 use Rector\NodeNameResolver\NodeNameResolver;
 use Rector\NodeRemoval\NodeRemover;
@@ -141,6 +140,8 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
      */
     private array $nodesToReturn = [];
 
+    private InfiniteLoopValidator $infiniteLoopValidator;
+
     /**
      * @required
      */
@@ -171,7 +172,8 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
         BetterNodeFinder $betterNodeFinder,
         NodeComparator $nodeComparator,
         CurrentFileProvider $currentFileProvider,
-        ChangedNodeAnalyzer $changedNodeAnalyzer
+        ChangedNodeAnalyzer $changedNodeAnalyzer,
+        InfiniteLoopValidator $infiniteLoopValidator
     ): void {
         $this->nodesToRemoveCollector = $nodesToRemoveCollector;
         $this->nodesToAddCollector = $nodesToAddCollector;
@@ -200,6 +202,7 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
         $this->nodeComparator = $nodeComparator;
         $this->currentFileProvider = $currentFileProvider;
         $this->changedNodeAnalyzer = $changedNodeAnalyzer;
+        $this->infiniteLoopValidator = $infiniteLoopValidator;
     }
 
     /**
@@ -275,34 +278,7 @@ abstract class AbstractRector extends NodeVisitorAbstract implements PhpRectorIn
 
             // is different node type? do not traverse children to avoid looping
             if (get_class($originalNode) !== get_class($node)) {
-                $createdByRule = $originalNode->getAttribute(AttributeKey::CREATED_BY_RULE);
-                // special case
-                if ($createdByRule === static::class && static::class !== DowngradeNullsafeToTernaryOperatorRector::class) {
-                    // does it contain the same node type as input?
-                    $hasNestedOriginalNodeType = $this->betterNodeFinder->findInstanceOf(
-                        $node,
-                        get_class($originalNode)
-                    );
-                    if ($hasNestedOriginalNodeType !== []) {
-                        throw new InfiniteLoopTraversingException(static::class);
-                    }
-                }
-
-                // hacking :)
-                $nodeTraverser = new NodeTraverser();
-                $nodeTraverser->addVisitor(new class(static::class) extends NodeVisitorAbstract {
-                    public function __construct(
-                        private string $rectorClass
-                    ) {
-                    }
-
-                    public function enterNode(Node $node)
-                    {
-                        $node->setAttribute(AttributeKey::CREATED_BY_RULE, $this->rectorClass);
-                        return $node;
-                    }
-                });
-                $nodeTraverser->traverse([$originalNode]);
+                $this->infiniteLoopValidator->process($node, $originalNode, static::class);
 
                 // search "infinite recursion" in https://github.com/nikic/PHP-Parser/blob/master/doc/component/Walking_the_AST.markdown
                 $originalNodeHash = spl_object_hash($originalNode);

src/Validation/InfiniteLoopValidator.php

@@ -0,0 +1,60 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Core\Validation;
+
+use PhpParser\Node;
+use PhpParser\NodeTraverser;
+use Rector\Core\Contract\Rector\RectorInterface;
+use Rector\Core\Exception\NodeTraverser\InfiniteLoopTraversingException;
+use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\Core\PhpParser\NodeVisitor\CreatedByRuleNodeVisitor;
+use Rector\DowngradePhp80\Rector\NullsafeMethodCall\DowngradeNullsafeToTernaryOperatorRector;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+
+final class InfiniteLoopValidator
+{
+    public function __construct(
+        private BetterNodeFinder $betterNodeFinder
+    ) {
+    }
+
+    /**
+     * @param class-string<RectorInterface> $rectorClass
+     */
+    public function process(Node $node, Node $originalNode, string $rectorClass): void
+    {
+        if ($rectorClass === DowngradeNullsafeToTernaryOperatorRector::class) {
+            return;
+        }
+
+        $createdByRule = $originalNode->getAttribute(AttributeKey::CREATED_BY_RULE);
+
+        // special case
+        if ($createdByRule === $rectorClass) {
+            // does it contain the same node type as input?
+            $originalNodeClass = get_class($originalNode);
+
+            $hasNestedOriginalNodeType = $this->betterNodeFinder->findInstanceOf($node, $originalNodeClass);
+            if ($hasNestedOriginalNodeType !== []) {
+                throw new InfiniteLoopTraversingException($rectorClass);
+            }
+        }
+
+        $this->decorateNode($originalNode, $rectorClass);
+    }
+
+    /**
+     * @param class-string<RectorInterface> $rectorClass
+     */
+    private function decorateNode(Node $node, string $rectorClass): void
+    {
+        $nodeTraverser = new NodeTraverser();
+
+        $createdByRuleNodeVisitor = new CreatedByRuleNodeVisitor($rectorClass);
+        $nodeTraverser->addVisitor($createdByRuleNodeVisitor);
+
+        $nodeTraverser->traverse([$node]);
+    }
+}