Conversation
ondrejmirtes
left a comment
There was a problem hiding this comment.
This is almost perfect, thank you :)
- Should we override and change
isAcceptedWithReasonBy? Maybe not, just asking. - We should clarify the relationship between NeverType and NonAcceptingNeverType. Ideally with tests in
TypeCombinatorTest::dataUnionanddataIntersect, and by implementing it in isSuperTypeOf/isSubTypeOf. - Another regression test can be written for phpstan/phpstan#6485.
|
As for your questions:
I think the PR changes in
That would actually defeat the purpose of As for any future plans - I think that we can unify the code to have a single |
I think not. From a theoretical standpoint,
👍
That's what I'm struggling with the most. In the ideal world in my head, they should be one, and I find it difficult to find where to draw the line between them. For example, with Maybe it would be wiser if
That's right. For instance, TypeScript's
Yes, I was thinking the proposed
Oh yes, right, I remember, that's exactly what happened in #2110 😅 what if we tread more carefully this time? It seems to me that most of the places that currently instantiate an explicit Also, what about here? |
1c2cfd9 to
9ac9ba3
Compare
|
Hi, I really appreciate your work and I brought it over the finish line :) If you feel like NonAcceptingNeverType should be used in more places instead of NeverType, feel free to do so. I kept NonAcceptingType extending NeverType, because why not. Let's not break existing The class inheritance relationship can be different to what |
|
Hi, thanks!
Sure, I'll try and follow up on this to see if it's feasible. I'll go and rebase #2485 as soon as I have a minute :) |
split from #2485, closes phpstan/phpstan#9133
The idea is that
nevershould accept no value at all, so that it can truly act as PHP's bottom type. In the light of that, the naming feels a bit strange:NeverTypealone should already be non-accepting. But I know that's not possible right now due to how it is used especially with generics.Perhaps we could rename
NonAcceptingNeverTypetoNeverTypeand turn the current (implicit)NeverTypeinto something likeUnknownType? Is it something worth revisiting in PHPStan 2.0?On a related note, are there other places where I should replace
NeverTypewithNonAcceptingNeverType? Maybe everywhere NeverType is instantiated as explicit?