Handle the expression expr & number for positive numbers
#1629
Conversation
This is a specific but very common use case, where the result a bitwise-and with a positive number results in the range 0-number.
| } | ||
| if ($leftNumberType instanceof ConstantIntegerType && $leftNumberType->getValue() >= 0) { | ||
| return IntegerRangeType::fromInterval(0, $leftNumberType->getValue()); | ||
| } |
There was a problem hiding this comment.
This logic isn't exactly correct. The type can also be IntegerRangeType where we're sure about being 0 or positive-int.
You can replace this logic with IntegerRangeType::fromInterval(0, null)->isSuperTypeOf($leftNumberType)->yes() and similar. Also make sure to test that :)
There was a problem hiding this comment.
That is surely a better way to do it, but that doesn't make my logic incorrect!😁
So your method catches in one go positive integer constants and ranges right? Even unions of those i suppose... But then how do i get the max value? I suppose something like intersect, but then i still have to check if i end up with a constant or a range? Is there a smart way to get the max?
There was a problem hiding this comment.
This is a very good point, I haven't realized that. And it's a big wormhole to fall into :)
Because you could write some method for that in TypeUtils and try to get the correct answer, but it means a lot of instanceof *Type questions which are bound to be wrong, same as this one here is wrong too.
Correct answer for this is to add a new specific use-case method on PHPStan\Type\Type interface, and implement it in all Type implementations, so that even types like UnionType and IntersectionType are forced to give the correct answer here.
Of course, the question is how this hypothetical method getIntegerMax() looks like and what it returns.
- It should probably return an
int, right? - What does it return if the interval is infinite, e.g. it's an integer, but it's
int<1, max>for example. - What does it return if it's not an integer, e.g. ConstantStringType or ObjectType...
There was a problem hiding this comment.
I gave this a thought and it seems a bit of an overkill for my original need, that is solve the type for expr & constant. Do you think you could accept the patch as is? Unless I miss something, the analysis is still correct, even though very partial compared to what could be done.. but still an improvement.
|
Yes, this is good enough. Thanks. |
This is a specific but very common use case, where the result a
bitwise-and with a positive number results in the range 0-number.