From ae238a12a14880da07c33e6817161ece59e685c0 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 3 Jan 2020 15:02:18 +0100 Subject: [PATCH] Parameter name remapping when inheriting phpDocs --- src/Analyser/NodeScopeResolver.php | 15 +- src/PhpDoc/PhpDocBlock.php | 156 ++++++++++++++++-- .../Php/PhpClassReflectionExtension.php | 18 +- .../Analyser/NodeScopeResolverTest.php | 6 + .../data/inheritdoc-parameter-remapping.php | 47 ++++++ .../Rules/Methods/CallMethodsRuleTest.php | 32 ++++ ...thod-with-phpDocs-implicit-inheritance.php | 46 ++++++ 7 files changed, 301 insertions(+), 19 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/inheritdoc-parameter-remapping.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 2121105c3f..462281b49b 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2535,12 +2535,22 @@ public function getPhpDocs(Scope $scope, Node\FunctionLike $functionLike): array throw new \PHPStan\ShouldNotHappenException(); } $functionName = $functionLike->name->name; + $positionalParameterNames = array_map(static function (Node\Param $param): string { + if (!$param->var instanceof Variable || !is_string($param->var->name)) { + throw new \PHPStan\ShouldNotHappenException(); + } + + return $param->var->name; + }, $functionLike->getParams()); $phpDocBlock = PhpDocBlock::resolvePhpDocBlockForMethod( $docComment, $scope->getClassReflection(), $trait, $functionLike->name->name, - $file + $file, + null, + $positionalParameterNames, + $positionalParameterNames ); if ($phpDocBlock !== null) { @@ -2572,6 +2582,9 @@ public function getPhpDocs(Scope $scope, Node\FunctionLike $functionLike): array } return $tag->getType(); }, $resolvedPhpDoc->getParamTags()); + if ($phpDocBlock !== null) { + $phpDocParameterTypes = $phpDocBlock->transformArrayKeysWithParameterNameMapping($phpDocParameterTypes); + } $nativeReturnType = $scope->getFunctionType($functionLike->getReturnType(), false, false); $phpDocReturnType = $this->getPhpDocReturnType($phpDocBlock, $resolvedPhpDoc, $nativeReturnType); $phpDocThrowType = $resolvedPhpDoc->getThrowsTag() !== null ? $resolvedPhpDoc->getThrowsTag()->getType() : null; diff --git a/src/PhpDoc/PhpDocBlock.php b/src/PhpDoc/PhpDocBlock.php index 9c2ab02293..be29ba125d 100644 --- a/src/PhpDoc/PhpDocBlock.php +++ b/src/PhpDoc/PhpDocBlock.php @@ -26,12 +26,24 @@ class PhpDocBlock /** @var bool */ private $explicit; + /** @var array */ + private $parameterNameMapping; + + /** + * @param string $docComment + * @param string $file + * @param \PHPStan\Reflection\ClassReflection $classReflection + * @param string|null $trait + * @param bool $explicit + * @param array $parameterNameMapping + */ private function __construct( string $docComment, string $file, ClassReflection $classReflection, ?string $trait, - bool $explicit + bool $explicit, + array $parameterNameMapping ) { $this->docComment = $docComment; @@ -39,6 +51,7 @@ private function __construct( $this->classReflection = $classReflection; $this->trait = $trait; $this->explicit = $explicit; + $this->parameterNameMapping = $parameterNameMapping; } public function getDocComment(): string @@ -66,13 +79,44 @@ public function isExplicit(): bool return $this->explicit; } + /** + * @template T + * @param array $array + * @return array + */ + public function transformArrayKeysWithParameterNameMapping(array $array): array + { + $newArray = []; + foreach ($array as $key => $value) { + if (!array_key_exists($key, $this->parameterNameMapping)) { + continue; + } + $newArray[$this->parameterNameMapping[$key]] = $value; + } + + return $newArray; + } + + /** + * @param string|null $docComment + * @param \PHPStan\Reflection\ClassReflection $classReflection + * @param string|null $trait + * @param string $propertyName + * @param string $file + * @param bool|null $explicit + * @param array $originalPositionalParameterNames + * @param array $newPositionalParameterNames + * @return self|null + */ public static function resolvePhpDocBlockForProperty( ?string $docComment, ClassReflection $classReflection, ?string $trait, string $propertyName, string $file, - ?bool $explicit = null + ?bool $explicit, + array $originalPositionalParameterNames, // unused + array $newPositionalParameterNames // unused ): ?self { return self::resolvePhpDocBlock( @@ -84,17 +128,32 @@ public static function resolvePhpDocBlockForProperty( 'hasNativeProperty', 'getNativeProperty', __FUNCTION__, - $explicit + $explicit, + [], + [] ); } + /** + * @param string|null $docComment + * @param \PHPStan\Reflection\ClassReflection $classReflection + * @param string|null $trait + * @param string $methodName + * @param string $file + * @param bool|null $explicit + * @param array $originalPositionalParameterNames + * @param array $newPositionalParameterNames + * @return self|null + */ public static function resolvePhpDocBlockForMethod( ?string $docComment, ClassReflection $classReflection, ?string $trait, string $methodName, string $file, - ?bool $explicit = null + ?bool $explicit, + array $originalPositionalParameterNames, + array $newPositionalParameterNames ): ?self { return self::resolvePhpDocBlock( @@ -106,10 +165,26 @@ public static function resolvePhpDocBlockForMethod( 'hasNativeMethod', 'getNativeMethod', __FUNCTION__, - $explicit + $explicit, + $originalPositionalParameterNames, + $newPositionalParameterNames ); } + /** + * @param string|null $docComment + * @param \PHPStan\Reflection\ClassReflection $classReflection + * @param string|null $trait + * @param string $name + * @param string $file + * @param string $hasMethodName + * @param string $getMethodName + * @param string $resolveMethodName + * @param bool|null $explicit + * @param array $originalPositionalParameterNames + * @param array $newPositionalParameterNames + * @return self|null + */ private static function resolvePhpDocBlock( ?string $docComment, ClassReflection $classReflection, @@ -119,7 +194,9 @@ private static function resolvePhpDocBlock( string $hasMethodName, string $getMethodName, string $resolveMethodName, - ?bool $explicit + ?bool $explicit, + array $originalPositionalParameterNames, + array $newPositionalParameterNames ): ?self { if ( @@ -135,7 +212,8 @@ private static function resolvePhpDocBlock( $hasMethodName, $getMethodName, $resolveMethodName, - $explicit ?? $docComment !== null + $explicit ?? $docComment !== null, + $originalPositionalParameterNames ); if ($phpDocBlockFromClass !== null) { return $phpDocBlockFromClass; @@ -149,7 +227,8 @@ private static function resolvePhpDocBlock( $hasMethodName, $getMethodName, $resolveMethodName, - $explicit ?? $docComment !== null + $explicit ?? $docComment !== null, + $originalPositionalParameterNames ); if ($phpDocBlockFromClass !== null) { return $phpDocBlockFromClass; @@ -157,11 +236,30 @@ private static function resolvePhpDocBlock( } } + $parameterNameMapping = []; + foreach ($originalPositionalParameterNames as $i => $parameterName) { + if (!array_key_exists($i, $newPositionalParameterNames)) { + continue; + } + $parameterNameMapping[$newPositionalParameterNames[$i]] = $parameterName; + } + return $docComment !== null - ? new self($docComment, $file, $classReflection, $trait, $explicit ?? true) + ? new self($docComment, $file, $classReflection, $trait, $explicit ?? true, $parameterNameMapping) : null; } + /** + * @param \PHPStan\Reflection\ClassReflection $classReflection + * @param string|null $trait + * @param string $name + * @param string $hasMethodName + * @param string $getMethodName + * @param string $resolveMethodName + * @param bool $explicit + * @param array $positionalParameterNames $positionalParameterNames + * @return self|null + */ private static function resolvePhpDocBlockRecursive( ClassReflection $classReflection, ?string $trait, @@ -169,7 +267,8 @@ private static function resolvePhpDocBlockRecursive( string $hasMethodName, string $getMethodName, string $resolveMethodName, - bool $explicit + bool $explicit, + array $positionalParameterNames = [] ): ?self { $phpDocBlockFromClass = self::resolvePhpDocBlockFromClass( @@ -178,7 +277,8 @@ private static function resolvePhpDocBlockRecursive( $hasMethodName, $getMethodName, $resolveMethodName, - $explicit + $explicit, + $positionalParameterNames ); if ($phpDocBlockFromClass !== null) { @@ -194,20 +294,32 @@ private static function resolvePhpDocBlockRecursive( $hasMethodName, $getMethodName, $resolveMethodName, - $explicit + $explicit, + $positionalParameterNames ); } return null; } + /** + * @param \PHPStan\Reflection\ClassReflection $classReflection + * @param string $name + * @param string $hasMethodName + * @param string $getMethodName + * @param string $resolveMethodName + * @param bool $explicit + * @param array $positionalParameterNames + * @return self|null + */ private static function resolvePhpDocBlockFromClass( ClassReflection $classReflection, string $name, string $hasMethodName, string $getMethodName, string $resolveMethodName, - bool $explicit + bool $explicit, + array $positionalParameterNames ): ?self { if ($classReflection->getFileNameWithPhpDocs() !== null && $classReflection->$hasMethodName($name)) { @@ -223,10 +335,22 @@ private static function resolvePhpDocBlockFromClass( return null; } - if ($parentReflection instanceof PhpPropertyReflection || $parentReflection instanceof ResolvedPropertyReflection || $parentReflection instanceof PhpMethodReflection || $parentReflection instanceof ResolvedMethodReflection) { + if ($parentReflection instanceof PhpPropertyReflection || $parentReflection instanceof ResolvedPropertyReflection) { + $traitReflection = $parentReflection->getDeclaringTrait(); + $positionalMethodParameterNames = []; + } elseif ($parentReflection instanceof PhpMethodReflection || $parentReflection instanceof ResolvedMethodReflection) { $traitReflection = $parentReflection->getDeclaringTrait(); + $methodVariants = $parentReflection->getVariants(); + $positionalMethodParameterNames = []; + if (count($methodVariants) === 1) { + $methodParameters = $methodVariants[0]->getParameters(); + foreach ($methodParameters as $methodParameter) { + $positionalMethodParameterNames[] = $methodParameter->getName(); + } + } } else { $traitReflection = null; + $positionalMethodParameterNames = []; } $trait = $traitReflection !== null @@ -240,7 +364,9 @@ private static function resolvePhpDocBlockFromClass( $trait, $name, $classReflection->getFileNameWithPhpDocs(), - $explicit + $explicit, + $positionalParameterNames, + $positionalMethodParameterNames ); } } diff --git a/src/Reflection/Php/PhpClassReflectionExtension.php b/src/Reflection/Php/PhpClassReflectionExtension.php index a4e40e9590..5fe7b368e2 100644 --- a/src/Reflection/Php/PhpClassReflectionExtension.php +++ b/src/Reflection/Php/PhpClassReflectionExtension.php @@ -222,7 +222,10 @@ private function createProperty( $declaringClassReflection, null, $propertyName, - $declaringClassReflection->getFileName() + $declaringClassReflection->getFileName(), + null, + [], + [] ); if ($phpDocBlock !== null) { $declaringTraitName = $this->findPropertyTrait( @@ -488,16 +491,22 @@ private function createMethod( $declaringTraitName = $this->findMethodTrait($methodReflection); $resolvedPhpDoc = $this->findMethodPhpDocIncludingAncestors($declaringClassName, $methodReflection->getName()); $stubPhpDocString = null; + $phpDocBlock = null; if ($resolvedPhpDoc === null) { if ($declaringClass->getFileName() !== false) { $docComment = $methodReflection->getDocComment(); - + $positionalParameterNames = array_map(static function (\ReflectionParameter $parameter): string { + return $parameter->getName(); + }, $methodReflection->getParameters()); $phpDocBlock = PhpDocBlock::resolvePhpDocBlockForMethod( $docComment, $declaringClass, $declaringTraitName, $methodReflection->getName(), - $declaringClass->getFileName() + $declaringClass->getFileName(), + null, + $positionalParameterNames, + $positionalParameterNames ); if ($phpDocBlock !== null) { @@ -544,6 +553,9 @@ private function createMethod( $phpDocBlockClassReflection->getActiveTemplateTypeMap() ); }, $resolvedPhpDoc->getParamTags()); + if ($phpDocBlock !== null) { + $phpDocParameterTypes = $phpDocBlock->transformArrayKeysWithParameterNameMapping($phpDocParameterTypes); + } $nativeReturnType = TypehintHelper::decideTypeFromReflection( $methodReflection->getReturnType(), null, diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index 1999addd54..250d5b94b5 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -9778,6 +9778,11 @@ public function dataBug2740(): array return $this->gatherAssertTypes(__DIR__ . '/data/bug-2740.php'); } + public function dataPhpDocInheritanceParameterRemapping(): array + { + return $this->gatherAssertTypes(__DIR__ . '/data/inheritdoc-parameter-remapping.php'); + } + /** * @dataProvider dataBug2574 * @dataProvider dataBug2577 @@ -9797,6 +9802,7 @@ public function dataBug2740(): array * @dataProvider dataComplexGenericsExample * @dataProvider dataBug2648 * @dataProvider dataBug2740 + * @dataProvider dataPhpDocInheritanceParameterRemapping * @param ConstantStringType $expectedType * @param Type $actualType */ diff --git a/tests/PHPStan/Analyser/data/inheritdoc-parameter-remapping.php b/tests/PHPStan/Analyser/data/inheritdoc-parameter-remapping.php new file mode 100644 index 0000000000..37fd8ea320 --- /dev/null +++ b/tests/PHPStan/Analyser/data/inheritdoc-parameter-remapping.php @@ -0,0 +1,47 @@ +doBar('1'); $baz->doBar(1); }; + +class Lorem +{ + + /** + * @param B $b + * @param C $c + * @param A $a + * @param D $d + */ + public function doLorem($a, $b, $c, $d) + { + + } + +} + +class Ipsum extends Lorem +{ + + public function doLorem($x, $y, $z, $d) + { + + } + +} + +function (Ipsum $ipsum, A $a, B $b, C $c, D $d): void { + $ipsum->doLorem($a, $b, $c, $d); + $ipsum->doLorem(1, 1, 1, 1); +}; + +class Dolor extends Ipsum +{ + + public function doLorem($g, $h, $i, $d) + { + + } + +} + +function (Dolor $ipsum, A $a, B $b, C $c, D $d): void { + $ipsum->doLorem($a, $b, $c, $d); + $ipsum->doLorem(1, 1, 1, 1); +};