use IntegerRangeType in modulo-operator#614
Conversation
|
Please squash and rebase your commits so there aren't any merge commits with master, but a clean linear history. Thanks. |
staabm
force-pushed
the
modulo-range-types
branch
from
c96bb9c to
f5fd4e2
Compare
staabm
force-pushed
the
modulo-range-types
branch
from
f5fd4e2 to
cb50241
Compare
|
@ondrejmirtes squashed and rebased |
| $rangeMax = null; | ||
| if ($rightType instanceof IntegerRangeType) { | ||
| $rangeMax = $rightType->getMax() !== null ? $rightType->getMax() - 1 : null; | ||
| } elseif ($rightType instanceof ConstantIntegerType) { |
There was a problem hiding this comment.
I think this will lead to creating int<X, max> when you have 1|2|3 which is wrong I guess.
There was a problem hiding this comment.
another great catch. added a test and a fix
There was a problem hiding this comment.
That's because instanceof *Type is almost always wrong and you can find edge-cases like that.. https://phpstan.org/developing-extensions/type-system#querying-a-specific-type
There was a problem hiding this comment.
Right now it's still wrong for unions of disjoint ranges like int<min, 3>|int<4, max>.
There was a problem hiding this comment.
got this case and a few more also covered. I guess you won't like that, I had to add a few more instanceof *Type calls.
I somehow get the feeling you let me built this thing intentionally and we will now refactor the logic regarding merging IntegerRangeType and ConstantIntegerType types (or unions of it) into either TypeUtils or IntegerRange class, right? ;)
There was a problem hiding this comment.
I guess it's fine like that. A correct approach is to add use-case based methods on PHPStan\Type\Type that would tell us exactly what we need here. Each Type implementation would return what it knows.
I'm preparing myself mentally for this refactoring for a long time, in the end PHPStan\Type\Type might have hundreds of methods, and it might be the only valid situation considered quality code in the universe 😂
staabm
changed the title
IntegerRangeType in modulo-operator
|
Thank you. |
3 participants
closes phpstan/phpstan#5442