Faster AddReturnTypeDeclarationBasedOnParentClassMethodRector (#4804)

* Faster ParentClassMethodTypeOverrideGuard * fix test * drop no longer used AstResolver dependency * fix phpstan
rectorphp · Aug 17, 2023 · 2604a47 · 2604a47
1 parent 81caf1e
commit 2604a47
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 50 deletions.
diff --git 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
@@ -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/...eturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/extended_class_no_type.php.inc b/...eturnTypeDeclarationBasedOnParentClassMethodRector/Fixture/extended_class_no_type.php.inc
diff --git a/...TypeDeclarationBasedOnParentClassMethodRector/Fixture/skip_extended_class_no_type.php.inc b/...TypeDeclarationBasedOnParentClassMethodRector/Fixture/skip_extended_class_no_type.php.inc
@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Fixture;
+
+use Rector\Tests\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationBasedOnParentClassMethodRector\Source\SomeClassWithoutReturnType;
+
+class MyClass extends SomeClassWithoutReturnType
+{
+    public function run()
+    {
+    }
+}
+
+?>
diff --git a/...Declaration/Rector/ClassMethod/AddReturnTypeDeclarationBasedOnParentClassMethodRector.php b/...Declaration/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(