From ba9aeb984900d04f387b5a0fcdfb8df65d7e5758 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Mon, 15 Jun 2026 11:03:13 +0000 Subject: [PATCH] Use genuine falsey narrowing (not the inverted truthy narrowing) for the consequent of `BooleanAnd` false-context conditional holders - In `BooleanAndHandler::specifyTypes()` false context, split the per-arm narrowings used to build conditional expression holders into a condition (antecedent) set and a holder (consequent) set. - The truthy-narrowing-swap fallback (added for `isset()` on array dim fetches) is now only applied to the antecedent set. `processBooleanConditionalTypes` inverts that set back to the truthy narrowing, so the swap is sound there. - The consequent set keeps the genuine falsey narrowing of each arm (including the mixed truthy-and-false re-derivation). When an arm has no sound falsey narrowing it contributes no consequent, instead of fabricating one by inverting its truthy narrowing. - This fixes over-narrowing where `$a !== null && $b === $a` falling through and then re-testing `$a !== null` wrongly narrowed `$b` to `null`: inverting the `===` truthy narrowing (`$b` to `$a`'s broad `string`) removed `string` from `$b`. The same fault affected every comparison value type (int/string) and both variables and property fetches; all are fixed by the single change. - The `BooleanOr` truthy path does not use the swap and was already correct. --- .../ExprHandler/BooleanAndHandler.php | 42 ++++++++------ tests/PHPStan/Analyser/nsrt/bug-14828.php | 55 +++++++++++++++++++ 2 files changed, 81 insertions(+), 16 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14828.php diff --git a/src/Analyser/ExprHandler/BooleanAndHandler.php b/src/Analyser/ExprHandler/BooleanAndHandler.php index 6a56274b37..383b03c7d6 100644 --- a/src/Analyser/ExprHandler/BooleanAndHandler.php +++ b/src/Analyser/ExprHandler/BooleanAndHandler.php @@ -109,29 +109,39 @@ public function specifyTypes(TypeSpecifier $typeSpecifier, Scope $scope, Expr $e $types = $this->conditionalExpressionHolderHelper->augmentDisjunctionTypes($scope, $rightScope, $leftNormalized, $rightNormalized, $expr->left, $expr->right, false, $types); } if ($context->false()) { - $leftTypesForHolders = $leftTypes; - $rightTypesForHolders = $rightTypes; + // Consequent (holder) narrowings projected by each holder: these must be + // the genuine falsey narrowing of the arm. When that is empty, the arm + // has no sound falsey narrowing and must not contribute a consequent. + $leftHolderTypes = $leftTypes; + $rightHolderTypes = $rightTypes; // In a mixed truthy-and-false context, re-derive empty holders from the falsey narrowing. if ($context->truthy()) { - if ($leftTypesForHolders->getSureTypes() === [] && $leftTypesForHolders->getSureNotTypes() === []) { - $leftTypesForHolders = $typeSpecifier->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createFalsey())->setRootExpr($expr); + if ($leftHolderTypes->getSureTypes() === [] && $leftHolderTypes->getSureNotTypes() === []) { + $leftHolderTypes = $typeSpecifier->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createFalsey())->setRootExpr($expr); } - if ($rightTypesForHolders->getSureTypes() === [] && $rightTypesForHolders->getSureNotTypes() === []) { - $rightTypesForHolders = $typeSpecifier->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createFalsey())->setRootExpr($expr); + if ($rightHolderTypes->getSureTypes() === [] && $rightHolderTypes->getSureNotTypes() === []) { + $rightHolderTypes = $typeSpecifier->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createFalsey())->setRootExpr($expr); } } - // For arms still empty (e.g. isset() on an array dim fetch), derive conditions - // from the truthy narrowing instead, swapping sure/sureNot types. - if ($leftTypesForHolders->getSureTypes() === [] && $leftTypesForHolders->getSureNotTypes() === []) { + // Condition (antecedent) narrowings: when an arm has no falsey narrowing + // (e.g. isset() on an array dim fetch), derive the condition from the truthy + // narrowing by swapping sure/sureNot types. This swap is only sound for the + // antecedent — processBooleanConditionalTypes inverts it back to the truthy + // narrowing. It must NOT feed the consequent: inverting a comparison's truthy + // narrowing (e.g. `$a === $b` narrowing `$a` to `$b`'s broad type) would + // over-narrow the consequent (see regression for `$x === $nonConstantString`). + $leftCondTypes = $leftHolderTypes; + $rightCondTypes = $rightHolderTypes; + if ($leftCondTypes->getSureTypes() === [] && $leftCondTypes->getSureNotTypes() === []) { $truthyLeftTypes = $typeSpecifier->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createTruthy()); if ($this->allExpressionsTrackable($truthyLeftTypes)) { - $leftTypesForHolders = new SpecifiedTypes($truthyLeftTypes->getSureNotTypes(), $truthyLeftTypes->getSureTypes()); + $leftCondTypes = new SpecifiedTypes($truthyLeftTypes->getSureNotTypes(), $truthyLeftTypes->getSureTypes()); } } - if ($rightTypesForHolders->getSureTypes() === [] && $rightTypesForHolders->getSureNotTypes() === []) { + if ($rightCondTypes->getSureTypes() === [] && $rightCondTypes->getSureNotTypes() === []) { $truthyRightTypes = $typeSpecifier->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createTruthy()); if ($this->allExpressionsTrackable($truthyRightTypes)) { - $rightTypesForHolders = new SpecifiedTypes($truthyRightTypes->getSureNotTypes(), $truthyRightTypes->getSureTypes()); + $rightCondTypes = new SpecifiedTypes($truthyRightTypes->getSureNotTypes(), $truthyRightTypes->getSureTypes()); } } $result = new SpecifiedTypes( @@ -142,10 +152,10 @@ public function specifyTypes(TypeSpecifier $typeSpecifier, Scope $scope, Expr $e $result = $result->setAlwaysOverwriteTypes(); } return $result->setNewConditionalExpressionHolders($this->conditionalExpressionHolderHelper->mergeConditionalHolders([ - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, false, true, $rightScope, $expr->right), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, false, true, $scope, $expr->left), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, true, true, $rightScope, $expr->right), - $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, true, true, $scope, $expr->left), + $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $leftCondTypes, $rightHolderTypes, false, true, $rightScope, $expr->right), + $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $rightCondTypes, $leftHolderTypes, false, true, $scope, $expr->left), + $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $leftCondTypes, $rightHolderTypes, true, true, $rightScope, $expr->right), + $this->conditionalExpressionHolderHelper->processBooleanConditionalTypes($scope, $rightCondTypes, $leftHolderTypes, true, true, $scope, $expr->left), ]))->setRootExpr($expr); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-14828.php b/tests/PHPStan/Analyser/nsrt/bug-14828.php new file mode 100644 index 0000000000..a3f991022e --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14828.php @@ -0,0 +1,55 @@ +p !== null && $c->q === $c->p) { + return; + } + + if ($c->p !== null) { + assertType('string|null', $c->q); + } + } + +}