Skip to content

Handle the expression expr & number for positive numbers#1629

Merged
ondrejmirtes merged 1 commit intophpstan:1.8.xfrom
thg2k:pr/bitwise_and_range
Aug 22, 2022
Merged

Handle the expression expr & number for positive numbers#1629
ondrejmirtes merged 1 commit intophpstan:1.8.xfrom
thg2k:pr/bitwise_and_range

Conversation

@thg2k
Copy link
Copy Markdown
Contributor

@thg2k thg2k commented Aug 16, 2022

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.

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());
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

  1. It should probably return an int, right?
  2. What does it return if the interval is infinite, e.g. it's an integer, but it's int<1, max> for example.
  3. What does it return if it's not an integer, e.g. ConstantStringType or ObjectType...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

@ondrejmirtes
Copy link
Copy Markdown
Member

Yes, this is good enough. Thanks.

@ondrejmirtes ondrejmirtes merged commit 62ac4d9 into phpstan:1.8.x Aug 22, 2022
@thg2k thg2k deleted the pr/bitwise_and_range branch August 22, 2022 13:30
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.

2 participants