From 1f252c8e55244cd0f5c61524429464fff773e3ba Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 5 Sep 2026 10:24:04 +0200 Subject: [PATCH 1/4] 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 Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA --- src/Analyser/ExprHandler/AssignHandler.php | 99 +++++++++++++++++++ tests/PHPStan/Analyser/nsrt/bug-11109.php | 40 ++++++++ .../Variables/DefinedVariableRuleTest.php | 9 ++ .../Rules/Variables/data/bug-11109.php | 16 +++ 4 files changed, 164 insertions(+) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-11109.php create mode 100644 tests/PHPStan/Rules/Variables/data/bug-11109.php diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 7e7759f1c17..65fe9946581 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -8,7 +8,12 @@ use PhpParser\Node\Expr; use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Assign; +use PhpParser\Node\Expr\AssignOp; use PhpParser\Node\Expr\AssignRef; +use PhpParser\Node\Expr\BinaryOp\BooleanAnd; +use PhpParser\Node\Expr\BinaryOp\BooleanOr; +use PhpParser\Node\Expr\BinaryOp\LogicalAnd; +use PhpParser\Node\Expr\BinaryOp\LogicalOr; use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\List_; @@ -96,6 +101,7 @@ use function array_slice; use function count; use function in_array; +use function is_array; use function is_float; use function is_int; use function is_nan; @@ -1218,6 +1224,29 @@ public function applyWrite( : $this->defaultNarrowingHelper->specifyTypesForNode($scope, $assignedExpr, TypeSpecifierContext::createFalsey()); $conditionalExpressions = $this->processSureTypesForConditionalExpressionsAfterAssign($nodeScopeResolver, $scope, $storage, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $impurePoints, $assignedExpr, $storedAssignedExprResult); $conditionalExpressions = $this->processSureNotTypesForConditionalExpressionsAfterAssign($nodeScopeResolver, $scope, $storage, $var->name, $conditionalExpressions, $falseySpecifiedTypes, $falseyType, $impurePoints, $assignedExpr, $storedAssignedExprResult); + + // An assignment inside a short-circuit operand may or may not have run + // (its variable is only maybe-defined after the RHS walk), but a truthy + // && (or falsey ||) guarantees the right operand was evaluated. The + // operand walk's truthy/falsey scope knows exactly which variables it + // defined - record their certainty as a consequence of the assigned + // boolean, e.g. "$bool = $x && ($var = 'foo'); if ($bool) { … $var … }". + if ( + $storedAssignedExprResult !== null + && ( + $assignedExpr instanceof BooleanAnd + || $assignedExpr instanceof BooleanOr + || $assignedExpr instanceof LogicalAnd + || $assignedExpr instanceof LogicalOr + ) + && self::containsVariableAssignment($assignedExpr) + ) { + if ($assignedExpr instanceof BooleanAnd || $assignedExpr instanceof LogicalAnd) { + $conditionalExpressions = $this->processDefinednessForConditionalExpressionsAfterAssign($conditionalExpressions, $var->name, $truthyType, $storedAssignedExprResult->getTruthyScope(), $scope); + } else { + $conditionalExpressions = $this->processDefinednessForConditionalExpressionsAfterAssign($conditionalExpressions, $var->name, $falseyType, $storedAssignedExprResult->getFalseyScope(), $scope); + } + } } foreach ([null, false, 0, 0.0, '', '0', []] as $falseyScalar) { @@ -1985,6 +2014,76 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(NodeSco return $conditionalExpressions; } + /** + * Records "if the assigned variable has $variableType, the target variable is + * certainly defined (with its branch-scope type)" holders for variables whose + * certainty is Yes in the given branch continuation scope of the RHS (the + * truthy scope of a `&&`, the falsey scope of a `||` - the scopes where every + * short-circuit operand was guaranteed evaluated) but only Maybe in the + * merged after-RHS scope. + * + * @param array $conditionalExpressions + * @return array + */ + private function processDefinednessForConditionalExpressionsAfterAssign( + array $conditionalExpressions, + string $variableName, + Type $variableType, + MutatingScope $branchScope, + MutatingScope $mergedScope, + ): array + { + foreach ($branchScope->expressionTypes as $exprString => $holder) { + if (!$holder->getCertainty()->yes()) { + continue; + } + $expr = $holder->getExpr(); + if (!$expr instanceof Variable || !is_string($expr->name) || $expr->name === $variableName) { + continue; + } + $mergedHolder = $mergedScope->expressionTypes[$exprString] ?? null; + if ($mergedHolder === null || !$mergedHolder->getCertainty()->maybe()) { + continue; + } + + $conditionalHolder = new ConditionalExpressionHolder([ + '$' . $variableName => ExpressionTypeHolder::createYes(new Variable($variableName), $variableType), + ], $holder); + $conditionalExpressions[(string) $exprString][$conditionalHolder->getKey()] = $conditionalHolder; + } + + return $conditionalExpressions; + } + + /** + * Whether the expression contains an assignment whose execution short-circuit + * evaluation may have skipped - a cheap AST gate so the truthy/falsey scope + * is only derived for boolean RHS expressions that can define a variable. + */ + private static function containsVariableAssignment(Node $node): bool + { + if ($node instanceof Assign || $node instanceof AssignOp || $node instanceof AssignRef) { + return true; + } + + foreach ($node->getSubNodeNames() as $subNodeName) { + $subNode = $node->$subNodeName; + if ($subNode instanceof Node) { + if (self::containsVariableAssignment($subNode)) { + return true; + } + } elseif (is_array($subNode)) { + foreach ($subNode as $subNodeItem) { + if ($subNodeItem instanceof Node && self::containsVariableAssignment($subNodeItem)) { + return true; + } + } + } + } + + return false; + } + /** * Current type of a conditional-holder expression, used to refine the holder's * projected type. Prefers the tracked scope state over the stored result, diff --git a/tests/PHPStan/Analyser/nsrt/bug-11109.php b/tests/PHPStan/Analyser/nsrt/bug-11109.php new file mode 100644 index 00000000000..9693317ec2b --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-11109.php @@ -0,0 +1,40 @@ +analyse([__DIR__ . '/data/bug-14418.php'], []); } + public function testBug11109(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-11109.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-11109.php b/tests/PHPStan/Rules/Variables/data/bug-11109.php new file mode 100644 index 00000000000..e97dcc67072 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-11109.php @@ -0,0 +1,16 @@ + Date: Sat, 5 Sep 2026 10:30:41 +0200 Subject: [PATCH 2/4] Record try-path definedness as conditional on the catch variable being undefined At a try/catch join where the catch variable is untracked on the non-catch path and certainly defined at the end of the catch body, its later definedness tells the joined paths apart. Record conditional expression holders with a certainty-No (and, for scopes where any variable can exist, a maybe-defined-null) condition on the catch variable, restoring the non-catch certainty and type of variables the join demoted to maybe-defined. To let such holders fire, the isset()-style falsey specification in MutatingScope::applySpecifiedTypes() now publishes the certainty change into the specified-expressions batch - rescuing the waiting holders around unsetExpression()'s invalidation, which would otherwise drop them at the very specification they wait for. A second catch clause merging into the join drops the holders via the conditional-expressions intersection, so multi-catch stays conservative. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA --- src/Analyser/MutatingScope.php | 25 ++++++ src/Analyser/StmtHandler/TryCatchHandler.php | 80 ++++++++++++++++++- tests/PHPStan/Analyser/nsrt/bug-6608.php | 23 ++++++ .../Variables/DefinedVariableRuleTest.php | 9 +++ .../PHPStan/Rules/Variables/data/bug-6608.php | 8 ++ 5 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-6608.php create mode 100644 tests/PHPStan/Rules/Variables/data/bug-6608.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 483d58981da..41d1a63c532 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -4092,12 +4092,37 @@ public function applySpecifiedTypes(SpecifiedTypes $specifiedTypes): self $expr = $issetExpr->getExpr(); if ($typeSpecification['sure']) { + $innerExprString = $scope->getNodeKey($expr); $scope = $scope->setExpressionCertaintyKeepingType( $expr, TrinaryLogic::createMaybe(), ); + $specifiedExpressions[$innerExprString] = ExpressionTypeHolder::createMaybe( + $expr, + $scope->expressionTypes[$innerExprString]->getType(), + ); } else { + $innerExprString = $scope->getNodeKey($expr); + // Holders conditioned on this expression being undefined (a + // certainty-No condition) wait for exactly the specification + // applied here, but unsetExpression()'s invalidation would drop + // them before the conditional-expressions matcher below could + // fire them - carve them out and re-add them afterwards. + $rescuedHolders = []; + foreach ($scope->conditionalExpressions as $targetExprString => $targetHolders) { + foreach ($targetHolders as $holderKey => $conditionalHolder) { + $conditionHolder = $conditionalHolder->getConditionExpressionTypeHolders()[$innerExprString] ?? null; + if ($conditionHolder === null || !$conditionHolder->getCertainty()->no()) { + continue; + } + $rescuedHolders[$targetExprString][$holderKey] = $conditionalHolder; + } + } $scope = $scope->unsetExpression($expr); + foreach ($rescuedHolders as $targetExprString => $targetHolders) { + $scope = $scope->addConditionalExpressions((string) $targetExprString, $targetHolders); + } + $specifiedExpressions[$innerExprString] = new ExpressionTypeHolder($expr, new ErrorType(), TrinaryLogic::createNo()); } $scopeIsWorkingCopy = false; diff --git a/src/Analyser/StmtHandler/TryCatchHandler.php b/src/Analyser/StmtHandler/TryCatchHandler.php index 5f3426d23b8..accaf465a85 100644 --- a/src/Analyser/StmtHandler/TryCatchHandler.php +++ b/src/Analyser/StmtHandler/TryCatchHandler.php @@ -8,7 +8,10 @@ use PhpParser\Node\Expr; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\TryCatch; +use PhpParser\Node\Expr\Variable; +use PHPStan\Analyser\ConditionalExpressionHolder; use PHPStan\Analyser\ExpressionResultStorage; +use PHPStan\Analyser\ExpressionTypeHolder; use PHPStan\Analyser\InternalStatementExitPoint; use PHPStan\Analyser\InternalStatementResult; use PHPStan\Analyser\InternalThrowPoint; @@ -26,7 +29,10 @@ use PHPStan\Node\Variable\VariableWrite; use PHPStan\Node\VariableAssignNode; use PHPStan\ShouldNotHappenException; +use PHPStan\TrinaryLogic; +use PHPStan\Type\ErrorType; use PHPStan\Type\NeverType; +use PHPStan\Type\NullType; use PHPStan\Type\ObjectType; use PHPStan\Type\TypeCombinator; use Throwable; @@ -239,7 +245,13 @@ public function processStmt( $catchScopeForFinally = $catchScopeResult->getScope(); $catchFlows[] = [$originalCatchType, VariableFlow::sequence($catchNode->var !== null ? VariableFlowBuilder::targetWrite($catchNode->var, VariableWrite::KIND_CATCH, $catchScopeForFinally, $storage) : null, $catchScopeResult->getVariableFlow())]; - $finalScope = $catchScopeResult->isAlwaysTerminating() ? $finalScope : $catchScopeResult->getScope()->mergeWith($finalScope); + if (!$catchScopeResult->isAlwaysTerminating()) { + $mergedScope = $catchScopeResult->getScope()->mergeWith($finalScope); + if ($variableName !== null && $finalScope !== null) { + $mergedScope = $this->addCatchVariableDefinednessConditionals($mergedScope, $finalScope, $catchScopeResult->getScope(), $variableName); + } + $finalScope = $mergedScope; + } $alwaysTerminating = $alwaysTerminating && $catchScopeResult->isAlwaysTerminating(); $hasYield = $hasYield || $catchScopeResult->hasYield(); $catchThrowPoints = $catchScopeResult->getThrowPoints(); @@ -323,4 +335,70 @@ public function processStmt( return new InternalStatementResult($finalScope, hasYield: $hasYield, isAlwaysTerminating: $alwaysTerminating, exitPoints: $exitPoints, throwPoints: array_merge($throwPoints, $throwPointsForLater), impurePoints: $impurePoints, variableFlow: VariableFlow::tryCatch($branchScopeResult->getVariableFlow(), $catchFlows, $finallyFlow)); } + /** + * The catch variable's definedness after the try/catch tells the two joined + * paths apart: when it is untracked on the non-catch path and certainly + * defined at the end of the catch body, "the catch variable is undefined" + * later implies the non-catch path ran. Record that as conditional + * expression holders with a certainty-No condition on the catch variable, + * restoring the non-catch certainty (and type) of variables the join + * demoted to maybe-defined - e.g. `isset($e) || $var instanceof \DateTime` + * evaluates `$var` only where `$e` is narrowed away. + */ + private function addCatchVariableDefinednessConditionals( + MutatingScope $mergedScope, + MutatingScope $nonCatchScope, + MutatingScope $catchEndScope, + string $variableName, + ): MutatingScope + { + $variableExprString = '$' . $variableName; + if (isset($nonCatchScope->expressionTypes[$variableExprString])) { + return $mergedScope; + } + + $catchVariableHolder = $catchEndScope->expressionTypes[$variableExprString] ?? null; + if ($catchVariableHolder === null || !$catchVariableHolder->getCertainty()->yes()) { + return $mergedScope; + } + + $conditions = [ + [ + $variableExprString => new ExpressionTypeHolder(new Variable($variableName), new ErrorType(), TrinaryLogic::createNo()), + ], + ]; + if ($catchVariableHolder->getType()->isNull()->no()) { + // In a scope where any variable can exist (e.g. the top level), the + // isset() machinery models "!isset($e)" on a maybe-defined variable as + // "maybe defined, null when defined" instead of unsetting it. That + // state excludes the catch path just the same - it guarantees a + // defined, non-null catch variable. + $conditions[] = [ + $variableExprString => ExpressionTypeHolder::createMaybe(new Variable($variableName), new NullType()), + ]; + } + foreach ($nonCatchScope->expressionTypes as $exprString => $holder) { + if (!$holder->getCertainty()->yes()) { + continue; + } + $expr = $holder->getExpr(); + if (!$expr instanceof Variable || !is_string($expr->name)) { + continue; + } + $mergedHolder = $mergedScope->expressionTypes[$exprString] ?? null; + if ($mergedHolder === null || !$mergedHolder->getCertainty()->maybe()) { + continue; + } + + $conditionalHolders = []; + foreach ($conditions as $condition) { + $conditionalHolder = new ConditionalExpressionHolder($condition, $holder); + $conditionalHolders[$conditionalHolder->getKey()] = $conditionalHolder; + } + $mergedScope = $mergedScope->addConditionalExpressions((string) $exprString, $conditionalHolders); + } + + return $mergedScope; + } + } diff --git a/tests/PHPStan/Analyser/nsrt/bug-6608.php b/tests/PHPStan/Analyser/nsrt/bug-6608.php new file mode 100644 index 00000000000..d213f1b5633 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-6608.php @@ -0,0 +1,23 @@ +analyse([__DIR__ . '/data/bug-11109.php'], []); } + public function testBug6608(): void + { + $this->cliArgumentsVariablesRegistered = true; + $this->polluteScopeWithLoopInitialAssignments = false; + $this->checkMaybeUndefinedVariables = true; + $this->polluteScopeWithAlwaysIterableForeach = true; + $this->analyse([__DIR__ . '/data/bug-6608.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/bug-6608.php b/tests/PHPStan/Rules/Variables/data/bug-6608.php new file mode 100644 index 00000000000..e1e82e75908 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/bug-6608.php @@ -0,0 +1,8 @@ + Date: Sat, 5 Sep 2026 14:46:46 +0200 Subject: [PATCH 3/4] Fix self-analysis and code style after definedness-holder changes The definedness-holder changes added three `(string)` casts on array keys documented as strings - load-bearing under strict_types because numeric-string keys become ints at runtime. Suppress them inline like ScopeOps does instead of growing the baseline counts. Also sort the TryCatchHandler imports. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA --- src/Analyser/ExprHandler/AssignHandler.php | 2 +- src/Analyser/MutatingScope.php | 2 +- src/Analyser/StmtHandler/TryCatchHandler.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 65fe9946581..cd3c8ec9d99 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -2049,7 +2049,7 @@ private function processDefinednessForConditionalExpressionsAfterAssign( $conditionalHolder = new ConditionalExpressionHolder([ '$' . $variableName => ExpressionTypeHolder::createYes(new Variable($variableName), $variableType), ], $holder); - $conditionalExpressions[(string) $exprString][$conditionalHolder->getKey()] = $conditionalHolder; + $conditionalExpressions[(string) $exprString][$conditionalHolder->getKey()] = $conditionalHolder; // @phpstan-ignore cast.useless } return $conditionalExpressions; diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 41d1a63c532..41ce5caac81 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -4120,7 +4120,7 @@ public function applySpecifiedTypes(SpecifiedTypes $specifiedTypes): self } $scope = $scope->unsetExpression($expr); foreach ($rescuedHolders as $targetExprString => $targetHolders) { - $scope = $scope->addConditionalExpressions((string) $targetExprString, $targetHolders); + $scope = $scope->addConditionalExpressions((string) $targetExprString, $targetHolders); // @phpstan-ignore cast.useless } $specifiedExpressions[$innerExprString] = new ExpressionTypeHolder($expr, new ErrorType(), TrinaryLogic::createNo()); } diff --git a/src/Analyser/StmtHandler/TryCatchHandler.php b/src/Analyser/StmtHandler/TryCatchHandler.php index accaf465a85..0256a90b36b 100644 --- a/src/Analyser/StmtHandler/TryCatchHandler.php +++ b/src/Analyser/StmtHandler/TryCatchHandler.php @@ -6,9 +6,9 @@ use Exception; use PhpParser\Node; use PhpParser\Node\Expr; +use PhpParser\Node\Expr\Variable; use PhpParser\Node\Stmt; use PhpParser\Node\Stmt\TryCatch; -use PhpParser\Node\Expr\Variable; use PHPStan\Analyser\ConditionalExpressionHolder; use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\ExpressionTypeHolder; @@ -395,7 +395,7 @@ private function addCatchVariableDefinednessConditionals( $conditionalHolder = new ConditionalExpressionHolder($condition, $holder); $conditionalHolders[$conditionalHolder->getKey()] = $conditionalHolder; } - $mergedScope = $mergedScope->addConditionalExpressions((string) $exprString, $conditionalHolders); + $mergedScope = $mergedScope->addConditionalExpressions((string) $exprString, $conditionalHolders); // @phpstan-ignore cast.useless } return $mergedScope; From 37602e945b6317c4e87bb0efd22307bbed0d42d4 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Sat, 5 Sep 2026 14:48:05 +0200 Subject: [PATCH 4/4] Remap conditional expressions through constant string concat-assigns `$var .= ''` used to invalidate every conditional-expression holder mentioning $var, so a format string built across `if (!empty($target))` lost its correlation with $target and a later `empty($target)` re-check could not select the matching string. Appending a fixed suffix is injective, so holders whose target or condition on $var is a single constant string are remapped through the append instead; everything else still falls back to invalidation. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01CGhnJQpUWRRpg6H5LuWJkA --- src/Analyser/ExprHandler/AssignHandler.php | 118 +++++++++++++++++- tests/PHPStan/Analyser/nsrt/bug-9854.php | 22 ++++ .../Functions/PrintfParametersRuleTest.php | 5 + .../PHPStan/Rules/Functions/data/bug-9854.php | 16 +++ 4 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-9854.php create mode 100644 tests/PHPStan/Rules/Functions/data/bug-9854.php diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index cd3c8ec9d99..3d681fdc03b 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -68,6 +68,7 @@ use PHPStan\Node\VariableAssignNode; use PHPStan\Node\VirtualNode; use PHPStan\Php\PhpVersion; +use PHPStan\Reflection\InitializerExprTypeResolver; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\ShouldNotHappenException; use PHPStan\TrinaryLogic; @@ -107,6 +108,7 @@ use function is_nan; use function is_string; use function spl_object_id; +use function str_contains; /** * @implements ExprHandler @@ -138,6 +140,7 @@ public function __construct( private StaticPropertyFetchHandler $staticPropertyFetchHandler, private MethodThrowPointHelper $methodThrowPointHelper, private PropertyHookThrowPointsResolver $propertyHookThrowPointsResolver, + private InitializerExprTypeResolver $initializerExprTypeResolver, ) { } @@ -1296,7 +1299,23 @@ public function applyWrite( } $nodeScopeResolver->callNodeCallback($nodeCallback, new VariableAssignNode($var, $assignedExpr), $scopeBeforeAssignEval, $storage); - + $remappedConditionalExpressions = []; + if ( + // only the concat-assign's own write remaps - an enclosing plain + // assignment of the same variable (`$s = $s .= 'x'`) sees the + // already-remapped holders and would remap them a second time + $isAssignOp + && $assignedExpr instanceof AssignOp\Concat + && $assignedExpr->var instanceof Variable + && $assignedExpr->var->name === $var->name + && $scope->getConditionalExpressions() !== [] + ) { + $remappedConditionalExpressions = $this->remapConditionalExpressionsThroughConcatAssign( + $scope->getConditionalExpressions(), + '$' . $var->name, + $valueResult->getType(), + ); + } $nativeType = $this->readAssignedValueType($nodeScopeResolver, $storedAssignedExprResult, $assignedExpr, $scope->doNotTreatPhpDocTypesAsCertain()); $scope = $scope->assignVariable( $var->name, @@ -1305,6 +1324,9 @@ public function applyWrite( TrinaryLogic::createYes(), [], ); + foreach ($remappedConditionalExpressions as $exprString => $holders) { + $scope = $scope->addConditionalExpressions((string) $exprString, $holders); // @phpstan-ignore cast.useless + } foreach ($conditionalExpressions as $exprString => $holders) { $scope = $scope->addConditionalExpressions((string) $exprString, $holders); } @@ -2014,6 +2036,100 @@ private function processSureNotTypesForConditionalExpressionsAfterAssign(NodeSco return $conditionalExpressions; } + /** + * `$var .= ` keeps the conditional-expression + * holders about $var alive by remapping them through the append instead of + * losing them to the write's invalidation. A consequence type about $var is + * concatenated with the appended constant; a condition on $var is remapped + * only when it is itself a single constant string - appending a fixed + * suffix is injective, so the remapped condition selects exactly the states + * the original condition did. Holders mentioning $var inside a composite + * expression, with a non-Yes certainty about $var, or with a non-constant + * condition on $var are left to the regular invalidation. This keeps e.g. a + * format string built across `if (!empty($target))` correlated with + * $target while further pieces are appended. + * + * @param array $conditionalExpressions + * @return array + */ + private function remapConditionalExpressionsThroughConcatAssign( + array $conditionalExpressions, + string $varExprString, + Type $appendedType, + ): array + { + $appendedConstantStrings = $appendedType->getConstantStrings(); + if (count($appendedConstantStrings) !== 1 || !$appendedType->equals($appendedConstantStrings[0])) { + return []; + } + $appendedConstantString = $appendedConstantStrings[0]; + + $remapped = []; + foreach ($conditionalExpressions as $targetExprString => $holders) { + $targetExprString = (string) $targetExprString; // @phpstan-ignore cast.useless + $targetIsVar = $targetExprString === $varExprString; + if (!$targetIsVar && str_contains($targetExprString, $varExprString)) { + // a composite target containing the variable ($var[0], f($var), ...) + continue; + } + + foreach ($holders as $holder) { + $holderTouchesVar = $targetIsVar; + $remappable = true; + $newConditions = []; + foreach ($holder->getConditionExpressionTypeHolders() as $conditionExprString => $conditionHolder) { + $conditionExprString = (string) $conditionExprString; // @phpstan-ignore cast.useless + if ($conditionExprString === $varExprString) { + $holderTouchesVar = true; + $conditionConstantStrings = $conditionHolder->getType()->getConstantStrings(); + if ( + !$conditionHolder->getCertainty()->yes() + || count($conditionConstantStrings) !== 1 + || !$conditionHolder->getType()->equals($conditionConstantStrings[0]) + ) { + $remappable = false; + break; + } + $newConditions[$conditionExprString] = ExpressionTypeHolder::createYes( + $conditionHolder->getExpr(), + $conditionConstantStrings[0]->append($appendedConstantString), + ); + continue; + } + + if (str_contains($conditionExprString, $varExprString)) { + $remappable = false; + break; + } + + $newConditions[$conditionExprString] = $conditionHolder; + } + if (!$remappable || !$holderTouchesVar) { + continue; + } + + $typeHolder = $holder->getTypeHolder(); + if ($targetIsVar) { + if (!$typeHolder->getCertainty()->yes()) { + // the append leaves the variable defined on every path - + // an undefined/maybe consequence cannot be carried over + continue; + } + $concatType = $this->initializerExprTypeResolver->resolveConcatType($typeHolder->getType(), $appendedType); + if ($concatType instanceof ErrorType) { + continue; + } + $typeHolder = ExpressionTypeHolder::createYes($typeHolder->getExpr(), $concatType); + } + + $newHolder = new ConditionalExpressionHolder($newConditions, $typeHolder); + $remapped[$targetExprString][$newHolder->getKey()] = $newHolder; + } + } + + return $remapped; + } + /** * Records "if the assigned variable has $variableType, the target variable is * certainly defined (with its branch-scope type)" holders for variables whose diff --git a/tests/PHPStan/Analyser/nsrt/bug-9854.php b/tests/PHPStan/Analyser/nsrt/bug-9854.php new file mode 100644 index 00000000000..29e9fc65e5c --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-9854.php @@ -0,0 +1,22 @@ +%s'; + +if (empty($target)) { + assertType('\'%s\'', $htmlLinkStructure); + return sprintf($htmlLinkStructure, $url, $linkText); +} + +assertType('\'%s\'', $htmlLinkStructure); +return sprintf($htmlLinkStructure, $url, $target, $linkText); diff --git a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php index d31344d8cdb..8f2f17c5f86 100644 --- a/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/PrintfParametersRuleTest.php @@ -160,4 +160,9 @@ public function testBug14567(): void $this->analyse([__DIR__ . '/data/bug-14567.php'], []); } + public function testBug9854(): void + { + $this->analyse([__DIR__ . '/data/bug-9854.php'], []); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/bug-9854.php b/tests/PHPStan/Rules/Functions/data/bug-9854.php new file mode 100644 index 00000000000..1f1e308c505 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/bug-9854.php @@ -0,0 +1,16 @@ +%s'; + +if (empty($target)) { + return sprintf($htmlLinkStructure, $url, $linkText); +} + +return sprintf($htmlLinkStructure, $url, $target, $linkText);