Fix false positives #118, #119, #130

phpstan · May 9, 2022 · 45ee104 · 45ee104
1 parent a83a4d2
commit 45ee104
Show file tree

Hide file tree

Showing 7 changed files with 110 additions and 41 deletions.
diff --git 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,56 +772,35 @@ 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,
 			$expression,
 			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
@@ -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<non-empty-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
@@ -0,0 +1,14 @@
+<?php
+
+declare(strict_types=1);
+
+namespace WebmozartAssertBug118;
+
+use DateTime;
+use Webmozart\Assert\Assert;
+
+function test(float $a, DateTime $b): void
+{
+	Assert::range($a, 0, 1);
+	Assert::range($b, 123456789, 9876543321);
+}
diff --git a/tests/Type/WebMozartAssert/data/bug-119.php b/tests/Type/WebMozartAssert/data/bug-119.php
@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace WebmozartAssertBug119;
+
+use DateTime;
+use Webmozart\Assert\Assert;
+
+function test(float $a, float $b, float $c, float $d, DateTime  $e): void
+{
+	Assert::greaterThan($a, 0);
+	Assert::greaterThanEq($b, 0);
+	Assert::lessThan($c, 0);
+	Assert::lessThanEq($d, 0);
+	Assert::greaterThanEq($e, 639828000);
+}
diff --git a/tests/Type/WebMozartAssert/data/bug-130.php b/tests/Type/WebMozartAssert/data/bug-130.php
@@ -0,0 +1,22 @@
+<?php declare(strict_types=1);
+
+namespace WebmozartAssertBug130;
+
+use Webmozart\Assert\Assert;
+
+class Bug130
+{
+	/** @var array<int, array{id: string, num_order: string}> */
+	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
@@ -120,6 +120,15 @@ public function allKeyExists(array $a, array $b, array $c, $d): void
 		\PHPStan\Testing\assertType('array<array&hasOffset(\'id\')>', $c);
 	}
 
+	/**
+	 * @param array<array> $a
+	 */
+	public function allCount(array $a): void
+	{
+		Assert::allCount($a, 2);
+		\PHPStan\Testing\assertType('array<non-empty-array>', $a);
+	}
+
 }
 
 class CollectionFoo

diff --git 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<array> $a
+	 */
+	public function allCount(array $a): void
+	{
+		Assert::allCount($a, 2);
+		Assert::allCount($a, 2);
+	}
+
 }
 
 interface Bar {};