-
Notifications
You must be signed in to change notification settings - Fork 548
MutatingScope: Remove unnecessary union #4780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| $types = [ | ||
| new ArrayType(new MixedType(), new MixedType()), | ||
| new ObjectType(ArrayAccess::class), | ||
| new NullType(), | ||
| ]; | ||
| if ($dimType->isInteger()->yes()) { | ||
| $types[] = new StringType(); | ||
| } | ||
| $offsetValueType = TypeCombinator::intersect($exprVarType, TypeCombinator::union(...$types)); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the actual change is here.
I am not sure why this no longer makes a difference.. maybe related to Type->setExistingOffset() and friends
| if ($type instanceof CompoundType) { | ||
| return $type->isAcceptedBy($this, $strictTypes); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is #4766 which makes sense on its own
ad53698 to
8f73a13
Compare
|
This pull request has been marked as ready for review. |
|
ok - lets see what you think about it. in case you are fine with the change I can add regression tests for the issue-bot reports |
I think this is the 3rd time this week, which made me think that I need #4766 to implement some other stuff.
Lets see how you feel about it :-)