Skip to content

Commit 3be0e83

Browse files
staabmphpstan-bot
authored andcommitted
Fix phpstan/phpstan#9426: Variable always defined when needed
- When no typeGuards exist, recover type guards from ourExpressionTypes for entries removed by the first loop whose type differs from merged - Only create conditional expressions for variables whose certainty changed from YES to MAYBE (restricting to Variable nodes) - This handles isset()-correlated variable definedness without affecting other conditional expression creation paths - New regression test in tests/PHPStan/Rules/Variables/data/bug-9426.php
1 parent 52704a4 commit 3be0e83

File tree

3 files changed

+100
-0
lines changed

3 files changed

+100
-0
lines changed

src/Analyser/MutatingScope.php

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3612,6 +3612,77 @@ private function createConditionalExpressions(
36123612
}
36133613

36143614
if (count($typeGuards) === 0) {
3615+
// Look for variables whose certainty changed from YES (in our scope) to
3616+
// MAYBE (in merged scope) because they don't exist in their scope.
3617+
// For each such variable, look for potential type guards among variables
3618+
// that were removed from newVariableTypes (because merged type == their
3619+
// type) but whose type differs between our and their scope.
3620+
// This handles cases like: if (isset($a['key'])) { $b = ...; }
3621+
// where $b should be conditionally defined based on $a's narrowed type.
3622+
$certaintyChangedVars = [];
3623+
foreach ($newVariableTypes as $exprString => $holder) {
3624+
if (!$holder->getCertainty()->yes()) {
3625+
continue;
3626+
}
3627+
if (!array_key_exists($exprString, $mergedExpressionTypes)) {
3628+
continue;
3629+
}
3630+
if ($mergedExpressionTypes[$exprString]->getCertainty()->yes()) {
3631+
continue;
3632+
}
3633+
if (!$holder->getExpr() instanceof Variable) {
3634+
continue;
3635+
}
3636+
$certaintyChangedVars[$exprString] = $holder;
3637+
}
3638+
3639+
if (count($certaintyChangedVars) > 0) {
3640+
$recoveredTypeGuards = [];
3641+
foreach ($ourExpressionTypes as $exprString => $holder) {
3642+
if ($holder->getExpr() instanceof VirtualNode) {
3643+
continue;
3644+
}
3645+
if (array_key_exists($exprString, $newVariableTypes)) {
3646+
continue;
3647+
}
3648+
if (!array_key_exists($exprString, $mergedExpressionTypes)) {
3649+
continue;
3650+
}
3651+
if (!$holder->getCertainty()->yes()) {
3652+
continue;
3653+
}
3654+
if (
3655+
array_key_exists($exprString, $theirExpressionTypes)
3656+
&& !$theirExpressionTypes[$exprString]->getCertainty()->yes()
3657+
) {
3658+
continue;
3659+
}
3660+
if ($mergedExpressionTypes[$exprString]->equalTypes($holder)) {
3661+
continue;
3662+
}
3663+
$recoveredTypeGuards[$exprString] = $holder;
3664+
}
3665+
3666+
foreach ($certaintyChangedVars as $exprString => $holder) {
3667+
foreach ($recoveredTypeGuards as $guardExprString => $guardHolder) {
3668+
if ($guardExprString === $exprString) {
3669+
continue;
3670+
}
3671+
if (
3672+
array_key_exists($exprString, $theirExpressionTypes)
3673+
&& $theirExpressionTypes[$exprString]->getCertainty()->yes()
3674+
&& array_key_exists($guardExprString, $theirExpressionTypes)
3675+
&& $theirExpressionTypes[$guardExprString]->getCertainty()->yes()
3676+
&& !$guardHolder->getType()->isSuperTypeOf($theirExpressionTypes[$guardExprString]->getType())->no()
3677+
) {
3678+
continue;
3679+
}
3680+
$conditionalExpression = new ConditionalExpressionHolder([$guardExprString => $guardHolder], $holder);
3681+
$conditionalExpressions[$exprString][$conditionalExpression->getKey()] = $conditionalExpression;
3682+
}
3683+
}
3684+
}
3685+
36153686
return $conditionalExpressions;
36163687
}
36173688

tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,4 +1517,13 @@ public function testBug11218(): void
15171517
$this->analyse([__DIR__ . '/data/bug-11218.php'], []);
15181518
}
15191519

1520+
public function testBug9426(): void
1521+
{
1522+
$this->cliArgumentsVariablesRegistered = true;
1523+
$this->polluteScopeWithLoopInitialAssignments = false;
1524+
$this->checkMaybeUndefinedVariables = true;
1525+
$this->polluteScopeWithAlwaysIterableForeach = true;
1526+
$this->analyse([__DIR__ . '/data/bug-9426.php'], []);
1527+
}
1528+
15201529
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug9426;
4+
5+
final class A
6+
{
7+
/**
8+
* @param array{something?: string} $a
9+
*/
10+
public function something(array $a): void
11+
{
12+
if (isset($a['something'])) {
13+
$b = new \DateTimeImmutable();
14+
}
15+
16+
$c = [
17+
!isset($a['something']) ? 'something' : $b,
18+
];
19+
}
20+
}

0 commit comments

Comments
 (0)