From ea70a59cf0d0cf755bf8e43619d531d143db7fbe Mon Sep 17 00:00:00 2001 From: VincentLanglet <9052536+VincentLanglet@users.noreply.github.com> Date: Sat, 16 May 2026 20:15:28 +0000 Subject: [PATCH] Fix narrowing to non-falsy-string in compound boolean conditions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When checking '' and '0' in a single || or && expression, the type narrowing to non-falsy-string was lost because the sequential removal of constant string types from the scope was lossy — removing '0' from 'string' cannot be represented, so the removal information was lost before '' could be removed (which would have enabled the '0' removal to succeed on the narrower non-empty-string type). Two fixes: 1. TypeCombinator::remove() now retries union type member removals in a loop, so that removing ''|'0' from string correctly yields non-falsy-string regardless of member ordering. 2. BooleanAndHandler/BooleanOrHandler now re-apply "lossy" sureNotType removals from the left arm after the right arm's narrowing succeeds, enabling the sequential per-arm path to also produce non-falsy-string. Closes https://github.com/phpstan/phpstan/issues/9114 --- .../ExprHandler/BooleanAndHandler.php | 34 +++++++++++++- src/Analyser/ExprHandler/BooleanOrHandler.php | 34 +++++++++++++- src/Type/TypeCombinator.php | 16 ++++++- tests/PHPStan/Analyser/nsrt/bug-9114.php | 47 +++++++++++++++++++ 4 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-9114.php diff --git a/src/Analyser/ExprHandler/BooleanAndHandler.php b/src/Analyser/ExprHandler/BooleanAndHandler.php index 5b672a2db5b..39b40f66a9f 100644 --- a/src/Analyser/ExprHandler/BooleanAndHandler.php +++ b/src/Analyser/ExprHandler/BooleanAndHandler.php @@ -15,12 +15,16 @@ use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; +use PHPStan\Analyser\SpecifiedTypes; +use PHPStan\Analyser\TypeSpecifier; +use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\BooleanAndNode; use PHPStan\Type\BooleanType; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\NeverType; use PHPStan\Type\Type; +use PHPStan\Type\TypeCombinator; use function array_merge; /** @@ -34,6 +38,7 @@ final class BooleanAndHandler implements ExprHandler public function __construct( private NodeScopeResolver $nodeScopeResolver, + private TypeSpecifier $typeSpecifier, ) { } @@ -99,15 +104,42 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $nodeScopeResolver->callNodeCallbackWithExpression($nodeCallback, new BooleanAndNode($expr, $leftTruthyScope), $scope, $storage, $context); + $leftTruthySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createTruthy()); + return new ExpressionResult( $leftMergedWithRightScope, hasYield: $leftResult->hasYield() || $rightResult->hasYield(), isAlwaysTerminating: $leftResult->isAlwaysTerminating(), throwPoints: array_merge($leftResult->getThrowPoints(), $rightResult->getThrowPoints()), impurePoints: array_merge($leftResult->getImpurePoints(), $rightResult->getImpurePoints()), - truthyScopeCallback: static fn (): MutatingScope => $rightResult->getScope()->filterByTruthyValue($expr->right), + truthyScopeCallback: static fn (): MutatingScope => self::computeTruthyScope($rightResult->getScope(), $expr->right, $leftTruthySpecifiedTypes, $leftTruthyScope), falseyScopeCallback: static fn (): MutatingScope => $leftMergedWithRightScope->filterByFalseyValue($expr), ); } + private static function computeTruthyScope( + MutatingScope $rightScope, + Expr $rightExpr, + SpecifiedTypes $leftTruthySpecifiedTypes, + MutatingScope $leftTruthyScope, + ): MutatingScope + { + $scope = $rightScope->filterByTruthyValue($rightExpr); + + foreach ($leftTruthySpecifiedTypes->getSureNotTypes() as [$exprNode, $sureNotType]) { + if (!$leftTruthyScope->getType($exprNode)->equals($rightScope->getType($exprNode))) { + continue; + } + + $exprType = $scope->getType($exprNode); + if (TypeCombinator::remove($exprType, $sureNotType) === $exprType) { + continue; + } + + $scope = $scope->removeTypeFromExpression($exprNode, $sureNotType); + } + + return $scope; + } + } diff --git a/src/Analyser/ExprHandler/BooleanOrHandler.php b/src/Analyser/ExprHandler/BooleanOrHandler.php index 9c828edb942..b43d25afe84 100644 --- a/src/Analyser/ExprHandler/BooleanOrHandler.php +++ b/src/Analyser/ExprHandler/BooleanOrHandler.php @@ -13,12 +13,16 @@ use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; +use PHPStan\Analyser\SpecifiedTypes; +use PHPStan\Analyser\TypeSpecifier; +use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\BooleanOrNode; use PHPStan\Type\BooleanType; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\NeverType; use PHPStan\Type\Type; +use PHPStan\Type\TypeCombinator; use function array_merge; /** @@ -32,6 +36,7 @@ final class BooleanOrHandler implements ExprHandler public function __construct( private NodeScopeResolver $nodeScopeResolver, + private TypeSpecifier $typeSpecifier, ) { } @@ -83,6 +88,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $nodeScopeResolver->callNodeCallbackWithExpression($nodeCallback, new BooleanOrNode($expr, $leftFalseyScope), $scope, $storage, $context); + $leftFalseySpecifiedTypes = $this->typeSpecifier->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createFalsey()); + return new ExpressionResult( $leftMergedWithRightScope, hasYield: $leftResult->hasYield() || $rightResult->hasYield(), @@ -90,8 +97,33 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex throwPoints: array_merge($leftResult->getThrowPoints(), $rightResult->getThrowPoints()), impurePoints: array_merge($leftResult->getImpurePoints(), $rightResult->getImpurePoints()), truthyScopeCallback: static fn (): MutatingScope => $leftMergedWithRightScope->filterByTruthyValue($expr), - falseyScopeCallback: static fn (): MutatingScope => $rightResult->getScope()->filterByFalseyValue($expr->right), + falseyScopeCallback: static fn (): MutatingScope => self::computeFalseyScope($rightResult->getScope(), $expr->right, $leftFalseySpecifiedTypes, $leftFalseyScope), ); } + private static function computeFalseyScope( + MutatingScope $rightScope, + Expr $rightExpr, + SpecifiedTypes $leftFalseySpecifiedTypes, + MutatingScope $leftFalseyScope, + ): MutatingScope + { + $scope = $rightScope->filterByFalseyValue($rightExpr); + + foreach ($leftFalseySpecifiedTypes->getSureNotTypes() as [$exprNode, $sureNotType]) { + if (!$leftFalseyScope->getType($exprNode)->equals($rightScope->getType($exprNode))) { + continue; + } + + $exprType = $scope->getType($exprNode); + if (TypeCombinator::remove($exprType, $sureNotType) === $exprType) { + continue; + } + + $scope = $scope->removeTypeFromExpression($exprNode, $sureNotType); + } + + return $scope; + } + } diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php index df18959ae24..fe35b522e81 100644 --- a/src/Type/TypeCombinator.php +++ b/src/Type/TypeCombinator.php @@ -64,8 +64,20 @@ public static function addNull(Type $type): Type public static function remove(Type $fromType, Type $typeToRemove): Type { if ($typeToRemove instanceof UnionType) { - foreach ($typeToRemove->getTypes() as $unionTypeToRemove) { - $fromType = self::remove($fromType, $unionTypeToRemove); + $typesToRemove = $typeToRemove->getTypes(); + $changed = true; + while ($changed && count($typesToRemove) > 0) { + $changed = false; + foreach ($typesToRemove as $key => $unionTypeToRemove) { + $newFromType = self::remove($fromType, $unionTypeToRemove); + if ($newFromType === $fromType) { + continue; + } + + $fromType = $newFromType; + unset($typesToRemove[$key]); + $changed = true; + } } return $fromType; } diff --git a/tests/PHPStan/Analyser/nsrt/bug-9114.php b/tests/PHPStan/Analyser/nsrt/bug-9114.php new file mode 100644 index 00000000000..d1015fcb2b8 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-9114.php @@ -0,0 +1,47 @@ +