Skip to content

Improve loose comparison on union type #3750

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

Merged
merged 7 commits into from
Dec 23, 2024
Merged

Improve loose comparison on union type #3750

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
33ff84d
Improve loose comparison on union type
staabm Dec 22, 2024
5a94e38
Improve loose comparison on intersection type
staabm Dec 22, 2024
07c6a0d
Improve loose comparison on array type
staabm Dec 22, 2024
b5ecab0
Improve loose comparison on uppercase/lowercase string
staabm Dec 22, 2024
684c599
Added regression test
staabm Dec 22, 2024
6bb6d92
Improve loose comparison on numeric string
VincentLanglet Dec 22, 2024
04091a1
Add BooleanType->toTrinaryLogic()
staabm Dec 23, 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
12 changes: 12 additions & 0 deletions src/TrinaryLogic.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,18 @@ public static function createFromBoolean(bool $value): self
return self::$registry[$yesNo] ??= new self($yesNo);
}

public static function createFromBooleanType(BooleanType $type): self
Copy link
Member

Choose a reason for hiding this comment

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

You had the same idea, nice! But I'd actually like BooleanType::toTrinaryLogic() more :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hehe I see. done

{
if ($type->isTrue()->yes()) {
return self::createYes();
}
if ($type->isFalse()->yes()) {
return self::createNo();
}

return self::createMaybe();
}

private static function create(int $value): self
{
self::$registry[$value] ??= new self($value);
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Accessory/AccessoryLowercaseStringType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use PHPStan\Type\BooleanType;
use PHPStan\Type\CompoundType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\ErrorType;
use PHPStan\Type\FloatType;
Expand Down Expand Up @@ -326,6 +327,10 @@ public function isScalar(): TrinaryLogic

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
{
if ($type->isString()->yes() && $type->isLowercaseString()->no()) {
return new ConstantBooleanType(false);
}

return new BooleanType();
}

Expand Down
4 changes: 4 additions & 0 deletions src/Type/Accessory/AccessoryNonEmptyStringType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use PHPStan\Type\BooleanType;
use PHPStan\Type\CompoundType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\ErrorType;
Expand Down Expand Up @@ -322,6 +323,9 @@ public function isScalar(): TrinaryLogic

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
{
if ($type->isString()->yes() && $type->isNonEmptyString()->no()) {
return new ConstantBooleanType(false);
}
return new BooleanType();
}

Expand Down
5 changes: 5 additions & 0 deletions src/Type/Accessory/AccessoryUppercaseStringType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use PHPStan\Type\BooleanType;
use PHPStan\Type\CompoundType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\ErrorType;
use PHPStan\Type\FloatType;
Expand Down Expand Up @@ -326,6 +327,10 @@ public function isScalar(): TrinaryLogic

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
{
if ($type->isString()->yes() && $type->isUppercaseString()->no()) {
return new ConstantBooleanType(false);
}

return new BooleanType();
}

Expand Down
5 changes: 5 additions & 0 deletions src/Type/Accessory/NonEmptyArrayType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use PHPStan\Type\AcceptsResult;
use PHPStan\Type\BooleanType;
use PHPStan\Type\CompoundType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantFloatType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\ErrorType;
Expand Down Expand Up @@ -402,6 +403,10 @@ public function isScalar(): TrinaryLogic

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
{
if ($type->isArray()->yes() && $type->isIterableAtLeastOnce()->no()) {
return new ConstantBooleanType(false);
}

return new BooleanType();
}

Expand Down
4 changes: 4 additions & 0 deletions src/Type/ArrayType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@ public function isConstantValue(): TrinaryLogic

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
{
if ($type->isInteger()->yes()) {
return new ConstantBooleanType(false);
}

return new BooleanType();
}

Expand Down
4 changes: 3 additions & 1 deletion src/Type/IntersectionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,9 @@ public function isScalar(): TrinaryLogic

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
{
return new BooleanType();
return $this->intersectResults(
static fn (Type $innerType): TrinaryLogic => TrinaryLogic::createFromBooleanType($innerType->looseCompare($type, $phpVersion))
)->toBooleanType();
}

public function isOffsetAccessible(): TrinaryLogic
Expand Down
4 changes: 3 additions & 1 deletion src/Type/UnionType.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,9 @@ public function isScalar(): TrinaryLogic

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
{
return new BooleanType();
return $this->unionResults(
static fn (Type $innerType): TrinaryLogic => TrinaryLogic::createFromBooleanType($innerType->looseCompare($type, $phpVersion))
)->toBooleanType();
}

public function isOffsetAccessible(): TrinaryLogic
Expand Down
98 changes: 98 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 @@ -653,4 +653,102 @@ public function sayInt(

}

/**
* @param true|1|"1" $looseOne
* @param false|0|"0" $looseZero
* @param false|1 $constMix
*/
public function sayConstUnion(
$looseOne,
$looseZero,
$constMix
): void
{
assertType('true', $looseOne == 1);
assertType('false', $looseOne == 0);
assertType('true', $looseOne == true);
assertType('false', $looseOne == false);
assertType('true', $looseOne == "1");
assertType('false', $looseOne == "0");
assertType('false', $looseOne == []);

assertType('false', $looseZero == 1);
assertType('true', $looseZero == 0);
assertType('false', $looseZero == true);
assertType('true', $looseZero == false);
assertType('false', $looseZero == "1");
assertType('true', $looseZero == "0");
assertType('bool', $looseZero == []);

assertType('bool', $constMix == 0);
assertType('bool', $constMix == 1);
assertType('bool', $constMix == true);
assertType('bool', $constMix == false);
assertType('bool', $constMix == "1");
assertType('bool', $constMix == "0");
assertType('bool', $constMix == []);

assertType('true', $looseOne == $looseOne);
assertType('true', $looseZero == $looseZero);
assertType('false', $looseOne == $looseZero);
assertType('false', $looseZero == $looseOne);
assertType('bool', $looseOne == $constMix);
assertType('bool', $constMix == $looseOne);
assertType('bool', $looseZero == $constMix);
assertType('bool', $constMix == $looseZero);
}

/**
* @param uppercase-string $upper
* @param lowercase-string $lower
* @param array{} $emptyArr
* @param non-empty-array $nonEmptyArr
* @param int<10, 20> $intRange
*/
public function sayIntersection(
string $upper,
string $lower,
string $s,
array $emptyArr,
array $nonEmptyArr,
array $arr,
int $i,
int $intRange,
): void
{
assertType('bool', '' == $upper);
assertType('bool', '0' == $upper);
assertType('false', 'a' == $upper);
assertType('false', 'abc' == $upper);
assertType('false', 'aBc' == $upper);
assertType('bool', strtoupper($s) == $upper);
assertType('bool', strtolower($s) == $upper);
assertType('bool', $upper == $lower);

assertType('bool', '0' == $lower);
assertType('false', 'A' == $lower);
assertType('false', 'ABC' == $lower);
assertType('false', 'AbC' == $lower);
assertType('bool', strtoupper($s) == $lower);
assertType('bool', strtolower($s) == $lower);
assertType('bool', $lower == $upper);

assertType('false', $arr == $i);
assertType('false', $nonEmptyArr == $i);
assertType('false', $arr == $intRange);
assertType('false', $nonEmptyArr == $intRange);
assertType('bool', $emptyArr == $nonEmptyArr); // should be false
assertType('false', $nonEmptyArr == $emptyArr);
assertType('bool', $arr == $nonEmptyArr);
assertType('bool', $nonEmptyArr == $arr);

assertType('bool', '' == $lower);
if ($lower != '') {
assertType('false', '' == $lower);
}
if ($upper != '') {
assertType('false', '' == $upper);
}
}

}
13 changes: 11 additions & 2 deletions tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,10 @@ public function testBug11694(): void
[
"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%</>.',
],
]);
}
Expand Down Expand Up @@ -227,4 +225,15 @@ public function testBug11694(): void
$this->analyse([__DIR__ . '/data/bug-11694.php'], $expectedErrors);
}

public function testBug8800(): void
{
$this->treatPhpDocTypesAsCertain = true;
$this->analyse([__DIR__ . '/data/bug-8800.php'], [
[
'Loose comparison using == between 0|1|false and 2 will always evaluate to false.',
9,
],
]);
}

}
11 changes: 11 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-8800.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php declare(strict_types=1);

namespace Bug8800;

class HelloWorld
{
public function sayHello(string $s): void
{
var_dump(preg_match('{[A-Z]}', $s) == 2);
}
}
Loading