Skip to content

Commit 623d90f

Browse files
TomasVotrubaTomasVotruba
authored
[exp] [unambiguous] Add RemoveReturnThisFromSetterClassMethodRector (#7624)
1 parent 157504c commit 623d90f

File tree

8 files changed

+288
-1
lines changed

8 files changed

+288
-1
lines changed

‎rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/different_set.php.inc‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
namespace Rector\Tests\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector\Fixture;
4+
5+
final class DifferentSet
6+
{
7+
private string $name;
8+
9+
public function setName(string $name): self
10+
{
11+
$this->name = 100;
12+
13+
return $this;
14+
}
15+
}
16+
17+
?>
18+
-----
19+
<?php
20+
21+
namespace Rector\Tests\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector\Fixture;
22+
23+
final class DifferentSet
24+
{
25+
private string $name;
26+
27+
public function setName(string $name): void
28+
{
29+
$this->name = 100;
30+
}
31+
}
32+
33+
?>

‎rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/fixture.php.inc‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
namespace Rector\Tests\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector\Fixture;
4+
5+
final class SomeClass
6+
{
7+
private string $name;
8+
9+
public function setName(string $name): self
10+
{
11+
$this->name = $name;
12+
13+
return $this;
14+
}
15+
}
16+
17+
?>
18+
-----
19+
<?php
20+
21+
namespace Rector\Tests\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector\Fixture;
22+
23+
final class SomeClass
24+
{
25+
private string $name;
26+
27+
public function setName(string $name): void
28+
{
29+
$this->name = $name;
30+
}
31+
}
32+
33+
?>

‎rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/skip_different_return.php.inc‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
namespace Rector\Tests\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector\Fixture;
4+
5+
final class SkipDifferentReturn
6+
{
7+
private string $name;
8+
9+
public function setName(string $name)
10+
{
11+
$this->name = $name;
12+
return $name;
13+
}
14+
}

‎rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/Fixture/skip_no_param.php.inc‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
namespace Rector\Tests\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector\Fixture;
4+
5+
final class SkipNoParam
6+
{
7+
private string $name;
8+
9+
public function setName(): self
10+
{
11+
$this->name = 'name';
12+
13+
return $this;
14+
}
15+
}

‎rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/RemoveReturnThisFromSetterClassMethodRectorTest.php‎

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\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class RemoveReturnThisFromSetterClassMethodRectorTest 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+
}

‎rules-tests/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector/config/configured_rule.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Rector\Config\RectorConfig;
6+
use Rector\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector;
7+
8+
return RectorConfig::configure()
9+
->withRules([RemoveReturnThisFromSetterClassMethodRector::class]);

‎rules/TypeDeclaration/NodeAnalyzer/ClassMethodAndPropertyAnalyzer.php‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
use PhpParser\Node\Expr\Assign;
88
use PhpParser\Node\Expr\PropertyFetch;
9+
use PhpParser\Node\Expr\Variable;
10+
use PhpParser\Node\Stmt;
911
use PhpParser\Node\Stmt\ClassMethod;
1012
use PhpParser\Node\Stmt\Expression;
1113
use PhpParser\Node\Stmt\Return_;
@@ -47,7 +49,39 @@ public function hasOnlyPropertyAssign(ClassMethod $classMethod, string $property
4749
return false;
4850
}
4951

50-
$onlyClassMethodStmt = $stmts[0] ?? null;
52+
$onlyClassMethodStmt = $stmts[0];
53+
return $this->isLocalPropertyVariableAssign($onlyClassMethodStmt, $propertyName);
54+
}
55+
56+
public function hasPropertyAssignWithReturnThis(ClassMethod $classMethod, string $propertyName): bool
57+
{
58+
$stmts = (array) $classMethod->stmts;
59+
if (count($stmts) !== 2) {
60+
return false;
61+
}
62+
63+
$possibleAssignStmt = $stmts[0];
64+
$possibleReturnThis = $stmts[1];
65+
66+
if (! $this->isLocalPropertyVariableAssign($possibleAssignStmt, $propertyName)) {
67+
return false;
68+
}
69+
70+
if (! $possibleReturnThis instanceof Return_) {
71+
return false;
72+
73+
}
74+
75+
$returnExpr = $possibleReturnThis->expr;
76+
if (! $returnExpr instanceof Variable) {
77+
return false;
78+
}
79+
80+
return $this->nodeNameResolver->isName($returnExpr, 'this');
81+
}
82+
83+
private function isLocalPropertyVariableAssign(Stmt $onlyClassMethodStmt, string $propertyName): bool
84+
{
5185
if (! $onlyClassMethodStmt instanceof Expression) {
5286
return false;
5387
}

‎rules/Unambiguous/Rector/Class_/RemoveReturnThisFromSetterClassMethodRector.php‎

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Unambiguous\Rector\Class_;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Identifier;
9+
use PhpParser\Node\Stmt\Class_;
10+
use Rector\Rector\AbstractRector;
11+
use Rector\TypeDeclaration\NodeAnalyzer\ClassMethodAndPropertyAnalyzer;
12+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
13+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
14+
15+
/**
16+
* @experimental since 2025-11
17+
*
18+
* @see \Rector\Tests\Unambiguous\Rector\Class_\RemoveReturnThisFromSetterClassMethodRector\RemoveReturnThisFromSetterClassMethodRectorTest
19+
*/
20+
final class RemoveReturnThisFromSetterClassMethodRector extends AbstractRector
21+
{
22+
public function __construct(
23+
private readonly ClassMethodAndPropertyAnalyzer $classMethodAndPropertyAnalyzer,
24+
) {
25+
26+
}
27+
28+
public function getRuleDefinition(): RuleDefinition
29+
{
30+
return new RuleDefinition(
31+
'Remove return $this from setter method, to make explicit setter without return value. Goal is to make code unambiguous with one way to set value',
32+
[
33+
new CodeSample(
34+
<<<'CODE_SAMPLE'
35+
class SomeClass
36+
{
37+
private $name;
38+
39+
public function setName(string $name): self
40+
{
41+
$this->name = $name;
42+
return $this;
43+
}
44+
}
45+
CODE_SAMPLE
46+
,
47+
<<<'CODE_SAMPLE'
48+
class SomeClass
49+
{
50+
private $name;
51+
52+
public function setName(string $name): void
53+
{
54+
$this->name = $name;
55+
}
56+
}
57+
CODE_SAMPLE
58+
),
59+
]
60+
);
61+
}
62+
63+
/**
64+
* @return array<class-string<Class_>>
65+
*/
66+
public function getNodeTypes(): array
67+
{
68+
return [Class_::class];
69+
}
70+
71+
/**
72+
* @param Class_ $node
73+
*/
74+
public function refactor(Node $node): ?Class_
75+
{
76+
$hasChanged = false;
77+
78+
foreach ($node->getMethods() as $classMethod) {
79+
if ($classMethod->isMagic()) {
80+
continue;
81+
}
82+
83+
// skip void return type
84+
if ($classMethod->returnType instanceof Identifier && $this->isName($classMethod->returnType, 'void')) {
85+
continue;
86+
}
87+
88+
if (count($classMethod->params) !== 1) {
89+
continue;
90+
}
91+
92+
$soleParam = $classMethod->params[0];
93+
94+
// magic spread
95+
if ($soleParam->variadic) {
96+
continue;
97+
}
98+
99+
$paramName = $this->getName($soleParam->var);
100+
if (! is_string($paramName)) {
101+
continue;
102+
}
103+
104+
if (! $this->classMethodAndPropertyAnalyzer->hasPropertyAssignWithReturnThis($classMethod, $paramName)) {
105+
continue;
106+
}
107+
108+
// remove 2nd stmts, that is "return $this;"
109+
unset($classMethod->stmts[1]);
110+
$classMethod->returnType = new Identifier('void');
111+
112+
$hasChanged = true;
113+
}
114+
115+
if (! $hasChanged) {
116+
return null;
117+
}
118+
119+
return $node;
120+
}
121+
}

0 commit comments

Comments
 (0)