Skip to content

Commit 5ff7aec

Browse files
phpstan-botVincentLangletclaude
authored
Pass only the right-hand side of ??= as the written expression to PropertyAssignNode and VariableAssignNode (#6943)
Co-authored-by: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7a343dd commit 5ff7aec

12 files changed

Lines changed: 542 additions & 168 deletions

File tree

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ private function doPrepareTarget(
760760
// 1b. build the write chain (Set*OffsetValueTypeExpr nesting) after
761761
// the root walk, so a property base's holder and current types are
762762
// read from its stored result instead of pricing the unwalked fetch
763-
$assignedPropertyExpr = $assignedExpr;
763+
$assignedPropertyExpr = $this->getWrittenExpr($assignedExpr);
764764
$chainVar = $originalVar;
765765
while ($chainVar instanceof ArrayDimFetch) {
766766
$varForSetOffsetValue = $chainVar->var;
@@ -1097,7 +1097,7 @@ private function doPrepareTarget(
10971097
if ($var instanceof ExistingArrayDimFetch) {
10981098
$originalVar = $var;
10991099
$dimFetchStack = [];
1100-
$assignedPropertyExpr = $assignedExpr;
1100+
$assignedPropertyExpr = $this->getWrittenExpr($assignedExpr);
11011101
while ($var instanceof ExistingArrayDimFetch) {
11021102
$varForSetOffsetValue = $var->getVar();
11031103
if ($varForSetOffsetValue instanceof PropertyFetch || $varForSetOffsetValue instanceof StaticPropertyFetch) {
@@ -1374,7 +1374,7 @@ public function applyWrite(
13741374
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($nodeScopeResolver, $scope, $storage, $var->name, $conditionalExpressions, $identicalSpecifiedTypes, $falseyType, $impurePoints, $assignedExpr, $storedAssignedExprResult);
13751375
}
13761376

1377-
$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval, $storage);
1377+
$nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $this->getWrittenExpr($assignedExpr)), $scopeBeforeAssignEval, $storage);
13781378

13791379
$nativeType = $this->readAssignedValueType($nodeScopeResolver, $storedAssignedExprResult, $assignedExpr, $scope->doNotTreatPhpDocTypesAsCertain());
13801380
$scope = $scope->assignVariable(
@@ -1596,7 +1596,7 @@ public function applyWrite(
15961596
if ($templateArgumentFrame !== null) {
15971597
$scope = $scope->addTemplateArgumentConstraints($this->templateArgumentObserver->collectSend($propertyReflection->getWritableType(), $assignedExprType));
15981598
}
1599-
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);
1599+
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $this->getWrittenExpr($assignedExpr), $isAssignOp), $scopeBeforeAssignEval, $storage);
16001600
if ($propertyReflection->canChangeTypeAfterAssignment()) {
16011601
if ($propertyReflection->hasNativeType()) {
16021602
$propertyNativeType = $propertyReflection->getNativeType();
@@ -1657,7 +1657,7 @@ public function applyWrite(
16571657
} else {
16581658
// fallback
16591659
$assignedExprType = $this->readAssignedValueType($nodeScopeResolver, $assignedValueResult, $assignedExpr, $scope);
1660-
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);
1660+
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $this->getWrittenExpr($assignedExpr), $isAssignOp), $scopeBeforeAssignEval, $storage);
16611661
$scope = $scope->assignExpression($var, $assignedExprType, $this->readAssignedValueType($nodeScopeResolver, $assignedValueResult, $assignedExpr, $scope->doNotTreatPhpDocTypesAsCertain()));
16621662
// simulate dynamic property assign by __set to get throw points
16631663
if (!$propertyHolderType->hasMethod('__set')->no()) {
@@ -1693,7 +1693,7 @@ public function applyWrite(
16931693
$scope = $scope->addTemplateArgumentConstraints($this->templateArgumentObserver->collectSend($propertyReflection->getWritableType(), $assignedExprType));
16941694
}
16951695
}
1696-
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);
1696+
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $this->getWrittenExpr($assignedExpr), $isAssignOp), $scopeBeforeAssignEval, $storage);
16971697
if ($propertyReflection !== null && $propertyReflection->canChangeTypeAfterAssignment()) {
16981698
if ($propertyReflection->hasNativeType()) {
16991699
$propertyNativeType = $propertyReflection->getNativeType();
@@ -1724,7 +1724,7 @@ public function applyWrite(
17241724
} else {
17251725
// fallback
17261726
$assignedExprType = $this->readAssignedValueType($nodeScopeResolver, $assignedValueResult, $assignedExpr, $scope);
1727-
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $assignedExpr, $isAssignOp), $scopeBeforeAssignEval, $storage);
1727+
$nodeScopeResolver->callNodeCallback($nodeCallback, new PropertyAssignNode($var, $this->getWrittenExpr($assignedExpr), $isAssignOp), $scopeBeforeAssignEval, $storage);
17281728
$scope = $scope->assignExpression($var, $assignedExprType, $this->readAssignedValueType($nodeScopeResolver, $assignedValueResult, $assignedExpr, $scope->doNotTreatPhpDocTypesAsCertain()));
17291729
}
17301730
} elseif ($kind === PreparedAssignTarget::KIND_LIST) {
@@ -2051,6 +2051,19 @@ private function assignDynamicVariable(MutatingScope $scope, ExpressionResult $n
20512051
return $scope;
20522052
}
20532053

2054+
/**
2055+
* `$lvalue ??= value` writes only `value` - the type of the whole
2056+
* expression also covers the non-null old value it keeps in place.
2057+
*/
2058+
private function getWrittenExpr(Expr $assignedExpr): Expr
2059+
{
2060+
if ($assignedExpr instanceof Expr\AssignOp\Coalesce) {
2061+
return $assignedExpr->expr;
2062+
}
2063+
2064+
return $assignedExpr;
2065+
}
2066+
20542067
private function unwrapAssign(Expr $expr): Expr
20552068
{
20562069
if ($expr instanceof Assign) {

‎src/Rules/Arrays/OffsetAccessValueAssignmentRule.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ public function processNode(Node $node, Scope $scope): array
5858
return [];
5959
}
6060

61-
if ($node instanceof Assign || $node instanceof Expr\AssignRef) {
61+
if ($node instanceof Assign || $node instanceof Expr\AssignRef || $node instanceof AssignOp\Coalesce) {
62+
// `$lvalue ??= value` writes only `value`
6263
$assignedValueType = $scope->getType($node->expr);
6364
} else {
6465
$assignedValueType = $scope->getType($node);

‎tests/PHPStan/Rules/Arrays/OffsetAccessValueAssignmentRuleTest.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,18 @@ public function testBug10783(): void
9090
$this->analyse([__DIR__ . '/../Variables/data/bug-10783.php'], []);
9191
}
9292

93+
public function testCoalesceAssign(): void
94+
{
95+
$this->analyse([__DIR__ . '/data/offset-access-value-assignment-coalesce.php'], [
96+
[
97+
'OffsetAccessValueAssignmentCoalesce\\Foo does not accept string.',
98+
40,
99+
],
100+
[
101+
'OffsetAccessValueAssignmentCoalesce\\Foo does not accept string.',
102+
53,
103+
],
104+
]);
105+
}
106+
93107
}

‎tests/PHPStan/Rules/Arrays/data/offset-access-value-assignment-coalesce.php‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace OffsetAccessValueAssignmentCoalesce;
4+
5+
/**
6+
* @implements \ArrayAccess<string, int>
7+
*/
8+
class Foo implements \ArrayAccess
9+
{
10+
public function offsetExists($offset): bool
11+
{
12+
return true;
13+
}
14+
15+
public function offsetGet($offset): ?string
16+
{
17+
return 'x';
18+
}
19+
20+
/**
21+
* @param string|null $offset
22+
* @param int $value
23+
*/
24+
public function offsetSet($offset, $value): void
25+
{
26+
}
27+
28+
public function offsetUnset($offset): void
29+
{
30+
}
31+
}
32+
33+
function coalesceValid(Foo $foo): void
34+
{
35+
$foo['x'] ??= 5;
36+
}
37+
38+
function coalesceInvalid(Foo $foo): void
39+
{
40+
$foo['x'] ??= 'boom';
41+
}
42+
43+
function ifValid(Foo $foo): void
44+
{
45+
if (null === $foo['x']) {
46+
$foo['x'] = 5;
47+
}
48+
}
49+
50+
function ifInvalid(Foo $foo): void
51+
{
52+
if (null === $foo['x']) {
53+
$foo['x'] = 'boom';
54+
}
55+
}

‎tests/PHPStan/Rules/Properties/TypesAssignedToPropertiesRuleTest.php‎

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ class TypesAssignedToPropertiesRuleTest extends RuleTestCase
1515

1616
private bool $checkNullables = true;
1717

18+
private bool $checkUnionTypes = true;
19+
1820
private bool $checkExplicitMixed = false;
1921

2022
private bool $checkImplicitMixed = false;
@@ -26,7 +28,7 @@ protected function getRule(): Rule
2628
self::createReflectionProvider(),
2729
checkNullables: $this->checkNullables,
2830
checkThisOnly: false,
29-
checkUnionTypes: true,
31+
checkUnionTypes: $this->checkUnionTypes,
3032
checkExplicitMixed: $this->checkExplicitMixed,
3133
checkImplicitMixed: $this->checkImplicitMixed,
3234
checkBenevolentUnionTypes: false,
@@ -1143,4 +1145,97 @@ public function testIntersectionWithTemplateMixed(): void
11431145
]);
11441146
}
11451147

1148+
public function testBug15228(): void
1149+
{
1150+
$this->analyse([__DIR__ . '/data/bug-15228.php'], [
1151+
[
1152+
'Property Bug15228\\CoalesceAssignDemo::$a (int|null) does not accept string.',
1153+
20,
1154+
],
1155+
[
1156+
'Property Bug15228\\CoalesceAssignDemo::$a (int|null) does not accept string.',
1157+
26,
1158+
],
1159+
[
1160+
'Property Bug15228\\CoalesceAssignDemo::$b (list<int>|null) does not accept list<string>.',
1161+
32,
1162+
],
1163+
[
1164+
'Property Bug15228\\CoalesceAssignDemo::$b (list<int>|null) does not accept list<string>.',
1165+
38,
1166+
],
1167+
[
1168+
'Static property Bug15228\\StaticCoalesceAssignDemo::$a (int|null) does not accept string.',
1169+
60,
1170+
],
1171+
[
1172+
'Static property Bug15228\\StaticCoalesceAssignDemo::$b (list<int>|null) does not accept list<string>.',
1173+
65,
1174+
],
1175+
[
1176+
'Property Bug15228\\OffsetCoalesceAssignDemo::$a (array<string, int>) does not accept array<string, int|string>.',
1177+
80,
1178+
],
1179+
[
1180+
'Property Bug15228\\OtherAssignOpsDemo::$a (int|null) does not accept string.',
1181+
98,
1182+
],
1183+
[
1184+
'Property Bug15228\\OtherAssignOpsDemo::$a (int|null) does not accept string.',
1185+
103,
1186+
],
1187+
[
1188+
'Property Bug15228\\OtherAssignOpsDemo::$r (int<0, 10>) does not accept int<5, 15>.',
1189+
108,
1190+
],
1191+
[
1192+
'Property Bug15228\\OtherAssignOpsDemo::$r (int<0, 10>) does not accept int<5, 15>.',
1193+
113,
1194+
],
1195+
]);
1196+
}
1197+
1198+
public function testBug15228WithoutCheckUnionTypes(): void
1199+
{
1200+
$this->checkUnionTypes = false;
1201+
$this->analyse([__DIR__ . '/data/bug-15228.php'], [
1202+
[
1203+
'Property Bug15228\\CoalesceAssignDemo::$a (int|null) does not accept string.',
1204+
20,
1205+
],
1206+
[
1207+
'Property Bug15228\\CoalesceAssignDemo::$a (int|null) does not accept string.',
1208+
26,
1209+
],
1210+
[
1211+
'Property Bug15228\\CoalesceAssignDemo::$b (list<int>|null) does not accept list<string>.',
1212+
32,
1213+
],
1214+
[
1215+
'Property Bug15228\\CoalesceAssignDemo::$b (list<int>|null) does not accept list<string>.',
1216+
38,
1217+
],
1218+
[
1219+
'Static property Bug15228\\StaticCoalesceAssignDemo::$a (int|null) does not accept string.',
1220+
60,
1221+
],
1222+
[
1223+
'Static property Bug15228\\StaticCoalesceAssignDemo::$b (list<int>|null) does not accept list<string>.',
1224+
65,
1225+
],
1226+
[
1227+
'Property Bug15228\\OffsetCoalesceAssignDemo::$a (array<string, int>) does not accept array<string, int|string>.',
1228+
80,
1229+
],
1230+
[
1231+
'Property Bug15228\\OtherAssignOpsDemo::$a (int|null) does not accept string.',
1232+
98,
1233+
],
1234+
[
1235+
'Property Bug15228\\OtherAssignOpsDemo::$a (int|null) does not accept string.',
1236+
103,
1237+
],
1238+
]);
1239+
}
1240+
11461241
}

‎tests/PHPStan/Rules/Properties/data/bug-15228.php‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15228;
4+
5+
class CoalesceAssignDemo
6+
{
7+
public ?int $a = null;
8+
9+
/** @var list<int>|null */
10+
public $b;
11+
12+
/** @return list<string> */
13+
private function strings(): array
14+
{
15+
return ['x'];
16+
}
17+
18+
public function coalesceInvalid(): void
19+
{
20+
$this->a ??= 'boom';
21+
}
22+
23+
public function ifInvalid(): void
24+
{
25+
if (null === $this->a) {
26+
$this->a = 'boom';
27+
}
28+
}
29+
30+
public function coalesceValid(): void
31+
{
32+
$this->b = $this->strings();
33+
$this->b ??= [1, 2];
34+
}
35+
36+
public function ifValid(): void
37+
{
38+
$this->b = $this->strings();
39+
if (null === $this->b) {
40+
$this->b = [1, 2];
41+
}
42+
}
43+
}
44+
45+
class StaticCoalesceAssignDemo
46+
{
47+
public static ?int $a = null;
48+
49+
/** @var list<int>|null */
50+
public static $b;
51+
52+
/** @return list<string> */
53+
private static function strings(): array
54+
{
55+
return ['x'];
56+
}
57+
58+
public static function coalesceInvalid(): void
59+
{
60+
self::$a ??= 'boom';
61+
}
62+
63+
public static function coalesceValid(): void
64+
{
65+
self::$b = self::strings();
66+
self::$b ??= [1, 2];
67+
}
68+
}
69+
70+
class OffsetCoalesceAssignDemo
71+
{
72+
/** @var array<string, int> */
73+
public array $a = [];
74+
75+
/** @var array<string, list<int>|null> */
76+
public array $b = [];
77+
78+
public function coalesceInvalid(): void
79+
{
80+
$this->a['x'] ??= 'boom';
81+
}
82+
83+
public function coalesceValid(): void
84+
{
85+
$this->b['x'] ??= [1, 2];
86+
}
87+
}
88+
89+
class OtherAssignOpsDemo
90+
{
91+
public ?int $a = null;
92+
93+
/** @var int<0, 10> */
94+
public int $r = 0;
95+
96+
public function concat(): void
97+
{
98+
$this->a .= 'x';
99+
}
100+
101+
public function concatExpanded(): void
102+
{
103+
$this->a = $this->a . 'x';
104+
}
105+
106+
public function plus(): void
107+
{
108+
$this->r += 5;
109+
}
110+
111+
public function plusExpanded(): void
112+
{
113+
$this->r = $this->r + 5;
114+
}
115+
}

0 commit comments

Comments
 (0)