From 45ee1042e80e8dc1a7fd17dad98fe3bdcd6fb139 Mon Sep 17 00:00:00 2001 From: Martin Herndl Date: Mon, 9 May 2022 10:53:03 +0200 Subject: [PATCH] Fix false positives #118, #119, #130 --- .../AssertTypeSpecifyingExtension.php | 61 ++++++------------- .../ImpossibleCheckTypeMethodCallRuleTest.php | 19 ++++++ tests/Type/WebMozartAssert/data/bug-118.php | 14 +++++ tests/Type/WebMozartAssert/data/bug-119.php | 17 ++++++ tests/Type/WebMozartAssert/data/bug-130.php | 22 +++++++ .../Type/WebMozartAssert/data/collection.php | 9 +++ .../WebMozartAssert/data/impossible-check.php | 9 +++ 7 files changed, 110 insertions(+), 41 deletions(-) create mode 100644 tests/Type/WebMozartAssert/data/bug-118.php create mode 100644 tests/Type/WebMozartAssert/data/bug-119.php create mode 100644 tests/Type/WebMozartAssert/data/bug-130.php diff --git a/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php b/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php index a2ff1ce..ceeef05 100644 --- a/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php +++ b/src/Type/WebMozartAssert/AssertTypeSpecifyingExtension.php @@ -8,6 +8,7 @@ use PhpParser\Node\Arg; use PhpParser\Node\Expr; use PhpParser\Node\Expr\Array_; +use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\ArrayItem; use PhpParser\Node\Expr\BinaryOp; use PhpParser\Node\Expr\BinaryOp\BooleanAnd; @@ -25,12 +26,9 @@ use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\Instanceof_; use PhpParser\Node\Expr\StaticCall; -use PhpParser\Node\Expr\Variable; use PhpParser\Node\Name; -use PhpParser\Node\Param; use PhpParser\Node\Scalar\LNumber; use PhpParser\Node\Scalar\String_; -use PhpParser\Node\Stmt\Return_; use PHPStan\Analyser\Scope; use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\TypeSpecifier; @@ -44,6 +42,7 @@ use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\IterableType; use PHPStan\Type\MixedType; +use PHPStan\Type\NeverType; use PHPStan\Type\ObjectType; use PHPStan\Type\StaticMethodTypeSpecifyingExtension; use PHPStan\Type\StringType; @@ -59,7 +58,6 @@ use function array_reduce; use function array_shift; use function count; -use function key; use function lcfirst; use function substr; @@ -774,33 +772,12 @@ private function handleAll( Scope $scope ): SpecifiedTypes { - $closureItemVariable = new Variable('item'); - $closureArgs = $node->getArgs(); - $closureArgs[0] = new Arg($closureItemVariable); - - $expression = new BooleanAnd( - new FuncCall(new Name('is_iterable'), [$node->getArgs()[0]]), - new Identical( - $node->getArgs()[0]->value, - new FuncCall( - new Name('array_filter'), - [ - new Arg($node->getArgs()[0]->value), - new Arg( - new Expr\Closure( - [ - 'static' => true, - 'params' => [new Param($closureItemVariable)], - 'stmts' => [ - new Return_(self::createExpression($scope, $methodName, $closureArgs)), - ], - ] - ) - ), - ] - ) - ) - ); + $args = $node->getArgs(); + $args[0] = new Arg(new ArrayDimFetch($args[0]->value, new LNumber(0))); + $expression = self::createExpression($scope, $methodName, $args); + if ($expression === null) { + return new SpecifiedTypes(); + } $specifiedTypes = $this->typeSpecifier->specifyTypesInCondition( $scope, @@ -808,22 +785,22 @@ private function handleAll( TypeSpecifierContext::createTruthy() ); - if (count($specifiedTypes->getSureTypes()) > 0) { - $sureTypes = $specifiedTypes->getSureTypes(); - $exprString = key($sureTypes); - [$exprNode, $type] = $sureTypes[$exprString]; + $sureNotTypes = $specifiedTypes->getSureNotTypes(); + foreach ($specifiedTypes->getSureTypes() as $exprStr => [$exprNode, $type]) { + if ($exprNode !== $args[0]->value) { + continue; + } + + $type = TypeCombinator::remove($type, $sureNotTypes[$exprStr][1] ?? new NeverType()); return $this->arrayOrIterable( $scope, - $exprNode, + $node->getArgs()[0]->value, static function () use ($type): Type { - return $type->getIterableValueType(); + return $type; } ); } - if (count($specifiedTypes->getSureNotTypes()) > 0) { - throw new ShouldNotHappenException(); - } return $specifiedTypes; } @@ -861,7 +838,9 @@ private function arrayOrIterable( return $this->typeSpecifier->create( $expr, $specifiedType, - TypeSpecifierContext::createTruthy() + TypeSpecifierContext::createTruthy(), + false, + $scope ); } diff --git a/tests/Type/WebMozartAssert/ImpossibleCheckTypeMethodCallRuleTest.php b/tests/Type/WebMozartAssert/ImpossibleCheckTypeMethodCallRuleTest.php index e56c0dd..a7dc0f9 100644 --- a/tests/Type/WebMozartAssert/ImpossibleCheckTypeMethodCallRuleTest.php +++ b/tests/Type/WebMozartAssert/ImpossibleCheckTypeMethodCallRuleTest.php @@ -64,6 +64,10 @@ public function testExtension(): void 'Call to static method Webmozart\Assert\Assert::nullOrStringNotEmpty() with non-empty-string|null will always evaluate to true.', 53, ], + [ + 'Call to static method Webmozart\Assert\Assert::allCount() with array and 2 will always evaluate to true.', + 76, + ], ]); } @@ -175,6 +179,21 @@ public function testBug85(): void $this->analyse([__DIR__ . '/data/bug-85.php'], []); } + public function testBug118(): void + { + $this->analyse([__DIR__ . '/data/bug-118.php'], []); + } + + public function testBug119(): void + { + $this->analyse([__DIR__ . '/data/bug-119.php'], []); + } + + public function testBug130(): void + { + $this->analyse([__DIR__ . '/data/bug-130.php'], []); + } + public static function getAdditionalConfigFiles(): array { return [ diff --git a/tests/Type/WebMozartAssert/data/bug-118.php b/tests/Type/WebMozartAssert/data/bug-118.php new file mode 100644 index 0000000..25486f5 --- /dev/null +++ b/tests/Type/WebMozartAssert/data/bug-118.php @@ -0,0 +1,14 @@ + */ + protected $orders = []; + + public function setOrders(array $orders): self + { + Assert::allCount($orders, 2); + Assert::allKeyExists($orders, 'id'); + Assert::allKeyExists($orders, 'num_order'); + + $this->orders = $orders; + + return $this; + } +} diff --git a/tests/Type/WebMozartAssert/data/collection.php b/tests/Type/WebMozartAssert/data/collection.php index 72ddaf2..6f16fb4 100644 --- a/tests/Type/WebMozartAssert/data/collection.php +++ b/tests/Type/WebMozartAssert/data/collection.php @@ -120,6 +120,15 @@ public function allKeyExists(array $a, array $b, array $c, $d): void \PHPStan\Testing\assertType('array', $c); } + /** + * @param array $a + */ + public function allCount(array $a): void + { + Assert::allCount($a, 2); + \PHPStan\Testing\assertType('array', $a); + } + } class CollectionFoo diff --git a/tests/Type/WebMozartAssert/data/impossible-check.php b/tests/Type/WebMozartAssert/data/impossible-check.php index 3a07fbb..93972b3 100644 --- a/tests/Type/WebMozartAssert/data/impossible-check.php +++ b/tests/Type/WebMozartAssert/data/impossible-check.php @@ -67,6 +67,15 @@ public function notSame(Bar $a, Bar $b): void Assert::notSame(Baz::create(), Baz::create()); } + /** + * @param array $a + */ + public function allCount(array $a): void + { + Assert::allCount($a, 2); + Assert::allCount($a, 2); + } + } interface Bar {};