From b02ee1444440ed995c17171c456feec2c56e0cb2 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 2 May 2020 16:08:50 +0200 Subject: [PATCH] Support for `@mixin` --- conf/config.level2.neon | 6 + conf/config.neon | 10 ++ src/PhpDoc/PhpDocNodeResolver.php | 16 +++ src/PhpDoc/ResolvedPhpDocBlock.php | 19 +++ src/PhpDoc/Tag/MixinTag.php | 34 +++++ src/Reflection/ClassReflection.php | 35 +++++ .../MixinMethodsClassReflectionExtension.php | 51 ++++++++ ...ixinPropertiesClassReflectionExtension.php | 51 ++++++++ src/Rules/Classes/MixinRule.php | 123 ++++++++++++++++++ tests/PHPStan/Rules/Classes/MixinRuleTest.php | 77 +++++++++++ tests/PHPStan/Rules/Classes/data/mixin.php | 79 +++++++++++ .../Rules/Methods/CallMethodsRuleTest.php | 25 ++++ tests/PHPStan/Rules/Methods/data/mixin.php | 65 +++++++++ .../Properties/AccessPropertiesRuleTest.php | 12 ++ tests/PHPStan/Rules/Properties/data/mixin.php | 54 ++++++++ 15 files changed, 657 insertions(+) create mode 100644 src/PhpDoc/Tag/MixinTag.php create mode 100644 src/Reflection/Mixin/MixinMethodsClassReflectionExtension.php create mode 100644 src/Reflection/Mixin/MixinPropertiesClassReflectionExtension.php create mode 100644 src/Rules/Classes/MixinRule.php create mode 100644 tests/PHPStan/Rules/Classes/MixinRuleTest.php create mode 100644 tests/PHPStan/Rules/Classes/data/mixin.php create mode 100644 tests/PHPStan/Rules/Methods/data/mixin.php create mode 100644 tests/PHPStan/Rules/Properties/data/mixin.php diff --git a/conf/config.level2.neon b/conf/config.level2.neon index b44a73faa7..9ff871b3d4 100644 --- a/conf/config.level2.neon +++ b/conf/config.level2.neon @@ -33,6 +33,12 @@ rules: - PHPStan\Rules\PhpDoc\WrongVariableNameInVarTagRule services: + - + class: PHPStan\Rules\Classes\MixinRule + arguments: + checkClassCaseSensitivity: %checkClassCaseSensitivity% + tags: + - phpstan.rules.rule - class: PHPStan\Rules\Functions\CallCallablesRule arguments: diff --git a/conf/config.neon b/conf/config.neon index ca09598d29..e652b256cf 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -476,6 +476,16 @@ services: - class: PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedSingleFileSourceLocatorRepository + - + class: PHPStan\Reflection\Mixin\MixinMethodsClassReflectionExtension + tags: + - phpstan.broker.methodsClassReflectionExtension + + - + class: PHPStan\Reflection\Mixin\MixinPropertiesClassReflectionExtension + tags: + - phpstan.broker.propertiesClassReflectionExtension + - class: PHPStan\Reflection\Php\PhpClassReflectionExtension arguments: diff --git a/src/PhpDoc/PhpDocNodeResolver.php b/src/PhpDoc/PhpDocNodeResolver.php index f455a90f50..074ee8a9f8 100644 --- a/src/PhpDoc/PhpDocNodeResolver.php +++ b/src/PhpDoc/PhpDocNodeResolver.php @@ -8,6 +8,7 @@ use PHPStan\PhpDoc\Tag\ImplementsTag; use PHPStan\PhpDoc\Tag\MethodTag; use PHPStan\PhpDoc\Tag\MethodTagParameter; +use PHPStan\PhpDoc\Tag\MixinTag; use PHPStan\PhpDoc\Tag\ParamTag; use PHPStan\PhpDoc\Tag\PropertyTag; use PHPStan\PhpDoc\Tag\ReturnTag; @@ -16,6 +17,7 @@ use PHPStan\PhpDoc\Tag\UsesTag; use PHPStan\PhpDoc\Tag\VarTag; use PHPStan\PhpDocParser\Ast\ConstExpr\ConstExprNullNode; +use PHPStan\PhpDocParser\Ast\PhpDoc\MixinTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode; use PHPStan\PhpDocParser\Ast\PhpDoc\TemplateTagValueNode; use PHPStan\PhpDocParser\Ast\PhpDoc\ThrowsTagValueNode; @@ -339,6 +341,20 @@ public function resolveThrowsTags(PhpDocNode $phpDocNode, NameScope $nameScope): return new ThrowsTag(TypeCombinator::union(...$types)); } + /** + * @param PhpDocNode $phpDocNode + * @param NameScope $nameScope + * @return array + */ + public function resolveMixinTags(PhpDocNode $phpDocNode, NameScope $nameScope): array + { + return array_map(function (MixinTagValueNode $mixinTagValueNode) use ($nameScope): MixinTag { + return new MixinTag( + $this->typeNodeResolver->resolve($mixinTagValueNode->type, $nameScope) + ); + }, $phpDocNode->getMixinTagValues()); + } + public function resolveDeprecatedTag(PhpDocNode $phpDocNode, NameScope $nameScope): ?\PHPStan\PhpDoc\Tag\DeprecatedTag { foreach ($phpDocNode->getDeprecatedTagValues() as $deprecatedTagValue) { diff --git a/src/PhpDoc/ResolvedPhpDocBlock.php b/src/PhpDoc/ResolvedPhpDocBlock.php index f345b3b7f7..7af3ba825f 100644 --- a/src/PhpDoc/ResolvedPhpDocBlock.php +++ b/src/PhpDoc/ResolvedPhpDocBlock.php @@ -3,6 +3,7 @@ namespace PHPStan\PhpDoc; use PHPStan\Analyser\NameScope; +use PHPStan\PhpDoc\Tag\MixinTag; use PHPStan\PhpDocParser\Ast\PhpDoc\PhpDocNode; use PHPStan\Type\Generic\TemplateTypeMap; @@ -57,6 +58,9 @@ class ResolvedPhpDocBlock /** @var \PHPStan\PhpDoc\Tag\ThrowsTag|false|null */ private $throwsTag = false; + /** @var array|false */ + private $mixinTags = false; + /** @var \PHPStan\PhpDoc\Tag\DeprecatedTag|false|null */ private $deprecatedTag = false; @@ -314,6 +318,21 @@ public function getThrowsTag(): ?\PHPStan\PhpDoc\Tag\ThrowsTag return $this->throwsTag; } + /** + * @return array + */ + public function getMixinTags(): array + { + if ($this->mixinTags === false) { + $this->mixinTags = $this->phpDocNodeResolver->resolveMixinTags( + $this->phpDocNode, + $this->nameScope + ); + } + + return $this->mixinTags; + } + public function getDeprecatedTag(): ?\PHPStan\PhpDoc\Tag\DeprecatedTag { if ($this->deprecatedTag === false) { diff --git a/src/PhpDoc/Tag/MixinTag.php b/src/PhpDoc/Tag/MixinTag.php new file mode 100644 index 0000000000..c9d7119418 --- /dev/null +++ b/src/PhpDoc/Tag/MixinTag.php @@ -0,0 +1,34 @@ +type = $type; + } + + public function getType(): Type + { + return $this->type; + } + + /** + * @param mixed[] $properties + * @return self + */ + public static function __set_state(array $properties): self + { + return new self( + $properties['type'] + ); + } + +} diff --git a/src/Reflection/ClassReflection.php b/src/Reflection/ClassReflection.php index c87f4c786a..2d27b7cd35 100644 --- a/src/Reflection/ClassReflection.php +++ b/src/Reflection/ClassReflection.php @@ -5,6 +5,7 @@ use PHPStan\PhpDoc\ResolvedPhpDocBlock; use PHPStan\PhpDoc\Tag\ExtendsTag; use PHPStan\PhpDoc\Tag\ImplementsTag; +use PHPStan\PhpDoc\Tag\MixinTag; use PHPStan\PhpDoc\Tag\TemplateTag; use PHPStan\Reflection\Php\PhpClassReflectionExtension; use PHPStan\Reflection\Php\PhpPropertyReflection; @@ -941,4 +942,38 @@ private function isValidAncestorType(Type $type, array $ancestorClasses): bool return in_array($reflection->getName(), $ancestorClasses, true); } + /** + * @return array + */ + public function getMixinTags(): array + { + $resolvedPhpDoc = $this->getResolvedPhpDoc(); + if ($resolvedPhpDoc === null) { + return []; + } + + return $resolvedPhpDoc->getMixinTags(); + } + + /** + * @return array + */ + public function getResolvedMixinTypes(): array + { + $types = []; + foreach ($this->getMixinTags() as $mixinTag) { + if (!$this->isGeneric()) { + $types[] = $mixinTag->getType(); + continue; + } + + $types[] = TemplateTypeHelper::resolveTemplateTypes( + $mixinTag->getType(), + $this->getActiveTemplateTypeMap() + ); + } + + return $types; + } + } diff --git a/src/Reflection/Mixin/MixinMethodsClassReflectionExtension.php b/src/Reflection/Mixin/MixinMethodsClassReflectionExtension.php new file mode 100644 index 0000000000..e62c830efb --- /dev/null +++ b/src/Reflection/Mixin/MixinMethodsClassReflectionExtension.php @@ -0,0 +1,51 @@ +findMethod($classReflection, $methodName) !== null; + } + + public function getMethod(ClassReflection $classReflection, string $methodName): MethodReflection + { + $method = $this->findMethod($classReflection, $methodName); + if ($method === null) { + throw new \PHPStan\ShouldNotHappenException(); + } + + return $method; + } + + private function findMethod(ClassReflection $classReflection, string $methodName): ?MethodReflection + { + $mixinTypes = $classReflection->getResolvedMixinTypes(); + foreach ($mixinTypes as $type) { + if (!$type->hasMethod($methodName)->yes()) { + continue; + } + + return $type->getMethod($methodName, new OutOfClassScope()); + } + + foreach ($classReflection->getParents() as $parentClass) { + $method = $this->findMethod($parentClass, $methodName); + if ($method === null) { + continue; + } + + return $method; + } + + return null; + } + +} diff --git a/src/Reflection/Mixin/MixinPropertiesClassReflectionExtension.php b/src/Reflection/Mixin/MixinPropertiesClassReflectionExtension.php new file mode 100644 index 0000000000..3dadfe2e4a --- /dev/null +++ b/src/Reflection/Mixin/MixinPropertiesClassReflectionExtension.php @@ -0,0 +1,51 @@ +findProperty($classReflection, $propertyName) !== null; + } + + public function getProperty(ClassReflection $classReflection, string $propertyName): PropertyReflection + { + $property = $this->findProperty($classReflection, $propertyName); + if ($property === null) { + throw new \PHPStan\ShouldNotHappenException(); + } + + return $property; + } + + private function findProperty(ClassReflection $classReflection, string $propertyName): ?PropertyReflection + { + $mixinTypes = $classReflection->getResolvedMixinTypes(); + foreach ($mixinTypes as $type) { + if (!$type->hasProperty($propertyName)->yes()) { + continue; + } + + return $type->getProperty($propertyName, new OutOfClassScope()); + } + + foreach ($classReflection->getParents() as $parentClass) { + $property = $this->findProperty($parentClass, $propertyName); + if ($property === null) { + continue; + } + + return $property; + } + + return null; + } + +} diff --git a/src/Rules/Classes/MixinRule.php b/src/Rules/Classes/MixinRule.php new file mode 100644 index 0000000000..9cdac3ff99 --- /dev/null +++ b/src/Rules/Classes/MixinRule.php @@ -0,0 +1,123 @@ + + */ +class MixinRule implements Rule +{ + + /** @var ReflectionProvider */ + private $reflectionProvider; + + /** @var \PHPStan\Rules\ClassCaseSensitivityCheck */ + private $classCaseSensitivityCheck; + + /** @var \PHPStan\Rules\Generics\GenericObjectTypeCheck */ + private $genericObjectTypeCheck; + + /** @var MissingTypehintCheck */ + private $missingTypehintCheck; + + /** @var bool */ + private $checkClassCaseSensitivity; + + public function __construct( + ReflectionProvider $reflectionProvider, + ClassCaseSensitivityCheck $classCaseSensitivityCheck, + GenericObjectTypeCheck $genericObjectTypeCheck, + MissingTypehintCheck $missingTypehintCheck, + bool $checkClassCaseSensitivity + ) + { + $this->reflectionProvider = $reflectionProvider; + $this->classCaseSensitivityCheck = $classCaseSensitivityCheck; + $this->genericObjectTypeCheck = $genericObjectTypeCheck; + $this->missingTypehintCheck = $missingTypehintCheck; + $this->checkClassCaseSensitivity = $checkClassCaseSensitivity; + } + + public function getNodeType(): string + { + return Node\Stmt\Class_::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!isset($node->namespacedName)) { + // anonymous class + return []; + } + + $className = (string) $node->namespacedName; + if (!$this->reflectionProvider->hasClass($className)) { + return []; + } + $classReflection = $this->reflectionProvider->getClass($className); + $mixinTags = $classReflection->getMixinTags(); + $errors = []; + foreach ($mixinTags as $mixinTag) { + $type = $mixinTag->getType(); + if (!$type->canCallMethods()->yes() || !$type->canAccessProperties()->yes()) { + $errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains non-object type %s.', $type->describe(VerbosityLevel::typeOnly())))->build(); + continue; + } + + if ( + $type instanceof ErrorType + || ($type instanceof NeverType && !$type->isExplicit()) + ) { + $errors[] = RuleErrorBuilder::message('PHPDoc tag @mixin contains unresolvable type.')->build(); + continue; + } + + $errors = array_merge($errors, $this->genericObjectTypeCheck->check( + $type, + 'PHPDoc tag @mixin contains generic type %s but class %s is not generic.', + 'Generic type %s in PHPDoc tag @mixin does not specify all template types of class %s: %s', + 'Generic type %s in PHPDoc tag @mixin specifies %d template types, but class %s supports only %d: %s', + 'Type %s in generic type %s in PHPDoc tag @mixin is not subtype of template type %s of class %s.' + )); + + foreach ($this->missingTypehintCheck->getNonGenericObjectTypesWithGenericClass($type) as [$innerName, $genericTypeNames]) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'PHPDoc tag @mixin contains generic %s but does not specify its types: %s', + $innerName, + implode(', ', $genericTypeNames) + ))->tip(MissingTypehintCheck::TURN_OFF_NON_GENERIC_CHECK_TIP)->build(); + } + + foreach ($type->getReferencedClasses() as $class) { + if (!$this->reflectionProvider->hasClass($class)) { + $errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains unknown class %s.', $class))->build(); + } elseif ($this->reflectionProvider->getClass($class)->isTrait()) { + $errors[] = RuleErrorBuilder::message(sprintf('PHPDoc tag @mixin contains invalid type %s.', $class))->build(); + } elseif ($this->checkClassCaseSensitivity) { + $errors = array_merge( + $errors, + $this->classCaseSensitivityCheck->checkClassNames([ + new ClassNameNodePair($class, $node), + ]) + ); + } + } + } + + return $errors; + } + +} diff --git a/tests/PHPStan/Rules/Classes/MixinRuleTest.php b/tests/PHPStan/Rules/Classes/MixinRuleTest.php new file mode 100644 index 0000000000..d60fa872d0 --- /dev/null +++ b/tests/PHPStan/Rules/Classes/MixinRuleTest.php @@ -0,0 +1,77 @@ + + */ +class MixinRuleTest extends RuleTestCase +{ + + protected function getRule(): Rule + { + $reflectionProvider = $this->createReflectionProvider(); + + return new MixinRule( + $reflectionProvider, + new ClassCaseSensitivityCheck($reflectionProvider), + new GenericObjectTypeCheck(), + new MissingTypehintCheck($reflectionProvider, true, true), + true + ); + } + + public function testRule(): void + { + $this->analyse([__DIR__ . '/data/mixin.php'], [ + [ + 'PHPDoc tag @mixin contains non-object type int.', + 16, + ], + [ + 'PHPDoc tag @mixin contains unresolvable type.', + 24, + ], + [ + 'PHPDoc tag @mixin contains generic type Exception but class Exception is not generic.', + 34, + ], + [ + 'Generic type Traversable in PHPDoc tag @mixin specifies 3 template types, but class Traversable supports only 2: TKey, TValue', + 34, + ], + [ + 'Type string in generic type ReflectionClass in PHPDoc tag @mixin is not subtype of template type T of object of class ReflectionClass.', + 34, + ], + [ + 'PHPDoc tag @mixin contains generic class ReflectionClass but does not specify its types: T', + 50, + 'You can turn this off by setting checkGenericClassInNonGenericObjectType: false in your %configurationFile%.', + ], + [ + 'PHPDoc tag @mixin contains unknown class MixinRule\UnknownestClass.', + 50, + ], + [ + 'PHPDoc tag @mixin contains invalid type MixinRule\FooTrait.', + 50, + ], + [ + 'PHPDoc tag @mixin contains unknown class MixinRule\U.', + 59, + ], + [ + 'Generic type MixinRule\Consecteur in PHPDoc tag @mixin does not specify all template types of class MixinRule\Consecteur: T, U', + 76, + ], + ]); + } + +} diff --git a/tests/PHPStan/Rules/Classes/data/mixin.php b/tests/PHPStan/Rules/Classes/data/mixin.php new file mode 100644 index 0000000000..1627a0258c --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/mixin.php @@ -0,0 +1,79 @@ + + * @mixin \Traversable + * @mixin \ReflectionClass + */ +class Lorem +{ + +} + +trait FooTrait +{ + +} + +/** + * @mixin \ReflectionClass + * @mixin \Iterator + * @mixin UnknownestClass + * @mixin FooTrait + */ +class Ipsum +{ +} + +/** + * @template T + * @mixin T + * @mixin U + */ +class Dolor +{ + +} + +/** + * @template T + * @template U + */ +class Consecteur +{ + +} + +/** + * @mixin Consecteur + */ +class Sit +{ + +} diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php index b8012afa74..c0ee961e50 100644 --- a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php +++ b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php @@ -1250,4 +1250,29 @@ public function testClosureCallInvocations(): void ]); } + public function testMixin(): void + { + $this->checkThisOnly = false; + $this->checkNullables = true; + $this->checkUnionTypes = true; + $this->analyse([__DIR__ . '/data/mixin.php'], [ + [ + 'Method MixinMethods\Foo::doFoo() invoked with 1 parameter, 0 required.', + 30, + ], + [ + 'Method MixinMethods\Foo::doFoo() invoked with 1 parameter, 0 required.', + 40, + ], + [ + 'Method Exception::getMessage() invoked with 1 parameter, 0 required.', + 61, + ], + [ + 'Call to an undefined method MixinMethods\GenericFoo::getMessagee().', + 62, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/mixin.php b/tests/PHPStan/Rules/Methods/data/mixin.php new file mode 100644 index 0000000000..3c8695cf34 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/mixin.php @@ -0,0 +1,65 @@ +doFoo(); + $bar->doFoo(1); +}; + +class Baz extends Bar +{ + +} + +function (Baz $baz): void { + $baz->doFoo(); + $baz->doFoo(1); +}; + +/** + * @template T + * @mixin T + */ +class GenericFoo +{ + +} + +class Test +{ + + /** + * @param GenericFoo<\Exception> $foo + */ + public function doFoo(GenericFoo $foo): void + { + echo $foo->getMessage(); + echo $foo->getMessage(1); + echo $foo->getMessagee(); + } + +} diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index 54f4f8363f..4f6a0c64d1 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -400,4 +400,16 @@ public function testClassExists(): void ]); } + public function testMixin(): void + { + $this->checkThisOnly = false; + $this->checkUnionTypes = true; + $this->analyse([__DIR__ . '/data/mixin.php'], [ + [ + 'Access to an undefined property MixinProperties\GenericFoo::$namee.', + 51, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Properties/data/mixin.php b/tests/PHPStan/Rules/Properties/data/mixin.php new file mode 100644 index 0000000000..e6ba546647 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/mixin.php @@ -0,0 +1,54 @@ +fooProp; +}; + +class Baz extends Bar +{ + +} + +function (Baz $baz): void { + $baz->fooProp; +}; + +/** + * @template T + * @mixin T + */ +class GenericFoo +{ + +} + +class Test +{ + + /** + * @param GenericFoo<\ReflectionClass> $foo + */ + public function doFoo(GenericFoo $foo): void + { + echo $foo->name; + echo $foo->namee; + } + +}