On Wed, Jun 22, 2022 at 6:01 PM Nicolas Grekas <nicolas.grekas+...@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 >
