[RFC] [Under Discussion] Auto-implement Stringable for string backed enums View as Markdown

On Wed, Jun 22, 2022 at 6:01 PM Nicolas Grekas nicolas.grekas+php@gmail.com
wrote:

Hi Benjamin and Derick,

I'm replying to both of you because I see some things in common in your
comments.

https://wiki.php.net/rfc/auto-implement_stringable_for_string_backed_enums

I would prefer if this was an explicit opt-in to have a __toString on a

backed enum. Maybe a special trait for enums that has the implementation,
so that a custom __toString cannot be implemented or a new syntax "enum Foo
: Stringable".

We can make this opt-in by simply allowing user-land to implement
Stringable.
This is a different solution to the problem the RFC tries to solve and I
would also personally be fine with.
I would then not try to limit which implementation of it is allowed by
the engine. Instead, I would give all powers to user-land to decide on
their own what makes sense for their use case. As a general principle, I
believe that empowering user-land is always a win vs trying to "save them
from themselves", as if we knew better than them what they want to achieve,
and especially how.

My concern is that auto implementing __toString, will lead to decreasing
type safety of enums in weak typing mode, since they get auto-casted to
string when passed to a function accepting strings. This effectively adds
more type juggling cases.

I don't share this concern: if an API accepts a string and the engine can
provide a string, let it do so. There is nothing inherently dangerous in
doing so. But we don't need to agree on that if the proposal above makes
sense to everybody :)

The example in the RFC about attributes accepting strings or Enums can be
solved by union types on the side of the library developers, it doesn't
need to be magically implemented by the engine.

I extensively explain in the RFC why this should not be on the side of lib
authors, but on the side of end-users. Please double check and let me know
your thoughts.

I don't fully agree with your argument in the RFC.

You mention as an exmaple that users want to pass PossibleRoles enum values
into the IsGranted attribute. But the way Symfony works as you know, you
can pass arbitrary role names here from the library POV, and applications
can add their own. So naturally the API is to pass a string. So
"PossibleRoles" can only be a user provided enum, it can never be a Symfony
type.

I don't see how PHP should provide a workaround for Symfony and its users
wanting to allow users to use an arbitrary enum as input to a function.

In this example, in Symfony code you only expect "any" BackedEnum and
cannot validate that it is a value of PossibleRoles enum. As such Rowan
suggestion to use constants is the way Symfony should recommend their users.

I don't consider "use strict mode" a good argument to avoid this problem,
because that has other downsides such as overcasting.

I'm 100% aligned with that.

2. It is not implemented for all enum types, only for string backed ones.

This change would now make different classes of enums, without them being
differently inherited classes.

This would also be solved by allowing user-land to implement Stringable on
all kind of enums. Would that make sense to you?

3. The main use case seems to be to prevent having to type ->value, which
   would otherwise indicate reliably whether an enum is used as string
   argument.

Yep, that's a "strict mode" approach and this RFC mostly applies to
non-strict mode.
There are also cases where using "->value" is just not possible. I mention
attributes in the RFC, but we also have a case in Symfony where defining
service definitions in yaml doesn't work with enums because there is no way
to express the "->value" part.

If that's the consensus, I'm fine updating the RFC to turn the vote into
whether "allowing user-land to implement Stringable on any kind of enums"
is desired or not.

Nicolas

Hi Nicolas, thanks for the RFC,

There are also cases where using "->value" is just not possible. I mention
attributes in the RFC,

which also mentions
https://wiki.php.net/rfc/fetch_property_in_const_expressions (but with
"For people that use non-strict mode, this extra “->value” is
boilerplate that they'd better remove")

but we also have a case in Symfony where defining
service definitions in yaml doesn't work with enums because there is no way
to express the "->value" part.

Symfony YAML has a !php/const X feature, which also works when X is
an Enum::CASE; how about a !php/enum_value feature?

Otherwise, I also like Rowan's suggestion of implementing "internal
cast handlers", so that non-strict users could call e.g.
takes_int(IntEnum::CASE) as well as
takes_string(StringEnum::CASE); but what about
takes_string(IntEnum::CASE), and
takes_Stringable(StringEnum::CASE)?

In any case, several people requested that it should require to be
opted-in explicitly; but then [for solutions other than "allowing
user-land to implement Stringable"] we probably also need a way to
test whether a BackedEnum [instance] is "coercible"?

Regards,

--
Guilliam Xavier

Hi

In case 1, I'd argue that the function should be switching to an Enum long term and dropping the string. For that, a union type is the optimal solution. Does that have BC implications for sub-classes? Well, yes, but so does any type improvement. This is a known problem space, with known solutions and migration strategies. It's conceptually no different from migrating from "this takes a string" to "this takes an array of strings": Widen the type, have transitional code, retighten the type. The time frame for that could be weeks, months, or years depending on the situation, but it's not a novel concept.

Exactly this. It might be painful for existing code, but I expect any
newly written and any updated code to handle enums "natively" and so
this is something that will solve itself over time.

On the other hand once the enum type safety is watered up by allowing
implicit string conversions, there is no easy way to revert this.

Best regards
Tim Düsterhu

On Wed, Jun 22, 2022 at 8:27 PM Larry Garfield larry@garfieldtech.com
wrote:

So I am firmly against making it easier to (mis)use enums in a situation
where constants are already the superior solution by every metric. The
only argument I see is making case 1, transitioning from a string to an
enum for a genuinely limited-case, easier. But in that case, the
transition is going to have to happen eventually anyway, and that means the
type is going to change at some point, and the same BC issue will appear,
just at a different time. Unless the intent is to then never change the
type and keep the function incorrectly typed (from the POV that it's
logically an enum, even though string typed was the best/correct type for
years) forever, in which case... use a set of constants.

Hi!
I'm with you on what you mentioned here.

But also, I think the need I understood arises from another case that is
neither 1 or 2.
When you have two domains the value might need to be represented as a
backed enum in one side and as a string in the other.
As far as I understood, this is the case, with applications that are in one
domain wants to have a proper enum for let's say the app roles as the
possible roles are just a limited set.
That application is using another library to configure the ACL using those
roles and this is another domain that does not have a limited value on the
role representation, it's just a string.
Naturally, you should just transform the enum instance to the string and
that should be done using the value property. But this does not work for
configurations done through attribute parameters.
And I think this is the only problem we should fix and that's fixable by
https://wiki.php.net/rfc/fetch_property_in_const_expressions

What you mentioned about developers using enum in the wrong way is
completely true and it's been a long effort for me in explaining this.
I was hoping it would be diminished somehow by increased popularity of
enums, now that they are supported by everyone. But the usages are also
increasing.

Regards,
Alex

The argument presented is that it's easier to type AppRoles::Admin than "admin", because the former provides you with an error if you typo something. That's a valid argument, but... not for using enums. It's an argument for using constants.

I wonder if the reality is that neither enums (as implemented) nor
constants are the right solution to to this.

What users want is some way to say "this value should be a string, but
in this context it should be one of this list of strings"; or,
sometimes, "is this string one of this list of strings?" Constants can't
do that - they give you a way of referring to the possible values, but
no tools for enforcing or testing against them. Backed enums can kinda
sorta do that with a bit of effort, using ->value and ::tryFrom, but
they're not really built for it.

A better fit would be some kind of "domain type", which would allow you
to write something vaguely like this:

domain SymfonyPermission: string;
domain AcmePermission: string { 'admin' | 'user' | 'bot' };

assert( in_domain('admin', SymfonyPermission) );
assert( in_domain('admin', AcmePermission) );

assert( in_domain('random stranger', SymfonyPermission) );
assert( ! in_domain('random stranger', AcmePermission) );

Domains can also be considered sets, which you could compare directly,
and maybe even calculate intersections, unions, etc:

assert( is_subset(AcmePermission, SymfonyPermission) );

The actual values would be ordinary strings, and type constraints would
just be checking the value passed against the domain:

function doSymfonyThing(SymfonyPermission $permission) {
    echo $permission; // no coercion needed, $permission is a string
}

function doAcmeThing(AcmePermission $permission) {
    doSymfonyThing($permission);
}

doAcmeThing('admin'); // no special syntax needed to "construct" or
"look up" an instance

Crucially, this solves the described problem of a library accepting an
infinite (or perhaps just very wide) set of values, and a consuming app
wanting to constrain that set within its own code.

It's one disadvantage is the typo-proofing and look up availability that
constants give, but you could always combine the two.

Regards,

--
Rowan Tommins
[IMSoP]

So I am firmly against making it easier to (mis)use enums in a
situation where constants are already the superior solution by every
metric. The only argument I see is making case 1, transitioning from a
string to an enum for a genuinely limited-case, easier. But in that
case, the transition is going to have to happen eventually anyway, and
that means the type is going to change at some point, and the same BC
issue will appear, just at a different time. Unless the intent is to
then never change the type and keep the function incorrectly typed
(from the POV that it's logically an enum, even though string typed was
the best/correct type for years) forever, in which case... use a set of
constants.

--Larry Garfield

As promised, a blog post on the topic that folks are welcome to link to when telling people no:

https://peakd.com/hive-168588/@crell/on-the-use-of-enums

--Larry Garfield