[PHPStan] Add rule to hoist resolveClassReflection() above foreach in Rector rules

phpstan.neon

@@ -4,6 +4,7 @@ includes:
 
 rules:
     - Rector\Utils\PHPStan\Rule\RegisterRelatedPolyfillRectorRule
+    - Rector\Utils\PHPStan\Rule\NoResolveClassReflectionInForeachRule
 
 parameters:
     level: 8

rules/DeadCode/Rector/ClassConst/RemoveUnusedPrivateClassConstantRector.php

@@ -67,6 +67,11 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
+        $classReflection = $this->reflectionResolver->resolveClassReflection($node);
+        if (! $classReflection instanceof ClassReflection) {
+            return null;
+        }
+
         $unusedClassConstKeys = [];
         foreach ($node->stmts as $key => $stmt) {
             if (! $stmt instanceof ClassConst) {
@@ -77,11 +82,6 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
-            $classReflection = $this->reflectionResolver->resolveClassReflection($stmt);
-            if (! $classReflection instanceof ClassReflection) {
-                continue;
-            }
-
             if ($this->classConstManipulator->hasClassConstFetch($stmt, $classReflection, $node)) {
                 continue;
             }

rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php

@@ -107,8 +107,12 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
+        $classReflection = $this->reflectionResolver->resolveClassReflection($node);
+        if (! $classReflection instanceof ClassReflection) {
+            return null;
+        }
+
         $hasChanged = false;
-        $classReflection = null;
 
         foreach ($node->getProperties() as $property) {
             if ($property->type instanceof Node) {
@@ -138,14 +142,6 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
-            if (! $classReflection instanceof ClassReflection) {
-                $classReflection = $this->reflectionResolver->resolveClassReflection($node);
-            }
-
-            if (! $classReflection instanceof ClassReflection) {
-                return null;
-            }
-
             if (! $this->makePropertyTypedGuard->isLegal($property, $classReflection, false)) {
                 continue;
             }

rules/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector.php

@@ -136,8 +136,12 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
+        $classReflection = $this->reflectionResolver->resolveClassReflection($node);
+        if (! $classReflection instanceof ClassReflection) {
+            return null;
+        }
+
         $hasChanged = false;
-        $classReflection = null;
 
         foreach ($node->getProperties() as $property) {
             // non-private property can be anything with not inline public configured
@@ -149,14 +153,6 @@ public function refactor(Node $node): ?Node
                 continue;
             }
 
-            if (! $classReflection instanceof ClassReflection) {
-                $classReflection = $this->reflectionResolver->resolveClassReflection($node);
-            }
-
-            if (! $classReflection instanceof ClassReflection) {
-                return null;
-            }
-
             if (! $this->makePropertyTypedGuard->isLegal($property, $classReflection, $this->inlinePublic)) {
                 continue;
             }

utils/phpstan/src/Rule/NoResolveClassReflectionInForeachRule.php

@@ -0,0 +1,104 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Rule;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Stmt\Foreach_;
+use PhpParser\NodeFinder;
+use PHPStan\Analyser\Scope;
+use PHPStan\Reflection\ClassReflection;
+use PHPStan\Rules\IdentifierRuleError;
+use PHPStan\Rules\Rule;
+use PHPStan\Rules\RuleErrorBuilder;
+use Rector\Contract\Rector\RectorInterface;
+
+/**
+ * Inside a Rector rule, the refactored class reflection is identical for every
+ * loop iteration, so resolving it inside a foreach() is wasted work. Hoist the
+ * call above the loop.
+ *
+ * @implements Rule<Foreach_>
+ * @see \Rector\Utils\PHPStan\Tests\Rule\NoResolveClassReflectionInForeachRule\NoResolveClassReflectionInForeachRuleTest
+ */
+final readonly class NoResolveClassReflectionInForeachRule implements Rule
+{
+    private const string ERROR_MESSAGE = 'resolveClassReflection() is called inside a foreach() loop. The class reflection is the same for every iteration, hoist the call above the loop.';
+
+    private const string METHOD_NAME = 'resolveClassReflection';
+
+    private NodeFinder $nodeFinder;
+
+    public function __construct()
+    {
+        $this->nodeFinder = new NodeFinder();
+    }
+
+    public function getNodeType(): string
+    {
+        return Foreach_::class;
+    }
+
+    /**
+     * @param Foreach_ $node
+     * @return list<IdentifierRuleError>
+     */
+    public function processNode(Node $node, Scope $scope): array
+    {
+        $classReflection = $scope->getClassReflection();
+        if (! $classReflection instanceof ClassReflection) {
+            return [];
+        }
+
+        // only Rector rules resolve the single refactored class per node
+        if (! $classReflection->implementsInterface(RectorInterface::class)) {
+            return [];
+        }
+
+        $ruleErrors = [];
+        foreach ($this->matchResolveClassReflectionCalls($node) as $methodCall) {
+            $ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE)
+                ->identifier('rector.resolveClassReflectionInForeach')
+                ->line($methodCall->getStartLine())
+                ->build();
+        }
+
+        return $ruleErrors;
+    }
+
+    /**
+     * @return MethodCall[]
+     */
+    private function matchResolveClassReflectionCalls(Foreach_ $foreach): array
+    {
+        // skip calls nested in an inner foreach, that inner one reports them
+        $nestedCallObjectIds = [];
+        foreach ($this->nodeFinder->findInstanceOf($foreach->stmts, Foreach_::class) as $nestedForeach) {
+            foreach ($this->nodeFinder->findInstanceOf($nestedForeach->stmts, MethodCall::class) as $nestedCall) {
+                $nestedCallObjectIds[spl_object_id($nestedCall)] = true;
+            }
+        }
+
+        $matchedCalls = [];
+        foreach ($this->nodeFinder->findInstanceOf($foreach->stmts, MethodCall::class) as $methodCall) {
+            if (! $methodCall->name instanceof Identifier) {
+                continue;
+            }
+
+            if ($methodCall->name->toString() !== self::METHOD_NAME) {
+                continue;
+            }
+
+            if (isset($nestedCallObjectIds[spl_object_id($methodCall)])) {
+                continue;
+            }
+
+            $matchedCalls[] = $methodCall;
+        }
+
+        return $matchedCalls;
+    }
+}

utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/NoResolveClassReflectionInForeachRuleTest.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NoResolveClassReflectionInForeachRule;
+
+use PHPStan\Rules\Rule;
+use PHPStan\Testing\RuleTestCase;
+use Rector\Utils\PHPStan\Rule\NoResolveClassReflectionInForeachRule;
+
+/**
+ * @extends RuleTestCase<NoResolveClassReflectionInForeachRule>
+ */
+final class NoResolveClassReflectionInForeachRuleTest extends RuleTestCase
+{
+    public function testCallInForeach(): void
+    {
+        $expectedErrorMessage = 'resolveClassReflection() is called inside a foreach() loop. The class reflection is the same for every iteration, hoist the call above the loop.';
+
+        $this->analyse([__DIR__ . '/Source/CallInForeach.php'], [[$expectedErrorMessage, 19]]);
+    }
+
+    public function testSkipCallOutsideForeach(): void
+    {
+        $this->analyse([__DIR__ . '/Source/SkipCallOutsideForeach.php'], []);
+    }
+
+    public function testSkipNonRectorClass(): void
+    {
+        $this->analyse([__DIR__ . '/Source/SkipNonRectorClass.php'], []);
+    }
+
+    protected function getRule(): Rule
+    {
+        return new NoResolveClassReflectionInForeachRule();
+    }
+}

utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/CallInForeach.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NoResolveClassReflectionInForeachRule\Source;
+
+use Rector\Contract\Rector\RectorInterface;
+
+final class CallInForeach implements RectorInterface
+{
+    public function __construct(
+        private readonly ReflectionResolver $reflectionResolver
+    ) {
+    }
+
+    public function run(array $stmts): void
+    {
+        foreach ($stmts as $stmt) {
+            $classReflection = $this->reflectionResolver->resolveClassReflection($stmt);
+            if ($classReflection === null) {
+                continue;
+            }
+        }
+    }
+}

utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/ReflectionResolver.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NoResolveClassReflectionInForeachRule\Source;
+
+use PhpParser\Node;
+
+final class ReflectionResolver
+{
+    public function resolveClassReflection(Node $node): ?object
+    {
+        return null;
+    }
+}

utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/SkipCallOutsideForeach.php

@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NoResolveClassReflectionInForeachRule\Source;
+
+use Rector\Contract\Rector\RectorInterface;
+
+final class SkipCallOutsideForeach implements RectorInterface
+{
+    public function __construct(
+        private readonly ReflectionResolver $reflectionResolver
+    ) {
+    }
+
+    public function run(array $stmts): void
+    {
+        $classReflection = $this->reflectionResolver->resolveClassReflection($stmts[0]);
+        if ($classReflection === null) {
+            return;
+        }
+
+        foreach ($stmts as $stmt) {
+            // reflection already resolved above the loop
+        }
+    }
+}

utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/SkipNonRectorClass.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Utils\PHPStan\Tests\Rule\NoResolveClassReflectionInForeachRule\Source;
+
+final class SkipNonRectorClass
+{
+    public function __construct(
+        private readonly ReflectionResolver $reflectionResolver
+    ) {
+    }
+
+    public function run(array $stmts): void
+    {
+        foreach ($stmts as $stmt) {
+            $classReflection = $this->reflectionResolver->resolveClassReflection($stmt);
+            if ($classReflection === null) {
+                continue;
+            }
+        }
+    }
+}