Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve loose comparison on integer #3748

Merged
merged 2 commits into from
Dec 21, 2024
Merged

Improve loose comparison on integer #3748

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
72ac21c
Improve loose comparison on integer
staabm Dec 21, 2024
7db5ac6
test integer ranges
staabm Dec 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/Php/PhpVersion.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions src/Type/IntegerType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -139,6 +140,18 @@ public function isScalar(): TrinaryLogic

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have a new todo for PHPStan 3.x: adjust all Type methods which depend on PhpVersion and turn them into using PhpVersions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this should be possible. It's possible to make PhpVersions from PhpVersion (without losing precision), but not vice-versa.

So places that only have PhpVersion could still call these methods by transforming it into PhpVersions first.

{
if ($type->isArray()->yes()) {
return new ConstantBooleanType(false);
}

if (
$phpVersion->nonNumericStringAndIntegerIsFalseOnLooseComparison()
&& $type->isString()->yes()
&& $type->isNumericString()->no()
) {
return new ConstantBooleanType(false);
}

return new BooleanType();
}

Expand Down
2 changes: 1 addition & 1 deletion src/Type/Traits/ConstantScalarTypeTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ignores

------ ------------------------------------------------------------------------------------------------------------- 
  Line   src/Type/Traits/ConstantScalarTypeTrait.php (in context of class PHPStan\Type\Constant\ConstantIntegerType)  
 ------ ------------------------------------------------------------------------------------------------------------- 
  62     Loose comparison using == between int and array{} will always evaluate to false.                             
         🪪  equal.alwaysFalse                                                                                        
 ------ -------------------------------------------------------------------------------------------------------------

return new ConstantBooleanType($this->getValue() == []); // phpcs:ignore
}

Expand Down
3 changes: 1 addition & 2 deletions tests/PHPStan/Analyser/nsrt/equal.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand All @@ -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');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved this case into the loose-comparison-* files, as it depends on php version

assertType('bool', $a == 1);
assertType('true', 1 == 1);
assertType('false', 1 == 0);
Expand Down
15 changes: 15 additions & 0 deletions tests/PHPStan/Analyser/nsrt/loose-comparisons-php7.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
22 changes: 22 additions & 0 deletions tests/PHPStan/Analyser/nsrt/loose-comparisons-php8.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}

}
52 changes: 52 additions & 0 deletions tests/PHPStan/Analyser/nsrt/loose-comparisons.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}

}
23 changes: 22 additions & 1 deletion tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use PHPStan\Rules\Rule;
use PHPStan\Testing\RuleTestCase;
use function array_merge;
use const PHP_VERSION_ID;

/**
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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%</>.',
],
]);
}
Comment on lines +179 to +192
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://3v4l.org/W28WC


$expectedErrors = array_merge($expectedErrors, [
[
'Loose comparison using == between \' 3\' and int<10, 20> will always evaluate to false.',
32,
Expand Down Expand Up @@ -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);
}

}
Loading