Handle BcMath\Number operators for simple cases #3787
Conversation
| assertType('true', $a or $b); | ||
| } | ||
|
|
||
| public function bcVsNever(Number $a): void |
There was a problem hiding this comment.
There's a bunch of inconsistencies in how never type is handled. I assume that all of these operations should result in *NEVER*, but I'm ignoring it for this PR.
|
This pull request has been marked as ready for review. |
ondrejmirtes
force-pushed
the
feature-bcmathNumberOperators
branch
from
90193e2 to
e7063f3
Compare
|
Hi, I'm sorry for taking such a long time to review this. I've separated part of this PR (253903a) because it's not really related to BcMath. |
|
|
||
| $this->analyse([__DIR__ . '/data/bcmath-number.php'], [ | ||
| [ | ||
| 'Binary operation "<<" between BcMath\\Number and BcMath\\Number results in an error.', |
There was a problem hiding this comment.
Personally I'd remove this method altogether (testBcMathNumber). We don't need to test all of these situations "results in an error" because presumably it's handled by a single code path. If you still want to test this error, please do it with a separate test file only with a single or a few expressions.
There was a problem hiding this comment.
You're right - it's sufficiently covered by NSRT. I removed the test case from InvalidBinaryOperationRuleTest and moved the file to NSRT directory.
|
Thank you! |
Partially fixes: phpstan/phpstan#12099
Fixes: phpstan/phpstan#7937, phpstan/phpstan#8555
It's loosely based on #3660
Here is a demonstration of how the operators behave with various types: https://3v4l.org/jYX5J#v8.4.3
I skipped the complicated stuff. The main issue is unions. IMO it's too difficult to handle unions correctly in the extension. It would probably make more sense for PHPStan to handle the unions itself and offload only non-unions to extensions (e.g.
X|int + X|floatneeds to know whatint + floatis).I also ignored a few preexisting bugs/inconsistencies (null and never types). They can be solved later.