Skip to content

Commit b4b7c2e

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Record definedness of short-circuit operand assignments as conditional on the assigned boolean
When a boolean && / || RHS contains an assignment inside an operand, a truthy && (or falsey ||) result guarantees every short-circuit operand was evaluated. Diff the RHS walk's truthy/falsey continuation scope against the merged after-RHS scope for variables whose certainty is Yes there but only Maybe after the merge, and record conditional expression holders so that narrowing the assigned variable later restores the certainty and type. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA
1 parent b3547b6 commit b4b7c2e

4 files changed

Lines changed: 164 additions & 0 deletions

File tree

‎src/Analyser/ExprHandler/AssignHandler.php‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,12 @@
88
use PhpParser\Node\Expr;
99
use PhpParser\Node\Expr\ArrayDimFetch;
1010
use PhpParser\Node\Expr\Assign;
11+
use PhpParser\Node\Expr\AssignOp;
1112
use PhpParser\Node\Expr\AssignRef;
13+
use PhpParser\Node\Expr\BinaryOp\BooleanAnd;
14+
use PhpParser\Node\Expr\BinaryOp\BooleanOr;
15+
use PhpParser\Node\Expr\BinaryOp\LogicalAnd;
16+
use PhpParser\Node\Expr\BinaryOp\LogicalOr;
1217
use PhpParser\Node\Expr\ConstFetch;
1318
use PhpParser\Node\Expr\FuncCall;
1419
use PhpParser\Node\Expr\List_;
@@ -94,6 +99,7 @@
9499
use function array_slice;
95100
use function count;
96101
use function in_array;
102+
use function is_array;
97103
use function is_int;
98104
use function is_string;
99105
use function spl_object_id;
@@ -1191,6 +1197,29 @@ public function applyWrite(
11911197
: $this->defaultNarrowingHelper->specifyTypesForNode($scope, $assignedExpr, TypeSpecifierContext::createFalsey());
11921198
$conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($nodeScopeResolver, $scope, $storage, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $impurePoints, $assignedExpr, $storedAssignedExprResult);
11931199
$conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($nodeScopeResolver, $scope, $storage, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $impurePoints, $assignedExpr, $storedAssignedExprResult);
1200+
1201+
// An assignment inside a short-circuit operand may or may not have run
1202+
// (its variable is only maybe-defined after the RHS walk), but a truthy
1203+
// && (or falsey ||) guarantees the right operand was evaluated. The
1204+
// operand walk's truthy/falsey scope knows exactly which variables it
1205+
// defined - record their certainty as a consequence of the assigned
1206+
// boolean, e.g. "$bool = $x && ($var = 'foo'); if ($bool) { … $var … }".
1207+
if (
1208+
$storedAssignedExprResult !== null
1209+
&& (
1210+
$assignedExpr instanceof BooleanAnd
1211+
|| $assignedExpr instanceof BooleanOr
1212+
|| $assignedExpr instanceof LogicalAnd
1213+
|| $assignedExpr instanceof LogicalOr
1214+
)
1215+
&& self::containsVariableAssignment($assignedExpr)
1216+
) {
1217+
if ($assignedExpr instanceof BooleanAnd || $assignedExpr instanceof LogicalAnd) {
1218+
$conditionalExpressions = $this->processDefinednessForConditionalExpressionsAfterAssign($conditionalExpressions, $var->name, $truthyType, $storedAssignedExprResult->getTruthyScope(), $scope);
1219+
} else {
1220+
$conditionalExpressions = $this->processDefinednessForConditionalExpressionsAfterAssign($conditionalExpressions, $var->name, $falseyType, $storedAssignedExprResult->getFalseyScope(), $scope);
1221+
}
1222+
}
11941223
}
11951224

11961225
foreach ([null, false, 0, 0.0, '', '0', []] as $falseyScalar) {
@@ -1958,6 +1987,76 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(NodeSco
19581987
return $conditionalExpressions;
19591988
}
19601989

1990+
/**
1991+
* Records "if the assigned variable has $variableType, the target variable is
1992+
* certainly defined (with its branch-scope type)" holders for variables whose
1993+
* certainty is Yes in the given branch continuation scope of the RHS (the
1994+
* truthy scope of a `&&`, the falsey scope of a `||` - the scopes where every
1995+
* short-circuit operand was guaranteed evaluated) but only Maybe in the
1996+
* merged after-RHS scope.
1997+
*
1998+
* @param array<string, ConditionalExpressionHolder[]> $conditionalExpressions
1999+
* @return array<string, ConditionalExpressionHolder[]>
2000+
*/
2001+
private function processDefinednessForConditionalExpressionsAfterAssign(
2002+
array $conditionalExpressions,
2003+
string $variableName,
2004+
Type $variableType,
2005+
MutatingScope $branchScope,
2006+
MutatingScope $mergedScope,
2007+
): array
2008+
{
2009+
foreach ($branchScope->expressionTypes as $exprString => $holder) {
2010+
if (!$holder->getCertainty()->yes()) {
2011+
continue;
2012+
}
2013+
$expr = $holder->getExpr();
2014+
if (!$expr instanceof Variable || !is_string($expr->name) || $expr->name === $variableName) {
2015+
continue;
2016+
}
2017+
$mergedHolder = $mergedScope->expressionTypes[$exprString] ?? null;
2018+
if ($mergedHolder === null || !$mergedHolder->getCertainty()->maybe()) {
2019+
continue;
2020+
}
2021+
2022+
$conditionalHolder = new ConditionalExpressionHolder([
2023+
'$' . $variableName => ExpressionTypeHolder::createYes(new Variable($variableName), $variableType),
2024+
], $holder);
2025+
$conditionalExpressions[(string) $exprString][$conditionalHolder->getKey()] = $conditionalHolder;
2026+
}
2027+
2028+
return $conditionalExpressions;
2029+
}
2030+
2031+
/**
2032+
* Whether the expression contains an assignment whose execution short-circuit
2033+
* evaluation may have skipped - a cheap AST gate so the truthy/falsey scope
2034+
* is only derived for boolean RHS expressions that can define a variable.
2035+
*/
2036+
private static function containsVariableAssignment(Node $node): bool
2037+
{
2038+
if ($node instanceof Assign || $node instanceof AssignOp || $node instanceof AssignRef) {
2039+
return true;
2040+
}
2041+
2042+
foreach ($node->getSubNodeNames() as $subNodeName) {
2043+
$subNode = $node->$subNodeName;
2044+
if ($subNode instanceof Node) {
2045+
if (self::containsVariableAssignment($subNode)) {
2046+
return true;
2047+
}
2048+
} elseif (is_array($subNode)) {
2049+
foreach ($subNode as $subNodeItem) {
2050+
if ($subNodeItem instanceof Node && self::containsVariableAssignment($subNodeItem)) {
2051+
return true;
2052+
}
2053+
}
2054+
}
2055+
}
2056+
2057+
return false;
2058+
}
2059+
19612060
/**
19622061
* Current type of a conditional-holder expression, used to refine the holder's
19632062
* projected type. Prefers the tracked scope state over the stored result,

‎tests/PHPStan/Analyser/nsrt/bug-11109.php‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug11109;
4+
5+
use PHPStan\TrinaryLogic;
6+
use function PHPStan\Testing\assertType;
7+
use function PHPStan\Testing\assertVariableCertainty;
8+
9+
function (): void {
10+
$bool = 1 == random_int(1, 2)
11+
&& 2 == random_int(1, 2)
12+
&& in_array(
13+
'foo',
14+
[$var = 'foo', 'bar']
15+
);
16+
17+
if ($bool) {
18+
// here $bool is true
19+
// so $var must be defined
20+
// because the 3rd condition above must have been fulfilled for the $bool to be true
21+
// so 2nd argument to in_array() must have been computed
22+
assertVariableCertainty(TrinaryLogic::createYes(), $var);
23+
assertType("'foo'", $var);
24+
throw new \Exception($var);
25+
}
26+
27+
assertVariableCertainty(TrinaryLogic::createMaybe(), $var);
28+
};
29+
30+
function (): void {
31+
$bool = 1 == random_int(1, 2)
32+
|| in_array(random_int(1, 2), [$var = 1, 3], true);
33+
34+
if (!$bool) {
35+
assertVariableCertainty(TrinaryLogic::createYes(), $var);
36+
assertType('1', $var);
37+
} else {
38+
assertVariableCertainty(TrinaryLogic::createMaybe(), $var);
39+
}
40+
};

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1768,4 +1768,13 @@ public function testBug14418(): void
17681768
$this->analyse([__DIR__ . '/data/bug-14418.php'], []);
17691769
}
17701770

1771+
public function testBug11109(): void
1772+
{
1773+
$this->cliArgumentsVariablesRegistered = true;
1774+
$this->polluteScopeWithLoopInitialAssignments = false;
1775+
$this->checkMaybeUndefinedVariables = true;
1776+
$this->polluteScopeWithAlwaysIterableForeach = true;
1777+
$this->analyse([__DIR__ . '/data/bug-11109.php'], []);
1778+
}
1779+
17711780
}

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php declare(strict_types = 1);
2+
3+
$bool = 1 == random_int(1, 2)
4+
&& 2 == random_int(1, 2)
5+
&& in_array(
6+
'foo',
7+
[$var = 'foo', 'bar']
8+
);
9+
10+
if ($bool) {
11+
// here $bool is true
12+
// so $var must be defined
13+
// because the 3rd condition above must have been fulfilled for the $bool to be true
14+
// so 2nd argument to in_array() must have been computed
15+
throw new \Exception($var);
16+
}

0 commit comments

Comments
 (0)