From 55ea2ae516df22a071ab873fdd6f748a3af0520e Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 24 Aug 2024 13:45:46 +0200 Subject: [PATCH] Bleeding edge - check type in `@property` tags --- conf/config.level2.neon | 10 + conf/config.neon | 5 + src/Reflection/ClassReflection.php | 2 +- src/Rules/Classes/PropertyTagCheck.php | 174 ++++++++++++++++++ src/Rules/Classes/PropertyTagRule.php | 30 +++ src/Rules/Classes/PropertyTagTraitRule.php | 39 ++++ .../Rules/Classes/PropertyTagRuleTest.php | 139 ++++++++++++++ .../Classes/PropertyTagTraitRuleTest.php | 51 +++++ .../Rules/Classes/data/property-tag-trait.php | 11 ++ .../Rules/Classes/data/property-tag.php | 93 ++++++++++ 10 files changed, 553 insertions(+), 1 deletion(-) create mode 100644 src/Rules/Classes/PropertyTagCheck.php create mode 100644 src/Rules/Classes/PropertyTagRule.php create mode 100644 src/Rules/Classes/PropertyTagTraitRule.php create mode 100644 tests/PHPStan/Rules/Classes/PropertyTagRuleTest.php create mode 100644 tests/PHPStan/Rules/Classes/PropertyTagTraitRuleTest.php create mode 100644 tests/PHPStan/Rules/Classes/data/property-tag-trait.php create mode 100644 tests/PHPStan/Rules/Classes/data/property-tag.php diff --git a/conf/config.level2.neon b/conf/config.level2.neon index 72ff318df0..0d9fd56ff5 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -49,6 +49,10 @@ rules: - PHPStan\Rules\PhpDoc\RequireExtendsDefinitionTraitRule conditionalTags: + PHPStan\Rules\Classes\PropertyTagRule: + phpstan.rules.rule: %featureToggles.absentTypeChecks% + PHPStan\Rules\Classes\PropertyTagTraitRule: + phpstan.rules.rule: %featureToggles.absentTypeChecks% PHPStan\Rules\Functions\IncompatibleArrowFunctionDefaultParameterTypeRule: phpstan.rules.rule: %featureToggles.closureDefaultParameterTypeRule% PHPStan\Rules\Functions\IncompatibleClosureDefaultParameterTypeRule: @@ -75,6 +79,12 @@ services: tags: - phpstan.rules.rule + - + class: PHPStan\Rules\Classes\PropertyTagRule + + - + class: PHPStan\Rules\Classes\PropertyTagTraitRule + - class: PHPStan\Rules\PhpDoc\RequireExtendsCheck arguments: diff --git a/conf/config.neon b/conf/config.neon index 688c2a4f5c..6ece4804c1 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -917,6 +917,11 @@ services: checkClassCaseSensitivity: %checkClassCaseSensitivity% absentTypeChecks: %featureToggles.absentTypeChecks% + - + class: PHPStan\Rules\Classes\PropertyTagCheck + arguments: + checkClassCaseSensitivity: %checkClassCaseSensitivity% + - class: PHPStan\Rules\Comparison\ConstantConditionRuleHelper arguments: diff --git a/src/Reflection/ClassReflection.php b/src/Reflection/ClassReflection.php index 390092a857..68752bfab1 100644 --- a/src/Reflection/ClassReflection.php +++ b/src/Reflection/ClassReflection.php @@ -1732,7 +1732,7 @@ public function getRequireImplementsTags(): array } /** - * @return array + * @return array */ public function getPropertyTags(): array { diff --git a/src/Rules/Classes/PropertyTagCheck.php b/src/Rules/Classes/PropertyTagCheck.php new file mode 100644 index 0000000000..abbc274698 --- /dev/null +++ b/src/Rules/Classes/PropertyTagCheck.php @@ -0,0 +1,174 @@ + + */ + public function check( + ClassReflection $classReflection, + ClassLike $node, + ): array + { + $errors = []; + foreach ($classReflection->getPropertyTags() as $propertyName => $propertyTag) { + $readableType = $propertyTag->getReadableType(); + $writableType = $propertyTag->getWritableType(); + + $types = []; + $tagName = '@property'; + if ($readableType !== null) { + if ($writableType !== null) { + if ($writableType->equals($readableType)) { + $types[] = $readableType; + } else { + $types[] = $readableType; + $types[] = $writableType; + } + } else { + $tagName = '@property-read'; + $types[] = $readableType; + } + } elseif ($writableType !== null) { + $tagName = '@property-write'; + $types[] = $writableType; + } else { + throw new ShouldNotHappenException(); + } + + foreach ($types as $type) { + foreach ($this->checkPropertyType($classReflection, $propertyName, $tagName, $type, $node) as $error) { + $errors[] = $error; + } + } + } + + return $errors; + } + + /** + * @return list + */ + private function checkPropertyType(ClassReflection $classReflection, string $propertyName, string $tagName, Type $type, ClassLike $node): array + { + if ($this->unresolvableTypeHelper->containsUnresolvableType($type)) { + return [ + RuleErrorBuilder::message(sprintf( + 'PHPDoc tag %s for property %s::$%s contains unresolvable type.', + $tagName, + $classReflection->getDisplayName(), + $propertyName, + ))->identifier('propertyTag.unresolvableType') + ->build(), + ]; + } + + $escapedClassName = SprintfHelper::escapeFormatString($classReflection->getDisplayName()); + $escapedPropertyName = SprintfHelper::escapeFormatString($propertyName); + $escapedTagName = SprintfHelper::escapeFormatString($tagName); + + $errors = $this->genericObjectTypeCheck->check( + $type, + sprintf('PHPDoc tag %s for property %s::$%s contains generic type %%s but %%s %%s is not generic.', $escapedTagName, $escapedClassName, $escapedPropertyName), + sprintf('Generic type %%s in PHPDoc tag %s for property %s::$%s does not specify all template types of %%s %%s: %%s', $escapedTagName, $escapedClassName, $escapedPropertyName), + sprintf('Generic type %%s in PHPDoc tag %s for property %s::$%s specifies %%d template types, but %%s %%s supports only %%d: %%s', $escapedTagName, $escapedClassName, $escapedPropertyName), + sprintf('Type %%s in generic type %%s in PHPDoc tag %s for property %s::$%s is not subtype of template type %%s of %%s %%s.', $escapedTagName, $escapedClassName, $escapedPropertyName), + sprintf('Call-site variance of %%s in generic type %%s in PHPDoc tag %s for property %s::$%s is in conflict with %%s template type %%s of %%s %%s.', $escapedTagName, $escapedClassName, $escapedPropertyName), + sprintf('Call-site variance of %%s in generic type %%s in PHPDoc tag %s for property %s::$%s is redundant, template type %%s of %%s %%s has the same variance.', $escapedTagName, $escapedClassName, $escapedPropertyName), + ); + + foreach ($this->missingTypehintCheck->getNonGenericObjectTypesWithGenericClass($type) as [$innerName, $genericTypeNames]) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'PHPDoc tag %s for property %s::$%s contains generic %s but does not specify its types: %s', + $tagName, + $classReflection->getDisplayName(), + $propertyName, + $innerName, + implode(', ', $genericTypeNames), + )) + ->identifier('missingType.generics') + ->build(); + } + + foreach ($this->missingTypehintCheck->getIterableTypesWithMissingValueTypehint($type) as $iterableType) { + $iterableTypeDescription = $iterableType->describe(VerbosityLevel::typeOnly()); + $errors[] = RuleErrorBuilder::message(sprintf( + '%s %s has PHPDoc tag %s for property $%s with no value type specified in iterable type %s.', + $classReflection->getClassTypeDescription(), + $classReflection->getDisplayName(), + $tagName, + $propertyName, + $iterableTypeDescription, + )) + ->tip(MissingTypehintCheck::MISSING_ITERABLE_VALUE_TYPE_TIP) + ->identifier('missingType.iterableValue') + ->build(); + } + + foreach ($this->missingTypehintCheck->getCallablesWithMissingSignature($type) as $callableType) { + $errors[] = RuleErrorBuilder::message(sprintf( + '%s %s has PHPDoc tag %s for property $%s with no signature specified for %s.', + $classReflection->getClassTypeDescription(), + $classReflection->getDisplayName(), + $tagName, + $propertyName, + $callableType->describe(VerbosityLevel::typeOnly()), + ))->identifier('missingType.callable')->build(); + } + + foreach ($type->getReferencedClasses() as $class) { + if (!$this->reflectionProvider->hasClass($class)) { + $errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag %s for property %s::$%s contains unknown class %s.', $tagName, $classReflection->getDisplayName(), $propertyName, $class)) + ->identifier('class.notFound') + ->discoveringSymbolsTip() + ->build(); + } elseif ($this->reflectionProvider->getClass($class)->isTrait()) { + $errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag %s for property %s::$%s contains invalid type %s.', $tagName, $classReflection->getDisplayName(), $propertyName, $class)) + ->identifier('propertyTag.trait') + ->build(); + } else { + $errors = array_merge( + $errors, + $this->classCheck->checkClassNames([ + new ClassNameNodePair($class, $node), + ], $this->checkClassCaseSensitivity), + ); + } + } + + return $errors; + } + +} diff --git a/src/Rules/Classes/PropertyTagRule.php b/src/Rules/Classes/PropertyTagRule.php new file mode 100644 index 0000000000..c1f002c3b3 --- /dev/null +++ b/src/Rules/Classes/PropertyTagRule.php @@ -0,0 +1,30 @@ + + */ +final class PropertyTagRule implements Rule +{ + + public function __construct(private PropertyTagCheck $check) + { + } + + public function getNodeType(): string + { + return InClassNode::class; + } + + public function processNode(Node $node, Scope $scope): array + { + return $this->check->check($node->getClassReflection(), $node->getOriginalNode()); + } + +} diff --git a/src/Rules/Classes/PropertyTagTraitRule.php b/src/Rules/Classes/PropertyTagTraitRule.php new file mode 100644 index 0000000000..cd3a54c9fd --- /dev/null +++ b/src/Rules/Classes/PropertyTagTraitRule.php @@ -0,0 +1,39 @@ + + */ +final class PropertyTagTraitRule implements Rule +{ + + public function __construct(private PropertyTagCheck $check, 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 []; + } + + return $this->check->check($this->reflectionProvider->getClass($traitName->toString()), $node); + } + +} diff --git a/tests/PHPStan/Rules/Classes/PropertyTagRuleTest.php b/tests/PHPStan/Rules/Classes/PropertyTagRuleTest.php new file mode 100644 index 0000000000..b5718fb844 --- /dev/null +++ b/tests/PHPStan/Rules/Classes/PropertyTagRuleTest.php @@ -0,0 +1,139 @@ + + */ +class PropertyTagRuleTest extends RuleTestCase +{ + + protected function getRule(): TRule + { + $reflectionProvider = $this->createReflectionProvider(); + + return new PropertyTagRule( + new PropertyTagCheck( + $reflectionProvider, + new ClassNameCheck( + new ClassCaseSensitivityCheck($reflectionProvider, true), + new ClassForbiddenNameCheck(self::getContainer()), + ), + new GenericObjectTypeCheck(), + new MissingTypehintCheck(true, true, true, true, []), + new UnresolvableTypeHelper(), + true, + ), + ); + } + + public function testRule(): void + { + $tipText = 'Learn more at https://phpstan.org/user-guide/discovering-symbols'; + $fooClassLine = 23; + + $this->analyse([__DIR__ . '/data/property-tag.php'], [ + [ + 'PHPDoc tag @property for property PropertyTag\Foo::$a contains unknown class PropertyTag\intt.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property for property PropertyTag\Foo::$b contains unknown class PropertyTag\intt.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property for property PropertyTag\Foo::$c contains unknown class PropertyTag\stringg.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property for property PropertyTag\Foo::$c contains unknown class PropertyTag\intt.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property for property PropertyTag\Foo::$d contains unknown class PropertyTag\intt.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property for property PropertyTag\Foo::$e contains unknown class PropertyTag\stringg.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property for property PropertyTag\Foo::$e contains unknown class PropertyTag\intt.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property-read for property PropertyTag\Foo::$f contains unknown class PropertyTag\intt.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property-write for property PropertyTag\Foo::$g contains unknown class PropertyTag\stringg.', + $fooClassLine, + $tipText, + ], + [ + 'PHPDoc tag @property for property PropertyTag\Bar::$unresolvable contains unresolvable type.', + 31, + ], + [ + 'PHPDoc tag @property for property PropertyTag\TestGenerics::$a contains generic type Exception but class Exception is not generic.', + 51, + ], + [ + 'Generic type PropertyTag\Generic in PHPDoc tag @property for property PropertyTag\TestGenerics::$b does not specify all template types of class PropertyTag\Generic: T, U', + 51, + ], + [ + 'Generic type PropertyTag\Generic in PHPDoc tag @property for property PropertyTag\TestGenerics::$c specifies 3 template types, but class PropertyTag\Generic supports only 2: T, U', + 51, + ], + [ + 'Type string in generic type PropertyTag\Generic in PHPDoc tag @property for property PropertyTag\TestGenerics::$d is not subtype of template type T of int of class PropertyTag\Generic.', + 51, + ], + [ + 'PHPDoc tag @property for property PropertyTag\MissingGenerics::$a contains generic class PropertyTag\Generic but does not specify its types: T, U', + 59, + ], + [ + 'Class PropertyTag\MissingIterableValue has PHPDoc tag @property for property $a with no value type specified in iterable type array.', + 67, + 'See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iterable-type', + ], + [ + 'Class PropertyTag\MissingCallableSignature has PHPDoc tag @property for property $a with no signature specified for callable.', + 75, + ], + [ + 'PHPDoc tag @property for property PropertyTag\NonexistentClasses::$a contains unknown class PropertyTag\Nonexistent.', + 85, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + [ + 'PHPDoc tag @property for property PropertyTag\NonexistentClasses::$b contains invalid type PropertyTagTrait\Foo.', + 85, + ], + [ + 'Class PropertyTag\Foo referenced with incorrect case: PropertyTag\fOO.', + 85, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Classes/PropertyTagTraitRuleTest.php b/tests/PHPStan/Rules/Classes/PropertyTagTraitRuleTest.php new file mode 100644 index 0000000000..c6e140604c --- /dev/null +++ b/tests/PHPStan/Rules/Classes/PropertyTagTraitRuleTest.php @@ -0,0 +1,51 @@ + + */ +class PropertyTagTraitRuleTest extends RuleTestCase +{ + + protected function getRule(): TRule + { + $reflectionProvider = $this->createReflectionProvider(); + + return new PropertyTagTraitRule( + new PropertyTagCheck( + $reflectionProvider, + new ClassNameCheck( + new ClassCaseSensitivityCheck($reflectionProvider, true), + new ClassForbiddenNameCheck(self::getContainer()), + ), + new GenericObjectTypeCheck(), + new MissingTypehintCheck(true, true, true, true, []), + new UnresolvableTypeHelper(), + true, + ), + $reflectionProvider, + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/property-tag-trait.php'], [ + [ + 'PHPDoc tag @property for property PropertyTagTrait\Foo::$foo contains unknown class PropertyTagTrait\intt.', + 8, + 'Learn more at https://phpstan.org/user-guide/discovering-symbols', + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Classes/data/property-tag-trait.php b/tests/PHPStan/Rules/Classes/data/property-tag-trait.php new file mode 100644 index 0000000000..c5a50f30dd --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/property-tag-trait.php @@ -0,0 +1,11 @@ + $a + * @property Generic $b + * @property Generic $c + * @property Generic $d + */ +class TestGenerics +{ + +} + +/** + * @property Generic $a + */ +class MissingGenerics +{ + +} + +/** + * @property Generic $a + */ +class MissingIterableValue +{ + +} + +/** + * @property Generic $a + */ +class MissingCallableSignature +{ + +} + +/** + * @property Nonexistent $a + * @property \PropertyTagTrait\Foo $b + * @property fOO $c + */ +class NonexistentClasses +{ + +} + + +// todo nonexistent class +// todo trait +// todo class name case