[Experiment] Rework child classes detection on DynamicSourceLocatorPr…

…ovider (#5735) * [Experiment] Rework child classes detection on DynamicSourceLocatorProvider * trigger by locate by identifier type * fix test * cs fix * collect classes * Trigger collect clases collection * clean up calls * only run on non-phpunit * Fix test * added back FamilyRelationsAnalyzer::getChildrenOfClassReflection() * cs fix * add fixture to skip change abstract method, with child different type * Fix * final touch: allow collect classes on OptimizedSingleFileSourceLocator on non-phpunit env * final touch: future note: comment * final touch: no need to collect classes on phpunit env * final touch: future note for infinite loop reason * final touch: rollback TestModifyReprintTest * final touch: clean up * really final touch: clean up * final touch: no need to collect class on single file locator * Really final touch: make reflectionProvider nullable to avoid BC break on usage * final touch: comment * Really final touch: add e2e test for proof * Really final touch: inject by PHPStanServicesFactory * final touch: use autowire() for consistency * really final touch: early create IdentifierType object before loop * final touch: cs fix
rectorphp · Apr 25, 2024 · 9bcded0 · 9bcded0
1 parent 8281a06
commit 9bcded0
Show file tree

Hide file tree

Showing 11 changed files with 198 additions and 3 deletions.
diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml
@@ -34,6 +34,7 @@ jobs:
                     - 'e2e/different-path-over-skip-config'
                     - 'e2e/invalid-paths'
                     - 'e2e/applied-polyfill-php80'
+                    - 'e2e/skip-add-return-has-child'
 
         name: End to end test - ${{ matrix.directory }}
 

diff --git a/e2e/skip-add-return-has-child/.gitignore b/e2e/skip-add-return-has-child/.gitignore
@@ -0,0 +1 @@
+/vendor
diff --git a/e2e/skip-add-return-has-child/composer.json b/e2e/skip-add-return-has-child/composer.json
@@ -0,0 +1,7 @@
+{
+    "require": {
+        "php": "^8.1"
+    },
+    "minimum-stability": "dev",
+    "prefer-stable": true
+}
diff --git a/e2e/skip-add-return-has-child/rector.php b/e2e/skip-add-return-has-child/rector.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+use Rector\Config\RectorConfig;
+use Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector;
+use Rector\ValueObject\PhpVersionFeature;
+
+return static function (RectorConfig $rectorConfig): void {
+    $rectorConfig->rule(ReturnTypeFromReturnNewRector::class);
+    $rectorConfig->phpVersion(PhpVersionFeature::STATIC_RETURN_TYPE);
+    $rectorConfig->paths([__DIR__ . '/src']);
+};
diff --git a/e2e/skip-add-return-has-child/src/SkipChangeAbstractMethodDifferentType.php b/e2e/skip-add-return-has-child/src/SkipChangeAbstractMethodDifferentType.php
@@ -0,0 +1,11 @@
+<?php
+
+namespace App;
+
+abstract class SkipChangeAbstractMethodDifferentType
+{
+    public function run()
+    {
+        return new static();
+    }
+}
diff --git a/e2e/skip-add-return-has-child/src/SkipChangeAbstractMethodDifferentTypeChild.php b/e2e/skip-add-return-has-child/src/SkipChangeAbstractMethodDifferentTypeChild.php
@@ -0,0 +1,11 @@
+<?php
+
+namespace App;
+
+class SkipChangeAbstractMethodDifferentTypeChild extends SkipChangeAbstractMethodDifferentType
+{
+    public function run(): string
+    {
+        return 'a';
+    }
+}
diff --git a/.../ReturnTypeFromReturnNewRector/Fixture/skip_change_abstract_method_different_type.php.inc b/.../ReturnTypeFromReturnNewRector/Fixture/skip_change_abstract_method_different_type.php.inc
@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector\Fixture;
+
+abstract class SkipChangeAbstractMethodDifferentType
+{
+    public function run()
+    {
+        return new static();
+    }
+}
+
+class SkipChangeAbstractMethodDifferentTypeChild extends SkipChangeAbstractMethodDifferentType
+{
+    public function run(): string
+    {
+        return 'a';
+    }
+}
diff --git a/src/DependencyInjection/LazyContainerFactory.php b/src/DependencyInjection/LazyContainerFactory.php
@@ -466,6 +466,13 @@ static function (Container $container): DynamicSourceLocatorProvider {
             }
         );
 
+        $rectorConfig->afterResolving(
+            DynamicSourceLocatorProvider::class,
+            static function (DynamicSourceLocatorProvider $dynamicSourceLocatorProvider, Container $container): void {
+                $dynamicSourceLocatorProvider->autowire($container->make(ReflectionProvider::class));
+            }
+        );
+
         // resetables
         $rectorConfig->tag(DynamicSourceLocatorProvider::class, ResetableInterface::class);
         $rectorConfig->tag(RenamedClassesDataCollector::class, ResetableInterface::class);

diff --git a/src/FamilyTree/Reflection/FamilyRelationsAnalyzer.php b/src/FamilyTree/Reflection/FamilyRelationsAnalyzer.php
@@ -10,15 +10,41 @@
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\ReflectionProvider;
 use Rector\NodeNameResolver\NodeNameResolver;
+use Rector\Util\Reflection\PrivatesAccessor;
 
 final readonly class FamilyRelationsAnalyzer
 {
     public function __construct(
         private ReflectionProvider $reflectionProvider,
         private NodeNameResolver $nodeNameResolver,
+        private PrivatesAccessor $privatesAccessor
     ) {
     }
 
+    /**
+     * @return ClassReflection[]
+     */
+    public function getChildrenOfClassReflection(ClassReflection $desiredClassReflection): array
+    {
+        if ($desiredClassReflection->isFinalByKeyword()) {
+            return [];
+        }
+
+        /** @var ClassReflection[] $classReflections */
+        $classReflections = $this->privatesAccessor->getPrivateProperty($this->reflectionProvider, 'classes');
+        $childrenClassReflections = [];
+
+        foreach ($classReflections as $classReflection) {
+            if (! $classReflection->isSubclassOf($desiredClassReflection->getName())) {
+                continue;
+            }
+
+            $childrenClassReflections[] = $classReflection;
+        }
+
+        return $childrenClassReflections;
+    }
+
     /**
      * @api
      * @return string[]

diff --git a/...solver/Reflection/BetterReflection/SourceLocatorProvider/DynamicSourceLocatorProvider.php b/...solver/Reflection/BetterReflection/SourceLocatorProvider/DynamicSourceLocatorProvider.php
@@ -4,11 +4,17 @@
 
 namespace Rector\NodeTypeResolver\Reflection\BetterReflection\SourceLocatorProvider;
 
+use PHPStan\BetterReflection\Identifier\IdentifierType;
+use PHPStan\BetterReflection\Reflector\DefaultReflector;
 use PHPStan\BetterReflection\SourceLocator\Type\AggregateSourceLocator;
 use PHPStan\BetterReflection\SourceLocator\Type\SourceLocator;
+use PHPStan\Broker\ClassNotFoundException;
+use PHPStan\File\CouldNotReadFileException;
 use PHPStan\Reflection\BetterReflection\SourceLocator\FileNodesFetcher;
+use PHPStan\Reflection\BetterReflection\SourceLocator\NewOptimizedDirectorySourceLocator;
 use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedDirectorySourceLocatorFactory;
 use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedSingleFileSourceLocator;
+use PHPStan\Reflection\ReflectionProvider;
 use Rector\Contract\DependencyInjection\ResetableInterface;
 use Rector\Testing\PHPUnit\StaticPHPUnitEnvironment;
 
@@ -29,12 +35,19 @@ final class DynamicSourceLocatorProvider implements ResetableInterface
 
     private ?AggregateSourceLocator $aggregateSourceLocator = null;
 
+    private ReflectionProvider $reflectionProvider;
+
     public function __construct(
         private readonly FileNodesFetcher $fileNodesFetcher,
         private readonly OptimizedDirectorySourceLocatorFactory $optimizedDirectorySourceLocatorFactory
     ) {
     }
 
+    public function autowire(ReflectionProvider $reflectionProvider): void
+    {
+        $this->reflectionProvider = $reflectionProvider;
+    }
+
     public function setFilePath(string $filePath): void
     {
         $this->filePaths = [$filePath];
@@ -66,6 +79,7 @@ public function provide(): SourceLocator
         }
 
         $sourceLocators = [];
+
         foreach ($this->filePaths as $file) {
             $sourceLocators[] = new OptimizedSingleFileSourceLocator($this->fileNodesFetcher, $file);
         }
@@ -74,9 +88,11 @@ public function provide(): SourceLocator
             $sourceLocators[] = $this->optimizedDirectorySourceLocatorFactory->createByDirectory($directory);
         }
 
-        $this->aggregateSourceLocator = new AggregateSourceLocator($sourceLocators);
+        $aggregateSourceLocator = $this->aggregateSourceLocator = new AggregateSourceLocator($sourceLocators);
+
+        $this->collectClasses($aggregateSourceLocator, $sourceLocators);
 
-        return $this->aggregateSourceLocator;
+        return $aggregateSourceLocator;
     }
 
     public function isPathsEmpty(): bool
@@ -93,4 +109,38 @@ public function reset(): void
         $this->directories = [];
         $this->aggregateSourceLocator = null;
     }
+
+    /**
+     * @param OptimizedSingleFileSourceLocator[]|NewOptimizedDirectorySourceLocator[] $sourceLocators
+     */
+    private function collectClasses(AggregateSourceLocator $aggregateSourceLocator, array $sourceLocators): void
+    {
+        if ($sourceLocators === []) {
+            return;
+        }
+
+        // no need to collect classes on single file, will auto collected
+        if (count($sourceLocators) === 1 && $sourceLocators[0] instanceof OptimizedSingleFileSourceLocator) {
+            return;
+        }
+
+        $reflector = new DefaultReflector($aggregateSourceLocator);
+        $identifierClass = new IdentifierType(IdentifierType::IDENTIFIER_CLASS);
+
+        foreach ($sourceLocators as $sourceLocator) {
+            // trigger collect "classes" on get class on locate identifier
+            try {
+                $reflections = $sourceLocator->locateIdentifiersByType($reflector, $identifierClass);
+
+                foreach ($reflections as $reflection) {
+                    // make 'classes' collection
+                    try {
+                        $this->reflectionProvider->getClass($reflection->getName());
+                    } catch (ClassNotFoundException) {
+                    }
+                }
+            } catch (CouldNotReadFileException) {
+            }
+        }
+    }
 }
diff --git a/src/VendorLocker/NodeVendorLocker/ClassMethodReturnTypeOverrideGuard.php b/src/VendorLocker/NodeVendorLocker/ClassMethodReturnTypeOverrideGuard.php
@@ -4,22 +4,30 @@
 
 namespace Rector\VendorLocker\NodeVendorLocker;
 
+use PhpParser\Node;
 use PhpParser\Node\Stmt\ClassMethod;
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\FunctionVariantWithPhpDocs;
 use PHPStan\Reflection\MethodReflection;
+use PHPStan\Reflection\ParametersAcceptorSelector;
+use PHPStan\Reflection\Php\PhpMethodReflection;
 use PHPStan\Type\MixedType;
+use PHPStan\Type\Type;
+use Rector\FamilyTree\Reflection\FamilyRelationsAnalyzer;
 use Rector\FileSystem\FilePathHelper;
 use Rector\NodeAnalyzer\MagicClassMethodAnalyzer;
 use Rector\NodeTypeResolver\PHPStan\ParametersAcceptorSelectorVariantsWrapper;
 use Rector\Reflection\ReflectionResolver;
+use Rector\TypeDeclaration\TypeInferer\ReturnTypeInferer;
 use Rector\VendorLocker\ParentClassMethodTypeOverrideGuard;
 
 final readonly class ClassMethodReturnTypeOverrideGuard
 {
     public function __construct(
+        private FamilyRelationsAnalyzer $familyRelationsAnalyzer,
         private ReflectionResolver $reflectionResolver,
+        private ReturnTypeInferer $returnTypeInferer,
         private ParentClassMethodTypeOverrideGuard $parentClassMethodTypeOverrideGuard,
         private FilePathHelper $filePathHelper,
         private MagicClassMethodAnalyzer $magicClassMethodAnalyzer
@@ -54,7 +62,46 @@ public function shouldSkipClassMethod(ClassMethod $classMethod, Scope $scope): b
             return true;
         }
 
-        return $classMethod->isFinal();
+        if ($classMethod->isFinal()) {
+            return false;
+        }
+
+        $childrenClassReflections = $this->familyRelationsAnalyzer->getChildrenOfClassReflection($classReflection);
+        if ($childrenClassReflections === []) {
+            return false;
+        }
+
+        if ($classMethod->returnType instanceof Node) {
+            return true;
+        }
+
+        $returnType = $this->returnTypeInferer->inferFunctionLike($classMethod);
+        return $this->hasChildrenDifferentTypeClassMethod($classMethod, $childrenClassReflections, $returnType);
+    }
+
+    /**
+     * @param ClassReflection[] $childrenClassReflections
+     */
+    private function hasChildrenDifferentTypeClassMethod(
+        ClassMethod $classMethod,
+        array $childrenClassReflections,
+        Type $returnType
+    ): bool {
+        $methodName = $classMethod->name->toString();
+        foreach ($childrenClassReflections as $childClassReflection) {
+            $methodReflection = $childClassReflection->getNativeMethod($methodName);
+            if (! $methodReflection instanceof PhpMethodReflection) {
+                continue;
+            }
+
+            $parametersAcceptor = ParametersAcceptorSelector::combineAcceptors($methodReflection->getVariants());
+            $childReturnType = $parametersAcceptor->getNativeReturnType();
+            if (! $returnType->isSuperTypeOf($childReturnType)->yes()) {
+                return true;
+            }
+        }
+
+        return false;
     }
 
     private function isReturnTypeChangeAllowed(ClassMethod $classMethod, Scope $scope): bool