Skip to content
/ rector-src Public

Commit bca5850

Browse files
TomasVotrubaactions-user
TomasVotruba
and
actions-user
authored
Improve parent call fixture in Php4ConstructorRector not to use KEY constant (#7642)
* improve parent call fixture in Php4ConstructorRector * etract MethodCallNameAnalyzer * restore fixture * [ci-review] Rector Rectify --------- Co-authored-by: GitHub Action <[email protected]>
1 parent b31f801 commit bca5850

File tree

9 files changed

+150
-79
lines changed

9 files changed

+150
-79
lines changed

‎composer.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@
9797
"files": [
9898
"tests/debug_functions.php",
9999
"rules-tests/Transform/Rector/FuncCall/FuncCallToMethodCallRector/Source/some_view_function.php",
100-
"rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Source/FunctionTyped.php"
100+
"rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Source/FunctionTyped.php",
101+
"rules-tests/Php70/Rector/ClassMethod/Php4ConstructorRector/Source/ParentClass.php"
101102
]
102103
},
103104
"scripts": {

‎phpstan.neon‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,3 +379,6 @@ parameters:
379379
-
380380
message: '#Property Rector\\PhpParser\\NodeTraverser\\AbstractImmutableNodeTraverser\:\:\$visitors \(list<PhpParser\\NodeVisitor>\) does not accept array<int\|string, PhpParser\\NodeVisitor>#'
381381
path: src/PhpParser/NodeTraverser/AbstractImmutableNodeTraverser.php
382+
383+
# handle next
384+
- '#Fetching deprecated class constant STMT_KEY of class Rector\\NodeTypeResolver\\Node\\AttributeKey#'

‎rules-tests/Php70/Rector/ClassMethod/Php4ConstructorRector/Fixture/method_call_parent.php.inc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
class SomeParentA
44
{
5-
public function SomeParentA()
5+
public function SomeParentA()
66
{
77
}
88
}
@@ -20,7 +20,7 @@ final class SomeChildB extends SomeParentA
2020

2121
class SomeParentA
2222
{
23-
public function __construct()
23+
public function __construct()
2424
{
2525
}
2626
}

‎rules-tests/Php70/Rector/ClassMethod/Php4ConstructorRector/Fixture/parent_constructor_call.php.inc‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<?php
2+
3+
final class SomeChildB extends ParentClass
4+
{
5+
public function SomeChildB()
6+
{
7+
$this->ParentClass();
8+
}
9+
}
10+
11+
?>
12+
-----
13+
<?php
14+
15+
final class SomeChildB extends ParentClass
16+
{
17+
public function __construct()
18+
{
19+
\ParentClass::__construct();
20+
}
21+
}
22+
23+
?>

‎rules-tests/Php70/Rector/ClassMethod/Php4ConstructorRector/Source/ParentClass.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?php
2+
3+
class ParentClass
4+
{
5+
public function ParentClass()
6+
{
7+
}
8+
}

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,13 +85,14 @@ public function getNodeTypes(): array
8585
public function refactor(Node $node): ?Node
8686
{
8787
$classMethods = $node->getMethods();
88-
8988
if ($classMethods === []) {
9089
return null;
9190
}
9291

93-
$filter = static fn (ClassMethod $classMethod): bool => $classMethod->isPrivate();
94-
$privateMethods = array_filter($classMethods, $filter);
92+
$privateMethods = array_filter(
93+
$classMethods,
94+
fn (ClassMethod $classMethod): bool => $classMethod->isPrivate()
95+
);
9596

9697
if ($privateMethods === []) {
9798
return null;

‎rules/Php70/NodeAnalyzer/MethodCallNameAnalyzer.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Php70\NodeAnalyzer;
6+
7+
use PhpParser\Node\Expr;
8+
use PhpParser\Node\Expr\MethodCall;
9+
use PhpParser\Node\Expr\Variable;
10+
use PhpParser\Node\Identifier;
11+
use PhpParser\Node\Name;
12+
use PhpParser\Node\Stmt\Class_;
13+
use PhpParser\Node\Stmt\ClassMethod;
14+
15+
final class MethodCallNameAnalyzer
16+
{
17+
public function isLocalMethodCallNamed(Expr $expr, string $desiredMethodName): bool
18+
{
19+
if (! $expr instanceof MethodCall) {
20+
return false;
21+
}
22+
23+
if (! $expr->var instanceof Variable) {
24+
return false;
25+
}
26+
27+
if ($expr->var->name !== 'this') {
28+
return false;
29+
}
30+
31+
if (! $expr->name instanceof Identifier) {
32+
return false;
33+
}
34+
35+
return $expr->name->toString() === $desiredMethodName;
36+
}
37+
38+
public function isParentMethodCall(Class_ $class, Expr $expr): bool
39+
{
40+
if (! $class->extends instanceof Name) {
41+
return false;
42+
}
43+
44+
$parentClassName = $class->extends->toString();
45+
if ($class->getMethod($parentClassName) instanceof ClassMethod) {
46+
return false;
47+
}
48+
49+
return $this->isLocalMethodCallNamed($expr, $parentClassName);
50+
}
51+
}

‎rules/Php70/NodeAnalyzer/Php4ConstructorClassMethodAnalyzer.php‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,12 @@
55
namespace Rector\Php70\NodeAnalyzer;
66

77
use PhpParser\Node\Stmt\ClassMethod;
8-
use PHPStan\Analyser\Scope;
98
use PHPStan\Reflection\ClassReflection;
109

1110
final class Php4ConstructorClassMethodAnalyzer
1211
{
13-
public function detect(ClassMethod $classMethod, Scope $scope): bool
12+
public function detect(ClassMethod $classMethod, ClassReflection $classReflection): bool
1413
{
15-
// catch only classes without namespace
16-
if ($scope->getNamespace() !== null) {
17-
return false;
18-
}
19-
2014
if ($classMethod->isAbstract()) {
2115
return false;
2216
}
@@ -25,11 +19,11 @@ public function detect(ClassMethod $classMethod, Scope $scope): bool
2519
return false;
2620
}
2721

28-
$classReflection = $scope->getClassReflection();
29-
if (! $classReflection instanceof ClassReflection) {
22+
if ($classReflection->isAnonymous()) {
3023
return false;
3124
}
3225

33-
return ! $classReflection->isAnonymous();
26+
$possiblePhp4MethodNames = [$classReflection->getName(), lcfirst($classReflection->getName())];
27+
return in_array($classMethod->name->toString(), $possiblePhp4MethodNames, true);
3428
}
3529
}

‎rules/Php70/Rector/ClassMethod/Php4ConstructorRector.php‎

Lines changed: 53 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
namespace Rector\Php70\Rector\ClassMethod;
66

77
use PhpParser\Node;
8-
use PhpParser\Node\Expr;
98
use PhpParser\Node\Expr\MethodCall;
109
use PhpParser\Node\Expr\StaticCall;
1110
use PhpParser\Node\Identifier;
@@ -18,7 +17,7 @@
1817
use PHPStan\Reflection\ClassReflection;
1918
use Rector\Enum\ObjectReference;
2019
use Rector\NodeCollector\ScopeResolver\ParentClassScopeResolver;
21-
use Rector\NodeTypeResolver\Node\AttributeKey;
20+
use Rector\Php70\NodeAnalyzer\MethodCallNameAnalyzer;
2221
use Rector\Php70\NodeAnalyzer\Php4ConstructorClassMethodAnalyzer;
2322
use Rector\PHPStan\ScopeFetcher;
2423
use Rector\Rector\AbstractRector;
@@ -35,7 +34,8 @@ final class Php4ConstructorRector extends AbstractRector implements MinPhpVersio
3534
{
3635
public function __construct(
3736
private readonly Php4ConstructorClassMethodAnalyzer $php4ConstructorClassMethodAnalyzer,
38-
private readonly ParentClassScopeResolver $parentClassScopeResolver
37+
private readonly ParentClassScopeResolver $parentClassScopeResolver,
38+
private readonly MethodCallNameAnalyzer $methodCallNameAnalyzer,
3939
) {
4040
}
4141

@@ -85,18 +85,10 @@ public function getNodeTypes(): array
8585
*/
8686
public function refactor(Node $node): Class_|null
8787
{
88-
$className = $this->getName($node);
89-
if (! is_string($className)) {
90-
return null;
91-
}
92-
93-
$psr4ConstructorMethod = $node->getMethod(lcfirst($className)) ?? $node->getMethod($className);
94-
if (! $psr4ConstructorMethod instanceof ClassMethod) {
95-
return null;
96-
}
97-
9888
$scope = ScopeFetcher::fetch($node);
99-
if (! $this->php4ConstructorClassMethodAnalyzer->detect($psr4ConstructorMethod, $scope)) {
89+
90+
// catch only classes without namespace
91+
if ($scope->getNamespace() !== null) {
10092
return null;
10193
}
10294

@@ -105,55 +97,65 @@ public function refactor(Node $node): Class_|null
10597
return null;
10698
}
10799

108-
// process parent call references first
109-
$this->processClassMethodStatementsForParentConstructorCalls($psr4ConstructorMethod, $scope);
110-
111-
// does it already have a __construct method?
112-
if (! $node->getMethod(MethodName::CONSTRUCT) instanceof ClassMethod) {
113-
$psr4ConstructorMethod->name = new Identifier(MethodName::CONSTRUCT);
114-
}
115-
116-
$classMethodStmts = $psr4ConstructorMethod->stmts;
117-
if ($classMethodStmts === null) {
100+
$className = $this->getName($node);
101+
if (! is_string($className)) {
118102
return null;
119103
}
120104

121-
$parentClassName = $classReflection->getParentClass() instanceof ClassReflection
122-
? $classReflection->getParentClass()
123-
->getName()
124-
: '';
105+
foreach ($node->stmts as $classStmtKey => $classStmt) {
106+
if (! $classStmt instanceof ClassMethod) {
107+
continue;
108+
}
125109

126-
foreach ($classMethodStmts as $classMethodStmt) {
127-
if (! $classMethodStmt instanceof Expression) {
128-
return null;
110+
if (! $this->php4ConstructorClassMethodAnalyzer->detect($classStmt, $classReflection)) {
111+
continue;
129112
}
130113

131-
if ($this->isLocalMethodCallNamed($classMethodStmt->expr, MethodName::CONSTRUCT)) {
132-
$stmtKey = $psr4ConstructorMethod->getAttribute(AttributeKey::STMT_KEY);
133-
unset($node->stmts[$stmtKey]);
114+
$psr4ConstructorMethod = $classStmt;
115+
116+
// process parent call references first
117+
$this->processClassMethodStatementsForParentConstructorCalls($psr4ConstructorMethod, $scope);
118+
119+
// does it already have a __construct method?
120+
if (! $node->getMethod(MethodName::CONSTRUCT) instanceof ClassMethod) {
121+
$psr4ConstructorMethod->name = new Identifier(MethodName::CONSTRUCT);
134122
}
135123

136-
if ($this->isLocalMethodCallNamed($classMethodStmt->expr, $parentClassName) && ! $node->getMethod(
137-
$parentClassName
138-
) instanceof ClassMethod) {
139-
/** @var MethodCall $expr */
140-
$expr = $classMethodStmt->expr;
141-
$classMethodStmt->expr = new StaticCall(new FullyQualified($parentClassName), new Identifier(
124+
foreach ((array) $psr4ConstructorMethod->stmts as $classMethodStmt) {
125+
if (! $classMethodStmt instanceof Expression) {
126+
continue;
127+
}
128+
129+
// remove delegating method
130+
if ($this->methodCallNameAnalyzer->isLocalMethodCallNamed(
131+
$classMethodStmt->expr,
142132
MethodName::CONSTRUCT
143-
), $expr->args);
133+
)) {
134+
unset($node->stmts[$classStmtKey]);
135+
}
136+
137+
if ($this->methodCallNameAnalyzer->isParentMethodCall($node, $classMethodStmt->expr)) {
138+
/** @var MethodCall $expr */
139+
$expr = $classMethodStmt->expr;
140+
141+
/** @var string $parentClassName */
142+
$parentClassName = $this->getParentClassName($node);
143+
144+
$classMethodStmt->expr = new StaticCall(new FullyQualified($parentClassName), new Identifier(
145+
MethodName::CONSTRUCT
146+
), $expr->args);
147+
}
144148
}
149+
150+
return $node;
145151
}
146152

147-
return $node;
153+
return null;
148154
}
149155

150156
private function processClassMethodStatementsForParentConstructorCalls(ClassMethod $classMethod, Scope $scope): void
151157
{
152-
if (! is_iterable($classMethod->stmts)) {
153-
return;
154-
}
155-
156-
foreach ($classMethod->stmts as $methodStmt) {
158+
foreach ((array) $classMethod->stmts as $methodStmt) {
157159
if (! $methodStmt instanceof Expression) {
158160
continue;
159161
}
@@ -197,24 +199,12 @@ private function processParentPhp4ConstructCall(StaticCall $staticCall, Scope $s
197199
$staticCall->name = new Identifier(MethodName::CONSTRUCT);
198200
}
199201

200-
private function isLocalMethodCallNamed(Expr $expr, string $name): bool
202+
private function getParentClassName(Class_ $class): ?string
201203
{
202-
if (! $expr instanceof MethodCall) {
203-
return false;
204-
}
205-
206-
if ($expr->var instanceof StaticCall) {
207-
return false;
208-
}
209-
210-
if ($expr->var instanceof MethodCall) {
211-
return false;
212-
}
213-
214-
if (! $this->isName($expr->var, 'this')) {
215-
return false;
204+
if (! $class->extends instanceof Node) {
205+
return null;
216206
}
217207

218-
return $this->isName($expr->name, $name);
208+
return $class->extends->toString();
219209
}
220210
}

0 commit comments

Comments
 (0)