diff --git a/rules-tests/TypeDeclaration/TypeGuardedClasses/Fixture/final_child_applied.php.inc b/rules-tests/TypeDeclaration/TypeGuardedClasses/Fixture/final_child_applied.php.inc new file mode 100644 index 00000000000..f96eb0707d2 --- /dev/null +++ b/rules-tests/TypeDeclaration/TypeGuardedClasses/Fixture/final_child_applied.php.inc @@ -0,0 +1,31 @@ + +----- + diff --git a/rules-tests/TypeDeclaration/TypeGuardedClasses/Fixture/skip_child_of_guarded.php.inc b/rules-tests/TypeDeclaration/TypeGuardedClasses/Fixture/skip_child_of_guarded.php.inc new file mode 100644 index 00000000000..305fa897895 --- /dev/null +++ b/rules-tests/TypeDeclaration/TypeGuardedClasses/Fixture/skip_child_of_guarded.php.inc @@ -0,0 +1,13 @@ +doTestFile($filePath); + } + + public static function provideData(): Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/rules-tests/TypeDeclaration/TypeGuardedClasses/config/configured_rule.php b/rules-tests/TypeDeclaration/TypeGuardedClasses/config/configured_rule.php new file mode 100644 index 00000000000..e823b8c5e04 --- /dev/null +++ b/rules-tests/TypeDeclaration/TypeGuardedClasses/config/configured_rule.php @@ -0,0 +1,22 @@ +typeGuardedClasses([GuardedRepository::class]); + + $rectorConfig->ruleWithConfiguration(AddReturnTypeDeclarationRector::class, [ + new AddReturnTypeDeclaration( + GuardedRepository::class, + 'getData', + new ArrayType(new MixedType(), new MixedType()) + ), + ]); +}; diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AbstractParamTypeByMethodCallTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AbstractParamTypeByMethodCallTypeRector.php index d53e90cfe9f..bfae44424e3 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AbstractParamTypeByMethodCallTypeRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AbstractParamTypeByMethodCallTypeRector.php @@ -99,6 +99,11 @@ abstract protected function isMatchingParamType(Type $type): bool; private function shouldSkipClassMethod(ClassMethod $classMethod): bool { + // user-guarded class: adding a param type here would break its child classes + if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($classMethod)) { + return true; + } + $isMissingParameterTypes = false; foreach ($classMethod->params as $param) { if ($param->type instanceof Node) { diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php index 924d31de9bf..5c9361152f1 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php @@ -20,6 +20,7 @@ use Rector\StaticTypeMapper\StaticTypeMapper; use Rector\TypeDeclaration\ValueObject\AddParamTypeDeclaration; use Rector\ValueObject\PhpVersionFeature; +use Rector\VendorLocker\ParentClassMethodTypeOverrideGuard; use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; use Webmozart\Assert\Assert; @@ -40,6 +41,7 @@ public function __construct( private readonly TypeComparator $typeComparator, private readonly PhpVersionProvider $phpVersionProvider, private readonly StaticTypeMapper $staticTypeMapper, + private readonly ParentClassMethodTypeOverrideGuard $parentClassMethodTypeOverrideGuard, ) { } @@ -85,6 +87,11 @@ public function refactor(Node $node): ?Node { $this->hasChanged = false; + // skip guarded classes, where adding a param type would break child classes + if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($node)) { + return null; + } + foreach ($node->getMethods() as $classMethod) { if ($this->shouldSkip($node, $classMethod)) { continue; diff --git a/rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationRector.php b/rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationRector.php index 0c2cd4568eb..26e5fc6eb30 100644 --- a/rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationRector.php +++ b/rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationRector.php @@ -89,6 +89,11 @@ public function refactor(Node $node): ?Node { $this->hasChanged = false; + // skip guarded classes, where adding a return type would break child classes + if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($node)) { + return null; + } + foreach ($this->methodReturnTypes as $methodReturnType) { $objectType = $methodReturnType->getObjectType(); if (! $this->isObjectType($node, $objectType)) { diff --git a/src/Config/RectorConfig.php b/src/Config/RectorConfig.php index 83d8a40206f..c914f347a44 100644 --- a/src/Config/RectorConfig.php +++ b/src/Config/RectorConfig.php @@ -326,6 +326,20 @@ public function treatClassesAsFinal(bool $treatClassesAsFinal = true): void SimpleParameterProvider::setParameter(Option::TREAT_CLASSES_AS_FINAL, $treatClassesAsFinal); } + /** + * Guard the listed classes and their descendants against method signature changes that would + * break child classes - e.g. adding a return type or a param type. Only non-final classes are + * guarded, as final classes cannot be extended. + * + * @param string[] $classes + */ + public function typeGuardedClasses(array $classes): void + { + Assert::allString($classes); + + SimpleParameterProvider::setParameter(Option::TYPE_GUARDED_CLASSES, $classes); + } + /** * @param string[] $extensions */ diff --git a/src/Configuration/Option.php b/src/Configuration/Option.php index 0fccaa3d555..158030f7c13 100644 --- a/src/Configuration/Option.php +++ b/src/Configuration/Option.php @@ -286,4 +286,10 @@ final class Option * @internal to allow process file without extension if explicitly registered */ public const string FILES_WITHOUT_EXTENSION = 'files_without_extension'; + + /** + * @internal Use @see \Rector\Config\RectorConfig::typeGuardedClasses() instead + * @var string + */ + public const string TYPE_GUARDED_CLASSES = 'type_guarded_classes'; } diff --git a/src/Configuration/RectorConfigBuilder.php b/src/Configuration/RectorConfigBuilder.php index b1a14dd6def..dea40d68bc0 100644 --- a/src/Configuration/RectorConfigBuilder.php +++ b/src/Configuration/RectorConfigBuilder.php @@ -162,6 +162,11 @@ final class RectorConfigBuilder private ?bool $isTreatClassesAsFinal = null; + /** + * @var string[] + */ + private array $typeGuardedClasses = []; + /** * @var RegisteredService[] */ @@ -388,6 +393,10 @@ public function __invoke(RectorConfig $rectorConfig): void $rectorConfig->newLineOnFluentCall($this->isFluentNewLine); } + if ($this->typeGuardedClasses !== []) { + $rectorConfig->typeGuardedClasses($this->typeGuardedClasses); + } + if ($this->isTreatClassesAsFinal !== null) { $rectorConfig->treatClassesAsFinal($this->isTreatClassesAsFinal); } @@ -1248,6 +1257,18 @@ public function withTreatClassesAsFinal(bool $isTreatClassesAsFinal = true): sel return $this; } + /** + * Guard the listed classes and their non-final descendants against method signature changes + * that would break child classes - e.g. adding a return type or a param type. + * + * @param string[] $typeGuardedClasses + */ + public function withTypeGuardedClasses(array $typeGuardedClasses): self + { + $this->typeGuardedClasses = $typeGuardedClasses; + return $this; + } + public function registerService(string $className, ?string $alias = null, ?string $tag = null): self { $this->registerServices[] = new RegisteredService($className, $alias, $tag); diff --git a/src/VendorLocker/NodeVendorLocker/ClassMethodParamVendorLockResolver.php b/src/VendorLocker/NodeVendorLocker/ClassMethodParamVendorLockResolver.php index 3dd34c32bd8..d3e3c6c738c 100644 --- a/src/VendorLocker/NodeVendorLocker/ClassMethodParamVendorLockResolver.php +++ b/src/VendorLocker/NodeVendorLocker/ClassMethodParamVendorLockResolver.php @@ -8,12 +8,14 @@ use PHPStan\Reflection\ClassReflection; use Rector\NodeNameResolver\NodeNameResolver; use Rector\Reflection\ReflectionResolver; +use Rector\VendorLocker\ParentClassMethodTypeOverrideGuard; final readonly class ClassMethodParamVendorLockResolver { public function __construct( private NodeNameResolver $nodeNameResolver, private ReflectionResolver $reflectionResolver, + private ParentClassMethodTypeOverrideGuard $parentClassMethodTypeOverrideGuard, ) { } @@ -23,6 +25,11 @@ public function isVendorLocked(ClassMethod $classMethod): bool return true; } + // user-guarded class: adding a param type here would break its child classes + if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($classMethod)) { + return true; + } + if ($classMethod->isPrivate()) { return false; } diff --git a/src/VendorLocker/NodeVendorLocker/ClassMethodReturnTypeOverrideGuard.php b/src/VendorLocker/NodeVendorLocker/ClassMethodReturnTypeOverrideGuard.php index 241f2ab419b..cfe23c40b5d 100644 --- a/src/VendorLocker/NodeVendorLocker/ClassMethodReturnTypeOverrideGuard.php +++ b/src/VendorLocker/NodeVendorLocker/ClassMethodReturnTypeOverrideGuard.php @@ -28,6 +28,11 @@ public function __construct( public function shouldSkipClassMethod(ClassMethod $classMethod, Scope $scope): bool { + // user-guarded class: adding a return type here would break its child classes + if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($classMethod)) { + return true; + } + if ($this->magicClassMethodAnalyzer->isUnsafeOverridden($classMethod)) { return true; } diff --git a/src/VendorLocker/ParentClassMethodTypeOverrideGuard.php b/src/VendorLocker/ParentClassMethodTypeOverrideGuard.php index 545ec182962..d1f9943311d 100644 --- a/src/VendorLocker/ParentClassMethodTypeOverrideGuard.php +++ b/src/VendorLocker/ParentClassMethodTypeOverrideGuard.php @@ -5,10 +5,13 @@ namespace Rector\VendorLocker; use PhpParser\Node; +use PhpParser\Node\Stmt\ClassLike; use PhpParser\Node\Stmt\ClassMethod; use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\MethodReflection; use PHPStan\Type\Type; +use Rector\Configuration\Option; +use Rector\Configuration\Parameter\SimpleParameterProvider; use Rector\NodeNameResolver\NodeNameResolver; use Rector\NodeTypeResolver\TypeComparator\TypeComparator; use Rector\Reflection\ClassReflectionAnalyzer; @@ -27,6 +30,39 @@ public function __construct( ) { } + /** + * Is the class user-guarded against method signature changes, via + * @see \Rector\Config\RectorConfig::typeGuardedClasses() + * + * A class is guarded when it - or any of its ancestors - is on the configured list and it is not + * final. Adding a return or param type to such a class is a breaking change for its child + * classes, so type-declaration rules must leave it untouched. Final classes are never guarded, + * as they cannot be extended. + */ + public function isTypeGuardedClass(ClassLike|ClassMethod $node): bool + { + $guardedClasses = SimpleParameterProvider::provideArrayParameter(Option::TYPE_GUARDED_CLASSES); + if ($guardedClasses === []) { + return false; + } + + $classReflection = $this->reflectionResolver->resolveClassReflection($node); + if (! $classReflection instanceof ClassReflection) { + return false; + } + + // final classes cannot be extended, so adding a type is never a breaking change + if ($classReflection->isFinalByKeyword()) { + return false; + } + + // covers the guarded class itself and all its descendants + return array_any( + $guardedClasses, + static fn (string $guardedClass): bool => $classReflection->is($guardedClass) + ); + } + public function hasParentClassMethod(ClassMethod|MethodReflection $classMethod): bool { try {