Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 9 additions & 3 deletions src/Type/Accessory/AccessoryNonFalsyStringType.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,15 @@
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\ErrorType;
use PHPStan\Type\FloatType;
use PHPStan\Type\GeneralizePrecision;
use PHPStan\Type\IntegerType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\IsSuperTypeOfResult;
use PHPStan\Type\NullType;
use PHPStan\Type\ObjectWithoutClassType;
use PHPStan\Type\StaticTypeFactory;
use PHPStan\Type\StringType;
use PHPStan\Type\Traits\MaybeCallableTypeTrait;
use PHPStan\Type\Traits\NonArrayTypeTrait;
Expand Down Expand Up @@ -347,8 +348,13 @@

public function looseCompare(Type $type, PhpVersion $phpVersion): BooleanType
{
$falseyTypes = StaticTypeFactory::falsey();
if ($falseyTypes->isSuperTypeOf($type)->yes()) {
$dominated = TypeCombinator::union(
new NullType(),
new ConstantBooleanType(false),
new ConstantStringType(''),
new ConstantArrayType([], []),
);
if ($dominated->isSuperTypeOf($type)->yes()) {

Check warning on line 357 in src/Type/Accessory/AccessoryNonFalsyStringType.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ new ConstantStringType(''), new ConstantArrayType([], []), ); - if ($dominated->isSuperTypeOf($type)->yes()) { + if (!$dominated->isSuperTypeOf($type)->no()) { return new ConstantBooleanType(false); }

Check warning on line 357 in src/Type/Accessory/AccessoryNonFalsyStringType.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\IsSuperTypeOfCalleeAndArgumentMutator": @@ @@ new ConstantStringType(''), new ConstantArrayType([], []), ); - if ($dominated->isSuperTypeOf($type)->yes()) { + if ($type->isSuperTypeOf($dominated)->yes()) { return new ConstantBooleanType(false); }

Check warning on line 357 in src/Type/Accessory/AccessoryNonFalsyStringType.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ new ConstantStringType(''), new ConstantArrayType([], []), ); - if ($dominated->isSuperTypeOf($type)->yes()) { + if (!$dominated->isSuperTypeOf($type)->no()) { return new ConstantBooleanType(false); }

Check warning on line 357 in src/Type/Accessory/AccessoryNonFalsyStringType.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\IsSuperTypeOfCalleeAndArgumentMutator": @@ @@ new ConstantStringType(''), new ConstantArrayType([], []), ); - if ($dominated->isSuperTypeOf($type)->yes()) { + if ($type->isSuperTypeOf($dominated)->yes()) { return new ConstantBooleanType(false); }
return new ConstantBooleanType(false);
}

Expand Down
6 changes: 4 additions & 2 deletions tests/PHPStan/Analyser/nsrt/loose-comparisons.php
Original file line number Diff line number Diff line change
Expand Up @@ -561,10 +561,12 @@ public function sayNonFalsyStr(
assertType('bool', $nonFalsyString == $true);
assertType('false', $nonFalsyString == $false);
assertType('bool', $nonFalsyString == $one);
assertType('false', $nonFalsyString == $zero);
assertType('bool', $nonFalsyString == $zero); // e.g. '0.0' == 0 is true (non-falsy numeric string compared numerically)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add a separate type assertion for the constant type '0.0' == 0 comparison.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Pushed. Added assertType('true', '0.0' == 0) and assertType('true', '0.0' == '0') right after the respective non-falsy-string assertions to prove with constant types that the comments are correct. All tests pass and make phpstan reports 0 errors.

assertType('true', '0.0' == 0);
assertType('bool', $nonFalsyString == $minusOne);
assertType('bool', $nonFalsyString == $oneStr);
assertType('false', $nonFalsyString == $zeroStr);
assertType('bool', $nonFalsyString == $zeroStr); // e.g. '0.0' == '0' is true (numeric strings compared numerically)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add a separate type assertion for the constant type '0.0' == '0' comparison.

assertType('true', '0.0' == '0');
assertType('bool', $nonFalsyString == $minusOneStr);
assertType('bool', $nonFalsyString == $plusOneStr);
assertType('false', $nonFalsyString == $null);
Expand Down
30 changes: 30 additions & 0 deletions tests/PHPStan/Rules/Comparison/ConstantLooseComparisonRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -248,4 +248,34 @@ public function testBug13098(): void
$this->analyse([__DIR__ . '/data/bug-13098.php'], []);
}

public function testBug14606(): void
{
$this->analyse([__DIR__ . '/data/bug-14606.php'], [
[
'Loose comparison using == between non-falsy-string and false will always evaluate to false.',
19,
'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 non-falsy-string and null will always evaluate to false.',
24,
'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 non-falsy-string and '' 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 non-falsy-string and array{} will always evaluate to false.',
34,
],
[
'Loose comparison using == between non-falsy-string and false|null will always evaluate to false.',
42,
'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%</>.',
],
]);
}

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

namespace Bug14606;

function nonFalsyStringLooseCompareInt(string $x): bool {
return !empty($x) && $x == 0; // may be true if $x is '0.0'
}

function nonFalsyStringLooseCompareFloat(string $x): bool {
return !empty($x) && $x == 0.0; // may be true if $x is '0.0'
}

function nonFalsyStringLooseCompareZeroString(string $x): bool {
return !empty($x) && $x == '0'; // may be true if $x is '0.0' (numeric strings compared numerically)
}

/** @param non-falsy-string $x */
function nonFalsyStringLooseCompareFalse(string $x): bool {
return $x == false; // always false: (bool)non-falsy-string is true
}

/** @param non-falsy-string $x */
function nonFalsyStringLooseCompareNull(string $x): bool {
return $x == null; // always false: non-falsy-string is non-empty
}

/** @param non-falsy-string $x */
function nonFalsyStringLooseCompareEmptyString(string $x): bool {
return $x == ''; // always false: non-falsy-string is non-empty
}

/** @param non-falsy-string $x */
function nonFalsyStringLooseCompareEmptyArray(string $x): bool {
return $x == []; // always false
}

/**
* @param non-falsy-string $x
* @param null|false $nullOrFalse
*/
function nonFalsyStringLooseCompareNullOrFalse(string $x, $nullOrFalse): bool {
return $x == $nullOrFalse; // always false
}
Loading