Enum collapsing #7655
Enum collapsing #7655
Conversation
orklah
added
the
release:fix
orklah
force-pushed
the
enumCollapsing
branch
2 times, most recently
from
1386e8b
to
94bc960
Compare
| @@ -334,6 +335,11 @@ public static function combine( | |||
| } | |||
|
|
|||
| if ($combination->named_object_types !== null) { | |||
| foreach ($combination->value_types as $key => $atomic_type) { | |||
| if ($atomic_type instanceof TEnumCase && isset($combination->named_object_types[$atomic_type->value])) { | |||
There was a problem hiding this comment.
Note that enum case can be represented as TClassConstant when it comes from docblock, e.g.
/** @var Code::Ok|int $code */
Ideally we'd want to check if the const is actually a enum case (by calling $codebase->classlikes->enumExists($atomic->value)), but $codebase is not available during scanning phase. I tried to pass it from TypeParser, but that leads to unrelated errors like SplFileInfo not existing prior to stub files scanning.
There was a problem hiding this comment.
That's not great :( Maybe we should have a different syntax for enum cases?
But thinking about it, it may not be too bad. The type from docblock will be collapsed at some point anyway, it may not be a problem
There was a problem hiding this comment.
Maybe we should have a different syntax for enum cases?
Well, why? Enum cases are constants, so it makes sense to use the same syntax.
The type from docblock will be collapsed at some point anyway
Doesn't seem to be happening, even in your branch: https://psalm.dev/r/1698218e19
There was a problem hiding this comment.
why
Well, exactly because of what you're describing. An Enum and a property of an enum should be collapsed but this is not the case for constants
There was a problem hiding this comment.
That doesn't mean we should introduce a new syntax for cases.
There was a problem hiding this comment.
Not sure what we can do then. I'll merge this for now, it's at least a good first step
orklah
force-pushed
the
enumCollapsing
branch
from
94bc960
to
9cc82d5
Compare
This improves the situation of #7654 but without fixing it.
It does fix another form of the same issue when using a if() instead of a coalesce assignment.
I tried to find why both forms doesn't do the same thing and it boils down to the differences between TernaryAnalyzer (which process coalesce and ternary operations) and IfElseAnalyzer(which process if block operations).
I tried to bring more consistency between the two but there are still major differences and I failed to find the solution.
Basically, there's just a missing Type::combineUnionType() somewhere, but I just couldn't find where to apply it.