Skip code changes on unresolvable/unknown classes (#4619)

* Added failling tests regarding unknown parents * Create skip_unknown_parent.php.inc * Create skip_unknown_parent.php.inc * Create skip_unknown_parent_class.php.inc * fix * fix * rename exception class
rectorphp · Aug 7, 2023 · 493e4f8 · 493e4f8
1 parent 73112a2
commit 493e4f8
Show file tree

Hide file tree

Showing 14 changed files with 118 additions and 12 deletions.
diff --git a/packages/VendorLocker/Exception/UnresolvableClassException.php b/packages/VendorLocker/Exception/UnresolvableClassException.php
@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\VendorLocker\Exception;
+
+final class UnresolvableClassException extends \Exception
+{
+}
diff --git a/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php b/packages/VendorLocker/ParentClassMethodTypeOverrideGuard.php
@@ -8,10 +8,13 @@
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\MethodReflection;
 use PHPStan\Type\Type;
+use Rector\Core\Exception\ShouldNotHappenException;
 use Rector\Core\Reflection\ReflectionResolver;
+use Rector\Core\Util\Reflection\PrivatesAccessor;
 use Rector\NodeNameResolver\NodeNameResolver;
 use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
 use Rector\StaticTypeMapper\StaticTypeMapper;
+use Rector\VendorLocker\Exception\UnresolvableClassException;
 
 final class ParentClassMethodTypeOverrideGuard
 {
@@ -20,30 +23,60 @@ public function __construct(
         private readonly ReflectionResolver $reflectionResolver,
         private readonly TypeComparator $typeComparator,
         private readonly StaticTypeMapper $staticTypeMapper,
+        private readonly PrivatesAccessor $privatesAccessor,
     ) {
     }
 
-    public function hasParentClassMethod(ClassMethod $classMethod): bool
+    public function hasParentClassMethod(ClassMethod $classMethod): ?bool
     {
-        return $this->getParentClassMethod($classMethod) instanceof MethodReflection;
+        try {
+            $parentClassMethod = $this->resolveParentClassMethod($classMethod);
+
+            return $parentClassMethod instanceof MethodReflection;
+        } catch (UnresolvableClassException) {
+            // we don't know all involved parents.
+            return null;
+        }
     }
 
     public function getParentClassMethod(ClassMethod $classMethod): ?MethodReflection
+    {
+        try {
+            $parentClassMethod = $this->resolveParentClassMethod($classMethod);
+
+            return $parentClassMethod;
+        } catch (UnresolvableClassException) {
+            // we don't know all involved parents.
+            throw new ShouldNotHappenException('Unable to resolve involved class. You are likely missing hasParentClassMethod() before calling getParentClassMethod().');
+        }
+    }
+
+    /**
+     * @throws UnresolvableClassException
+     */
+    private function resolveParentClassMethod(ClassMethod $classMethod): ?MethodReflection
     {
         $classReflection = $this->reflectionResolver->resolveClassReflection($classMethod);
         if (! $classReflection instanceof ClassReflection) {
-            return null;
+            // we can't resolve the class, so we don't know.
+            throw new UnresolvableClassException();
         }
 
         /** @var string $methodName */
         $methodName = $this->nodeNameResolver->getName($classMethod);
-        $parentClassReflection = $classReflection->getParentClass();
-        while ($parentClassReflection instanceof ClassReflection) {
+        $currentClassReflection = $classReflection;
+        while ($this->hasClassParent($currentClassReflection)) {
+            $parentClassReflection = $currentClassReflection->getParentClass();
+            if (!$parentClassReflection instanceof ClassReflection) {
+                // per AST we have a parent class, but our reflection classes are not able to load its class definition/signature
+                throw new UnresolvableClassException();
+            }
+
             if ($parentClassReflection->hasNativeMethod($methodName)) {
                 return $parentClassReflection->getNativeMethod($methodName);
             }
 
-            $parentClassReflection = $parentClassReflection->getParentClass();
+            $currentClassReflection = $parentClassReflection;
         }
 
         foreach ($classReflection->getInterfaces() as $interfaceReflection) {
@@ -57,6 +90,14 @@ public function getParentClassMethod(ClassMethod $classMethod): ?MethodReflectio
         return null;
     }
 
+    private function hasClassParent(ClassReflection $classReflection):bool {
+        // XXX rework this hack, after https://github.com/phpstan/phpstan-src/pull/2563 landed
+        $nativeReflection = $classReflection->getNativeReflection();
+        $betterReflectionClass = $this->privatesAccessor->getPrivateProperty($nativeReflection, 'betterReflectionClass');
+        $parentClassName = $this->privatesAccessor->getPrivateProperty($betterReflectionClass, 'parentClassName');
+        return $parentClassName !== null;
+    }
+
     public function shouldSkipReturnTypeChange(ClassMethod $classMethod, Type $parentType): bool
     {
         if ($classMethod->returnType === null) {

diff --git a/...on/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc b/...on/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Fixture/skip_unknown_parent.php.inc
@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ParamTypeByMethodCallTypeRector\Fixture;
+
+use Rector\Tests\TypeDeclaration\Rector\ClassMethod\ParamTypeByMethodCallTypeRector\Source\GoOneWayInterface;
+use Rector\Tests\TypeDeclaration\Rector\ClassMethod\ParamTypeByMethodCallTypeRector\Source\SomeTypedService;
+
+final class SkipUnknownParent extends UnknownParentClass
+{
+    public function __construct(
+        private SomeTypedService $someTypedService
+    ) {
+    }
+
+    public function go($value)
+    {
+        $this->someTypedService->run($value);
+    }
+}
diff --git a/...on/Rector/ClassMethod/ReturnTypeFromStrictParamRector/Fixture/skip_unknown_parent.php.inc b/...on/Rector/ClassMethod/ReturnTypeFromStrictParamRector/Fixture/skip_unknown_parent.php.inc
@@ -0,0 +1,9 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictParamRector\Fixture;
+
+class SkipUnknownParent extends UnknownParentClass {
+    public function doFoo(SkipParentOverridden $param) {
+        return $param;
+    }
+}
diff --git a/...ion/Rector/ClassMethod/StrictArrayParamDimFetchRector/Fixture/skip_unknown_parent.php.inc b/...ion/Rector/ClassMethod/StrictArrayParamDimFetchRector/Fixture/skip_unknown_parent.php.inc
@@ -0,0 +1,13 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\StrictArrayParamDimFetchRector\Fixture;
+
+use Rector\Tests\TypeDeclaration\Rector\ClassMethod\StrictArrayParamDimFetchRector\Source\SomeInterface;
+
+class SkipUnknownParent extends UnknownParentClass
+{
+    public function go($item)
+    {
+        return $item['name'];
+    }
+}
diff --git a/...or/Param/ParamTypeFromStrictTypedPropertyRector/Fixture/skip_unknown_parent_class.php.inc b/...or/Param/ParamTypeFromStrictTypedPropertyRector/Fixture/skip_unknown_parent_class.php.inc
@@ -0,0 +1,13 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\Param\ParamTypeFromStrictTypedPropertyRector\Fixture;
+
+final class SkipUnknownParentClass extends UnknownParentClass
+{
+    private array $items = [];
+
+    public function redirect(string $path, $args = [])
+    {
+        $this->items = $args;
+    }
+}
diff --git a/rules/Privatization/Guard/ParentPropertyLookupGuard.php b/rules/Privatization/Guard/ParentPropertyLookupGuard.php
@@ -48,6 +48,7 @@ public function isLegal(Property $property, ?ClassReflection $classReflection):
             return false;
         }
 
+        // XXX rework this hack, after https://github.com/phpstan/phpstan-src/pull/2563 landed
         $nativeReflection = $classReflection->getNativeReflection();
         $betterReflectionClass = $this->privatesAccessor->getPrivateProperty(
             $nativeReflection,

diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeFromPropertyTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeFromPropertyTypeRector.php
@@ -123,7 +123,7 @@ function (Node $node) use ($paramName): bool {
                 continue;
             }
 
-            if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node)) {
+            if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node) !== false) {
                 return null;
             }
 

diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector.php
@@ -146,7 +146,7 @@ private function shouldSkipClassMethod(ClassMethod $classMethod): bool
             return true;
         }
 
-        return $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($classMethod);
+        return $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($classMethod) !== false;
     }
 
     private function mirrorParamType(

diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnTypeFromStrictParamRector.php
@@ -202,7 +202,7 @@ private function shouldSkipNode(ClassMethod|Function_|Closure $node): bool
         }
 
         if ($node instanceof ClassMethod) {
-            if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node)) {
+            if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node) !== false) {
                 return true;
             }
 

diff --git a/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php b/rules/TypeDeclaration/Rector/ClassMethod/StrictArrayParamDimFetchRector.php
@@ -76,7 +76,7 @@ public function refactor(Node $node): ?Node
     {
         $hasChanged = false;
 
-        if ($node instanceof ClassMethod && $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node)) {
+        if ($node instanceof ClassMethod && $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node) !== false) {
             return null;
         }
 

diff --git a/rules/TypeDeclaration/Rector/ClassMethod/StrictStringParamConcatRector.php b/rules/TypeDeclaration/Rector/ClassMethod/StrictStringParamConcatRector.php
@@ -75,7 +75,7 @@ public function getNodeTypes(): array
      */
     public function refactor(Node $node): ?Node
     {
-        if ($node instanceof ClassMethod && $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node)) {
+        if ($node instanceof ClassMethod && $this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($node) !== false) {
             return null;
         }
 

diff --git a/rules/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector.php b/rules/TypeDeclaration/Rector/Param/ParamTypeFromStrictTypedPropertyRector.php
@@ -115,7 +115,7 @@ private function decorateParamWithType(ClassMethod $classMethod, Param $param, S
             return null;
         }
 
-        if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($classMethod)) {
+        if ($this->parentClassMethodTypeOverrideGuard->hasParentClassMethod($classMethod) !== false) {
             return null;
         }
 

diff --git a/src/PhpParser/Node/Value/ValueResolver.php b/src/PhpParser/Node/Value/ValueResolver.php
@@ -333,6 +333,7 @@ private function resolveClassFromSelfStaticParent(ClassConstFetch $classConstFet
             );
         }
 
+        // XXX rework this hack, after https://github.com/phpstan/phpstan-src/pull/2563 landed
         // ensure parent class name still resolved even not autoloaded
         $nativeReflection = $classReflection->getNativeReflection();
         $betterReflectionClass = $this->privatesAccessor->getPrivateProperty(