From 1e6e8bc12260c06dc07c0b56b9bb2bd23c9dcc3d Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Fri, 16 Oct 2026 12:14:16 +0200 Subject: [PATCH] Skip static const fetch overridden by child class in ForbiddenStaticClassConstFetchRule --- README.md | 23 ++++ config/static-rules.neon | 6 + .../ClassConstantDeclarationCollector.php | 47 ++++++++ .../StaticClassConstFetchCollector.php | 57 +++++++++ .../ForbiddenStaticClassConstFetchRule.php | 108 +++++++++++++++--- .../Fixture/ConstNotOverriddenByChild.php | 20 ++++ .../Fixture/SkipConstOverriddenByChild.php | 18 +++ .../Fixture/SkipStaticClass.php | 11 ++ ...ForbiddenStaticClassConstFetchRuleTest.php | 23 ++++ .../config/configured_rule.neon | 12 +- 10 files changed, 307 insertions(+), 18 deletions(-) create mode 100644 src/Collector/ClassConstantDeclarationCollector.php create mode 100644 src/Collector/StaticClassConstFetchCollector.php create mode 100644 tests/Rules/ForbiddenStaticClassConstFetchRule/Fixture/ConstNotOverriddenByChild.php create mode 100644 tests/Rules/ForbiddenStaticClassConstFetchRule/Fixture/SkipConstOverriddenByChild.php create mode 100644 tests/Rules/ForbiddenStaticClassConstFetchRule/Fixture/SkipStaticClass.php diff --git a/README.md b/README.md index 07990b22e..4a861aa6d 100644 --- a/README.md +++ b/README.md @@ -747,6 +747,29 @@ class SomeClass
+Skipped when a child class overrides the constant, as late static binding then changes the value on purpose. `static::class` is always allowed. + +```php +class ParentClass +{ + protected const SOME_CONST = 'parent'; + + public function run() + { + return static::SOME_CONST; + } +} + +class ChildClass extends ParentClass +{ + protected const SOME_CONST = 'child'; +} +``` + +:+1: + +
+ ### NoDynamicNameRule Use explicit names over dynamic ones diff --git a/config/static-rules.neon b/config/static-rules.neon index 80ccbf8aa..990d220f3 100644 --- a/config/static-rules.neon +++ b/config/static-rules.neon @@ -15,6 +15,10 @@ conditionalTags: phpstan.rules.rule: %symplify.static% Symplify\PHPStanRules\Rules\ForbiddenStaticClassConstFetchRule: phpstan.rules.rule: %symplify.static% + Symplify\PHPStanRules\Collector\StaticClassConstFetchCollector: + phpstan.collector: %symplify.static% + Symplify\PHPStanRules\Collector\ClassConstantDeclarationCollector: + phpstan.collector: %symplify.static% Symplify\PHPStanRules\Rules\Complexity\ForbiddenArrayMethodCallRule: phpstan.rules.rule: %symplify.static% @@ -41,6 +45,8 @@ services: - Symplify\PHPStanRules\Rules\ForbiddenMultipleClassLikeInOneFileRule - Symplify\PHPStanRules\Rules\ForbiddenStaticClassConstFetchRule + - Symplify\PHPStanRules\Collector\StaticClassConstFetchCollector + - Symplify\PHPStanRules\Collector\ClassConstantDeclarationCollector - Symplify\PHPStanRules\Rules\Complexity\ForbiddenArrayMethodCallRule # docblock diff --git a/src/Collector/ClassConstantDeclarationCollector.php b/src/Collector/ClassConstantDeclarationCollector.php new file mode 100644 index 000000000..b1ce88857 --- /dev/null +++ b/src/Collector/ClassConstantDeclarationCollector.php @@ -0,0 +1,47 @@ + + */ +final class ClassConstantDeclarationCollector implements Collector +{ + public function getNodeType(): string + { + return Class_::class; + } + + /** + * @return array{string, string|null, string[]}|null the class name, its parent class name and the + * constant names it declares + */ + public function processNode(Node $node, Scope $scope): ?array + { + if (! $node->namespacedName instanceof Name) { + return null; + } + + $parentClassName = $node->extends instanceof Name ? $node->extends->toString() : null; + + $constantNames = []; + foreach ($node->getConstants() as $classConst) { + foreach ($classConst->consts as $const) { + $constantNames[] = $const->name->toString(); + } + } + + return [$node->namespacedName->toString(), $parentClassName, $constantNames]; + } +} diff --git a/src/Collector/StaticClassConstFetchCollector.php b/src/Collector/StaticClassConstFetchCollector.php new file mode 100644 index 000000000..820972489 --- /dev/null +++ b/src/Collector/StaticClassConstFetchCollector.php @@ -0,0 +1,57 @@ + + */ +final class StaticClassConstFetchCollector implements Collector +{ + public function getNodeType(): string + { + return ClassConstFetch::class; + } + + /** + * @return array{string, string, int}|null the class the fetch lives in, the constant name and its line + */ + public function processNode(Node $node, Scope $scope): ?array + { + if (! $node->class instanceof Name) { + return null; + } + + if ($node->class->toString() !== 'static') { + return null; + } + + // "static::class" is late static binding magic, not an overridable constant + if (! $node->name instanceof Identifier || $node->name->toLowerString() === 'class') { + return null; + } + + $classReflection = $scope->getClassReflection(); + if (! $classReflection instanceof ClassReflection) { + return null; + } + + // abstract class is designed to be extended, so static fetch is expected + if ($classReflection->isAbstract()) { + return null; + } + + return [$classReflection->getName(), $node->name->toString(), $node->getStartLine()]; + } +} diff --git a/src/Rules/ForbiddenStaticClassConstFetchRule.php b/src/Rules/ForbiddenStaticClassConstFetchRule.php index 32d4869a4..03587f1a8 100644 --- a/src/Rules/ForbiddenStaticClassConstFetchRule.php +++ b/src/Rules/ForbiddenStaticClassConstFetchRule.php @@ -5,16 +5,18 @@ namespace Symplify\PHPStanRules\Rules; use PhpParser\Node; -use PhpParser\Node\Expr\ClassConstFetch; -use PhpParser\Node\Name; use PHPStan\Analyser\Scope; -use PHPStan\Reflection\ClassReflection; +use PHPStan\Node\CollectedDataNode; +use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use Symplify\PHPStanRules\Collector\ClassConstantDeclarationCollector; +use Symplify\PHPStanRules\Collector\StaticClassConstFetchCollector; /** - * @implements Rule * @see \Symplify\PHPStanRules\Tests\Rules\ForbiddenStaticClassConstFetchRule\ForbiddenStaticClassConstFetchRuleTest + * + * @implements Rule */ final class ForbiddenStaticClassConstFetchRule implements Rule { @@ -25,29 +27,103 @@ final class ForbiddenStaticClassConstFetchRule implements Rule public function getNodeType(): string { - return ClassConstFetch::class; + return CollectedDataNode::class; } /** - * @param ClassConstFetch $node + * @param CollectedDataNode $node + * @return list */ public function processNode(Node $node, Scope $scope): array { - if (! $node->class instanceof Name) { - return []; + [$constantNamesByClass, $parentClassByClass] = $this->resolveClassMaps($node); + + /** @var array> $fetchesByFilePath */ + $fetchesByFilePath = $node->get(StaticClassConstFetchCollector::class); + + $ruleErrors = []; + + foreach ($fetchesByFilePath as $filePath => $fetches) { + foreach ($fetches as [$className, $constantName, $line]) { + // a child class overriding the constant makes late static binding meaningful, so keep it + if ($this->isOverriddenInChildClass($className, $constantName, $constantNamesByClass, $parentClassByClass)) { + continue; + } + + $ruleErrors[] = RuleErrorBuilder::message(self::ERROR_MESSAGE) + ->identifier('symplify.forbiddenStaticClassConstFetch') + ->file($filePath) + ->line($line) + ->build(); + } + } + + return $ruleErrors; + } + + /** + * @return array{array, array} constant names and parent class per class + */ + private function resolveClassMaps(CollectedDataNode $collectedDataNode): array + { + /** @var array> $declarationsByFilePath */ + $declarationsByFilePath = $collectedDataNode->get(ClassConstantDeclarationCollector::class); + + $constantNamesByClass = []; + $parentClassByClass = []; + + foreach ($declarationsByFilePath as $declarations) { + foreach ($declarations as [$className, $parentClassName, $constantNames]) { + $constantNamesByClass[$className] = $constantNames; + $parentClassByClass[$className] = $parentClassName; + } } - if ($node->class->toString() !== 'static') { - return []; + return [$constantNamesByClass, $parentClassByClass]; + } + + /** + * @param array $constantNamesByClass + * @param array $parentClassByClass + */ + private function isOverriddenInChildClass( + string $parentClassName, + string $constantName, + array $constantNamesByClass, + array $parentClassByClass + ): bool { + foreach ($constantNamesByClass as $declaringClassName => $constantNames) { + if ($declaringClassName === $parentClassName) { + continue; + } + + if (! in_array($constantName, $constantNames, true)) { + continue; + } + + if ($this->isSubclassOf($declaringClassName, $parentClassName, $parentClassByClass)) { + return true; + } } - $classReflection = $scope->getClassReflection(); - if ($classReflection instanceof ClassReflection && $classReflection->isAbstract()) { - return []; + return false; + } + + /** + * @param array $parentClassByClass + */ + private function isSubclassOf(string $className, string $ancestorClassName, array $parentClassByClass): bool + { + $currentClassName = $parentClassByClass[$className] ?? null; + + while ($currentClassName !== null) { + if ($currentClassName === $ancestorClassName) { + return true; + } + + $currentClassName = $parentClassByClass[$currentClassName] ?? null; } - return [RuleErrorBuilder::message(self::ERROR_MESSAGE) - ->identifier('symplify.forbiddenStaticClassConstFetch') - ->build()]; + return false; } } diff --git a/tests/Rules/ForbiddenStaticClassConstFetchRule/Fixture/ConstNotOverriddenByChild.php b/tests/Rules/ForbiddenStaticClassConstFetchRule/Fixture/ConstNotOverriddenByChild.php new file mode 100644 index 000000000..0076d0e95 --- /dev/null +++ b/tests/Rules/ForbiddenStaticClassConstFetchRule/Fixture/ConstNotOverriddenByChild.php @@ -0,0 +1,20 @@ +getByType(ForbiddenStaticClassConstFetchRule::class); } + + /** + * @return list> + */ + #[Override] + protected function getCollectors(): array + { + return [ + self::getContainer()->getByType(StaticClassConstFetchCollector::class), + self::getContainer()->getByType(ClassConstantDeclarationCollector::class), + ]; + } } diff --git a/tests/Rules/ForbiddenStaticClassConstFetchRule/config/configured_rule.neon b/tests/Rules/ForbiddenStaticClassConstFetchRule/config/configured_rule.neon index 7edb61a51..b33f1755a 100644 --- a/tests/Rules/ForbiddenStaticClassConstFetchRule/config/configured_rule.neon +++ b/tests/Rules/ForbiddenStaticClassConstFetchRule/config/configured_rule.neon @@ -1,5 +1,13 @@ includes: - ../../../config/included_services.neon -rules: - - Symplify\PHPStanRules\Rules\ForbiddenStaticClassConstFetchRule +services: + - + class: Symplify\PHPStanRules\Rules\ForbiddenStaticClassConstFetchRule + tags: [phpstan.rules.rule] + - + class: Symplify\PHPStanRules\Collector\StaticClassConstFetchCollector + tags: [phpstan.collector] + - + class: Symplify\PHPStanRules\Collector\ClassConstantDeclarationCollector + tags: [phpstan.collector]