-
Notifications
You must be signed in to change notification settings - Fork 548
More precise types #4772
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
More precise types #4772
Conversation
src/Type/UnionTypeHelper.php
Outdated
| * @param Type[] $types | ||
| * @return Type[] | ||
| * @template T of Type | ||
| * @param list<T>|array<T> $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.
list<T>|array<T> does not make sense, should be just array<T>. The return type condition would still work.
But could we instead always make sure that list is passed here?
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.
But could we instead always make sure that list is passed here?
to make that work I either need to ignore some errors, or narrow the constructor parameters of e.g. UnionType, IntersectionType etc. (which you might not want because BC break)
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.
did the change, please have a look whether you like it
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.
or narrow the constructor parameters of e.g. UnionType, IntersectionType etc. (which you might not want because BC break)
I think it's okay because mostly people should be using TypeCombinator. It's only okay to do new UnionType() if you have the exact array and you know the types are already normalized.
| if ($tempTypes === []) { | ||
| if ($benevolentUnionObject instanceof TemplateBenevolentUnionType) { | ||
| return $benevolentUnionObject->withTypes($types); | ||
| return $benevolentUnionObject->withTypes(array_values($types)); | ||
| } | ||
|
|
||
| return new BenevolentUnionType($types, true); | ||
| return new BenevolentUnionType(array_values($types), true); | ||
| } | ||
| } | ||
|
|
||
| return new UnionType($types, true); | ||
| return new UnionType(array_values($types), true); | ||
| } |
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.
we might phpdoc-cast this parameter instead of doing a runtime array_values to reduce overhead
|
Thank you! |
first found of fixes to get TypeCombinator::union() / intersect() ready for no-named-arguments