From 67282d216f8808d8269121ed542c42a75dbfe851 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Sat, 12 Feb 2022 22:38:23 +0100 Subject: [PATCH] Normalize specified types before intersection --- src/Analyser/SpecifiedTypes.php | 32 +++- src/Analyser/TypeSpecifier.php | 4 + .../Analyser/NodeScopeResolverTest.php | 1 + tests/PHPStan/Analyser/TypeSpecifierTest.php | 39 +++++ tests/PHPStan/Analyser/data/bug-6329.php | 160 ++++++++++++++++++ 5 files changed, 230 insertions(+), 6 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-6329.php diff --git a/src/Analyser/SpecifiedTypes.php b/src/Analyser/SpecifiedTypes.php index 92963fb8264..af9af5ed5d5 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,21 @@ public function unionWith(SpecifiedTypes $other): self return new self($sureTypeUnion, $sureNotTypeUnion); } + private function normalize(): self + { + $sureTypes = $this->sureTypes; + $sureNotTypes = []; + + foreach ($this->sureNotTypes as $exprString => [$exprNode, $type]) { + if (!isset($sureTypes[$exprString])) { + $sureNotTypes[$exprString] = [$exprNode, $type]; + continue; + } + + $sureTypes[$exprString][1] = TypeCombinator::remove($sureTypes[$exprString][1], $type); + } + + return new self($sureTypes, $sureNotTypes, $this->overwrite, $this->newConditionalExpressionHolders); + } + } diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index b59275d157c..a2509cc1c4b 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -482,6 +482,8 @@ public function specifyTypesInCondition( $argType = $scope->getType($expr->right->getArgs()[0]->value); if ($argType->isArray()->yes()) { $result = $result->unionWith($this->create($expr->right->getArgs()[0]->value, new NonEmptyArrayType(), $context, false, $scope)); + } else { + $result = $result->unionWith($this->create($expr->right->getArgs()[0]->value, new ConstantArrayType([], []), $context->negate(), false, $scope)); } } } @@ -501,6 +503,8 @@ public function specifyTypesInCondition( $argType = $scope->getType($expr->right->getArgs()[0]->value); if ($argType instanceof StringType) { $result = $result->unionWith($this->create($expr->right->getArgs()[0]->value, new AccessoryNonEmptyStringType(), $context, false, $scope)); + } else { + $result = $result->unionWith($this->create($expr->right->getArgs()[0]->value, new ConstantStringType(''), $context->negate(), false, $scope)); } } } diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index 3535aac6dc0..0795d0586b9 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 7c8aa8986ba..68a4133b6d0 100644 --- a/tests/PHPStan/Analyser/TypeSpecifierTest.php +++ b/tests/PHPStan/Analyser/TypeSpecifierTest.php @@ -965,6 +965,45 @@ 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' => '~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' => '~null'], + ], ]; } diff --git a/tests/PHPStan/Analyser/data/bug-6329.php b/tests/PHPStan/Analyser/data/bug-6329.php new file mode 100644 index 00000000000..27590b3d7ac --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-6329.php @@ -0,0 +1,160 @@ + 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); + } +}