From b96f17bed78cf9e16446a3c8f5a3701a695d474e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 17 Dec 2024 11:41:41 +0100 Subject: [PATCH 1/7] Readonly classes cannot be combined with `#[AllowDynamicProperties]` --- Makefile | 3 + conf/config.level0.neon | 1 + src/Rules/Classes/ClassAttributesRule.php | 32 +++++- src/Rules/Traits/TraitAttributesRule.php | 61 ++++++++++++ .../Rules/Classes/ClassAttributesRuleTest.php | 24 +++++ .../PHPStan/Rules/Classes/data/bug-12281.php | 16 +++ .../Rules/Traits/TraitAttributesRuleTest.php | 99 +++++++++++++++++++ tests/PHPStan/Rules/Traits/data/bug-12011.php | 26 +++++ tests/PHPStan/Rules/Traits/data/bug-12281.php | 12 +++ .../Rules/Traits/data/trait-attributes.php | 21 ++++ 10 files changed, 294 insertions(+), 1 deletion(-) create mode 100644 src/Rules/Traits/TraitAttributesRule.php create mode 100644 tests/PHPStan/Rules/Classes/data/bug-12281.php create mode 100644 tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php create mode 100644 tests/PHPStan/Rules/Traits/data/bug-12011.php create mode 100644 tests/PHPStan/Rules/Traits/data/bug-12281.php create mode 100644 tests/PHPStan/Rules/Traits/data/trait-attributes.php diff --git a/Makefile b/Makefile index ad6075e3bf..e93608e7b5 100644 --- a/Makefile +++ b/Makefile @@ -87,6 +87,9 @@ lint: --exclude tests/PHPStan/Rules/Properties/data/non-abstract-hooked-properties-in-class.php \ --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-in-class.php \ --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php \ + --exclude tests/PHPStan/Rules/Classes/data/bug-12281.php \ + --exclude tests/PHPStan/Rules/Traits/data/bug-12281.php \ + --exclude tests/PHPStan/Rules/Traits/data/bug-12011.php \ src tests cs: diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 3964727b8d..c84cf8f5f7 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -103,6 +103,7 @@ rules: - PHPStan\Rules\Regexp\RegularExpressionPatternRule - PHPStan\Rules\Traits\ConflictingTraitConstantsRule - PHPStan\Rules\Traits\ConstantsInTraitsRule + - PHPStan\Rules\Traits\TraitAttributesRule - PHPStan\Rules\Types\InvalidTypesInUnionRule - PHPStan\Rules\Variables\UnsetRule - PHPStan\Rules\Whitespace\FileWhitespaceRule diff --git a/src/Rules/Classes/ClassAttributesRule.php b/src/Rules/Classes/ClassAttributesRule.php index afe9786db0..190be4331b 100644 --- a/src/Rules/Classes/ClassAttributesRule.php +++ b/src/Rules/Classes/ClassAttributesRule.php @@ -8,6 +8,9 @@ use PHPStan\Node\InClassNode; use PHPStan\Rules\AttributesCheck; use PHPStan\Rules\Rule; +use PHPStan\Rules\RuleErrorBuilder; +use function count; +use function sprintf; /** * @implements Rule @@ -28,12 +31,39 @@ public function processNode(Node $node, Scope $scope): array { $classLikeNode = $node->getOriginalNode(); - return $this->attributesCheck->check( + $errors = $this->attributesCheck->check( $scope, $classLikeNode->attrGroups, Attribute::TARGET_CLASS, 'class', ); + + $classReflection = $node->getClassReflection(); + if ( + $classReflection->isReadOnly() + || $classReflection->isEnum() + || $classReflection->isInterface() + ) { + $typeName = 'readonly class'; + $identifier = 'class.allowDynamicPropertiesReadonly'; + if ($classReflection->isEnum()) { + $typeName = 'enum'; + $identifier = 'enum.allowDynamicProperties'; + } + if ($classReflection->isInterface()) { + $typeName = 'interface'; + $identifier = 'interface.allowDynamicProperties'; + } + + if (count($classReflection->getNativeReflection()->getAttributes('AllowDynamicProperties')) > 0) { + $errors[] = RuleErrorBuilder::message(sprintf('Attribute class AllowDynamicProperties cannot be used with %s.', $typeName)) + ->identifier($identifier) + ->nonIgnorable() + ->build(); + } + } + + return $errors; } } diff --git a/src/Rules/Traits/TraitAttributesRule.php b/src/Rules/Traits/TraitAttributesRule.php new file mode 100644 index 0000000000..dddf566666 --- /dev/null +++ b/src/Rules/Traits/TraitAttributesRule.php @@ -0,0 +1,61 @@ + + */ +final class TraitAttributesRule implements Rule +{ + + public function __construct( + private AttributesCheck $attributesCheck, + private ReflectionProvider $reflectionProvider, + ) + { + } + + public function getNodeType(): string + { + return Node\Stmt\Trait_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + $traitName = $node->namespacedName; + if ($traitName === null) { + return []; + } + + if (!$this->reflectionProvider->hasClass($traitName->toString())) { + return []; + } + + $errors = $this->attributesCheck->check( + $scope, + $node->attrGroups, + Attribute::TARGET_CLASS, + 'class', + ); + + $classReflection = $this->reflectionProvider->getClass($traitName->toString()); + if (count($classReflection->getNativeReflection()->getAttributes('AllowDynamicProperties')) > 0) { + $errors[] = RuleErrorBuilder::message('Attribute class AllowDynamicProperties cannot be used with trait.') + ->identifier('class.allowDynamicPropertiesTrait') + ->nonIgnorable() + ->build(); + } + + return $errors; + } + +} diff --git a/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php b/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php index 1d72b629a8..18e7c7a1fd 100644 --- a/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php @@ -167,4 +167,28 @@ public function testBug12011(): void ]); } + public function testBug12281(): void + { + if (PHP_VERSION_ID < 80200) { + $this->markTestSkipped('Test requires PHP 8.2.'); + } + + $this->checkExplicitMixed = true; + $this->checkImplicitMixed = true; + $this->analyse([__DIR__ . '/data/bug-12281.php'], [ + [ + 'Attribute class AllowDynamicProperties cannot be used with readonly class.', + 05, + ], + [ + 'Attribute class AllowDynamicProperties cannot be used with enum.', + 12, + ], + [ + 'Attribute class AllowDynamicProperties cannot be used with interface.', + 15, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Classes/data/bug-12281.php b/tests/PHPStan/Rules/Classes/data/bug-12281.php new file mode 100644 index 0000000000..293d9e5e41 --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/bug-12281.php @@ -0,0 +1,16 @@ += 8.2 + +namespace Bug12281; + +#[\AllowDynamicProperties] +readonly class BlogData { /* … */ } + +/** @readonly */ +#[\AllowDynamicProperties] +class BlogDataPhpdoc { /* … */ } + +#[\AllowDynamicProperties] +enum BlogDataEnum { /* … */ } + +#[\AllowDynamicProperties] +interface BlogDataInterface { /* … */ } diff --git a/tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php b/tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php new file mode 100644 index 0000000000..046dbba631 --- /dev/null +++ b/tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php @@ -0,0 +1,99 @@ + + */ +class TraitAttributesRuleTest extends RuleTestCase +{ + + private bool $checkExplicitMixed = false; + + private bool $checkImplicitMixed = false; + + protected function getRule(): Rule + { + $reflectionProvider = $this->createReflectionProvider(); + return new TraitAttributesRule( + new AttributesCheck( + $reflectionProvider, + new FunctionCallParametersCheck( + new RuleLevelHelper($reflectionProvider, true, false, true, $this->checkExplicitMixed, $this->checkImplicitMixed, false), + new NullsafeCheck(), + new PhpVersion(80000), + new UnresolvableTypeHelper(), + new PropertyReflectionFinder(), + true, + true, + true, + true, + ), + new ClassNameCheck( + new ClassCaseSensitivityCheck($reflectionProvider, false), + new ClassForbiddenNameCheck(self::getContainer()), + ), + true, + ), + $reflectionProvider, + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/trait-attributes.php'], [ + [ + 'Attribute class TraitAttributes\AbstractAttribute is abstract.', + 8, + ], + [ + 'Attribute class TraitAttributes\MyTargettedAttribute does not have the class target.', + 20, + ], + ]); + } + + public function testBug12011(): void + { + if (PHP_VERSION_ID < 80300) { + $this->markTestSkipped('Test requires PHP 8.3.'); + } + + $this->checkExplicitMixed = true; + $this->checkImplicitMixed = true; + + $this->analyse([__DIR__ . '/data/bug-12011.php'], [ + [ + 'Parameter #1 $name of attribute class Bug12011Trait\Table constructor expects string|null, int given.', + 8, + ], + ]); + } + + public function testBug12281(): void + { + $this->analyse([__DIR__ . '/data/bug-12281.php'], [ + [ + 'Attribute class AllowDynamicProperties cannot be used with trait.', + 11, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Traits/data/bug-12011.php b/tests/PHPStan/Rules/Traits/data/bug-12011.php new file mode 100644 index 0000000000..c25bc5c111 --- /dev/null +++ b/tests/PHPStan/Rules/Traits/data/bug-12011.php @@ -0,0 +1,26 @@ += 8.3 + +namespace Bug12011Trait; + +use Attribute; + + +#[Table(self::TABLE_NAME)] +trait MyTrait +{ + private const int TABLE_NAME = 'table'; +} + +class X { + use MyTrait; +} + +#[Attribute(Attribute::TARGET_CLASS)] +final class Table +{ + public function __construct( + public readonly string|null $name = null, + public readonly string|null $schema = null, + ) { + } +} diff --git a/tests/PHPStan/Rules/Traits/data/bug-12281.php b/tests/PHPStan/Rules/Traits/data/bug-12281.php new file mode 100644 index 0000000000..79f0f3e76e --- /dev/null +++ b/tests/PHPStan/Rules/Traits/data/bug-12281.php @@ -0,0 +1,12 @@ += 8.2 + +namespace Bug12281Traits; + +#[\AllowDynamicProperties] +enum BlogDataEnum { /* … */ } // reported by ClassAttributesRule + +#[\AllowDynamicProperties] +interface BlogDataInterface { /* … */ } // reported by ClassAttributesRule + +#[\AllowDynamicProperties] +trait BlogDataTrait { /* … */ } diff --git a/tests/PHPStan/Rules/Traits/data/trait-attributes.php b/tests/PHPStan/Rules/Traits/data/trait-attributes.php new file mode 100644 index 0000000000..ea7acde5b8 --- /dev/null +++ b/tests/PHPStan/Rules/Traits/data/trait-attributes.php @@ -0,0 +1,21 @@ + Date: Thu, 19 Dec 2024 21:37:02 +0100 Subject: [PATCH 2/7] Update TraitAttributesRule.php Co-authored-by: Markus Staab --- src/Rules/Traits/TraitAttributesRule.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Rules/Traits/TraitAttributesRule.php b/src/Rules/Traits/TraitAttributesRule.php index dddf566666..0206c245b3 100644 --- a/src/Rules/Traits/TraitAttributesRule.php +++ b/src/Rules/Traits/TraitAttributesRule.php @@ -50,7 +50,7 @@ public function processNode(Node $node, Scope $scope): array $classReflection = $this->reflectionProvider->getClass($traitName->toString()); if (count($classReflection->getNativeReflection()->getAttributes('AllowDynamicProperties')) > 0) { $errors[] = RuleErrorBuilder::message('Attribute class AllowDynamicProperties cannot be used with trait.') - ->identifier('class.allowDynamicPropertiesTrait') + ->identifier('trait.allowDynamicProperties') ->nonIgnorable() ->build(); } From 0b750a9d11b0f6a0bfc484e3015ab2ef3af2bf3e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 19 Dec 2024 22:01:07 +0100 Subject: [PATCH 3/7] work in progress.. crashing atm --- src/Rules/Traits/TraitAttributesRule.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/Rules/Traits/TraitAttributesRule.php b/src/Rules/Traits/TraitAttributesRule.php index 0206c245b3..efd33c21ef 100644 --- a/src/Rules/Traits/TraitAttributesRule.php +++ b/src/Rules/Traits/TraitAttributesRule.php @@ -4,11 +4,13 @@ use Attribute; use PhpParser\Node; +use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\Scope; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\AttributesCheck; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\ShouldNotHappenException; use function count; /** @@ -39,6 +41,12 @@ public function processNode(Node $node, Scope $scope): array if (!$this->reflectionProvider->hasClass($traitName->toString())) { return []; } + $classReflection = $this->reflectionProvider->getClass($traitName->toString()); + + if (!$scope instanceof MutatingScope) { + throw new ShouldNotHappenException(); + } + $scope = $scope->enterTrait($classReflection); $errors = $this->attributesCheck->check( $scope, @@ -47,7 +55,6 @@ public function processNode(Node $node, Scope $scope): array 'class', ); - $classReflection = $this->reflectionProvider->getClass($traitName->toString()); if (count($classReflection->getNativeReflection()->getAttributes('AllowDynamicProperties')) > 0) { $errors[] = RuleErrorBuilder::message('Attribute class AllowDynamicProperties cannot be used with trait.') ->identifier('trait.allowDynamicProperties') From ff83891b4524b9c7fdbf091c48356ec35efcb512 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 19 Dec 2024 22:44:40 +0100 Subject: [PATCH 4/7] fix class fake --- src/Rules/Traits/TraitAttributesRule.php | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/Rules/Traits/TraitAttributesRule.php b/src/Rules/Traits/TraitAttributesRule.php index efd33c21ef..f11a7de5c1 100644 --- a/src/Rules/Traits/TraitAttributesRule.php +++ b/src/Rules/Traits/TraitAttributesRule.php @@ -4,6 +4,7 @@ use Attribute; use PhpParser\Node; +use PhpParser\Node\Stmt\Class_; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\Scope; use PHPStan\Reflection\ReflectionProvider; @@ -41,12 +42,15 @@ public function processNode(Node $node, Scope $scope): array if (!$this->reflectionProvider->hasClass($traitName->toString())) { return []; } - $classReflection = $this->reflectionProvider->getClass($traitName->toString()); + $traitClassReflection = $this->reflectionProvider->getClass($traitName->toString()); if (!$scope instanceof MutatingScope) { throw new ShouldNotHappenException(); } - $scope = $scope->enterTrait($classReflection); + $fakeClass = new Class_(null, [new Node\Stmt\TraitUse([$traitName])], ['startLine' => 1, 'endLine' => 1]); + $fakeClassReflection = $this->reflectionProvider->getAnonymousClassReflection($fakeClass, $scope); + $scope = $scope->enterClass($fakeClassReflection); + $scope = $scope->enterTrait($traitClassReflection); $errors = $this->attributesCheck->check( $scope, @@ -55,7 +59,7 @@ public function processNode(Node $node, Scope $scope): array 'class', ); - if (count($classReflection->getNativeReflection()->getAttributes('AllowDynamicProperties')) > 0) { + if (count($traitClassReflection->getNativeReflection()->getAttributes('AllowDynamicProperties')) > 0) { $errors[] = RuleErrorBuilder::message('Attribute class AllowDynamicProperties cannot be used with trait.') ->identifier('trait.allowDynamicProperties') ->nonIgnorable() From b1cebc2e1f5133b1bd68ebb4401947c5f4d0e324 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 20 Dec 2024 09:43:44 +0100 Subject: [PATCH 5/7] Use InTraitNode instead Some attribute arguments actually depend on using class instead of the trait So we have to analyse the attributes in context of each class that uses the trait --- src/Rules/Traits/TraitAttributesRule.php | 33 ++++--------------- .../Rules/Traits/TraitAttributesRuleTest.php | 3 -- tests/PHPStan/Rules/Traits/data/bug-12281.php | 7 ++++ .../Rules/Traits/data/trait-attributes.php | 9 +++++ 4 files changed, 22 insertions(+), 30 deletions(-) diff --git a/src/Rules/Traits/TraitAttributesRule.php b/src/Rules/Traits/TraitAttributesRule.php index f11a7de5c1..2b9fa768d0 100644 --- a/src/Rules/Traits/TraitAttributesRule.php +++ b/src/Rules/Traits/TraitAttributesRule.php @@ -4,62 +4,41 @@ use Attribute; use PhpParser\Node; -use PhpParser\Node\Stmt\Class_; -use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\Scope; -use PHPStan\Reflection\ReflectionProvider; +use PHPStan\Node\InTraitNode; use PHPStan\Rules\AttributesCheck; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; -use PHPStan\ShouldNotHappenException; use function count; /** - * @implements Rule + * @implements Rule */ final class TraitAttributesRule implements Rule { public function __construct( private AttributesCheck $attributesCheck, - private ReflectionProvider $reflectionProvider, ) { } public function getNodeType(): string { - return Node\Stmt\Trait_::class; + return InTraitNode::class; } public function processNode(Node $node, Scope $scope): array { - $traitName = $node->namespacedName; - if ($traitName === null) { - return []; - } - - if (!$this->reflectionProvider->hasClass($traitName->toString())) { - return []; - } - $traitClassReflection = $this->reflectionProvider->getClass($traitName->toString()); - - if (!$scope instanceof MutatingScope) { - throw new ShouldNotHappenException(); - } - $fakeClass = new Class_(null, [new Node\Stmt\TraitUse([$traitName])], ['startLine' => 1, 'endLine' => 1]); - $fakeClassReflection = $this->reflectionProvider->getAnonymousClassReflection($fakeClass, $scope); - $scope = $scope->enterClass($fakeClassReflection); - $scope = $scope->enterTrait($traitClassReflection); - + $originalNode = $node->getOriginalNode(); $errors = $this->attributesCheck->check( $scope, - $node->attrGroups, + $originalNode->attrGroups, Attribute::TARGET_CLASS, 'class', ); - if (count($traitClassReflection->getNativeReflection()->getAttributes('AllowDynamicProperties')) > 0) { + if (count($node->getTraitReflection()->getNativeReflection()->getAttributes('AllowDynamicProperties')) > 0) { $errors[] = RuleErrorBuilder::message('Attribute class AllowDynamicProperties cannot be used with trait.') ->identifier('trait.allowDynamicProperties') ->nonIgnorable() diff --git a/tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php b/tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php index 046dbba631..a3be5ead04 100644 --- a/tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php +++ b/tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php @@ -5,7 +5,6 @@ use PHPStan\Php\PhpVersion; use PHPStan\Rules\AttributesCheck; use PHPStan\Rules\ClassCaseSensitivityCheck; -use PHPStan\Rules\Classes\ClassAttributesRule; use PHPStan\Rules\ClassForbiddenNameCheck; use PHPStan\Rules\ClassNameCheck; use PHPStan\Rules\FunctionCallParametersCheck; @@ -14,7 +13,6 @@ use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleLevelHelper; -use PHPStan\Rules\Traits\TraitAttributesRule; use PHPStan\Testing\RuleTestCase; use const PHP_VERSION_ID; @@ -51,7 +49,6 @@ protected function getRule(): Rule ), true, ), - $reflectionProvider, ); } diff --git a/tests/PHPStan/Rules/Traits/data/bug-12281.php b/tests/PHPStan/Rules/Traits/data/bug-12281.php index 79f0f3e76e..da7d088f1a 100644 --- a/tests/PHPStan/Rules/Traits/data/bug-12281.php +++ b/tests/PHPStan/Rules/Traits/data/bug-12281.php @@ -10,3 +10,10 @@ interface BlogDataInterface { /* … */ } // reported by ClassAttributesRule #[\AllowDynamicProperties] trait BlogDataTrait { /* … */ } + +class Uses +{ + + use BlogDataTrait; + +} diff --git a/tests/PHPStan/Rules/Traits/data/trait-attributes.php b/tests/PHPStan/Rules/Traits/data/trait-attributes.php index ea7acde5b8..67906a2dfe 100644 --- a/tests/PHPStan/Rules/Traits/data/trait-attributes.php +++ b/tests/PHPStan/Rules/Traits/data/trait-attributes.php @@ -19,3 +19,12 @@ class MyTargettedAttribute {} #[MyTargettedAttribute] trait MyTrait3 {} + +class Uses +{ + + use MyTrait; + use MyTrait2; + use MyTrait3; + +} From b2b833c3d01c77b8f43f799b80b6ed0c65d68c1b Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 20 Dec 2024 09:49:52 +0100 Subject: [PATCH 6/7] Fix --- tests/PHPStan/Rules/Traits/data/bug-12011.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Traits/data/bug-12011.php b/tests/PHPStan/Rules/Traits/data/bug-12011.php index c25bc5c111..32b09d38d3 100644 --- a/tests/PHPStan/Rules/Traits/data/bug-12011.php +++ b/tests/PHPStan/Rules/Traits/data/bug-12011.php @@ -8,7 +8,7 @@ #[Table(self::TABLE_NAME)] trait MyTrait { - private const int TABLE_NAME = 'table'; + private const int TABLE_NAME = 1; } class X { From b35a845f43b3b02f9cd700785762a066c76bea9a Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 20 Dec 2024 09:52:56 +0100 Subject: [PATCH 7/7] This file can still be linted --- Makefile | 1 - 1 file changed, 1 deletion(-) diff --git a/Makefile b/Makefile index e93608e7b5..d8566bfdb0 100644 --- a/Makefile +++ b/Makefile @@ -89,7 +89,6 @@ lint: --exclude tests/PHPStan/Rules/Properties/data/hooked-properties-without-bodies-in-class.php \ --exclude tests/PHPStan/Rules/Classes/data/bug-12281.php \ --exclude tests/PHPStan/Rules/Traits/data/bug-12281.php \ - --exclude tests/PHPStan/Rules/Traits/data/bug-12011.php \ src tests cs: