[PropertyInfo] Support for intersection types #42098
Conversation
69d72d0 to
38d8741
Compare
|
@nicolas-grekas Have you made up your mind yet? |
|
Nope. Maybe @dunglas has? Otherwise can you make a proposal? I'm AFK right now 😅 |
|
My proposal is this PR. Our current strategy of returning an array of types does not really work well anymore if we want to distinguish union from intersection types. I think, we can find a better representation for 5.4, but for 4.4, I just want PropertyInfo to not fall apart if we encounter an intersection. |
|
This should probably be for 5.4 instead. |
I tend to disagree here. The component would currently throw cryptic errors if presented with an intersection type. We can certainly argue about what it should do instead, but I don't think we should leave it as is in 4.4. |
38d8741 to
2bf839d
Compare
|
i think a list of types fits our semantic generally; can we think of something that would conflict in practice? ref #37971 also :) i agree about the compat issue. |
|
ok, fair enough. |
Signed-off-by: Alexander M. Turek <me@derrabus.de>
2bf839d to
9070047
Compare
| { | ||
| return [ | ||
| ['nothing', null], | ||
| ['collection', [new Type(Type::BUILTIN_TYPE_OBJECT, false, 'Traversable'), new Type(Type::BUILTIN_TYPE_OBJECT, false, 'Countable')]], |
There was a problem hiding this comment.
i tend to believe the intersection type should be preserved as a single type;
[new Type(Type::BUILTIN_TYPE_OBJECT, false, 'Traversable&Countable')]
There was a problem hiding this comment.
Maybe on a higher branch, since this PR is motivated by the need to generate a proper message.
Follow up welcome.
|
Thank you @derrabus. |
I was a bit unsure how to fix this one. PropertyInfo does not know about the concept of intersection types yet. With this PR, the component will behave exactly the same for an intersection type as for a union: You will get an array of all atomic types.