From 2604a47c32dd8606349a1724122977c8b2fa6bf9 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 17 Aug 2023 11:00:02 +0200 Subject: [PATCH] Faster AddReturnTypeDeclarationBasedOnParentClassMethodRector (#4804) * Faster ParentClassMethodTypeOverrideGuard * fix test * drop no longer used AstResolver dependency * fix phpstan --- .../ParentClassMethodTypeOverrideGuard.php | 25 ++++++++++------ phpstan.neon | 5 ++++ .../Fixture/extended_class_no_type.php.inc | 29 ------------------- .../skip_extended_class_no_type.php.inc | 14 +++++++++ ...larationBasedOnParentClassMethodRector.php | 26 +++++++++-------- 5 files changed, 49 insertions(+), 50 deletions(-) delete mode 100644 rules-tests/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/extended_class_no_type.php.inc create mode 100644 rules-tests/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/skip_extended_class_no_type.php.inc diff --git a/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php b/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php index b393a1907f2..bdf9a574197 100644 --- a/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php +++ b/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php @@ -13,6 +13,7 @@ use Rector\NodeNameResolver\NodeNameResolver; use Rector\NodeTypeResolver\TypeComparator\TypeComparator; use Rector\StaticTypeMapper\StaticTypeMapper; +use Rector\Tests\Naming\Rector\Foreach_\RenameForeachValueVariableToMatchMethodCallReturnTypeRector\Source\Method; use Rector\VendorLocker\Exception\UnresolvableClassException; final class ParentClassMethodTypeOverrideGuard @@ -26,7 +27,7 @@ public function __construct( ) { } - public function hasParentClassMethod(ClassMethod $classMethod): bool + public function hasParentClassMethod(ClassMethod|MethodReflection $classMethod): bool { try { $parentClassMethod = $this->resolveParentClassMethod($classMethod); @@ -39,7 +40,7 @@ public function hasParentClassMethod(ClassMethod $classMethod): bool } } - public function getParentClassMethod(ClassMethod $classMethod): ?MethodReflection + public function getParentClassMethod(ClassMethod|MethodReflection $classMethod): ?MethodReflection { try { return $this->resolveParentClassMethod($classMethod); @@ -63,16 +64,22 @@ public function shouldSkipReturnTypeChange(ClassMethod $classMethod, Type $paren return $this->typeComparator->areTypesEqual($currentReturnType, $parentType); } - private function resolveParentClassMethod(ClassMethod $classMethod): ?MethodReflection + private function resolveParentClassMethod(ClassMethod|MethodReflection $classMethod): ?MethodReflection { - $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod); - if (! $classReflection instanceof ClassReflection) { - // we can't resolve the class, so we don't know. - throw new UnresolvableClassException(); + if ($classMethod instanceof ClassMethod) { + $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod); + if (! $classReflection instanceof ClassReflection) { + // we can't resolve the class, so we don't know. + throw new UnresolvableClassException(); + } + + /** @var string $methodName */ + $methodName = $this->nodeNameResolver->getName($classMethod); + } else { + $classReflection = $classMethod->getDeclaringClass(); + $methodName = $classMethod->getName(); } - /** @var string $methodName */ - $methodName = $this->nodeNameResolver->getName($classMethod); $currentClassReflection = $classReflection; while ($this->hasClassParent($currentClassReflection)) { $parentClassReflection = $currentClassReflection->getParentClass(); diff --git a/phpstan.neon b/phpstan.neon index bb3c01dee5b..4367e30d57e 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -692,3 +692,8 @@ parameters: path: packages/Config/RectorConfig.php - '#Method Rector\\Core\\PhpParser\\NodeTraverser\\RectorNodeTraverser\:\:__construct\(\) has parameter \$phpRectors with no value type specified in iterable type iterable#' + + # method signature kept for symmetry of hasParentClassMethod() with getParentClassMethod() + - + message: '#Parameters should use "PhpParser\\Node\\Stmt\\ClassMethod" types as the only types passed to this method#' + path: packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/extended_class_no_type.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/extended_class_no_type.php.inc deleted file mode 100644 index 9cd782c4f10..00000000000 --- a/rules-tests/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/extended_class_no_type.php.inc +++ /dev/null @@ -1,29 +0,0 @@ - ------ - diff --git a/rules-tests/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/skip_extended_class_no_type.php.inc b/rules-tests/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/skip_extended_class_no_type.php.inc new file mode 100644 index 00000000000..83fa60415c6 --- /dev/null +++ b/rules-tests/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/skip_extended_class_no_type.php.inc @@ -0,0 +1,14 @@ + diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector.php index 4aac8a0ddfd..b430dd68523 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector.php @@ -8,6 +8,7 @@ use PhpParser\Node\Stmt\Class_; use PhpParser\Node\Stmt\ClassMethod; use PHPStan\Reflection\MethodReflection; +use PHPStan\Reflection\ParametersAcceptorSelector; use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; @@ -30,7 +31,6 @@ final class AddReturnTypeDeclarationBasedOnParentClassMethodRector extends Abstr { public function __construct( private readonly ParentClassMethodTypeOverrideGuard $parentClassMethodTypeOverrideGuard, - private readonly AstResolver $astResolver, private readonly PhpVersionProvider $phpVersionProvider, ) { } @@ -124,26 +124,28 @@ public function refactor(Node $node): ?Node private function getReturnTypeRecursive(ClassMethod $classMethod): ?Type { $returnType = $classMethod->getReturnType(); + if ($returnType !== null) { + return $this->staticTypeMapper->mapPhpParserNodePHPStanType($returnType); + } - if ($returnType === null) { - $parentMethodReflection = $this->parentClassMethodTypeOverrideGuard->getParentClassMethod($classMethod); - if (! $parentMethodReflection instanceof MethodReflection) { + $parentMethodReflection = $this->parentClassMethodTypeOverrideGuard->getParentClassMethod($classMethod); + while ($parentMethodReflection instanceof MethodReflection) { + if ($parentMethodReflection->isPrivate()) { return null; } - $parentClassMethod = $this->astResolver->resolveClassMethodFromMethodReflection($parentMethodReflection); - if (! $parentClassMethod instanceof ClassMethod) { - return null; + $parentReturnType = ParametersAcceptorSelector::selectSingle($parentMethodReflection->getVariants())->getReturnType(); + if (!$parentReturnType instanceof MixedType) { + return $parentReturnType; } - - if ($parentClassMethod->isPrivate()) { - return null; + if ($parentReturnType->isExplicitMixed()) { + return $parentReturnType; } - return $this->getReturnTypeRecursive($parentClassMethod); + $parentMethodReflection = $this->parentClassMethodTypeOverrideGuard->getParentClassMethod($parentMethodReflection); } - return $this->staticTypeMapper->mapPhpParserNodePHPStanType($returnType); + return null; } private function processClassMethodReturnType(