Skip to content

Commit b88ca9c

Browse files
phpstan-botphpstan-bot
authored
Do not treat conditional-expression entries about a property fetch as proof it is initialized (#6183)
1 parent c2dbd45 commit b88ca9c

5 files changed

Lines changed: 176 additions & 18 deletions

File tree

‎src/Analyser/IssetabilityDescriptor.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ public function resolve(MutatingScope $scope, bool $useNativeTypes, Expr $expr):
125125
$propertyReflection = $reflectionResolver($scope);
126126
if ($propertyReflection === null) {
127127
return new IssetabilityResolution(
128-
IssetabilityLinkInfo::property(null, $propertyFetch, false, false, TrinaryLogic::createNo(), new NeverType(), new NeverType(), false, false, false, false, false, false, false, false),
128+
IssetabilityLinkInfo::property(null, $propertyFetch, false, false, TrinaryLogic::createNo(), new NeverType(), new NeverType(), false, false, false, false, false, false, false),
129129
$inner,
130130
);
131131
}
@@ -148,7 +148,6 @@ public function resolve(MutatingScope $scope, bool $useNativeTypes, Expr $expr):
148148
$propertyReflection->getWritableType(),
149149
$hasNativeType ? $propertyReflection->getNativeType() : new NeverType(),
150150
$scope->hasExpressionType($propertyFetch)->yes(),
151-
isset($scope->getConditionalExpressions()[$scope->getNodeKey($propertyFetch)]),
152151
$initializedThisProperty,
153152
$nativeReflection !== null,
154153
$nativeReflection !== null && $nativeReflection->isPromoted(),

‎src/Analyser/IssetabilityLinkInfo.php‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ private function __construct(
4040
private ?TrinaryLogic $isVirtual = null,
4141
private ?Type $nativeType = null,
4242
private bool $hasExpressionTypeOfFetch = false,
43-
private bool $hasConditionalExpressionsOfFetch = false,
4443
private bool $initializedThisProperty = false,
4544
private bool $nativeReflectionExists = false,
4645
private bool $nativeIsPromoted = false,
@@ -80,7 +79,6 @@ public static function property(
8079
Type $writableType,
8180
Type $nativeType,
8281
bool $hasExpressionTypeOfFetch,
83-
bool $hasConditionalExpressionsOfFetch,
8482
bool $initializedThisProperty,
8583
bool $nativeReflectionExists,
8684
bool $nativeIsPromoted,
@@ -99,7 +97,6 @@ public static function property(
9997
isVirtual: $isVirtual,
10098
nativeType: $nativeType,
10199
hasExpressionTypeOfFetch: $hasExpressionTypeOfFetch,
102-
hasConditionalExpressionsOfFetch: $hasConditionalExpressionsOfFetch,
103100
initializedThisProperty: $initializedThisProperty,
104101
nativeReflectionExists: $nativeReflectionExists,
105102
nativeIsPromoted: $nativeIsPromoted,
@@ -248,18 +245,6 @@ public function hasExpressionTypeOfFetch(): bool
248245
return $this->hasExpressionTypeOfFetch;
249246
}
250247

251-
/**
252-
* Whether the scope holds conditional-expression entries about the fetch.
253-
* Such entries exist only when the fetch was narrowed in an evaluated
254-
* condition - and evaluating a condition READS the fetch, which would have
255-
* thrown on an uninitialized typed property. A typed-property read
256-
* witnesses initialization.
257-
*/
258-
public function hasConditionalExpressionsOfFetch(): bool
259-
{
260-
return $this->hasConditionalExpressionsOfFetch;
261-
}
262-
263248
public function isInitializedThisProperty(): bool
264249
{
265250
return $this->initializedThisProperty;

‎src/Analyser/IssetabilityResolution.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ public function isSet(callable $typeCallback, ?bool $result = null): ?bool
9797
$link->hasNativeType()
9898
&& !$link->isVirtual()->yes()
9999
&& !$link->hasExpressionTypeOfFetch()
100-
&& !$link->hasConditionalExpressionsOfFetch()
101100
&& !$link->nativeHasDefaultValue()
102101
&& (!$link->nativeReflectionExists() || !$link->nativeIsPromoted() || (!$link->nativeIsReadOnly() && !$link->nativeIsHooked()))
103102
) {

‎tests/PHPStan/Rules/Variables/NullCoalesceRuleTest.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -597,4 +597,18 @@ public function testNullCoalesceAssignRightSideScope(): void
597597
]);
598598
}
599599

600+
public function testBug15046(): void
601+
{
602+
$this->analyse([__DIR__ . '/data/bug-15046.php'], [
603+
[
604+
'Property Bug15046\\BothBranches::$answer on left side of ?? is not nullable nor uninitialized.',
605+
140,
606+
],
607+
[
608+
'Property Bug15046\\ConditionMetAgain::$answer (int) on left side of ?? is not nullable.',
609+
157,
610+
],
611+
]);
612+
}
613+
600614
}

‎tests/PHPStan/Rules/Variables/data/bug-15046.php‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15046;
4+
5+
class Example {
6+
7+
protected int $answer;
8+
9+
public function echo_answer(?int $my_answer = null): void {
10+
// If the following conditional property initialisation is removed, the issue disappears.
11+
if ($my_answer !== null)
12+
$this->answer = $my_answer;
13+
// At this point, $this->answer may still be uninitialised, so that `??` is not unnecessary
14+
echo "The answer is ", self::format_answer($this->answer ?? null), ".\n";
15+
}
16+
17+
private static function format_answer(?int $the_answer): string {
18+
return (string) ($the_answer ?? 'unknown');
19+
}
20+
21+
}
22+
23+
class StaticProperty
24+
{
25+
26+
protected static int $answer;
27+
28+
public static function echoAnswer(?int $myAnswer = null): void
29+
{
30+
if ($myAnswer !== null) {
31+
self::$answer = $myAnswer;
32+
}
33+
34+
echo (self::$answer ?? null);
35+
}
36+
37+
}
38+
39+
class BooleanCondition
40+
{
41+
42+
protected int $answer;
43+
44+
public function echoAnswer(bool $cond): void
45+
{
46+
if ($cond) {
47+
$this->answer = 1;
48+
}
49+
50+
echo ($this->answer ?? null);
51+
}
52+
53+
}
54+
55+
class CoalesceAssign
56+
{
57+
58+
protected int $answer;
59+
60+
public function echoAnswer(?int $myAnswer = null): void
61+
{
62+
if ($myAnswer !== null) {
63+
$this->answer = $myAnswer;
64+
}
65+
66+
$this->answer ??= null;
67+
}
68+
69+
}
70+
71+
class Ternary
72+
{
73+
74+
protected int $answer;
75+
76+
public function echoAnswer(?int $myAnswer = null): void
77+
{
78+
$myAnswer !== null ? ($this->answer = $myAnswer) : null;
79+
80+
echo ($this->answer ?? null);
81+
}
82+
83+
}
84+
85+
class SwitchStatement
86+
{
87+
88+
protected int $answer;
89+
90+
public function echoAnswer(int $myAnswer): void
91+
{
92+
switch ($myAnswer) {
93+
case 1:
94+
$this->answer = 1;
95+
break;
96+
}
97+
98+
echo ($this->answer ?? null);
99+
}
100+
101+
}
102+
103+
class Inner
104+
{
105+
106+
public int $deep;
107+
108+
}
109+
110+
class NestedChain
111+
{
112+
113+
protected Inner $inner;
114+
115+
public function echoAnswer(?int $myAnswer = null): void
116+
{
117+
$this->inner = new Inner();
118+
if ($myAnswer !== null) {
119+
$this->inner->deep = $myAnswer;
120+
}
121+
122+
echo ($this->inner->deep ?? null);
123+
}
124+
125+
}
126+
127+
class BothBranches
128+
{
129+
130+
protected int $answer;
131+
132+
public function echoAnswer(bool $cond): void
133+
{
134+
if ($cond) {
135+
$this->answer = 1;
136+
} else {
137+
$this->answer = 2;
138+
}
139+
140+
echo ($this->answer ?? null);
141+
}
142+
143+
}
144+
145+
class ConditionMetAgain
146+
{
147+
148+
protected int $answer;
149+
150+
public function echoAnswer(?int $myAnswer = null): void
151+
{
152+
if ($myAnswer !== null) {
153+
$this->answer = $myAnswer;
154+
}
155+
156+
if ($myAnswer !== null) {
157+
echo ($this->answer ?? null);
158+
}
159+
}
160+
161+
}

0 commit comments

Comments
 (0)