diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml index 639d1c387f1..3d05513459b 100644 --- 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 new file mode 100644 index 00000000000..61ead86667c --- /dev/null +++ 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 new file mode 100644 index 00000000000..5468cd74606 --- /dev/null +++ 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 new file mode 100644 index 00000000000..3d4005d5bc6 --- /dev/null +++ b/e2e/skip-add-return-has-child/rector.php @@ -0,0 +1,13 @@ +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 new file mode 100644 index 00000000000..1f4fdbb769b --- /dev/null +++ b/e2e/skip-add-return-has-child/src/SkipChangeAbstractMethodDifferentType.php @@ -0,0 +1,11 @@ +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 index a2a5282a60b..4813d2039a9 100644 --- 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/src/NodeTypeResolver/Reflection/BetterReflection/SourceLocatorProvider/DynamicSourceLocatorProvider.php b/src/NodeTypeResolver/Reflection/BetterReflection/SourceLocatorProvider/DynamicSourceLocatorProvider.php index 86a71b58ce2..8aeebfafae0 100644 --- a/src/NodeTypeResolver/Reflection/BetterReflection/SourceLocatorProvider/DynamicSourceLocatorProvider.php +++ b/src/NodeTypeResolver/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 index f8e032d3bc1..246bc330102 100644 --- 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