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.

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

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Dec 21, 2024

@@ -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

@@ -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                                                                                        
 ------ ------------------------------------------------------------------------------------------------------------- 

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

Choose a reason for hiding this comment

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

@staabm staabm marked this pull request as ready for review December 21, 2024 17:13
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@@ -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.

@ondrejmirtes ondrejmirtes merged commit aa4a123 into phpstan:2.1.x Dec 21, 2024
425 of 426 checks passed
@ondrejmirtes
Copy link
Member

Awesome, thank you!

@staabm staabm deleted the int-loose branch December 22, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants