Added new type-safe API that allows validating a given value against a given `class-string<Enum>`
This patch introduces two new methods:
Enum::isValidEnumValue()Enum::assertValidEnumValue()
The Enum::isValidEnumValue() is still wonky due to https://github.com/vimeo/psalm/issues/5372, but
overall, this allows for matching against other Enum sub-types, by validating that a given $value
is effectively a subtype of T within an Enum<T>.
The previous approach did not work, because static method types are not coupled with the class they are put on (they are effectively namespaced functions, and not much else).
Detailed explanation of what was done is available commit-by-commit, but the idea is that it is impossible to validate T for a given Enum<T> when a static method is used to do so, because static methods, by definition, are not coupled to the class they live on.
Therefore, I had to be a bit more creative: Enum::assertValidEnumValue(ConcreteEnumClass::class, $value) is to be used instead.
TODOs:
- [ ] discuss deprecation, and if they are needed at all
- [ ] https://github.com/vimeo/psalm/issues/5372
- [ ] discuss naming of new API
/cc @michaelpetri @guidobonuzzi @DavideBicego
Hey! Just to be sure I understand, these 2 new methods are only useful for static analysis right?
If we can avoid that that would be great though (I'm not sold on deprecating current methods which are simpler to use).
How about:
public static function isValid($value, ?string $enumType = null)
The new parameter would be optional, and only those using static analysis would go through the trouble of providing the class name. Would that work?
Heads up: spent a lot of time debugging issues around this today, and will certainly work more on the patch as soon as I can, but I think what I'll do for now is removing <T> from these existing methods, so we fix the immediate issue (will open a new PR for that).
(I'm not sold on deprecating current methods which are simpler to use).
Agree, probably better to just leave them be.
The new parameter would be optional, and only those using static analysis would go through the trouble of providing the class name. Would that work?
That is an excellent question, and I do indeed need to try it out! :+1:
I recently stumbled upon this as well. If there is something I can help with @Ocramius, let me know.