Skip to content

Commit 7259b57

Browse files
committed
Fix phpstan/phpstan#14117: Variable might not be defined false positive with same condition
- Fixed createConditionalExpressions in MutatingScope to preserve variables in newVariableTypes when their type matches but certainty differs between branches - This ensures conditional expressions are created to track that a variable is definitely defined when the same condition holds as when it was assigned - Removed now-unnecessary @phpstan-ignore variable.undefined comments in OptimizedDirectorySourceLocator.php that were working around this bug - New regression test in tests/PHPStan/Rules/Variables/data/bug-14117.php
1 parent f08de42 commit 7259b57

4 files changed

Lines changed: 80 additions & 2 deletions

File tree

src/Analyser/MutatingScope.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4094,6 +4094,14 @@ private function createConditionalExpressions(
40944094
continue;
40954095
}
40964096

4097+
if (
4098+
array_key_exists($exprString, $newVariableTypes)
4099+
&& $newVariableTypes[$exprString]->equalTypes($holder)
4100+
&& !$newVariableTypes[$exprString]->getCertainty()->equals($holder->getCertainty())
4101+
) {
4102+
continue;
4103+
}
4104+
40974105
unset($newVariableTypes[$exprString]);
40984106
}
40994107

src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
127127
return null;
128128
}
129129

130-
[$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); // @phpstan-ignore variable.undefined
130+
[$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier);
131131
$classReflection = $this->nodeToReflection($reflector, $fetchedClassNode);
132132
$this->cache->save($reflectionCacheKey, $variableCacheKey, $classReflection->exportToCache());
133133

@@ -171,7 +171,7 @@ public function locateIdentifier(Reflector $reflector, Identifier $identifier):
171171
return null;
172172
}
173173

174-
[$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier); // @phpstan-ignore variable.undefined
174+
[$reflectionCacheKey, $variableCacheKey] = $this->getCacheKeys($file, $identifier);
175175
$constantReflection = $this->nodeToReflection(
176176
$reflector,
177177
$fetchedConstantNode,

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,4 +1333,23 @@ public function testBug5477(): void
13331333
$this->analyse([__DIR__ . '/data/bug-5477.php'], []);
13341334
}
13351335

1336+
public function testBug14117(): void
1337+
{
1338+
$this->cliArgumentsVariablesRegistered = true;
1339+
$this->polluteScopeWithLoopInitialAssignments = false;
1340+
$this->checkMaybeUndefinedVariables = true;
1341+
$this->polluteScopeWithAlwaysIterableForeach = true;
1342+
1343+
$this->analyse([__DIR__ . '/data/bug-14117.php'], [
1344+
[
1345+
'Variable $value might not be defined.',
1346+
33,
1347+
],
1348+
[
1349+
'Variable $value might not be defined.',
1350+
49,
1351+
],
1352+
]);
1353+
}
1354+
13361355
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug14117;
4+
5+
function foo(): void {
6+
$key = rand(0, 2);
7+
8+
if ($key === 2) {
9+
$value = 'test';
10+
}
11+
12+
if ($key === 1) {
13+
$value = 'test';
14+
}
15+
16+
if ($key === 1) {
17+
echo $value;
18+
}
19+
}
20+
21+
function bar(): void {
22+
$key = rand(0, 2);
23+
24+
if ($key === 2) {
25+
$value = 'two';
26+
}
27+
28+
if ($key === 1) {
29+
$value = 'one';
30+
}
31+
32+
if ($key === 2) {
33+
echo $value;
34+
}
35+
}
36+
37+
function baz(): void {
38+
$key = rand(0, 3);
39+
40+
if ($key === 1) {
41+
$value = 'one';
42+
}
43+
44+
if ($key === 2) {
45+
$value = 'two';
46+
}
47+
48+
if ($key === 3) {
49+
echo $value; // this one SHOULD report "might not be defined" because $key === 3 doesn't guarantee either earlier block ran
50+
}
51+
}

0 commit comments

Comments
 (0)