From 05dfb04eec9b8e1c72571f140a3f2f61d876615e Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Tue, 17 Dec 2024 11:41:41 +0100 Subject: [PATCH] Readonly classes cannot be combined with `#[AllowDynamicProperties]` --- Makefile | 2 + conf/config.level0.neon | 1 + src/Rules/Classes/ClassAttributesRule.php | 32 +++++- src/Rules/Traits/TraitAttributesRule.php | 63 +++++++++++ .../Rules/Classes/ClassAttributesRuleTest.php | 24 +++++ .../PHPStan/Rules/Classes/data/bug-12281.php | 16 +++ .../Rules/Traits/TraitAttributesRuleTest.php | 100 ++++++++++++++++++ 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, 296 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 11807088ca..748ba49063 100644 --- a/Makefile +++ b/Makefile @@ -77,6 +77,8 @@ lint: --exclude tests/PHPStan/Rules/Classes/data/extends-readonly-class.php \ --exclude tests/PHPStan/Rules/Classes/data/instantiation-promoted-properties.php \ --exclude tests/PHPStan/Rules/Classes/data/bug-11592.php \ + --exclude tests/PHPStan/Rules/Classes/data/bug-12281.php \ + --exclude tests/PHPStan/Rules/Traits/data/bug-12281.php \ src tests cs: diff --git a/conf/config.level0.neon b/conf/config.level0.neon index 15f1048b86..21a7286de8 100644 --- a/conf/config.level0.neon +++ b/conf/config.level0.neon @@ -117,6 +117,7 @@ rules: - PHPStan\Rules\Properties\ReadOnlyPropertyRule - 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..6b792628d9 --- /dev/null +++ b/src/Rules/Traits/TraitAttributesRule.php @@ -0,0 +1,63 @@ + + */ +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 f0deeee108..aeed0f0231 100644 --- a/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php +++ b/tests/PHPStan/Rules/Classes/ClassAttributesRuleTest.php @@ -168,4 +168,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..51b77e4332 --- /dev/null +++ b/tests/PHPStan/Rules/Traits/TraitAttributesRuleTest.php @@ -0,0 +1,100 @@ + + */ +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, true, false), + new NullsafeCheck(), + new PhpVersion(80000), + new UnresolvableTypeHelper(), + new PropertyReflectionFinder(), + true, + 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 @@ +