diff --git a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/different_set.php.inc b/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/different_set.php.inc deleted file mode 100644 index 14dba59ab3a..00000000000 --- a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/different_set.php.inc +++ /dev/null @@ -1,33 +0,0 @@ -name = 100; - - return $this; - } -} - -?> ------ -name = 100; - } -} - -?> diff --git a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/fixture.php.inc b/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/fixture.php.inc deleted file mode 100644 index 3e32a9f42da..00000000000 --- a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,33 +0,0 @@ -nameValue = $name; - - return $this; - } -} - -?> ------ -nameValue = $name; - } -} - -?> diff --git a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/skip_different_return.php.inc b/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/skip_different_return.php.inc deleted file mode 100644 index ab9f475a845..00000000000 --- a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/skip_different_return.php.inc +++ /dev/null @@ -1,14 +0,0 @@ -name = $name; - return $name; - } -} diff --git a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/skip_no_param.php.inc b/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/skip_no_param.php.inc deleted file mode 100644 index 282656ae823..00000000000 --- a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/skip_no_param.php.inc +++ /dev/null @@ -1,15 +0,0 @@ -name = 'name'; - - return $this; - } -} diff --git a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/RemoveReturnThisFromSetterClassMethodRectorTest.php b/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/RemoveReturnThisFromSetterClassMethodRectorTest.php deleted file mode 100644 index 8942233ac14..00000000000 --- a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/RemoveReturnThisFromSetterClassMethodRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -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/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/config/configured_rule.php b/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/config/configured_rule.php deleted file mode 100644 index 4d26a96f805..00000000000 --- a/rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/config/configured_rule.php +++ /dev/null @@ -1,9 +0,0 @@ -withRules([RemoveReturnThisFromSetterClassMethodRector::class]); diff --git a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/fixture.php.inc b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/fixture.php.inc deleted file mode 100644 index 46e12a13933..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/fixture.php.inc +++ /dev/null @@ -1,36 +0,0 @@ -setName('John') - ->setSurname('Doe'); - } -} - -?> ------ -setName('John'); - $someSetterClass->setSurname('Doe'); - return $someSetterClass; - } -} - -?> diff --git a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_getter_setter.php.inc b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_getter_setter.php.inc deleted file mode 100644 index 8ee1f312eed..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_getter_setter.php.inc +++ /dev/null @@ -1,18 +0,0 @@ -getter() - ->setName('John') - ->setSurname('Doe'); - } -} diff --git a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_mocks.php.inc b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_mocks.php.inc deleted file mode 100644 index 7832cdff7a3..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_mocks.php.inc +++ /dev/null @@ -1,16 +0,0 @@ -createMock(SomeSetterClass::class) - ->expects($this->once()) - ->method('some'); - } -} diff --git a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_mocks_on_variable.php.inc b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_mocks_on_variable.php.inc deleted file mode 100644 index 991c9fc08f2..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_mocks_on_variable.php.inc +++ /dev/null @@ -1,17 +0,0 @@ -createMock(SomeSetterClass::class); - $someVariable->expects($this->once()) - ->method('some') - ->with(1); - } -} diff --git a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_return_different_object.php.inc b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_return_different_object.php.inc deleted file mode 100644 index e35c779ec74..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_return_different_object.php.inc +++ /dev/null @@ -1,15 +0,0 @@ -setName('John') - ->setSurname('Doe'); - } -} diff --git a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_sole_setter.php.inc b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_sole_setter.php.inc deleted file mode 100644 index 9a8d204d64e..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Fixture/skip_sole_setter.php.inc +++ /dev/null @@ -1,14 +0,0 @@ -setName('John'); - } -} diff --git a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/FluentSettersToStandaloneCallMethodRectorTest.php b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/FluentSettersToStandaloneCallMethodRectorTest.php deleted file mode 100644 index 0a20edee79e..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/FluentSettersToStandaloneCallMethodRectorTest.php +++ /dev/null @@ -1,28 +0,0 @@ -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/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Source/GetterSetterClass.php b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Source/GetterSetterClass.php deleted file mode 100644 index 0ca827146dc..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Source/GetterSetterClass.php +++ /dev/null @@ -1,11 +0,0 @@ -name = $name; - return $this; - } - - public function setSurname(?string $surname): self - { - $this->surname = $surname; - return $this; - } -} diff --git a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Source/SurnameCaller.php b/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Source/SurnameCaller.php deleted file mode 100644 index 426cf94f53b..00000000000 --- a/rules-tests/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector/Source/SurnameCaller.php +++ /dev/null @@ -1,11 +0,0 @@ -withRules([FluentSettersToStandaloneCallMethodRector::class]); diff --git a/rules/TypeDeclaration/NodeAnalyzer/ClassMethodAndPropertyAnalyzer.php b/rules/TypeDeclaration/NodeAnalyzer/ClassMethodAndPropertyAnalyzer.php index 5ccea80d790..1a548462e93 100644 --- a/rules/TypeDeclaration/NodeAnalyzer/ClassMethodAndPropertyAnalyzer.php +++ b/rules/TypeDeclaration/NodeAnalyzer/ClassMethodAndPropertyAnalyzer.php @@ -6,7 +6,6 @@ use PhpParser\Node\Expr\Assign; use PhpParser\Node\Expr\PropertyFetch; -use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\ClassMethod; use PhpParser\Node\Stmt\Expression; @@ -52,33 +51,6 @@ public function hasOnlyPropertyAssign(ClassMethod $classMethod, string $property return $this->isLocalPropertyVariableAssign($onlyClassMethodStmt, $propertyName); } - public function hasPropertyAssignWithReturnThis(ClassMethod $classMethod): bool - { - $stmts = (array) $classMethod->stmts; - if (count($stmts) !== 2) { - return false; - } - - $possibleAssignStmt = $stmts[0]; - $possibleReturnThis = $stmts[1]; - - if (! $this->isLocalPropertyVariableAssign($possibleAssignStmt, null)) { - return false; - } - - if (! $possibleReturnThis instanceof Return_) { - return false; - - } - - $returnExpr = $possibleReturnThis->expr; - if (! $returnExpr instanceof Variable) { - return false; - } - - return $this->nodeNameResolver->isName($returnExpr, 'this'); - } - private function isLocalPropertyVariableAssign(Stmt $onlyClassMethodStmt, ?string $propertyName): bool { if (! $onlyClassMethodStmt instanceof Expression) { diff --git a/rules/Unambiguous/NodeAnalyzer/FluentMethodCallsCollector.php b/rules/Unambiguous/NodeAnalyzer/FluentMethodCallsCollector.php deleted file mode 100644 index 94387aedd78..00000000000 --- a/rules/Unambiguous/NodeAnalyzer/FluentMethodCallsCollector.php +++ /dev/null @@ -1,60 +0,0 @@ -var instanceof MethodCall) { - return []; - } - - /** @var MethodCall[] $methodCalls */ - $methodCalls = []; - - $currentMethodCall = $firstMethodCall; - $classNameObjectType = null; - while ($currentMethodCall instanceof MethodCall) { - if ($currentMethodCall->isFirstClassCallable()) { - return []; - } - - // must be exactly one argument - if (count($currentMethodCall->getArgs()) !== 1) { - return []; - } - - $objectType = $this->nodeTypeResolver->getType($currentMethodCall->var); - if (! $objectType instanceof ObjectType) { - return []; - } - - if ($classNameObjectType === null) { - $classNameObjectType = $objectType->getClassName(); - } elseif ($classNameObjectType !== $objectType->getClassName()) { - return []; - } - - $methodCalls[] = $currentMethodCall; - $currentMethodCall = $currentMethodCall->var; - } - - return $methodCalls; - } -} diff --git a/rules/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector.php b/rules/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector.php index 4b8c8654979..14d1f5de0eb 100644 --- a/rules/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector.php +++ b/rules/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector.php @@ -5,26 +5,18 @@ namespace Rector\Unambiguous\Rector\Class_; use PhpParser\Node; -use PhpParser\Node\Identifier; use PhpParser\Node\Stmt\Class_; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\Rector\AbstractRector; -use Rector\TypeDeclaration\NodeAnalyzer\ClassMethodAndPropertyAnalyzer; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @experimental since 2025-11 - * - * @see \Rector\Tests\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector\RemoveReturnThisFromSetterClassMethodRectorTest + * @deprecated This rule is deprecated, as removing "return $this" from a setter needs a more complex approach that depends on the use case - the setter may be part of a fluent public API that callers rely on. */ -final class RemoveReturnThisFromSetterClassMethodRector extends AbstractRector +final class RemoveReturnThisFromSetterClassMethodRector extends AbstractRector implements DeprecatedInterface { - public function __construct( - private readonly ClassMethodAndPropertyAnalyzer $classMethodAndPropertyAnalyzer, - ) { - - } - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( @@ -73,49 +65,9 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?Class_ { - $hasChanged = false; - - foreach ($node->getMethods() as $classMethod) { - if ($classMethod->isMagic()) { - continue; - } - - // skip void return type - if ($classMethod->returnType instanceof Identifier && $this->isName($classMethod->returnType, 'void')) { - continue; - } - - if (count($classMethod->params) !== 1) { - continue; - } - - $soleParam = $classMethod->params[0]; - - // magic spread - if ($soleParam->variadic) { - continue; - } - - $paramName = $this->getName($soleParam->var); - if (! is_string($paramName)) { - continue; - } - - if (! $this->classMethodAndPropertyAnalyzer->hasPropertyAssignWithReturnThis($classMethod)) { - continue; - } - - // remove 2nd stmts, that is "return $this;" - unset($classMethod->stmts[1]); - $classMethod->returnType = new Identifier('void'); - - $hasChanged = true; - } - - if (! $hasChanged) { - return null; - } - - return $node; + throw new ShouldNotHappenException(sprintf( + '"%s" rule is deprecated, as removing "return $this" from a setter needs a more complex approach that depends on the use case', + self::class + )); } } diff --git a/rules/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector.php b/rules/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector.php index 4b94cd32c40..8b35f23eb0b 100644 --- a/rules/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector.php +++ b/rules/Unambiguous/Rector/Expression/FluentSettersToStandaloneCallMethodRector.php @@ -5,37 +5,19 @@ namespace Rector\Unambiguous\Rector\Expression; use PhpParser\Node; -use PhpParser\Node\Expr; -use PhpParser\Node\Expr\Assign; -use PhpParser\Node\Expr\MethodCall; -use PhpParser\Node\Expr\New_; -use PhpParser\Node\Expr\Variable; -use PhpParser\Node\Name; -use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\Expression; use PhpParser\Node\Stmt\Return_; -use PHPStan\Reflection\ClassReflection; -use PHPStan\Type\ObjectType; -use Rector\Naming\Naming\PropertyNaming; -use Rector\NodeTypeResolver\Node\AttributeKey; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; +use Rector\Exception\ShouldNotHappenException; use Rector\Rector\AbstractRector; -use Rector\Unambiguous\NodeAnalyzer\FluentMethodCallsCollector; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; /** - * @experimental since 2025-11 - * - * @see \Rector\Tests\Unambiguous\Rector\Expression\FluentSettersToStandaloneCallMethodRector\FluentSettersToStandaloneCallMethodRectorTest + * @deprecated This rule is deprecated, as breaking a fluent setter chain into standalone calls needs a more complex approach that depends on the use case - the safe transformation differs per method return semantics. */ -final class FluentSettersToStandaloneCallMethodRector extends AbstractRector +final class FluentSettersToStandaloneCallMethodRector extends AbstractRector implements DeprecatedInterface { - public function __construct( - private readonly PropertyNaming $propertyNaming, - private readonly FluentMethodCallsCollector $fluentMethodCallsCollector - ) { - } - public function getRuleDefinition(): RuleDefinition { return new RuleDefinition( @@ -85,95 +67,9 @@ public function getNodeTypes(): array */ public function refactor(Node $node): ?array { - if (! $node->expr instanceof MethodCall) { - return null; - } - - $methodCalls = $this->fluentMethodCallsCollector->resolve($node->expr); - - // at least 2 method calls - if (count($methodCalls) < 1) { - return null; - } - - $lastMethodCall = end($methodCalls); - $rootExpr = $lastMethodCall->var; - - if (! $rootExpr instanceof New_) { - return null; - } - - if ($this->shouldSkipForVendorOrInternal($node->expr)) { - return null; - } - - $variableName = $this->resolveVariableName($rootExpr); - $someVariable = new Variable($variableName); - $firstAssign = new Assign($someVariable, $rootExpr); - - return $this->createStmts($firstAssign, $methodCalls, $someVariable, $node); - } - - private function resolveVariableName(Expr $expr): string - { - if (! $expr instanceof New_) { - return 'someVariable'; - } - - if ($expr->class instanceof Name) { - return $this->propertyNaming->fqnToVariableName($expr->class); - } - - return 'someVariable'; - } - - private function shouldSkipForVendorOrInternal(MethodCall $firstMethodCall): bool - { - $callerType = $this->getType($firstMethodCall); - if ($callerType instanceof ObjectType) { - $classReflection = $callerType->getClassReflection(); - if (! $classReflection instanceof ClassReflection) { - return false; - } - - $fileName = $classReflection->getFileName(); - if ($fileName === null || str_contains($fileName, 'vendor')) { - return true; - } - } - - return false; - } - - /** - * @param MethodCall[] $methodCalls - * @return Stmt[] - */ - private function createStmts( - Assign $firstAssign, - array $methodCalls, - Variable $someVariable, - Expression|Return_ $firstStmt - ): array { - $stmts = [new Expression($firstAssign)]; - - // revert to normal order - $methodCalls = array_reverse($methodCalls); - - foreach ($methodCalls as $methodCall) { - $methodCall->var = $someVariable; - // inlines indent and removes () around first expr - $methodCall->setAttribute(AttributeKey::ORIGINAL_NODE, null); - $stmts[] = new Expression($methodCall); - } - - if ($firstStmt instanceof Return_) { - $firstStmt->expr = $someVariable; - $stmts[] = $firstStmt; - } - - $firstStmt->expr = $someVariable; - - return $stmts; + throw new ShouldNotHappenException(sprintf( + '"%s" rule is deprecated, as breaking a fluent setter chain needs a more complex approach that depends on the use case', + self::class + )); } }