Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113544
To: internals@lists.php.net
References: <2ff5377c-3718-feb1-ba20-96df47357414@mabe.berlin>
 <CAPyj-LCZwon475=8RKjoDabosznGanDw1dtcvh9=Ofbm5ycyoQ@mail.gmail.com>
Message-ID: <9f234d2a-7875-2871-6726-e7b8175099f3@mabe.berlin>
Date: Mon, 15 Mar 2021 18:37:38 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.0
MIME-Version: 1.0
In-Reply-To: <CAPyj-LCZwon475=8RKjoDabosznGanDw1dtcvh9=Ofbm5ycyoQ@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Subject: Re: [PHP-DEV] Inconsistency in enumerations RFC
From: marc@mabe.berlin (Marc)

Hi Ilija - thanks for your answers

On 14.03.21 23:18, Ilija Tovilo wrote:
> 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:
>> ``` interface  UnitEnum{
>>     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:
>> ``` 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.
> 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
>