Skip to content

Commit d6fb9bf

Browse files
TomasVotrubaTomasVotruba
authored
[dead-code] Add RemoveParentDelegatingConstructorRector (#7769)
* [dead-code] Add RemoveParentDelegatingConstructorRector * enable rule * skip type override * skip fcc
1 parent 083c728 commit d6fb9bf

File tree

11 files changed

+367
-1
lines changed

11 files changed

+367
-1
lines changed

‎phpstan.neon‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,11 @@ parameters:
326326
- '#expects array<PhpParser\\Node\\Stmt>, array<PhpParser\\Node> given#'
327327
- '#should return non\-empty\-string but returns string#'
328328

329+
# known non-empty class method
330+
-
331+
message: '#Offset 0 might not exist on array<PhpParser\\Node\\Stmt>\|null#'
332+
path: rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector.php
333+
329334
# false positive, can accept non-class string
330335
- '#Parameter \#1 \$name of method PHPStan\\BetterReflection\\Reflection\\Adapter\\ReflectionClass\:\:getAttributes\(\) expects class\-string\|null, string given#'
331336

‎rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_different_args.php.inc‎

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\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\Fixture;
4+
5+
use PhpParser\Node\Scalar\String_;
6+
7+
final class SomeDifferentArgs extends String_
8+
{
9+
public function __construct($value, $attributes)
10+
{
11+
parent::__construct($value, []);
12+
}
13+
}

‎rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_different_count.php.inc‎

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\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\Fixture;
4+
5+
use PhpParser\Node\Scalar\String_;
6+
7+
final class SkipDifferentCount extends String_
8+
{
9+
public function __construct($value)
10+
{
11+
parent::__construct($value, []);
12+
}
13+
}

‎rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_different_type.php.inc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\Fixture;
6+
7+
use PhpParser\NodeTraverser;
8+
use Rector\Comments\NodeVisitor\CommentRemovingNodeVisitor;
9+
10+
final class SkipDifferentType extends NodeTraverser
11+
{
12+
public function __construct(CommentRemovingNodeVisitor $commentRemovingNodeVisitor)
13+
{
14+
parent::__construct($commentRemovingNodeVisitor);
15+
}
16+
}

‎rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/skip_first_class_callable.php.inc‎

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\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\Fixture;
4+
5+
use PhpParser\Node\Scalar\String_;
6+
7+
final class SkipFirstClassCallable extends String_
8+
{
9+
public function __construct($value, $attributes)
10+
{
11+
parent::__construct(...);
12+
}
13+
}

‎rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/Fixture/some_class.php.inc‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\Fixture;
4+
5+
use PhpParser\Node\Scalar\String_;
6+
7+
final class SomeClass extends String_
8+
{
9+
public function __construct($value, $attributes)
10+
{
11+
parent::__construct($value, $attributes);
12+
}
13+
}
14+
15+
?>
16+
-----
17+
<?php
18+
19+
namespace Rector\Tests\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\Fixture;
20+
21+
use PhpParser\Node\Scalar\String_;
22+
23+
final class SomeClass extends String_
24+
{
25+
}
26+
27+
?>

‎rules-tests/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/RemoveParentDelegatingConstructorRectorTest.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\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class RemoveParentDelegatingConstructorRectorTest 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/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector/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\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector;
7+
8+
return RectorConfig::configure()
9+
->withRules([RemoveParentDelegatingConstructorRector::class]);

‎rules/DeadCode/Rector/ClassMethod/RemoveParentDelegatingConstructorRector.php‎

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\DeadCode\Rector\ClassMethod;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Arg;
9+
use PhpParser\Node\Expr\StaticCall;
10+
use PhpParser\Node\Expr\Variable;
11+
use PhpParser\Node\Stmt;
12+
use PhpParser\Node\Stmt\ClassMethod;
13+
use PhpParser\Node\Stmt\Expression;
14+
use PhpParser\NodeVisitor;
15+
use PHPStan\Reflection\ClassReflection;
16+
use PHPStan\Reflection\ExtendedMethodReflection;
17+
use Rector\Enum\ObjectReference;
18+
use Rector\PHPStan\ScopeFetcher;
19+
use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
20+
use Rector\Rector\AbstractRector;
21+
use Rector\StaticTypeMapper\StaticTypeMapper;
22+
use Rector\ValueObject\MethodName;
23+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
24+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
25+
26+
/**
27+
* @see \Rector\Tests\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector\RemoveParentDelegatingConstructorRectorTest
28+
*/
29+
final class RemoveParentDelegatingConstructorRector extends AbstractRector
30+
{
31+
public function __construct(
32+
private readonly StaticTypeMapper $staticTypeMapper,
33+
) {
34+
}
35+
36+
public function getRuleDefinition(): RuleDefinition
37+
{
38+
return new RuleDefinition(
39+
'Remove constructor that only delegates call to parent class with same values',
40+
[
41+
new CodeSample(
42+
<<<'CODE_SAMPLE'
43+
class Node
44+
{
45+
public function __construct(array $attributes)
46+
{
47+
}
48+
}
49+
50+
class SomeParent extends Node
51+
{
52+
public function __construct(array $attributes)
53+
{
54+
parent::__construct($attributes);
55+
}
56+
}
57+
CODE_SAMPLE
58+
,
59+
<<<'CODE_SAMPLE'
60+
class Node
61+
{
62+
public function __construct(array $attributes)
63+
{
64+
}
65+
}
66+
67+
class SomeParent extends Node
68+
{
69+
}
70+
CODE_SAMPLE
71+
),
72+
]
73+
);
74+
}
75+
76+
/**
77+
* @return array<class-string<Node>>
78+
*/
79+
public function getNodeTypes(): array
80+
{
81+
return [ClassMethod::class];
82+
}
83+
84+
/**
85+
* @param ClassMethod $node
86+
*/
87+
public function refactor(Node $node): ?int
88+
{
89+
if (! $this->isName($node, MethodName::CONSTRUCT)) {
90+
return null;
91+
}
92+
93+
if ($node->stmts === null || count($node->stmts) !== 1) {
94+
return null;
95+
}
96+
97+
$parentMethodReflection = $this->matchParentConstructorReflection($node);
98+
if (! $parentMethodReflection instanceof ExtendedMethodReflection) {
99+
return null;
100+
}
101+
102+
$soleStmt = $node->stmts[0];
103+
$parentCallArgs = $this->matchParentConstructorCallArgs($soleStmt);
104+
if ($parentCallArgs === null) {
105+
return null;
106+
}
107+
108+
// match count and order
109+
if (! $this->isParameterAndArgCountAndOrderIdentical($node)) {
110+
return null;
111+
}
112+
113+
// match parameter types and parent constructor types
114+
if (! $this->areConstructorAndParentParameterTypesMatching($node, $parentMethodReflection)) {
115+
return null;
116+
}
117+
118+
return NodeVisitor::REMOVE_NODE;
119+
}
120+
121+
private function matchParentConstructorReflection(ClassMethod $classMethod): ?ExtendedMethodReflection
122+
{
123+
$scope = ScopeFetcher::fetch($classMethod);
124+
125+
$classReflection = $scope->getClassReflection();
126+
if (! $classReflection instanceof ClassReflection) {
127+
return null;
128+
}
129+
130+
$parentClassReflection = $classReflection->getParentClass();
131+
if (! $parentClassReflection instanceof ClassReflection) {
132+
return null;
133+
}
134+
135+
if (! $parentClassReflection->hasConstructor()) {
136+
return null;
137+
}
138+
139+
return $parentClassReflection->getConstructor();
140+
}
141+
142+
/**
143+
* Looking for parent::__construct()
144+
*
145+
* @return Arg[]|null
146+
*/
147+
private function matchParentConstructorCallArgs(Stmt $stmt): ?array
148+
{
149+
if (! $stmt instanceof Expression) {
150+
return null;
151+
}
152+
153+
if (! $stmt->expr instanceof StaticCall) {
154+
return null;
155+
}
156+
157+
$staticCall = $stmt->expr;
158+
if ($staticCall->isFirstClassCallable()) {
159+
return null;
160+
}
161+
162+
if (! $this->isName($staticCall->class, ObjectReference::PARENT)) {
163+
return null;
164+
}
165+
166+
if (! $this->isName($staticCall->name, MethodName::CONSTRUCT)) {
167+
return null;
168+
}
169+
170+
return $staticCall->getArgs();
171+
}
172+
173+
private function isParameterAndArgCountAndOrderIdentical(ClassMethod $classMethod): bool
174+
{
175+
$soleStmt = $classMethod->stmts[0];
176+
177+
$parentCallArgs = $this->matchParentConstructorCallArgs($soleStmt);
178+
if ($parentCallArgs === null) {
179+
return false;
180+
}
181+
182+
$constructorParams = $classMethod->getParams();
183+
if (count($constructorParams) !== count($parentCallArgs)) {
184+
return false;
185+
}
186+
187+
// match passed names in the same order
188+
$paramNames = [];
189+
foreach ($constructorParams as $constructorParam) {
190+
$paramNames[] = $this->getName($constructorParam->var);
191+
}
192+
193+
$argNames = [];
194+
foreach ($parentCallArgs as $parentCallArg) {
195+
$argValue = $parentCallArg->value;
196+
if (! $argValue instanceof Variable) {
197+
return false;
198+
}
199+
200+
$argNames[] = $this->getName($argValue);
201+
}
202+
203+
return $paramNames === $argNames;
204+
}
205+
206+
private function areConstructorAndParentParameterTypesMatching(
207+
ClassMethod $classMethod,
208+
ExtendedMethodReflection $extendedMethodReflection
209+
): bool {
210+
foreach ($classMethod->getParams() as $position => $param) {
211+
$parameterType = $param->type;
212+
213+
// no type override
214+
if ($parameterType === null) {
215+
continue;
216+
}
217+
218+
$parametersSelector = $extendedMethodReflection->getOnlyVariant();
219+
220+
foreach ($parametersSelector->getParameters() as $index => $parameterReflection) {
221+
if ($index !== $position) {
222+
continue;
223+
}
224+
225+
$parentParameterType = $this->staticTypeMapper->mapPHPStanTypeToPhpParserNode(
226+
$parameterReflection->getType(),
227+
TypeKind::PARAM
228+
);
229+
230+
if (! $this->nodeComparator->areNodesEqual($parameterType, $parentParameterType)) {
231+
return false;
232+
}
233+
}
234+
}
235+
236+
return true;
237+
}
238+
}

‎src/Config/Level/DeadCodeLevel.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use Rector\DeadCode\Rector\ClassMethod\RemoveArgumentFromDefaultParentCallRector;
1818
use Rector\DeadCode\Rector\ClassMethod\RemoveEmptyClassMethodRector;
1919
use Rector\DeadCode\Rector\ClassMethod\RemoveNullTagValueNodeRector;
20+
use Rector\DeadCode\Rector\ClassMethod\RemoveParentDelegatingConstructorRector;
2021
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedConstructorParamRector;
2122
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodParameterRector;
2223
use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodRector;
@@ -132,7 +133,10 @@ final class DeadCodeLevel
132133
RemoveConditionExactReturnRector::class,
133134
RemoveDeadStmtRector::class,
134135
UnwrapFutureCompatibleIfPhpVersionRector::class,
136+
135137
RemoveParentCallWithoutParentRector::class,
138+
RemoveParentDelegatingConstructorRector::class,
139+
136140
RemoveDeadConditionAboveReturnRector::class,
137141
RemoveDeadLoopRector::class,
138142

0 commit comments

Comments
 (0)