Various improvements (#2291)

* add value object cache * exact return type * exact variable naming * use stmts aware * add known stmt aware * re-use create array * add cache closure * misc
rectorphp · May 11, 2022 · 55be345 · 55be345
1 parent 17d200d
commit 55be345
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 50 deletions.
diff --git a/phpstan.neon b/phpstan.neon
@@ -162,6 +162,12 @@ parameters:
             message: '#There should be no empty class#'
             paths:
                 - packages/StaticTypeMapper/ValueObject/Type/*Type.php
+                - */Exception/*
+
+        -
+            message: '#Namespace "Rector\\(.*?)\\Rector" is only reserved for "Rector"\. Move the class somewhere else#'
+            paths:
+                - */Contract/*
 
         # generics nullable bugs
         - '#Method (.*?) should return (.*?)\|null but returns PhpParser\\Node\|null#'
@@ -377,13 +383,12 @@ parameters:
 
         - '#Cognitive complexity for "Rector\\Core\\NodeManipulator\\ClassMethodAssignManipulator\:\:collectReferenceVariableNames\(\)" is 13, keep it under 10#'
 
+        # caching
         -
             message: '#Use separate function calls with readable variable names#'
-            path: src/DependencyInjection/Loader/Configurator/RectorServiceConfigurator.php
-
-#        -
-#            message: '#Use separate function calls with readable variable names#'
-#            path: rules/CodingStyle/Rector/ClassMethod/OrderAttributesRector.php
+            paths:
+                - src/DependencyInjection/Loader/Configurator/RectorServiceConfigurator.php
+                - rules/Renaming/NodeManipulator/ClassRenamer.php
 
         # on purpose to make use of worker paralle pattern
         -

diff --git a/rules/CodeQuality/Rector/FuncCall/CompactToVariablesRector.php b/rules/CodeQuality/Rector/FuncCall/CompactToVariablesRector.php
@@ -119,9 +119,8 @@ private function refactorAssignedArray(Assign $assign, FuncCall $funcCall, Expr
             return null;
         }
 
-        $currentStatement = $this->betterNodeFinder->resolveCurrentStatement($funcCall);
-
-        if (! $currentStatement instanceof Stmt) {
+        $currentStmt = $this->betterNodeFinder->resolveCurrentStatement($funcCall);
+        if (! $currentStmt instanceof Stmt) {
             return null;
         }
 
@@ -155,7 +154,7 @@ private function refactorAssignedArray(Assign $assign, FuncCall $funcCall, Expr
         $assignVariable = $firstArg->value;
         $preAssign = new Assign($assignVariable, $array);
 
-        $this->nodesToAddCollector->addNodeBeforeNode($preAssign, $currentStatement);
+        $this->nodesToAddCollector->addNodeBeforeNode($preAssign, $currentStmt);
 
         return $expr;
     }

diff --git a/rules/DowngradePhp81/Rector/FuncCall/DowngradeArrayIsListRector.php b/rules/DowngradePhp81/Rector/FuncCall/DowngradeArrayIsListRector.php
@@ -26,6 +26,8 @@
  */
 final class DowngradeArrayIsListRector extends AbstractRector
 {
+    private ?Closure $cachedClosure = null;
+
     public function __construct(
         private readonly InlineCodeParser $inlineCodeParser,
         private readonly FunctionExistsFunCallAnalyzer $functionExistsFunCallAnalyzer,
@@ -94,6 +96,10 @@ public function refactor(Node $node): ?FuncCall
 
     private function createClosure(): Closure
     {
+        if ($this->cachedClosure instanceof Closure) {
+            return clone $this->cachedClosure;
+        }
+
         $stmts = $this->inlineCodeParser->parse(__DIR__ . '/../../snippet/array_is_list_closure.php.inc');
 
         /** @var Expression $expression */
@@ -104,6 +110,8 @@ private function createClosure(): Closure
             throw new ShouldNotHappenException();
         }
 
+        $this->cachedClosure = $expr;
+
         return $expr;
     }
 

diff --git a/rules/DowngradePhp81/Rector/FuncCall/DowngradeFirstClassCallableSyntaxRector.php b/rules/DowngradePhp81/Rector/FuncCall/DowngradeFirstClassCallableSyntaxRector.php
@@ -92,7 +92,7 @@ private function createCallback(FuncCall|MethodCall|StaticCall $node): Expr
         $class = $node->class instanceof Name ? new ClassConstFetch($node->class, 'class') : $node->class;
         $method = $node->name instanceof Identifier ? new String_($node->name->toString()) : $node->name;
 
-        return new Array_([new ArrayItem($class), new ArrayItem($method)]);
+        return $this->nodeFactory->createArray([$class, $method]);
     }
 
     private function createClosureFromCallableCall(Expr $expr): StaticCall

diff --git a/rules/EarlyReturn/Rector/If_/ChangeAndIfToEarlyReturnRector.php b/rules/EarlyReturn/Rector/If_/ChangeAndIfToEarlyReturnRector.php
@@ -7,17 +7,14 @@
 use PhpParser\Node;
 use PhpParser\Node\Expr;
 use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
-use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Break_;
-use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Continue_;
 use PhpParser\Node\Stmt\Else_;
 use PhpParser\Node\Stmt\ElseIf_;
-use PhpParser\Node\Stmt\Foreach_;
-use PhpParser\Node\Stmt\Function_;
 use PhpParser\Node\Stmt\If_;
 use PhpParser\Node\Stmt\Return_;
+use Rector\Core\Contract\PhpParser\Node\StmtsAwareInterface;
 use Rector\Core\NodeManipulator\IfManipulator;
 use Rector\Core\Rector\AbstractRector;
 use Rector\EarlyReturn\NodeFactory\InvertedIfFactory;
@@ -85,26 +82,16 @@ public function canDrive(Car $car)
      */
     public function getNodeTypes(): array
     {
-        return [
-            Function_::class,
-            ClassMethod::class,
-            Closure::class,
-            Foreach_::class,
-            If_::class,
-            Else_::class,
-            ElseIf_::class,
-        ];
+        return [StmtsAwareInterface::class];
     }
 
     /**
-     * @param Function_|ClassMethod|Closure|Foreach_|If_|Else_|ElseIf_ $node
+     * @param StmtsAwareInterface $node
      */
     public function refactor(Node $node): ?Node
     {
-        /** @var Stmt[]|null $stmts */
-        $stmts = $node->stmts;
-
-        if ($stmts === null) {
+        $stmts = (array) $node->stmts;
+        if ($stmts === []) {
             return null;
         }
 
@@ -217,7 +204,7 @@ private function processReplaceIfs(
         return array_merge($result, [$ifNextReturnClone]);
     }
 
-    private function shouldSkip(Node $parentNode, If_ $if, ?Stmt $nexStmt): bool
+    private function shouldSkip(StmtsAwareInterface $stmtsAware, If_ $if, ?Stmt $nexStmt): bool
     {
         if (! $this->ifManipulator->isIfWithOnlyOneStmt($if)) {
             return true;
@@ -231,11 +218,11 @@ private function shouldSkip(Node $parentNode, If_ $if, ?Stmt $nexStmt): bool
             return true;
         }
 
-        if ($this->isParentIfReturnsVoidOrParentIfHasNextNode($parentNode)) {
+        if ($this->isParentIfReturnsVoidOrParentIfHasNextNode($stmtsAware)) {
             return true;
         }
 
-        if ($this->isNestedIfInLoop($if, $parentNode)) {
+        if ($this->isNestedIfInLoop($if, $stmtsAware)) {
             return true;
         }
 
@@ -262,23 +249,23 @@ private function isIfStmtExprUsedInNextReturn(If_ $if, Return_ $return): bool
         return false;
     }
 
-    private function isParentIfReturnsVoidOrParentIfHasNextNode(Node $parentNode): bool
+    private function isParentIfReturnsVoidOrParentIfHasNextNode(StmtsAwareInterface $stmtsAware): bool
     {
-        if (! $parentNode instanceof If_) {
+        if (! $stmtsAware instanceof If_) {
             return false;
         }
 
-        $nextParent = $parentNode->getAttribute(AttributeKey::NEXT_NODE);
+        $nextParent = $stmtsAware->getAttribute(AttributeKey::NEXT_NODE);
         return $nextParent instanceof Node;
     }
 
-    private function isNestedIfInLoop(If_ $if, ?Node $parentNode): bool
+    private function isNestedIfInLoop(If_ $if, StmtsAwareInterface $stmtsAware): bool
     {
         if (! $this->contextAnalyzer->isInLoop($if)) {
             return false;
         }
 
-        return $parentNode instanceof If_ || $parentNode instanceof Else_ || $parentNode instanceof ElseIf_;
+        return $stmtsAware instanceof If_ || $stmtsAware instanceof Else_ || $stmtsAware instanceof ElseIf_;
     }
 
     private function isLastIfOrBeforeLastReturn(If_ $if, ?Stmt $nextStmt): bool

diff --git a/rules/Php70/Rector/FuncCall/NonVariableToVariableOnFunctionCallRector.php b/rules/Php70/Rector/FuncCall/NonVariableToVariableOnFunctionCallRector.php
@@ -102,13 +102,12 @@ public function refactor(Node $node): ?Node
 
             $replacements = $this->getReplacementsFor($argument, $currentScope, $scopeNode);
 
-            $currentStatement = $this->betterNodeFinder->resolveCurrentStatement($node);
-
-            if (! $currentStatement instanceof Stmt) {
+            $currentStmt = $this->betterNodeFinder->resolveCurrentStatement($node);
+            if (! $currentStmt instanceof Stmt) {
                 continue;
             }
 
-            $this->nodesToAddCollector->addNodeBeforeNode($replacements->getAssign(), $currentStatement);
+            $this->nodesToAddCollector->addNodeBeforeNode($replacements->getAssign(), $currentStmt);
 
             $node->args[$key]->value = $replacements->getVariable();
 

diff --git a/rules/Php81/Rector/Class_/SpatieEnumClassToEnumRector.php b/rules/Php81/Rector/Class_/SpatieEnumClassToEnumRector.php
@@ -6,6 +6,7 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\Enum_;
 use PHPStan\Type\ObjectType;
 use Rector\Core\Rector\AbstractRector;
 use Rector\Core\ValueObject\PhpVersionFeature;
@@ -73,7 +74,7 @@ public function getNodeTypes(): array
     /**
      * @param Class_ $node
      */
-    public function refactor(Node $node): ?Node
+    public function refactor(Node $node): ?Enum_
     {
         if (! $this->isObjectType($node, new ObjectType('Spatie\Enum\Enum'))) {
             return null;

diff --git a/rules/Renaming/NodeManipulator/ClassRenamer.php b/rules/Renaming/NodeManipulator/ClassRenamer.php
@@ -44,6 +44,11 @@ final class ClassRenamer
      */
     private array $alreadyProcessedClasses = [];
 
+    /**
+     * @var array<string, OldToNewType[]>
+     */
+    private array $oldToNewTypesByCacheKey = [];
+
     public function __construct(
         private readonly BetterNodeFinder $betterNodeFinder,
         private readonly SimpleCallableNodeTraverser $simpleCallableNodeTraverser,
@@ -64,11 +69,7 @@ public function __construct(
      */
     public function renameNode(Node $node, array $oldToNewClasses): ?Node
     {
-        $oldToNewTypes = [];
-        foreach ($oldToNewClasses as $oldClass => $newClass) {
-            $oldToNewTypes[] = new OldToNewType(new ObjectType($oldClass), new FullyQualifiedObjectType($newClass));
-        }
-
+        $oldToNewTypes = $this->createOldToNewTypes($oldToNewClasses);
         $this->refactorPhpDoc($node, $oldToNewTypes, $oldToNewClasses);
 
         if ($node instanceof Name) {
@@ -432,4 +433,29 @@ private function shouldRemoveUseName(string $last, string $newNameLastName, bool
     {
         return $last === $newNameLastName && $importNames;
     }
+
+    /**
+     * @param array<string, string> $oldToNewClasses
+     * @return OldToNewType[]
+     */
+    private function createOldToNewTypes(array $oldToNewClasses): array
+    {
+        $cacheKey = md5(serialize($oldToNewClasses));
+
+        if (isset($this->oldToNewTypesByCacheKey[$cacheKey])) {
+            return $this->oldToNewTypesByCacheKey[$cacheKey];
+        }
+
+        $oldToNewTypes = [];
+
+        foreach ($oldToNewClasses as $oldClass => $newClass) {
+            $oldObjectType = new ObjectType($oldClass);
+            $newObjectType = new FullyQualifiedObjectType($newClass);
+            $oldToNewTypes[] = new OldToNewType($oldObjectType, $newObjectType);
+        }
+
+        $this->oldToNewTypesByCacheKey[$cacheKey] = $oldToNewTypes;
+
+        return $oldToNewTypes;
+    }
 }
diff --git a/rules/Renaming/Rector/FileWithoutNamespace/PseudoNamespaceToNamespaceRector.php b/rules/Renaming/Rector/FileWithoutNamespace/PseudoNamespaceToNamespaceRector.php
@@ -90,10 +90,7 @@ public function refactor(Node $node): ?Node
 
             // add a new namespace?
             if ($this->newNamespace !== null) {
-                $namespace = new Namespace_(new Name($this->newNamespace));
-                $namespace->stmts = $stmts;
-
-                return $namespace;
+                return new Namespace_(new Name($this->newNamespace), $stmts);
             }
         }