fix union/intersect involving enum case#3907
Conversation
|
The SubtractableType methods can be basically made no-op in EnumCaseObjectType. Looking at the logic you're iffing-out: $subtractedType = $type->getSubtractedType() === null
? $subtractedType
: self::union($type->getSubtractedType(), $subtractedType);
if ($subtractedType instanceof NeverType) {
$subtractedType = null;
}
return $type->changeSubtractedType($subtractedType);If |
|
@ondrejmirtes Interesting idea, but I didn't manage to get it working (infinite recursion). Looking at $subtractedType = $type->getSubtractedType() === null
? $subtractedType
: self::union($type->getSubtractedType(), $subtractedType);
if ($subtractedType instanceof NeverType) {
$subtractedType = null;
}
return $type->changeSubtractedType($subtractedType);to be equivalent to public function changeSubtractedType(?Type $subtractedType): Type
{
if ($subtractedType === null) {
return $this;
}
return TypeCombinator::remove($this, $subtractedType);
}And then there's $subtractedType = $b->getSubtractedType();
if ($subtractedType === null) {
return $a->getTypeWithoutSubtractedType();
}to be equivalent to $subtractedTypeTmp = self::intersect($a->getTypeWithoutSubtractedType(), $a->getSubtractedType());
if ($b->isSuperTypeOf($subtractedTypeTmp)->yes()) {
return $a->getTypeWithoutSubtractedType();
}
$subtractedType = new MixedType(false, $b);It could be done by returning Unfortunately, I didn't find a way to neatly break the infinite recursion. |
|
Alright, another idea - instead of
|
|
@ondrejmirtes The |
|
Thanks! Feel free to mark it as ready. |
|
This pull request has been marked as ready for review. |
|
Thank you! |
Fixes https://phpstan.org/r/2bf03c1e-d38f-4d1d-9d67-0b4ace5e1629
IMO the real issue is that
EnumCaseObjectTypeshould not beSubtractableType(i.e. there is nothing to subtract from it, other than itself which should then beNeverType). Unfortunately, it inheritsSubtractableTypeimplementation fromObjectType. I checked the other types and the only other suspicious "subtractable" type I found isErrorType(it doesn't subtract anything).I'm not sure what the best way to fix it is. Phpstorm found just 15 usages of
instanceof SubtractableType. So a quick and dirty fix along the lines I did could probably be feasible (at least in the short term). I was thinking e.g. addingisReallySubtractable(): booltoSubtractableType, or havingisSubtractableType(Type): boolsomewhere, ... For now I did just the minimal changes necessary to make the tests pass, but the other usages may also be causing bugs.OFC a proper fix would be to get rid of the inheritance. Since PHPStorm reports
instanceof ...Typeas errors, I assume that is the long-term plan. But given thatObjectTypeis >1600 lines, I don't think that I have enough experience to tackle that.Please let me know what you think. If you say that I should just wait until the inheritance is removed then that's fine with me.