diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 01ab4208a3..fe4ec9b65d 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -727,6 +727,7 @@ public function specifyTypesInCondition( if ($context->false()) { $leftTypesForHolders = $leftTypes; $rightTypesForHolders = $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 = $this->specifyTypesInCondition($scope, $expr->left, TypeSpecifierContext::createFalsey())->setRootExpr($expr); @@ -735,6 +736,20 @@ public function specifyTypesInCondition( $rightTypesForHolders = $this->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() === []) { + $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(), @@ -747,6 +762,10 @@ public function specifyTypesInCondition( $this->processBooleanConditionalTypes($scope, $rightTypesForHolders, false, $leftTypesForHolders, false, $scope), $this->processBooleanConditionalTypes($scope, $leftTypesForHolders, true, $rightTypesForHolders, true, $rightScope), $this->processBooleanConditionalTypes($scope, $rightTypesForHolders, true, $leftTypesForHolders, true, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypesForHolders, false, $rightTypesForHolders, true, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypesForHolders, false, $leftTypesForHolders, true, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypesForHolders, true, $rightTypesForHolders, false, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypesForHolders, true, $leftTypesForHolders, false, $scope), ))->setRootExpr($expr); } @@ -800,6 +819,10 @@ public function specifyTypesInCondition( $this->processBooleanConditionalTypes($scope, $rightTypes, false, $leftTypes, false, $scope), $this->processBooleanConditionalTypes($scope, $leftTypes, true, $rightTypes, true, $rightScope), $this->processBooleanConditionalTypes($scope, $rightTypes, true, $leftTypes, true, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypes, false, $rightTypes, true, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypes, false, $leftTypes, true, $scope), + $this->processBooleanConditionalTypes($scope, $leftTypes, true, $rightTypes, false, $rightScope), + $this->processBooleanConditionalTypes($scope, $rightTypes, true, $leftTypes, false, $scope), ))->setRootExpr($expr); } @@ -2143,6 +2166,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) diff --git a/tests/PHPStan/Analyser/nsrt/boolean-and-conditional-holders-mixed-context.php b/tests/PHPStan/Analyser/nsrt/boolean-and-conditional-holders-mixed-context.php new file mode 100644 index 0000000000..c64c60002e --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/boolean-and-conditional-holders-mixed-context.php @@ -0,0 +1,61 @@ + $data + */ +function issetVarKey(array $data, string $key): void +{ + if ((isset($data[$key]) && !is_string($data[$key])) === true) { + return; + } + + assertType('string', $data[$key] ?? 'fallback'); +} + +/** + * @param array $data + */ +function issetConstKey(array $data): void +{ + if ((isset($data['k']) && !is_string($data['k'])) === true) { + return; + } + + assertType('string', $data['k'] ?? 'fallback'); +} + +/** + * @param array $data + */ +function arrayKeyExistsMixed(array $data): void +{ + if ((array_key_exists('k', $data) && !is_string($data['k'])) === true) { + return; + } + + assertType('string', $data['k'] ?? 'fallback'); +} + +/** + * @param mixed $y + */ +function simpleBoolMixed(bool $a, $y): void +{ + if (($a && !is_string($y)) === true) { + return; + } + + if ($a) { + assertType('string', $y); + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-10644.php b/tests/PHPStan/Analyser/nsrt/bug-10644.php new file mode 100644 index 0000000000..91a9e07075 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-10644.php @@ -0,0 +1,127 @@ + $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'] ?? ''); +} + +/** + * @param mixed $y + */ +function testSimpleBool(bool $a, $y): void +{ + if ($a && !is_string($y)) { + throw new \Exception(); + } + + if ($a) { + assertType("string", $y); + } + assertType("mixed", $y); +} + +/** + * @param mixed $y + */ +function testSimpleInt(bool $a, $y): void +{ + if ($a && !is_int($y)) { + throw new \Exception(); + } + + if ($a) { + assertType("int", $y); + } +} + +/** + * @param mixed $y + */ +function testSimpleArray(bool $a, $y): void +{ + if ($a && !is_array($y)) { + throw new \Exception(); + } + + if ($a) { + assertType("array", $y); + } +} + +/** + * @param mixed $y + */ +function testNotNull(?int $x, $y): void +{ + if ($x !== null && !is_string($y)) { + throw new \Exception(); + } + + if ($x !== null) { + assertType("string", $y); + } +} + +/** + * @param mixed $x + * @param mixed $y + */ +function testInstanceof($x, $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']); + } +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-11918.php b/tests/PHPStan/Analyser/nsrt/bug-11918.php new file mode 100644 index 0000000000..9e5cab77a4 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-11918.php @@ -0,0 +1,18 @@ +|string|false> $options + */ +function testArrayKeyExistsCoalesce(array $options): void +{ + if (array_key_exists('a', $options) && !is_string($options['a'])) { + exit(1); + } + + $a = $options['a'] ?? 'fallback'; + assertType('string', $a); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-14455.php b/tests/PHPStan/Analyser/nsrt/bug-14455.php new file mode 100644 index 0000000000..d6b6c8b92f --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-14455.php @@ -0,0 +1,27 @@ + $aggregation + * @param non-falsy-string $type + */ +function testTriviallyTrueConditionSkipped(array $aggregation, string $type): void +{ + if (empty($aggregation['field']) && $type === 'filter') { + return; + } + + assertType("array", $aggregation); + assertType('non-falsy-string', $type); + + if ($type === 'filter') { + assertType("non-empty-array&hasOffset('field')", $aggregation); + } else { + assertType("array", $aggregation); + } + + assertType('non-falsy-string', $type); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-3385.php b/tests/PHPStan/Analyser/nsrt/bug-3385.php new file mode 100644 index 0000000000..dc371da847 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-3385.php @@ -0,0 +1,40 @@ +sayHello() === $otherGreeter->sayHello(); + } + +} + +function isGreeterDifferent(?Greeter $greeterA, ?Greeter $greeterB): bool +{ + if ($greeterA === null && $greeterB !== null) { + return true; + } + + if ($greeterA !== null && $greeterB === null) { + return true; + } + + if ($greeterA === null && $greeterB === null) { + return false; + } + + assertType('Bug3385\Greeter', $greeterA); + assertType('Bug3385\Greeter', $greeterB); + + return $greeterA->isEqualTo($greeterB); +} diff --git a/tests/PHPStan/Analyser/nsrt/bug-6202.php b/tests/PHPStan/Analyser/nsrt/bug-6202.php new file mode 100644 index 0000000000..580196103e --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-6202.php @@ -0,0 +1,21 @@ + $array + */ + public function sayHello(array $array): void + { + if (isset($array['mightExist']) && !is_string($array['mightExist'])) { + throw new \Exception('Has to be string if set'); + } + assertType('string', $array['mightExist'] ?? ''); + } + +} diff --git a/tests/PHPStan/Analyser/nsrt/negated-boolean-and-conditional-holders.php b/tests/PHPStan/Analyser/nsrt/negated-boolean-and-conditional-holders.php new file mode 100644 index 0000000000..5b15a705b2 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/negated-boolean-and-conditional-holders.php @@ -0,0 +1,72 @@ + $data + */ +function negatedIsset(array $data): void +{ + if (!(isset($data['subtitle']) && !is_string($data['subtitle']))) { + // key is either unset or a string + assertType('string', $data['subtitle'] ?? 'fallback'); + if (isset($data['subtitle'])) { + assertType('string', $data['subtitle']); + } + } +} + +/** + * @param mixed $y + */ +function negatedSimpleBool(bool $a, $y): void +{ + if (!($a && !is_string($y))) { + if ($a) { + assertType('string', $y); + } + } +} + +/** + * @param mixed $y + */ +function negatedSimpleInt(bool $a, $y): void +{ + if (!($a && !is_int($y))) { + if ($a) { + assertType('int', $y); + } + } +} + +/** + * @param mixed $y + */ +function negatedNotNull(?int $x, $y): void +{ + if (!($x !== null && !is_string($y))) { + if ($x !== null) { + assertType('string', $y); + } + } +} + +/** + * @param array $data + */ +function negatedArrayKeyExists(array $data): void +{ + if (!(array_key_exists('subtitle', $data) && !is_string($data['subtitle']))) { + if (array_key_exists('subtitle', $data)) { + assertType('string', $data['subtitle']); + } + } +}