Skip to content

Commit 5e383da

Browse files
authored
Improve ClassPropertyAssignToConstructorPromotionRector not to use STMT_KEY attribute value (#7645)
* improve ClassPropertyAssignToConstructorPromotionRector not to use stmts key * move stmt key to property candidate value object
1 parent c7053b2 commit 5e383da

File tree

3 files changed

+41
-22
lines changed

3 files changed

+41
-22
lines changed

rules/Php80/NodeAnalyzer/PromotedPropertyCandidateResolver.php

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,25 @@ public function resolveFromClass(
4747
}
4848

4949
$propertyPromotionCandidates = [];
50-
foreach ($class->getProperties() as $property) {
51-
$propertyCount = count($property->props);
52-
if ($propertyCount !== 1) {
50+
foreach ($class->stmts as $classStmtPosition => $classStmt) {
51+
if (! $classStmt instanceof Property) {
5352
continue;
5453
}
5554

56-
$propertyPromotionCandidate = $this->matchPropertyPromotionCandidate($property, $constructClassMethod);
55+
if (count($classStmt->props) !== 1) {
56+
continue;
57+
}
58+
59+
$propertyPromotionCandidate = $this->matchPropertyPromotionCandidate(
60+
$classStmt,
61+
$constructClassMethod,
62+
$classStmtPosition
63+
);
5764
if (! $propertyPromotionCandidate instanceof PropertyPromotionCandidate) {
5865
continue;
5966
}
6067

61-
if (! $allowModelBasedClasses && $this->hasModelTypeCheck($property, ClassName::JMS_TYPE)) {
68+
if (! $allowModelBasedClasses && $this->hasModelTypeCheck($classStmt, ClassName::JMS_TYPE)) {
6269
continue;
6370
}
6471

@@ -80,7 +87,8 @@ private function hasModelTypeCheck(Class_|Property $node, string $modelType): bo
8087

8188
private function matchPropertyPromotionCandidate(
8289
Property $property,
83-
ClassMethod $constructClassMethod
90+
ClassMethod $constructClassMethod,
91+
int $propertyStmtPosition
8492
): ?PropertyPromotionCandidate {
8593
if ($property->flags === 0) {
8694
return null;
@@ -92,7 +100,7 @@ private function matchPropertyPromotionCandidate(
92100
$firstParamAsVariable = $this->resolveFirstParamUses($constructClassMethod);
93101

94102
// match property name to assign in constructor
95-
foreach ((array) $constructClassMethod->stmts as $stmt) {
103+
foreach ((array) $constructClassMethod->stmts as $assignStmtPosition => $stmt) {
96104
if (! $stmt instanceof Expression) {
97105
continue;
98106
}
@@ -127,7 +135,12 @@ private function matchPropertyPromotionCandidate(
127135
continue;
128136
}
129137

130-
return new PropertyPromotionCandidate($property, $matchedParam, $stmt);
138+
return new PropertyPromotionCandidate(
139+
$property,
140+
$matchedParam,
141+
$propertyStmtPosition,
142+
$assignStmtPosition
143+
);
131144
}
132145

133146
return null;

rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
use Rector\Naming\PropertyRenamer\PropertyPromotionRenamer;
2727
use Rector\Naming\VariableRenamer;
2828
use Rector\NodeAnalyzer\ParamAnalyzer;
29-
use Rector\NodeTypeResolver\Node\AttributeKey;
3029
use Rector\NodeTypeResolver\TypeComparator\TypeComparator;
3130
use Rector\Php80\DocBlock\PropertyPromotionDocBlockMerger;
3231
use Rector\Php80\Guard\MakePropertyPromotionGuard;
@@ -209,22 +208,17 @@ public function refactor(Node $node): ?Node
209208
continue;
210209
}
211210

212-
if ($property->type instanceof Node
213-
&& $param->type instanceof Node
214-
&& $property->hooks !== []
215-
&& ! $this->nodeComparator->areNodesEqual($property->type, $param->type)) {
211+
if ($this->shouldSkipPropertyOrParam($property, $param)) {
216212
continue;
217213
}
218214

219215
$hasChanged = true;
220216

221217
// remove property from class
222-
$propertyStmtKey = $property->getAttribute(AttributeKey::STMT_KEY);
223-
unset($node->stmts[$propertyStmtKey]);
218+
unset($node->stmts[$promotionCandidate->getPropertyStmtPosition()]);
224219

225220
// remove assign in constructor
226-
$assignStmtPosition = $promotionCandidate->getStmtPosition();
227-
unset($constructClassMethod->stmts[$assignStmtPosition]);
221+
unset($constructClassMethod->stmts[$promotionCandidate->getConstructorAssignStmtPosition()]);
228222

229223
$oldParamName = $this->getName($param);
230224
$this->variableRenamer->renameVariableInFunctionLike($constructClassMethod, $oldParamName, $propertyName);
@@ -367,4 +361,12 @@ private function isCallableTypeIdentifier(?Node $node): bool
367361
{
368362
return $node instanceof Identifier && $this->isName($node, 'callable');
369363
}
364+
365+
private function shouldSkipPropertyOrParam(Property $property, Param $param): bool
366+
{
367+
return $property->type instanceof Node
368+
&& $param->type instanceof Node
369+
&& $property->hooks !== []
370+
&& ! $this->nodeComparator->areNodesEqual($property->type, $param->type);
371+
}
370372
}

rules/Php80/ValueObject/PropertyPromotionCandidate.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,16 @@
66

77
use PhpParser\Node\Expr\Variable;
88
use PhpParser\Node\Param;
9-
use PhpParser\Node\Stmt\Expression;
109
use PhpParser\Node\Stmt\Property;
1110
use Rector\Exception\ShouldNotHappenException;
12-
use Rector\NodeTypeResolver\Node\AttributeKey;
1311

1412
final readonly class PropertyPromotionCandidate
1513
{
1614
public function __construct(
1715
private Property $property,
1816
private Param $param,
19-
private Expression $expression,
17+
private int $propertyStmtPosition,
18+
private int $constructorAssignStmtPosition,
2019
) {
2120
}
2221

@@ -45,8 +44,13 @@ public function getParamName(): string
4544
return $paramVar->name;
4645
}
4746

48-
public function getStmtPosition(): int
47+
public function getPropertyStmtPosition(): int
4948
{
50-
return $this->expression->getAttribute(AttributeKey::STMT_KEY);
49+
return $this->propertyStmtPosition;
50+
}
51+
52+
public function getConstructorAssignStmtPosition(): int
53+
{
54+
return $this->constructorAssignStmtPosition;
5155
}
5256
}

0 commit comments

Comments
 (0)