Refacto LooseComparisonHelper

[VincentLanglet]

I'll use the logic from LooseComparisonHelper in https://github.com/phpstan/phpstan-src/pull/3484

After the refacto, compareConstantScalars doesn't seems so useful and we could have the pattern

[$leftValue, $rightValue] = LooseComparisonHelper::getConstantScalarValuesForComparison($this, $type, $phpVersion);

and then do the ==, < or <= comparison.

Still to avoid BC break, I deprecate the method compareConstantScalars in 1.12.x to remove it in 2.0.x.

cc @staabm since you wrote this code.

[ondrejmirtes]

These things should be handled by a new method on Type, not by a new method on a helper.

[VincentLanglet]

These things should be handled by a new method on Type, not by a new method on a helper.

Hi @ondrejmirtes, I'm not sure to understand.

Currently there is a LooseComparisonHelper:: compareConstantScalars which

• converts the scalar values for comparison
• then do a == comparison

Since the same logic is needed for <=, <, ... I thought that creating another method wasn't useful and that I could rely on compareConstantScalars with a small refacto like I did.

If I introduce a new method on Type, what do you have in mind ?

• If a new method is introduce for <=, <, ... it will basically be a duplicate of the current LooseComparisonHelper:: compareConstantScalars ; does it mean that such method should be deprecated/removed ?
• I could find weird having a method ConstantScalarType::getValueForScalarComparison which returns an array of values (because based on phpVersion and values I may cast the left or right values) but I don't see a better way

Or I could do something like ConstantScalarType::compareToScalar

public function compareToScalar(ConstantScalarType $type, callable $cb): bool

with usages like $this->compareToScalar($type, fn static ($v1, $v2) => $v1 < $v2);, but both NullType and ConstantScalarTypeTrait would have the same implementation.

Also I was needed confirmation about if removing LooseComparisonHelper::compareConstantScalars on this PR was considered as a BC break or (If I understood correctly) it won't since there is no @api.

[VincentLanglet]

These things should be handled by a new method on Type, not by a new method on a helper.

Friendly ping @ondrejmirtes, I think https://github.com/phpstan/phpstan-src/pull/3485#issuecomment-2376291229 explains correctly my issue ; could you elaborate your request change.

[$leftValue, $rightValue] = LooseComparisonHelper::getConstantScalarValuesForComparison($this, $otherType, $phpVersion);

was useful cause I could then do

$leftValue === $rightValue
$leftValue < $rightValue
...

if I need to compute Type:: isSmallerThanOrEqual, Type::looseCompare, ...

I don't see which method I should add to Type since looseCompare, isSmallerThanOrEqual already exists ; I just refacto the existing Helper method to return the scalar values instead of the comparison.

Also, since getConstantScalarValuesForComparison

• Sometimes modify the $leftType
• Sometimes modify the $rightType
• Is only useful for scalars

I don't see a signature for Type insterface.

The only things which could be done is moving the code from the Helper to the ConstantScalarTypeTrait... not sure it's useful.

[VincentLanglet]

@staabm Maybe do you understand how I should refacto this ?

[VincentLanglet]

These things should be handled by a new method on Type, not by a new method on a helper.

Hi, I'd try to try moving this (and https://github.com/phpstan/phpstan-src/pull/3484) forward. Could you explain which method/signature you have in mind ?