Fix InvalidComparisonOperationRule for UnionType #4168
Conversation
VincentLanglet
force-pushed
the
fix/3364
branch
from
92d6809 to
de7d558
Compare
VincentLanglet
force-pushed
the
fix/3364
branch
from
de7d558 to
4857b6a
Compare
| if ($this->isNumberType($scope, $node->left) && $this->isNumberType($scope, $node->right)) { | ||
| $isLeftNumberType = $this->isNumberType($scope, $node->left); | ||
| $isRightNumberType = $this->isNumberType($scope, $node->right); | ||
| if (($isLeftNumberType && $isRightNumberType) || (!$isLeftNumberType && !$isRightNumberType)) { |
There was a problem hiding this comment.
I never used it, i'll try.
Offtopic @ondrejmirtes I saw you merged/review multiple of my PRs. I dunno if you plan a release. I just wanted to inform you I dont have access to a computer until Wednesday. So if you want to include one of my pr (with requested changes) feel free to push on it and if any of these PR introduce a regression I wont be able to fix it before Wednesday.
It's maybe not a big issue for you, just wanted to inform you
There was a problem hiding this comment.
It's fine, thank you for letting me know. I don't think there will be a major issue, PHPStan is continuously tested on several real-world projects (millions of LoC) so I know about the real issues pretty soon.
If something still comes up that a lot of people have problem with, they can wait on a one-before-last version before they upgrade.
There was a problem hiding this comment.
Simplification was easy finally
e14567b
|
Thank you! |
|
@VincentLanglet please see issue bot results for related regression tests: |
Yeah, I sometime forget... Do you think it closes phpstan/phpstan#9386, it's unclear to me... |
|
@VincentLanglet Unfortunately I don't know how to do it in a secure way. |
|
Looks like it does close phpstan/phpstan#9386. |
me too. no worries. |
@ondrejmirtes I guess you fear someone could steal the bot token or similar things? maybe the issue-bot could just emit a pull request annotation with a link to the job-summary in case changes are present. this can be done just with a "echo", and without any further api tokens |
3 participants
I'm not sure about the purpose of the InvalidComparisonOperationRule which seams to forbid comparison between number and object or number and arrays. Technically something like
$int == $arrayis supported by php (and 0 == [] for instance).Maybe the message
should be improved, but that's another topic.
Looking at the implementation, both
isPossiblyNullableObjectTypeandisPossiblyNullableArrayTypeshouldn't use the RuleLevelHelper, because it will removed non-array/non-object on lower level and this is the reason why an error happen on level 6 (and then disappear in level 7).Finally, I notice that currently a comparison between
intandobject|arrayis not reported.This is easy to fix, but since I dunno the whole purpose of this rule, I prefer to ask you first @ondrejmirtes if I should fix it too.
Closes phpstan/phpstan#3364