From 22f0c05bea20636490a2dd82c754acb91c343e57 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 a5df088329d..4163dcb4304 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_; @@ -90,6 +95,7 @@ use function array_slice; use function count; use function in_array; +use function is_array; use function is_int; use function is_string; use function spl_object_id; @@ -1157,6 +1163,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) { @@ -1845,6 +1874,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 @@ +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 96f4301f6df..11bbbe703bc 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3827,12 +3827,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 ac4612abc84..63688e561ae 100644 --- a/src/Analyser/StmtHandler/TryCatchHandler.php +++ b/src/Analyser/StmtHandler/TryCatchHandler.php @@ -6,7 +6,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\InternalStatementResult; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; @@ -18,7 +21,10 @@ use PHPStan\Node\FinallyExitPointsNode; 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; @@ -213,7 +219,13 @@ public function processStmt( $catchScopeResult = $nodeScopeResolver->processStmtNodesInternal($catchNode, $catchNode->stmts, $catchScope->enterCatchType($catchType, $variableName), $storage, $nodeCallback, $context); $catchScopeForFinally = $catchScopeResult->getScope(); - $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(); @@ -274,4 +286,70 @@ public function processStmt( return new InternalStatementResult($finalScope, hasYield: $hasYield, isAlwaysTerminating: $alwaysTerminating, exitPoints: $exitPoints, throwPoints: array_merge($throwPoints, $throwPointsForLater), impurePoints: $impurePoints); } + /** + * 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 4163dcb4304..158b4882491 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -1909,7 +1909,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 11bbbe703bc..d122d3efa83 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3855,7 +3855,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 63688e561ae..439d6eb4f5d 100644 --- a/src/Analyser/StmtHandler/TryCatchHandler.php +++ b/src/Analyser/StmtHandler/TryCatchHandler.php @@ -4,9 +4,9 @@ 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; @@ -346,7 +346,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 864ebf74fac200988f34bce293293970c16f1f6e 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 | 117 ++++++++++++++++++ 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(+) 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 158b4882491..249199568bb 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -63,6 +63,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; @@ -99,6 +100,7 @@ use function is_int; use function is_string; use function spl_object_id; +use function str_contains; /** * @implements ExprHandler @@ -125,6 +127,7 @@ public function __construct( private StaticPropertyFetchHandler $staticPropertyFetchHandler, private MethodThrowPointHelper $methodThrowPointHelper, private PropertyHookThrowPointsResolver $propertyHookThrowPointsResolver, + private InitializerExprTypeResolver $initializerExprTypeResolver, ) { } @@ -1235,7 +1238,27 @@ 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(), + ); + } $scope = $scope->assignVariable($var->name, $type, $this->readAssignedValueType($nodeScopeResolver, $storedAssignedExprResult, $assignedExpr, $scope->doNotTreatPhpDocTypesAsCertain()), 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); } @@ -1874,6 +1897,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);