Skip to content

Commit 328062c

Browse files
author
ILJA MANYUTIN
committed
[Php80] Skip promotion only when property is assigned null on ClassPropertyAssignToConstructorPromotionRector(#9809)
1 parent b6e9f5d commit 328062c

3 files changed

Lines changed: 54 additions & 23 deletions

File tree

rules-tests/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector/Fixture/skip_wider_property_type.php.inc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ namespace Rector\Tests\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromo
44

55
final class SkipWiderPropertyType
66
{
7-
public private(set) ?object $entity = null;
7+
private(set) ?object $entity;
88

99
public function __construct(object $entity)
1010
{
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
namespace Rector\Tests\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector\Fixture;
4+
5+
final class WiderPropertyTypeWithoutNullAssign
6+
{
7+
private(set) ?object $entity;
8+
9+
public function __construct(object $entity)
10+
{
11+
$this->entity = $entity;
12+
}
13+
14+
}
15+
16+
?>
17+
-----
18+
<?php
19+
20+
namespace Rector\Tests\Php80\Rector\Class_\ClassPropertyAssignToConstructorPromotionRector\Fixture;
21+
22+
final class WiderPropertyTypeWithoutNullAssign
23+
{
24+
public function __construct(private(set) ?object $entity)
25+
{
26+
}
27+
28+
}
29+
30+
?>

rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php

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

77
use PhpParser\Node;
88
use PhpParser\Node\Expr;
9+
use PhpParser\Node\Expr\Assign;
910
use PhpParser\Node\Expr\PropertyFetch;
1011
use PhpParser\Node\Expr\Variable;
1112
use PhpParser\Node\FunctionLike;
@@ -35,6 +36,7 @@
3536
use Rector\Php80\DocBlock\PropertyPromotionDocBlockMerger;
3637
use Rector\Php80\Guard\MakePropertyPromotionGuard;
3738
use Rector\Php80\NodeAnalyzer\PromotedPropertyCandidateResolver;
39+
use Rector\PhpParser\Node\BetterNodeFinder;
3840
use Rector\PhpParser\Node\Value\ValueResolver;
3941
use Rector\PHPStanStaticTypeMapper\Enum\TypeKind;
4042
use Rector\Rector\AbstractRector;
@@ -98,6 +100,7 @@ public function __construct(
98100
private readonly PhpDocInfoFactory $phpDocInfoFactory,
99101
private readonly StaticTypeMapper $staticTypeMapper,
100102
private readonly ValueResolver $valueResolver,
103+
private readonly BetterNodeFinder $betterNodeFinder,
101104
) {
102105
}
103106

@@ -220,7 +223,7 @@ public function refactor(Node $node): ?Node
220223
continue;
221224
}
222225

223-
if ($this->shouldSkipWiderPropertyType($property, $param)) {
226+
if ($this->shouldSkipPropertyAssignedNull($node, $propertyName)) {
224227
continue;
225228
}
226229

@@ -484,31 +487,29 @@ private function shouldUsePropertyTypeForPromotedParam(Property $property, Param
484487
return $this->typeComparator->areTypesEqual($type, $paramTypeWithoutNull);
485488
}
486489

487-
/**
488-
* Promotion shares one type. When processUnionType would copy the property type onto the
489-
* param and that type is strictly wider (e.g. ?object vs object), skip — otherwise the
490-
* constructor contract is loosened. Narrowing (object vs ?object) stays allowed.
491-
* Interface vs implementation does not fire here: shouldUsePropertyTypeForPromotedParam
492-
* is false when the non-null base types differ.
493-
*/
494-
private function shouldSkipWiderPropertyType(Property $property, Param $param): bool
490+
private function shouldSkipPropertyAssignedNull(Class_ $class, string $propertyName): bool
495491
{
496-
if (! $this->shouldUsePropertyTypeForPromotedParam($property, $param)) {
497-
return false;
498-
}
492+
return (bool) $this->betterNodeFinder->findFirst(
493+
$class,
494+
function (Node $node) use ($propertyName): bool {
495+
if (! $node instanceof Assign) {
496+
return false;
497+
}
499498

500-
if (! $property->type instanceof Node || ! $param->type instanceof Node) {
501-
return false;
502-
}
499+
if (! $node->var instanceof PropertyFetch) {
500+
return false;
501+
}
503502

504-
$propertyType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($property->type);
505-
$paramType = $this->staticTypeMapper->mapPhpParserNodePHPStanType($param->type);
503+
if (! $this->isName($node->var->var, 'this')) {
504+
return false;
505+
}
506506

507-
if ($param->default instanceof Expr) {
508-
$paramType = TypeCombinator::union($paramType, $this->getType($param->default));
509-
}
507+
if (! $this->isName($node->var->name, $propertyName)) {
508+
return false;
509+
}
510510

511-
return $this->typeComparator->isSubtype($paramType, $propertyType)
512-
&& ! $this->typeComparator->areTypesEqual($propertyType, $paramType);
511+
return $this->valueResolver->isNull($node->expr);
512+
}
513+
);
513514
}
514515
}

0 commit comments

Comments
 (0)