diff --git a/phpstan.neon b/phpstan.neon index aed17b5168d..fa25c0a2eb2 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -4,6 +4,7 @@ includes: rules: - Rector\Utils\PHPStan\Rule\RegisterRelatedPolyfillRectorRule + - Rector\Utils\PHPStan\Rule\NoResolveClassReflectionInForeachRule parameters: level: 8 diff --git a/rules/DeadCode/Rector/ClassConst/RemoveUnusedPrivateClassConstantRector.php b/rules/DeadCode/Rector/ClassConst/RemoveUnusedPrivateClassConstantRector.php index 07ad4e1a5e8..d685412baac 100644 --- a/rules/DeadCode/Rector/ClassConst/RemoveUnusedPrivateClassConstantRector.php +++ b/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; } diff --git a/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php b/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php index 6713642b2e1..b5e9c878ece 100644 --- a/rules/TypeDeclaration/Rector/Class_/PropertyTypeFromStrictSetterGetterRector.php +++ b/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; } diff --git a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector.php b/rules/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector.php index dd9e943fa85..7a6cc4def96 100644 --- a/rules/TypeDeclaration/Rector/Property/TypedPropertyFromAssignsRector.php +++ b/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; } diff --git a/utils/phpstan/src/Rule/NoResolveClassReflectionInForeachRule.php b/utils/phpstan/src/Rule/NoResolveClassReflectionInForeachRule.php new file mode 100644 index 00000000000..0666675ca95 --- /dev/null +++ b/utils/phpstan/src/Rule/NoResolveClassReflectionInForeachRule.php @@ -0,0 +1,104 @@ + + * @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 + */ + 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; + } +} diff --git a/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/NoResolveClassReflectionInForeachRuleTest.php b/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/NoResolveClassReflectionInForeachRuleTest.php new file mode 100644 index 00000000000..281a256126d --- /dev/null +++ b/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/NoResolveClassReflectionInForeachRuleTest.php @@ -0,0 +1,37 @@ + + */ +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(); + } +} diff --git a/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/CallInForeach.php b/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/CallInForeach.php new file mode 100644 index 00000000000..f3010ad1ddf --- /dev/null +++ b/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/CallInForeach.php @@ -0,0 +1,25 @@ +reflectionResolver->resolveClassReflection($stmt); + if ($classReflection === null) { + continue; + } + } + } +} diff --git a/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/ReflectionResolver.php b/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/ReflectionResolver.php new file mode 100644 index 00000000000..a42f73952a0 --- /dev/null +++ b/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/ReflectionResolver.php @@ -0,0 +1,15 @@ +reflectionResolver->resolveClassReflection($stmts[0]); + if ($classReflection === null) { + return; + } + + foreach ($stmts as $stmt) { + // reflection already resolved above the loop + } + } +} diff --git a/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/SkipNonRectorClass.php b/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/SkipNonRectorClass.php new file mode 100644 index 00000000000..8ba629b7976 --- /dev/null +++ b/utils/phpstan/tests/Rule/NoResolveClassReflectionInForeachRule/Source/SkipNonRectorClass.php @@ -0,0 +1,23 @@ +reflectionResolver->resolveClassReflection($stmt); + if ($classReflection === null) { + continue; + } + } + } +}