diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 334b96f68e..834de82a23 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -735,6 +735,18 @@ public function specifyTypesInCondition( $rightTypesForHolders = $this->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createFalsey())->setRootExpr($expr); } } + if ($leftTypesForHolders->getSureTypes() === [] && $leftTypesForHolders->getSureNotTypes() === []) { + $truthyLeftTypes = $this->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createTruthy()); + if ($this->allExpressionsTrackable($truthyLeftTypes)) { + $leftTypesForHolders = new SpecifiedTypes($truthyLeftTypes->getSureNotTypes(), $truthyLeftTypes->getSureTypes()); + } + } + if ($rightTypesForHolders->getSureTypes() === [] && $rightTypesForHolders->getSureNotTypes() === []) { + $truthyRightTypes = $this->specifyTypesInCondition($rightScope, $expr->right, TypeSpecifierContext::createTruthy()); + if ($this->allExpressionsTrackable($truthyRightTypes)) { + $rightTypesForHolders = new SpecifiedTypes($truthyRightTypes->getSureNotTypes(), $truthyRightTypes->getSureTypes()); + } + } $result = new SpecifiedTypes( $types->getSureTypes(), $types->getSureNotTypes(), @@ -743,10 +755,14 @@ public function specifyTypesInCondition( $result = $result->setAlwaysOverwriteTypes(); } return $result->setNewConditionalExpressionHolders(array_merge( - $this->processBooleanNotSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, $rightScope), - $this->processBooleanNotSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, $scope), - $this->processBooleanSureConditionalTypes($scope, $leftTypesForHolders, $rightTypesForHolders, $rightScope), - $this->processBooleanSureConditionalTypes($scope, $rightTypesForHolders, $leftTypesForHolders, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypesForHolders->getSureNotTypes(), false, $rightTypesForHolders->getSureNotTypes(), false, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypesForHolders->getSureNotTypes(), false, $leftTypesForHolders->getSureNotTypes(), false, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypesForHolders->getSureTypes(), true, $rightTypesForHolders->getSureTypes(), true, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypesForHolders->getSureTypes(), true, $leftTypesForHolders->getSureTypes(), true, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypesForHolders->getSureNotTypes(), false, $rightTypesForHolders->getSureTypes(), true, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypesForHolders->getSureNotTypes(), false, $leftTypesForHolders->getSureTypes(), true, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypesForHolders->getSureTypes(), true, $rightTypesForHolders->getSureNotTypes(), false, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypesForHolders->getSureTypes(), true, $leftTypesForHolders->getSureNotTypes(), false, $scope), ))->setRootExpr($expr); } @@ -796,10 +812,14 @@ public function specifyTypesInCondition( $result = $result->setAlwaysOverwriteTypes(); } return $result->setNewConditionalExpressionHolders(array_merge( - $this->processBooleanNotSureConditionalTypes($scope, $leftTypes, $rightTypes, $rightScope), - $this->processBooleanNotSureConditionalTypes($scope, $rightTypes, $leftTypes, $scope), - $this->processBooleanSureConditionalTypes($scope, $leftTypes, $rightTypes, $rightScope), - $this->processBooleanSureConditionalTypes($scope, $rightTypes, $leftTypes, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypes->getSureNotTypes(), false, $rightTypes->getSureNotTypes(), false, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypes->getSureNotTypes(), false, $leftTypes->getSureNotTypes(), false, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypes->getSureTypes(), true, $rightTypes->getSureTypes(), true, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypes->getSureTypes(), true, $leftTypes->getSureTypes(), true, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypes->getSureNotTypes(), false, $rightTypes->getSureTypes(), true, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypes->getSureNotTypes(), false, $leftTypes->getSureTypes(), true, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypes->getSureTypes(), true, $rightTypes->getSureNotTypes(), false, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypes->getSureTypes(), true, $leftTypes->getSureNotTypes(), false, $scope), ))->setRootExpr($expr); } @@ -2064,20 +2084,26 @@ private function augmentDisjunctionTypes( } /** + * @param array $conditionTypes + * @param array $holderTypes * @return array */ - private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes, Scope $rightScope): array + private function processBooleanConditionalTypes(Scope $scope, array $conditionTypes, bool $conditionsAreSure, array $holderTypes, bool $holdersAreSure, Scope $rightScope): array { $conditionExpressionTypes = []; - foreach ($leftTypes->getSureTypes() as $exprString => [$expr, $type]) { + foreach ($conditionTypes as $exprString => [$expr, $type]) { if (!$this->isTrackableExpression($expr)) { continue; } - $scopeType = $scope->getType($expr); - $conditionType = TypeCombinator::remove($scopeType, $type); - if ($scopeType->equals($conditionType)) { - continue; + if ($conditionsAreSure) { + $scopeType = $scope->getType($expr); + $conditionType = TypeCombinator::remove($scopeType, $type); + if ($scopeType->equals($conditionType)) { + continue; + } + } else { + $conditionType = TypeCombinator::intersect($scope->getType($expr), $type); } $conditionExpressionTypes[$exprString] = ExpressionTypeHolder::createYes( @@ -2088,7 +2114,7 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes if (count($conditionExpressionTypes) > 0) { $holders = []; - foreach ($rightTypes->getSureTypes() as $exprString => [$expr, $type]) { + foreach ($holderTypes as $exprString => [$expr, $type]) { if (!$this->isTrackableExpression($expr)) { continue; } @@ -2110,9 +2136,12 @@ private function processBooleanSureConditionalTypes(Scope $scope, SpecifiedTypes } $targetScope = $expr instanceof Expr\Variable ? $scope : $rightScope; + $holderType = $holdersAreSure + ? TypeCombinator::intersect($targetScope->getType($expr), $type) + : TypeCombinator::remove($targetScope->getType($expr), $type); $holder = new ConditionalExpressionHolder( $conditions, - ExpressionTypeHolder::createYes($expr, TypeCombinator::intersect($targetScope->getType($expr), $type)), + ExpressionTypeHolder::createYes($expr, $holderType), ); $holders[$exprString][$holder->getKey()] = $holder; } @@ -2134,6 +2163,22 @@ private function isTrackableExpression(Expr $expr): bool || $expr instanceof Expr\StaticPropertyFetch; } + private function allExpressionsTrackable(SpecifiedTypes $types): bool + { + foreach ($types->getSureTypes() as [$expr]) { + if (!$this->isTrackableExpression($expr)) { + return false; + } + } + foreach ($types->getSureNotTypes() as [$expr]) { + if (!$this->isTrackableExpression($expr)) { + return false; + } + } + + return $types->getSureTypes() !== [] || $types->getSureNotTypes() !== []; + } + /** * Flatten a deep BooleanOr chain into leaf expressions and process them * without recursive filterByFalseyValue calls. This reduces O(n^2) to O(n) @@ -2261,60 +2306,6 @@ private function specifyTypesForFlattenedBooleanAnd( return (new SpecifiedTypes($sureTypes, $sureNotTypes))->setRootExpr($expr); } - /** - * @return array - */ - private function processBooleanNotSureConditionalTypes(Scope $scope, SpecifiedTypes $leftTypes, SpecifiedTypes $rightTypes, Scope $rightScope): array - { - $conditionExpressionTypes = []; - foreach ($leftTypes->getSureNotTypes() as $exprString => [$expr, $type]) { - if (!$this->isTrackableExpression($expr)) { - continue; - } - - $conditionExpressionTypes[$exprString] = ExpressionTypeHolder::createYes( - $expr, - TypeCombinator::intersect($scope->getType($expr), $type), - ); - } - - if (count($conditionExpressionTypes) > 0) { - $holders = []; - foreach ($rightTypes->getSureNotTypes() as $exprString => [$expr, $type]) { - if (!$this->isTrackableExpression($expr)) { - continue; - } - - if (!isset($holders[$exprString])) { - $holders[$exprString] = []; - } - - $conditions = $conditionExpressionTypes; - foreach (array_keys($conditions) as $conditionExprString) { - if ($conditionExprString !== $exprString) { - continue; - } - unset($conditions[$conditionExprString]); - } - - if (count($conditions) === 0) { - continue; - } - - $targetScope = $expr instanceof Expr\Variable ? $scope : $rightScope; - $holder = new ConditionalExpressionHolder( - $conditions, - ExpressionTypeHolder::createYes($expr, TypeCombinator::remove($targetScope->getType($expr), $type)), - ); - $holders[$exprString][$holder->getKey()] = $holder; - } - - return $holders; - } - - return []; - } - /** * @return array{Expr, ConstantScalarType, Type}|null */ diff --git a/tests/PHPStan/Analyser/nsrt/bug-10644.php b/tests/PHPStan/Analyser/nsrt/bug-10644.php new file mode 100644 index 0000000000..ac9aed3ce2 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-10644.php @@ -0,0 +1,111 @@ + $data + */ +function testIssetCoalesce(array $data): void +{ + if (isset($data['subtitle']) && !is_string($data['subtitle'])) { + throw new \InvalidArgumentException('Subtitle must be a string'); + } + + if (isset($data['subtitle'])) { + assertType("string", $data['subtitle']); + } + assertType("string", $data['subtitle'] ?? ''); +} + +function testSimpleBool(bool $a, mixed $y): void +{ + if ($a && !is_string($y)) { + throw new \Exception(); + } + + if ($a) { + assertType("string", $y); + } + assertType("mixed", $y); +} + +function testSimpleInt(bool $a, mixed $y): void +{ + if ($a && !is_int($y)) { + throw new \Exception(); + } + + if ($a) { + assertType("int", $y); + } +} + +function testSimpleArray(bool $a, mixed $y): void +{ + if ($a && !is_array($y)) { + throw new \Exception(); + } + + if ($a) { + assertType("array", $y); + } +} + +function testNotNull(?int $x, mixed $y): void +{ + if ($x !== null && !is_string($y)) { + throw new \Exception(); + } + + if ($x !== null) { + assertType("string", $y); + } +} + +function testInstanceof(mixed $x, mixed $y): void +{ + if ($x instanceof \stdClass && !is_int($y)) { + throw new \Exception(); + } + + if ($x instanceof \stdClass) { + assertType("int", $y); + } +} + +/** + * @param array $data + */ +function testIssetMultipleKeys(array $data): void +{ + if (isset($data['a']) && !is_string($data['a'])) { + throw new \Exception(); + } + if (isset($data['b']) && !is_int($data['b'])) { + throw new \Exception(); + } + + if (isset($data['a'])) { + assertType("string", $data['a']); + } + if (isset($data['b'])) { + assertType("int", $data['b']); + } + assertType("string", $data['a'] ?? ''); + assertType("int", $data['b'] ?? 0); +} + +/** + * @param array $data + */ +function testArrayKeyExists(array $data): void +{ + if (array_key_exists('subtitle', $data) && !is_string($data['subtitle'])) { + throw new \Exception(); + } + if (array_key_exists('subtitle', $data)) { + assertType("string", $data['subtitle']); + } +}