Improve loose comparison on integer

phpstan · Dec 21, 2024 · aa4a123 · aa4a123
1 parent 4d2883b
commit aa4a123
Show file tree

Hide file tree

Showing 8 changed files with 131 additions and 4 deletions.
diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php
@@ -284,6 +284,11 @@ public function castsNumbersToStringsOnLooseComparison(): bool
 		return $this->versionId >= 80000;
 	}
 
+	public function nonNumericStringAndIntegerIsFalseOnLooseComparison(): bool
+	{
+		return $this->versionId >= 80000;
+	}
+
 	public function supportsCallableInstanceMethods(): bool
 	{
 		return $this->versionId < 80000;

diff --git a/src/Type/IntegerType.php b/src/Type/IntegerType.php
@@ -10,6 +10,7 @@
 use PHPStan\Type\Accessory\AccessoryNumericStringType;
 use PHPStan\Type\Accessory\AccessoryUppercaseStringType;
 use PHPStan\Type\Constant\ConstantArrayType;
+use PHPStan\Type\Constant\ConstantBooleanType;
 use PHPStan\Type\Constant\ConstantIntegerType;
 use PHPStan\Type\Traits\NonArrayTypeTrait;
 use PHPStan\Type\Traits\NonCallableTypeTrait;
@@ -139,6 +140,18 @@ public function isScalar(): TrinaryLogic
 
 	public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
 	{
+		if ($type->isArray()->yes()) {
+			return new ConstantBooleanType(false);
+		}
+
+		if (
+			$phpVersion->nonNumericStringAndIntegerIsFalseOnLooseComparison()
+			&& $type->isString()->yes()
+			&& $type->isNumericString()->no()
+		) {
+			return new ConstantBooleanType(false);
+		}
+
 		return new BooleanType();
 	}
 

diff --git a/src/Type/Traits/ConstantScalarTypeTrait.php b/src/Type/Traits/ConstantScalarTypeTrait.php
@@ -58,7 +58,7 @@ public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
 		}
 
 		if ($type->isConstantArray()->yes() && $type->isIterableAtLeastOnce()->no()) {
-			// @phpstan-ignore equal.notAllowed, equal.invalid
+			// @phpstan-ignore equal.notAllowed, equal.invalid, equal.alwaysFalse
 			return new ConstantBooleanType($this->getValue() == []); // phpcs:ignore
 		}
 

diff --git a/tests/PHPStan/Analyser/nsrt/equal.php b/tests/PHPStan/Analyser/nsrt/equal.php
@@ -135,7 +135,7 @@ public static function createStdClass(): \stdClass
 class Baz
 {
 
-	public function doFoo(string $a, int $b, float $c): void
+	public function doFoo(string $a, float $c): void
 	{
 		$nullableA = $a;
 		if (rand(0, 1)) {
@@ -152,7 +152,6 @@ public function doFoo(string $a, int $b, float $c): void
 		assertType('false', 'a' != 'a');
 		assertType('true', 'a' != 'b');
 
-		assertType('bool', $b == 'a');
 		assertType('bool', $a == 1);
 		assertType('true', 1 == 1);
 		assertType('false', 1 == 0);

diff --git a/tests/PHPStan/Analyser/nsrt/loose-comparisons-php7.php b/tests/PHPStan/Analyser/nsrt/loose-comparisons-php7.php
@@ -46,4 +46,19 @@ public function sayEmptyStr(
 	{
 		assertType('true', $emptyStr == $zero);
 	}
+
+	/**
+	 * @param 'php' $phpStr
+	 * @param '' $emptyStr
+	 */
+	public function sayInt(
+		$emptyStr,
+		$phpStr,
+		int $int
+	): void
+	{
+		assertType('bool', $int == $emptyStr);
+		assertType('bool', $int == $phpStr);
+		assertType('bool', $int == 'a');
+	}
 }
diff --git a/tests/PHPStan/Analyser/nsrt/loose-comparisons-php8.php b/tests/PHPStan/Analyser/nsrt/loose-comparisons-php8.php
@@ -46,4 +46,26 @@ public function sayEmptyStr(
 	{
 		assertType('false', $emptyStr == $zero); // PHP8+ only
 	}
+
+	/**
+	 * @param 'php' $phpStr
+	 * @param '' $emptyStr
+	 * @param int<10, 20> $intRange
+	 */
+	public function sayInt(
+		$emptyStr,
+		$phpStr,
+		int $int,
+		int $intRange
+	): void
+	{
+		assertType('false', $int == $emptyStr);
+		assertType('false', $int == $phpStr);
+		assertType('false', $int == 'a');
+
+		assertType('false', $intRange == $emptyStr);
+		assertType('false', $intRange == $phpStr);
+		assertType('false', $intRange == 'a');
+	}
+
 }
diff --git a/tests/PHPStan/Analyser/nsrt/loose-comparisons.php b/tests/PHPStan/Analyser/nsrt/loose-comparisons.php
@@ -601,4 +601,56 @@ public function sayEmptyStr(
 		assertType('false', $emptyStr == $phpStr);
 		assertType('true', $emptyStr == $emptyStr);
 	}
+
+	/**
+	 * @param true $true
+	 * @param false $false
+	 * @param 1 $one
+	 * @param 0 $zero
+	 * @param -1 $minusOne
+	 * @param '1' $oneStr
+	 * @param '0' $zeroStr
+	 * @param '-1' $minusOneStr
+	 * @param '+1' $plusOneStr
+	 * @param null $null
+	 * @param array{} $emptyArr
+	 * @param 'php' $phpStr
+	 * @param '' $emptyStr
+	 * @param int<10, 20> $intRange
+	 */
+	public function sayInt(
+		$true,
+		$false,
+		$one,
+		$zero,
+		$minusOne,
+		$oneStr,
+		$zeroStr,
+		$minusOneStr,
+		$plusOneStr,
+		$null,
+		$emptyArr,
+		array $array,
+		int $int,
+		int $intRange,
+	): void
+	{
+		assertType('bool', $int == $true);
+		assertType('bool', $int == $false);
+		assertType('bool', $int == $one);
+		assertType('bool', $int == $zero);
+		assertType('bool', $int == $minusOne);
+		assertType('bool', $int == $oneStr);
+		assertType('bool', $int == $zeroStr);
+		assertType('bool', $int == $minusOneStr);
+		assertType('bool', $int == $plusOneStr);
+		assertType('bool', $int == $null);
+		assertType('false', $int == $emptyArr);
+		assertType('false', $int == $array);
+
+		assertType('false', $intRange == $emptyArr);
+		assertType('false', $intRange == $array);
+
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php b/tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php
@@ -4,6 +4,7 @@
 
 use PHPStan\Rules\Rule;
 use PHPStan\Testing\RuleTestCase;
+use function array_merge;
 use const PHP_VERSION_ID;
 
 /**
@@ -142,7 +143,7 @@ public function testTreatPhpDocTypesAsCertain(bool $treatPhpDocTypesAsCertain, a
 
 	public function testBug11694(): void
 	{
-		$this->analyse([__DIR__ . '/data/bug-11694.php'], [
+		$expectedErrors = [
 			[
 				'Loose comparison using == between 3 and int<10, 20> will always evaluate to false.',
 				17,
@@ -173,6 +174,24 @@ public function testBug11694(): void
 				27,
 				'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
 			],
+		];
+
+		if (PHP_VERSION_ID >= 80000) {
+			$expectedErrors = array_merge($expectedErrors, [
+				[
+					"Loose comparison using == between '13foo' and int<10, 20> will always evaluate to false.",
+					29,
+					'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
+				],
+				[
+					"Loose comparison using == between int<10, 20> and '13foo' will always evaluate to false.",
+					30,
+					'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
+				],
+			]);
+		}
+
+		$expectedErrors = array_merge($expectedErrors, [
 			[
 				'Loose comparison using == between \' 3\' and int<10, 20> will always evaluate to false.',
 				32,
@@ -204,6 +223,8 @@ public function testBug11694(): void
 				'Because the type is coming from a PHPDoc, you can turn off this check by setting <fg=cyan>treatPhpDocTypesAsCertain: false</> in your <fg=cyan>%configurationFile%</>.',
 			],
 		]);
+
+		$this->analyse([__DIR__ . '/data/bug-11694.php'], $expectedErrors);
 	}
 
 }