Skip to content

Commit 356d61b

Browse files
authored
[TypeDeclaration] Add typeGuardedClasses() to guard against breaking method signature changes (#8135)
Introduces ->withTypeGuardedClasses([...]) / RectorConfig::typeGuardedClasses([...]). Listed classes and their non-final descendants are left untouched by type-declaration rules, since adding a return or param type to an extendable class is a breaking change for its child overrides. Final classes are never guarded (cannot be extended). Logic lives in ParentClassMethodTypeOverrideGuard::isTypeGuardedClass() and is applied at the shared choke points, covering the return and param rule families: - ClassMethodReturnTypeOverrideGuard::shouldSkipClassMethod() (return family) - ClassMethodParamVendorLockResolver::isVendorLocked() (param completer path) - AbstractParamTypeByMethodCallTypeRector::shouldSkipClassMethod() (by-method-call param family) - AddReturnTypeDeclarationRector / AddParamTypeDeclarationRector (configurable rules) When no classes are configured the guard short-circuits, so existing behaviour is unchanged.
1 parent 6b50c34 commit 356d61b

14 files changed

Lines changed: 213 additions & 0 deletions
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
namespace Rector\Tests\TypeDeclaration\TypeGuardedClasses\Fixture;
4+
5+
use Rector\Tests\TypeDeclaration\TypeGuardedClasses\Source\GuardedRepository;
6+
7+
final class FinalChildApplied extends GuardedRepository
8+
{
9+
public function getData()
10+
{
11+
return [];
12+
}
13+
}
14+
15+
?>
16+
-----
17+
<?php
18+
19+
namespace Rector\Tests\TypeDeclaration\TypeGuardedClasses\Fixture;
20+
21+
use Rector\Tests\TypeDeclaration\TypeGuardedClasses\Source\GuardedRepository;
22+
23+
final class FinalChildApplied extends GuardedRepository
24+
{
25+
public function getData(): array
26+
{
27+
return [];
28+
}
29+
}
30+
31+
?>
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
namespace Rector\Tests\TypeDeclaration\TypeGuardedClasses\Fixture;
4+
5+
use Rector\Tests\TypeDeclaration\TypeGuardedClasses\Source\GuardedRepository;
6+
7+
class SkipChildOfGuarded extends GuardedRepository
8+
{
9+
public function getData()
10+
{
11+
return [];
12+
}
13+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\TypeDeclaration\TypeGuardedClasses\Source;
6+
7+
class GuardedRepository
8+
{
9+
public function getData()
10+
{
11+
return [];
12+
}
13+
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\TypeDeclaration\TypeGuardedClasses;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class TypeGuardedClassesTest extends AbstractRectorTestCase
12+
{
13+
#[DataProvider('provideData')]
14+
public function test(string $filePath): void
15+
{
16+
$this->doTestFile($filePath);
17+
}
18+
19+
public static function provideData(): Iterator
20+
{
21+
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
22+
}
23+
24+
public function provideConfigFilePath(): string
25+
{
26+
return __DIR__ . '/config/configured_rule.php';
27+
}
28+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use PHPStan\Type\ArrayType;
6+
use PHPStan\Type\MixedType;
7+
use Rector\Config\RectorConfig;
8+
use Rector\Tests\TypeDeclaration\TypeGuardedClasses\Source\GuardedRepository;
9+
use Rector\TypeDeclaration\Rector\ClassMethod\AddReturnTypeDeclarationRector;
10+
use Rector\TypeDeclaration\ValueObject\AddReturnTypeDeclaration;
11+
12+
return static function (RectorConfig $rectorConfig): void {
13+
$rectorConfig->typeGuardedClasses([GuardedRepository::class]);
14+
15+
$rectorConfig->ruleWithConfiguration(AddReturnTypeDeclarationRector::class, [
16+
new AddReturnTypeDeclaration(
17+
GuardedRepository::class,
18+
'getData',
19+
new ArrayType(new MixedType(), new MixedType())
20+
),
21+
]);
22+
};

‎rules/TypeDeclaration/Rector/ClassMethod/AbstractParamTypeByMethodCallTypeRector.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,11 @@ abstract protected function isMatchingParamType(Type $type): bool;
9999

100100
private function shouldSkipClassMethod(ClassMethod $classMethod): bool
101101
{
102+
// user-guarded class: adding a param type here would break its child classes
103+
if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($classMethod)) {
104+
return true;
105+
}
106+
102107
$isMissingParameterTypes = false;
103108
foreach ($classMethod->params as $param) {
104109
if ($param->type instanceof Node) {

‎rules/TypeDeclaration/Rector/ClassMethod/AddParamTypeDeclarationRector.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Rector\StaticTypeMapper\StaticTypeMapper;
2121
use Rector\TypeDeclaration\ValueObject\AddParamTypeDeclaration;
2222
use Rector\ValueObject\PhpVersionFeature;
23+
use Rector\VendorLocker\ParentClassMethodTypeOverrideGuard;
2324
use Symplify\RuleDocGenerator\ValueObject\CodeSample\ConfiguredCodeSample;
2425
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
2526
use Webmozart\Assert\Assert;
@@ -40,6 +41,7 @@ public function __construct(
4041
private readonly TypeComparator $typeComparator,
4142
private readonly PhpVersionProvider $phpVersionProvider,
4243
private readonly StaticTypeMapper $staticTypeMapper,
44+
private readonly ParentClassMethodTypeOverrideGuard $parentClassMethodTypeOverrideGuard,
4345
) {
4446
}
4547

@@ -85,6 +87,11 @@ public function refactor(Node $node): ?Node
8587
{
8688
$this->hasChanged = false;
8789

90+
// skip guarded classes, where adding a param type would break child classes
91+
if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($node)) {
92+
return null;
93+
}
94+
8895
foreach ($node->getMethods() as $classMethod) {
8996
if ($this->shouldSkip($node, $classMethod)) {
9097
continue;

‎rules/TypeDeclaration/Rector/ClassMethod/AddReturnTypeDeclarationRector.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ public function refactor(Node $node): ?Node
8989
{
9090
$this->hasChanged = false;
9191

92+
// skip guarded classes, where adding a return type would break child classes
93+
if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($node)) {
94+
return null;
95+
}
96+
9297
foreach ($this->methodReturnTypes as $methodReturnType) {
9398
$objectType = $methodReturnType->getObjectType();
9499
if (! $this->isObjectType($node, $objectType)) {

‎src/Config/RectorConfig.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,20 @@ public function treatClassesAsFinal(bool $treatClassesAsFinal = true): void
326326
SimpleParameterProvider::setParameter(Option::TREAT_CLASSES_AS_FINAL, $treatClassesAsFinal);
327327
}
328328

329+
/**
330+
* Guard the listed classes and their descendants against method signature changes that would
331+
* break child classes - e.g. adding a return type or a param type. Only non-final classes are
332+
* guarded, as final classes cannot be extended.
333+
*
334+
* @param string[] $classes
335+
*/
336+
public function typeGuardedClasses(array $classes): void
337+
{
338+
Assert::allString($classes);
339+
340+
SimpleParameterProvider::setParameter(Option::TYPE_GUARDED_CLASSES, $classes);
341+
}
342+
329343
/**
330344
* @param string[] $extensions
331345
*/

‎src/Configuration/Option.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,4 +286,10 @@ final class Option
286286
* @internal to allow process file without extension if explicitly registered
287287
*/
288288
public const string FILES_WITHOUT_EXTENSION = 'files_without_extension';
289+
290+
/**
291+
* @internal Use @see \Rector\Config\RectorConfig::typeGuardedClasses() instead
292+
* @var string
293+
*/
294+
public const string TYPE_GUARDED_CLASSES = 'type_guarded_classes';
289295
}

0 commit comments

Comments
 (0)