Hi I'm trying out the new enumeration support for PHP 8.1 using
https://github.com/php/php-src/pull/6489 thought to implement a polyfill
based on class constants and reflection. Doing so I come across the
newly defined interfaces:
public string$name;
public staticfunction cases(): array <http://www.php.net/array>;
}
interface BackedEnumextends UnitEnum{
public string$value;
public staticfunction from(int|string$scalar): static;
public staticfunction tryFrom(int|string$scalar): ?static;
}
``` My interest here is on BackedEnum which (currently) supports
int|string. First of all the interface defines a "public string $value"
but in fact the type needs to be "int|string". As properties are not
supported in interfaces this is a pure documentation issue in the RFC
itself. Secondly the arguments name $scalar is not really a good name -
now as we have named arguments - $value would be much more meaningful.
But most interestingly I get the feeling that this should be split into
several typed interfaces because this makes wrong impression especially
that "tryFrom" would except int|string but actually it doesn't.
Additionally, if we want to support other types later on this would blow
up the types even more. Example: ``` enum Test:string { case TEST = '1';
} Test::tryFrom('unknown'); // `NULL` Test::tryFrom(1);// TypeError ```
The TypeError doesn't make sense here as it matches the type and it even
matches the string representation of an existing type. I think it would
be much clearer if there would be two different interfaces like:
``` interface IntEnum extends UnitEnum { public int $value; public
static function from(int$value): static; public static function
tryFrom(int$value): ?static; }
interface StringEnum extends UnitEnum { public string $value; public
static function from(string $value): static; public static function
tryFrom(string $value): ?static; } ``` or else the above example should
return a `NULL` or even case matching the string representation instead of
TypeError as it matches the defined argument types. Thoughts? Thanks, Marc
Hi Marc
Thanks for testing the enum feature.
Hi I'm trying out the new enumeration support for PHP 8.1 using
https://github.com/php/php-src/pull/6489 thought to implement a polyfill
based on class constants and reflection. Doing so I come across the
newly defined interfaces:public string$name; public staticfunction cases(): array <http://www.php.net/array>; } interface BackedEnumextends UnitEnum{ public string$value; public staticfunction from(int|string$scalar): static; public staticfunction tryFrom(int|string$scalar): ?static; } ``` My interest here is on BackedEnum which (currently) supports int|string. First of all the interface defines a "public string $value" but in fact the type needs to be "int|string". As properties are not supported in interfaces this is a pure documentation issue in the RFC itself.
We intentionally put it in the interface for clarity, even though it's
not technically possible yet.
Secondly the arguments name $scalar is not really a good name -
now as we have named arguments - $value would be much more meaningful.
I think you might be looking at an older implementation. I only fixed
the naming recently (~2 weeks ago). The parameter is called $value, as
you suggested.
But most interestingly I get the feeling that this should be split into
several typed interfaces because this makes wrong impression especially
that "tryFrom" would except int|string but actually it doesn't.
Additionally, if we want to support other types later on this would blow
up the types even more. Example:enum Test:string { case TEST = '1'; } Test::tryFrom('unknown'); // `NULL` Test::tryFrom(1);// TypeError
The TypeError doesn't make sense here as it matches the type and it even
matches the string representation of an existing type.
try/tryFrom will coerce the given value under non-strict_types. With
strict_types enabled it will type error. I think this behavior is
consistent with the rest of PHP.
I think it would
be much clearer if there would be two different interfaces like:static function from(int$value): static; public static function tryFrom(int$value): ?static; } interface StringEnum extends UnitEnum { public string $value; public static function from(string $value): static; public static function tryFrom(string $value): ?static; } ``` or else the above example should return a `NULL` or even case matching the string representation instead of TypeError as it matches the defined argument types.
I guess the interface would be more accurate. I'm not sure if that's a
huge win or just unnecessarily bloats the API. I think using
BackedEnum as a type hint should be relatively rare in practice.
Optimally we could add an associated type or generic type to
BackedEnum but that's currently not possible.
Ilija
Hi Ilija - thanks for your answers
Hi Marc
Thanks for testing the enum feature.
Hi I'm trying out the new enumeration support for PHP 8.1 using
https://github.com/php/php-src/pull/6489 thought to implement a polyfill
based on class constants and reflection. Doing so I come across the
newly defined interfaces:public string$name; public staticfunction cases(): array <http://www.php.net/array>; } interface BackedEnumextends UnitEnum{ public string$value; public staticfunction from(int|string$scalar): static; public staticfunction tryFrom(int|string$scalar): ?static; } ``` My interest here is on BackedEnum which (currently) supports int|string. First of all the interface defines a "public string $value" but in fact the type needs to be "int|string". As properties are not supported in interfaces this is a pure documentation issue in the RFC itself.
We intentionally put it in the interface for clarity, even though it's
not technically possible yet.Secondly the arguments name $scalar is not really a good name -
now as we have named arguments - $value would be much more meaningful.
I think you might be looking at an older implementation. I only fixed
the naming recently (~2 weeks ago). The parameter is called $value, as
you suggested.
nice +1
But most interestingly I get the feeling that this should be split into
several typed interfaces because this makes wrong impression especially
that "tryFrom" would except int|string but actually it doesn't.
Additionally, if we want to support other types later on this would blow
up the types even more. Example:enum Test:string { case TEST = '1'; } Test::tryFrom('unknown'); // `NULL` Test::tryFrom(1);// TypeError
The TypeError doesn't make sense here as it matches the type and it even
matches the string representation of an existing type.
try/tryFrom will coerce the given value under non-strict_types. With
strict_types enabled it will type error. I think this behavior is
consistent with the rest of PHP.I think it would
be much clearer if there would be two different interfaces like:static function from(int$value): static; public static function tryFrom(int$value): ?static; } interface StringEnum extends UnitEnum { public string $value; public static function from(string $value): static; public static function tryFrom(string $value): ?static; } ``` or else the above example should return a `NULL` or even case matching the string representation instead of TypeError as it matches the defined argument types.
I guess the interface would be more accurate. I'm not sure if that's a
huge win or just unnecessarily bloats the API. I think using
BackedEnum as a type hint should be relatively rare in practice.
Optimally we could add an associated type or generic type to
BackedEnum but that's currently not possible.
I think it would be at least be helpful for example on mapping an
enumeration from database to PHP as it's possible with doctrine types
and probably also for static analyzers without the need to special case
enumeration objects.
I also don't think one additional interface bloats up the API or waiting
for generics helps as nobody knows if this will ever be a thing in PHP.
Would this actually require a follow-up RFC or could this be done as a
small implementation detail change ?
Ilija
Hi Marc
I think it would
be much clearer if there would be two different interfaces like:static function from(int$value): static; public static function tryFrom(int$value): ?static; } interface StringEnum extends UnitEnum { public string $value; public static function from(string $value): static; public static function tryFrom(string $value): ?static; } ``` or else the above example should return a `NULL` or even case matching the string representation instead of TypeError as it matches the defined argument types.
I guess the interface would be more accurate. I'm not sure if that's a
huge win or just unnecessarily bloats the API. I think using
BackedEnum as a type hint should be relatively rare in practice.
Optimally we could add an associated type or generic type to
BackedEnum but that's currently not possible.I think it would be at least be helpful for example on mapping an
enumeration from database to PHP as it's possible with doctrine types
and probably also for static analyzers without the need to special case
enumeration objects.
Can you provide a concrete example where the current interface is
inadequate? I can't imagine a situation where you'd want to accept all
integer BackedEnums but string BackedEnums. If we ever expand the
types that backed enums accept the number of interfaces will grow
linearly.
I also don't think one additional interface bloats up the API or waiting
for generics helps as nobody knows if this will ever be a thing in PHP.Would this actually require a follow-up RFC or could this be done as a
small implementation detail change ?
Historically, things like this have been done before if nobody
objects. But that seems unjustified at this point given the lack of
concrete reasoning.
Ilija
Hi Marc
I think it would
be much clearer if there would be two different interfaces like:static function from(int$value): static; public static function tryFrom(int$value): ?static; } interface StringEnum extends UnitEnum { public string $value; public static function from(string $value): static; public static function tryFrom(string $value): ?static; } ``` or else the above example should return a `NULL` or even case matching the string representation instead of TypeError as it matches the defined argument types.
I guess the interface would be more accurate. I'm not sure if that's a
huge win or just unnecessarily bloats the API. I think using
BackedEnum as a type hint should be relatively rare in practice.
Optimally we could add an associated type or generic type to
BackedEnum but that's currently not possible.I think it would be at least be helpful for example on mapping an
enumeration from database to PHP as it's possible with doctrine types
and probably also for static analyzers without the need to special case
enumeration objects.
Can you provide a concrete example where the current interface is
inadequate? I can't imagine a situation where you'd want to accept all
integer BackedEnums but string BackedEnums. If we ever expand the
types that backed enums accept the number of interfaces will grow
linearly.
One example that comes to my mind would be enumeration support as
doctrine type.
This is a modified and simplified example of a real world doctrine type
using marc-mabe/php-enum
abstract class AbstractEnumType extends Type {
/** @return class-string<BackedEnum> */
abstract public function getEnum(): string;
public function convertToDatabaseValue($phpValue, AbstractPlatform
$platform): ?string {
$enum = $this->getEnum();
if (!$phpValue instanceof $enum) {
throw new RuntimeException('Unexpected $phpValue');
}
return (string)$phpValue->value;
}
// This currently requires reflection
public function convertToPHPValue($dbValue, AbstractPlatform
$platform): BackedEnum {
$enum = $this->getEnum();
$refl = new ReflectionEnum($enum);
if ($refl->getBackingType()->__toString() === 'int') {
return $enum::from((int)$dbValue);
}
return $enum::from($dbValue);
}
// This would be cleaner and faster
public function convertToPHPValue($dbValue, AbstractPlatform
$platform): IntEnum|StringEnum {
$enum = $this->getEnum();
if ($enum instanceof IntEnum) {
return $enum::from((int)$dbValue);
}
return $enum::from((string)$dbValue);
}
}
##########################
Another example would be an input filter
class EnumFilter extends Laminas\Filter\AbstractFilter {
/** @var class-string<BackedEnum> */
private string $enum;
public function filter($value) {
$enum = $this->enum;
$refl = new ReflectionEnum($enum);
if ($refl->getBackingType()->__toString() === 'int') {
return $enum::tryFrom((int)$value);
}
return $enum::tryFrom((string)$value);
}
// vs
public function filter($value) {
$enum = $this->enum;
if ($enum instanceof IntEnum) {
return $enum::tryFrom((int)$value);
}
return $enum::tryFrom((string)$value);
}
}
########################################
I can also think of static analyzers to detect type of from(), tryFrom()
and $enum->value based on the implemented interface instead of requiring
them to special case integer vs string backed enums.
I also don't think one additional interface bloats up the API or waiting
for generics helps as nobody knows if this will ever be a thing in PHP.Would this actually require a follow-up RFC or could this be done as a
small implementation detail change ?
Historically, things like this have been done before if nobody
objects. But that seems unjustified at this point given the lack of
concrete reasoning.
Sure, this needs a concrete and approved reason. I try my best ;)
Ilija
Thanks,
Marc