diff --git a/src/Analyser/SpecifiedTypes.php b/src/Analyser/SpecifiedTypes.php index 92963fb826..0feb361336 100644 --- a/src/Analyser/SpecifiedTypes.php +++ b/src/Analyser/SpecifiedTypes.php @@ -58,28 +58,31 @@ public function getNewConditionalExpressionHolders(): array /** @api */ public function intersectWith(SpecifiedTypes $other): self { + $normalized = $this->normalize(); + $otherNormalized = $other->normalize(); + $sureTypeUnion = []; $sureNotTypeUnion = []; - foreach ($this->sureTypes as $exprString => [$exprNode, $type]) { - if (!isset($other->sureTypes[$exprString])) { + foreach ($normalized->sureTypes as $exprString => [$exprNode, $type]) { + if (!isset($otherNormalized->sureTypes[$exprString])) { continue; } $sureTypeUnion[$exprString] = [ $exprNode, - TypeCombinator::union($type, $other->sureTypes[$exprString][1]), + TypeCombinator::union($type, $otherNormalized->sureTypes[$exprString][1]), ]; } - foreach ($this->sureNotTypes as $exprString => [$exprNode, $type]) { - if (!isset($other->sureNotTypes[$exprString])) { + foreach ($normalized->sureNotTypes as $exprString => [$exprNode, $type]) { + if (!isset($otherNormalized->sureNotTypes[$exprString])) { continue; } $sureNotTypeUnion[$exprString] = [ $exprNode, - TypeCombinator::intersect($type, $other->sureNotTypes[$exprString][1]), + TypeCombinator::intersect($type, $otherNormalized->sureNotTypes[$exprString][1]), ]; } @@ -117,4 +120,26 @@ public function unionWith(SpecifiedTypes $other): self return new self($sureTypeUnion, $sureNotTypeUnion); } + public function inverse(): self + { + return new self($this->sureNotTypes, $this->sureTypes, $this->overwrite, $this->newConditionalExpressionHolders); + } + + private function normalize(): self + { + $sureTypes = $this->sureTypes; + $sureNotTypes = []; + + foreach ($this->sureNotTypes as $exprString => [$exprNode, $sureNotType]) { + if (!isset($sureTypes[$exprString])) { + $sureNotTypes[$exprString] = [$exprNode, $sureNotType]; + continue; + } + + $sureTypes[$exprString][1] = TypeCombinator::remove($sureTypes[$exprString][1], $sureNotType); + } + + return new self($sureTypes, $sureNotTypes, $this->overwrite, $this->newConditionalExpressionHolders); + } + } diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index b59275d157..44bc014119 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -666,8 +666,12 @@ public function specifyTypesInCondition( return $this->handleDefaultTruthyOrFalseyContext($context, $expr, $scope); } elseif ($expr instanceof BooleanAnd || $expr instanceof LogicalAnd) { + if (!$scope instanceof MutatingScope) { + throw new ShouldNotHappenException(); + } $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context); - $rightTypes = $this->specifyTypesInCondition($scope, $expr->right, $context); + $leftScope = $scope->filterBySpecifiedTypes($context->true() ? $leftTypes : $leftTypes->inverse()); + $rightTypes = $this->specifyTypesInCondition($leftScope, $expr->right, $context); $types = $context->true() ? $leftTypes->unionWith($rightTypes) : $leftTypes->intersectWith($rightTypes); if ($context->false()) { return new SpecifiedTypes( @@ -683,8 +687,12 @@ public function specifyTypesInCondition( return $types; } elseif ($expr instanceof BooleanOr || $expr instanceof LogicalOr) { + if (!$scope instanceof MutatingScope) { + throw new ShouldNotHappenException(); + } $leftTypes = $this->specifyTypesInCondition($scope, $expr->left, $context); - $rightTypes = $this->specifyTypesInCondition($scope, $expr->right, $context); + $leftScope = $scope->filterBySpecifiedTypes($context->true() ? $leftTypes->inverse() : $leftTypes); + $rightTypes = $this->specifyTypesInCondition($leftScope, $expr->right, $context); $types = $context->true() ? $leftTypes->intersectWith($rightTypes) : $leftTypes->unionWith($rightTypes); if ($context->true()) { return new SpecifiedTypes( diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index 3535aac6dc..0795d0586b 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -692,6 +692,7 @@ public function dataFileAsserts(): iterable if (PHP_VERSION_ID >= 80000) { yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6308.php'); } + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6329.php'); if (PHP_VERSION_ID >= 70400) { yield from $this->gatherAssertTypes(__DIR__ . '/../Rules/Comparison/data/bug-6473.php'); diff --git a/tests/PHPStan/Analyser/TypeSpecifierTest.php b/tests/PHPStan/Analyser/TypeSpecifierTest.php index 7c8aa8986b..d86f095c54 100644 --- a/tests/PHPStan/Analyser/TypeSpecifierTest.php +++ b/tests/PHPStan/Analyser/TypeSpecifierTest.php @@ -8,6 +8,7 @@ use PhpParser\Node\Expr\BinaryOp\Identical; use PhpParser\Node\Expr\BinaryOp\NotIdentical; use PhpParser\Node\Expr\BooleanNot; +use PhpParser\Node\Expr\ConstFetch; use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\Variable; @@ -965,6 +966,62 @@ public function dataCondition(): array ], [], ], + [ + new Expr\BinaryOp\BooleanOr( + new Expr\BinaryOp\BooleanAnd( + $this->createFunctionCall('is_string', 'a'), + new NotIdentical(new String_(''), new Variable('a')), + ), + new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')), + ), + ['$a' => 'non-empty-string|null'], + ['$a' => '~null'], + ], + [ + new Expr\BinaryOp\BooleanOr( + new Expr\BinaryOp\BooleanAnd( + $this->createFunctionCall('is_string', 'a'), + new Expr\BinaryOp\Greater( + $this->createFunctionCall('strlen', 'a'), + new LNumber(0), + ), + ), + new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')), + ), + ['$a' => 'non-empty-string|null'], + ['$a' => '~non-empty-string|null'], + ], + [ + new Expr\BinaryOp\BooleanOr( + new Expr\BinaryOp\BooleanAnd( + $this->createFunctionCall('is_array', 'a'), + new Expr\BinaryOp\Greater( + $this->createFunctionCall('count', 'a'), + new LNumber(0), + ), + ), + new Identical(new Expr\ConstFetch(new Name('null')), new Variable('a')), + ), + ['$a' => 'non-empty-array|null'], + ['$a' => '~non-empty-array|null'], + ], + [ + new Expr\BinaryOp\BooleanAnd( + $this->createFunctionCall('is_array', 'foo'), + new Identical( + new FuncCall( + new Name('array_filter'), + [new Arg(new Variable('foo')), new Arg(new String_('is_string')), new Arg(new ConstFetch(new Name('ARRAY_FILTER_USE_KEY')))], + ), + new Variable('foo'), + ), + ), + [ + '$foo' => 'array', + 'array_filter($foo, \'is_string\', ARRAY_FILTER_USE_KEY)' => 'array', + ], + [], + ], ]; } diff --git a/tests/PHPStan/Analyser/data/bug-6329.php b/tests/PHPStan/Analyser/data/bug-6329.php new file mode 100644 index 0000000000..c6b49e87e4 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-6329.php @@ -0,0 +1,185 @@ + 0 || null === $a) { + assertType('non-empty-string|null', $a); + } + + if (null === $a || is_string($a) && strlen($a) > 0) { + assertType('non-empty-string|null', $a); + } +} + + +/** + * @param mixed $a + */ +function int1($a): void +{ + if (is_int($a) && 0 !== $a || null === $a) { + assertType('int|int<1, max>|null', $a); + } + + if (0 !== $a && is_int($a) || null === $a) { + assertType('int|int<1, max>|null', $a); + } + + if (null === $a || is_int($a) && 0 !== $a) { + assertType('int|int<1, max>|null', $a); + } + + if (null === $a || 0 !== $a && is_int($a)) { + assertType('int|int<1, max>|null', $a); + } +} + +/** + * @param mixed $a + */ +function int2($a): void +{ + if (is_int($a) && $a > 0 || null === $a) { + assertType('int<1, max>|null', $a); + } + + if (null === $a || is_int($a) && $a > 0) { + assertType('int<1, max>|null', $a); + } +} + + +/** + * @param mixed $a + */ +function true($a): void +{ + if (is_bool($a) && false !== $a || null === $a) { + assertType('true|null', $a); + } + + if (false !== $a && is_bool($a) || null === $a) { + assertType('true|null', $a); + } + + if (null === $a || is_bool($a) && false !== $a) { + assertType('true|null', $a); + } + + if (null === $a || false !== $a && is_bool($a)) { + assertType('true|null', $a); + } +} + +/** + * @param mixed $a + */ +function nonEmptyArray1($a): void +{ + if (is_array($a) && [] !== $a || null === $a) { + assertType('non-empty-array|null', $a); + } + + if ([] !== $a && is_array($a) || null === $a) { + assertType('non-empty-array|null', $a); + } + + if (null === $a || is_array($a) && [] !== $a) { + assertType('non-empty-array|null', $a); + } + + if (null === $a || [] !== $a && is_array($a)) { + assertType('non-empty-array|null', $a); + } +} + +/** + * @param mixed $a + */ +function nonEmptyArray2($a): void +{ + if (is_array($a) && count($a) > 0 || null === $a) { + assertType('non-empty-array|null', $a); + } + + if (null === $a || is_array($a) && count($a) > 0) { + assertType('non-empty-array|null', $a); + } +} + +/** + * @param mixed $a + * @param mixed $b + * @param mixed $c + */ +function inverse($a, $b, $c): void +{ + if ((!is_string($a) || '' === $a) && null !== $a) { + } else { + assertType('non-empty-string|null', $a); + } + + if ((!is_int($b) || $b <= 0) && null !== $b) { + } else { + assertType('int<1, max>|null', $b); + } + + if (null !== $c && (!is_array($c) || count($c) <= 0)) { + } else { + assertType('non-empty-array|null', $c); + } +} + +/** + * @param mixed $a + * @param mixed $b + * @param mixed $c + * @param mixed $d + */ +function combinations($a, $b, $c, $d): void +{ + if (is_string($a) && '' !== $a || is_int($a) && $a > 0 || null === $a) { + assertType('int<1, max>|non-empty-string|null', $a); + } + if ((!is_string($b) || '' === $b) && (!is_int($b) || $b <= 0) && null !== $b) { + } else { + assertType('int<1, max>|non-empty-string|null', $b); + } + + if (is_array($c) && $c === array_filter($c, 'is_string', ARRAY_FILTER_USE_KEY) || null === $c) { + assertType('array|null', $c); + } + if ((!is_array($d) || $d !== array_filter($d, 'is_string', ARRAY_FILTER_USE_KEY)) && null !== $d) { + } else { + assertType('array|null', $d); + } +}