Re: [PHP-DEV] Proposal for a RFC

This is only part of a thread. view whole thread
  106783
August 29, 2019 03:09 kontakt@beberlei.de (Benjamin Eberlei)
On Sun, May 5, 2019 at 5:08 PM Nicolas Grekas grekas@gmail.com>
wrote:

> Le sam. 4 mai 2019 à 18:37, Marco Pivetta <ocramius@gmail.com> a écrit : > > > Hi Steven, > > > > As it currently stands, the array cast is the only operation capable of > > exposing object state without triggering any kind of access guards: it is > > very much required for anything that works with reflection and typed > > properties, and possibly the only operation in PHP that operates on state > > without some contraption intercepting its execution. > > > > It is also necessary to distinguish dynamic properties from declared > object > > state. > > > > For comparison, all of the following have side-effects due to the > > complexity of the language: > > > > * isset() > > * unset() > > * property read > > * property write > > * ReflectionProperty#getValue() > > * ReflectionProperty#setValue() > > > > Overall, this is problematic, and introduces more magic that I'd gladly > > avoid, in an endpoint used to work around all the engine magic (the only > > stable one so far). > > > > From my end, this sounds like a bad idea, because it removes one of the > > very very few referentially transparent guarantees (if not the only one, > > when dealing with objects) from the language. > > > > Greets, > > > > Marco > > > > I want to weight in with what Marco expressed. I have the very same > concerns and they are major ones for many use cases. Mine is VarDumper. > > Please don't do this the way it is described. >
Because Steve asked for wiki access to work on an RFC for his proposal, I went back and re-read and just wanted to come in his support as I think it would be a great addition to the existing __magic functionality, I want to add a few more arguments why __toArray() should be added regardless of the objections from Marco & Nicolas: 1. PHP already has __debugInfo() which affects the behavior of var_dump() allowing users to control what gets exposed to it in a debugging context. This is new magic method would affect Symfony's VarDumper+ VarCloner in a similar consistent way that gives class owners back control of what gets exposed. One primary benefit of OOP is encapsulation and guarding against access of internal state from the outside, so it is questionable anyways why an idiomatic and simple to use syntax like (array) $object would expose internal state and a developer would have no means of preventing that. 2. Adding __toArray would not constitute a backwards compatibility break, because all existing code does not implement __toArray and would keep the same exact behavior. 3. You can't argue against the concept of magic methods overall by citing your own use case that is objectively an extremely magic use of the existing behavior of (array) $object itself. I can see a very good case to change (array) $object to return only public properties as part of the general push to cleanup inconsistent behavior in PHP 8. As Nikita mentioned, it might make sense to add a function that is specifically designed for your both use-cases instead and that would honestly make this part of the language cleaner in the end and we can get rid of this * protected and nullbyte private variable returning behavior once and for all.. 4. ReflectionProperty::getValue() does not trigger a guard clause if you call setAccessible(true) first, so I don't think the objection that this is the only way to access private data is true here. Correct me if I forgot an edge case that prevents this. 5. Arguments of API design that toSomethingString() is somehow "better" than using __toString() and therefore the addition of __toArray() is bad pit one paradigm (explicit OOP vs magic methods) and should be avoided in an RFC discussion. We are also not discussing to remove functions in PHP because we now have classes and methods and this falls into the same category. Its a subject design decision and shouldn't influence the expansion of an existing feature/paradigm that one doesn't use themselves.
> Nicolas >
  106786
August 29, 2019 05:44 ocramius@gmail.com (Marco Pivetta)
Hey Benjamin,

On Thu, Aug 29, 2019, 05:09 Benjamin Eberlei <kontakt@beberlei.de> wrote:

> > > On Sun, May 5, 2019 at 5:08 PM Nicolas Grekas grekas@gmail.com> > wrote: > >> Le sam. 4 mai 2019 à 18:37, Marco Pivetta <ocramius@gmail.com> a écrit : >> >> > Hi Steven, >> > >> > As it currently stands, the array cast is the only operation capable of >> > exposing object state without triggering any kind of access guards: it >> is >> > very much required for anything that works with reflection and typed >> > properties, and possibly the only operation in PHP that operates on >> state >> > without some contraption intercepting its execution. >> > >> > It is also necessary to distinguish dynamic properties from declared >> object >> > state. >> > >> > For comparison, all of the following have side-effects due to the >> > complexity of the language: >> > >> > * isset() >> > * unset() >> > * property read >> > * property write >> > * ReflectionProperty#getValue() >> > * ReflectionProperty#setValue() >> > >> > Overall, this is problematic, and introduces more magic that I'd gladly >> > avoid, in an endpoint used to work around all the engine magic (the only >> > stable one so far). >> > >> > From my end, this sounds like a bad idea, because it removes one of the >> > very very few referentially transparent guarantees (if not the only one, >> > when dealing with objects) from the language. >> > >> > Greets, >> > >> > Marco >> > >> >> I want to weight in with what Marco expressed. I have the very same >> concerns and they are major ones for many use cases. Mine is VarDumper. >> >> Please don't do this the way it is described. >> > > Because Steve asked for wiki access to work on an RFC for his proposal, I > went back and re-read and just wanted to come in his support as I think it > would be a great addition to the existing __magic functionality, > > I want to add a few more arguments why __toArray() should be added > regardless of the objections from Marco & Nicolas: > > 1. PHP already has __debugInfo() which affects the behavior of var_dump() > allowing users to control what gets exposed to it in a debugging context. > This is new magic method would affect Symfony's VarDumper+ VarCloner in a > similar consistent way that gives class owners back control of what gets > exposed. One primary benefit of OOP is encapsulation and guarding against > access of internal state from the outside, so it is questionable anyways > why an idiomatic and simple to use syntax like (array) $object would expose > internal state and a developer would have no means of preventing that. >
__debugInfo is already arguably added complexity and debugging time for those looking at a dump result and not understanding it. From my PoV, I'd love to also see __debugInfo gone, since it only ever caused me to lose hair.
> 2. Adding __toArray would not constitute a backwards compatibility break, > because all existing code does not implement __toArray and would keep the > same exact behavior. >
It would break any code relying on current `(array)` semantics against the general `object` type. Can certainly propose it for 8.x, and then we go on a hunt for `(array)` casts, and disallowing them explicitly (cs, static analysis).
> 3. You can't argue against the concept of magic methods overall by citing > your own use case that is objectively an extremely magic use of the > existing behavior of (array) $object itself. I can see a very good case to > change (array) $object to return only public properties as part of the > general push to cleanup inconsistent behavior in PHP 8. As Nikita > mentioned, it might make sense to add a function that is specifically > designed for your both use-cases instead and that would honestly make this > part of the language cleaner in the end and we can get rid of this * > protected and nullbyte private variable returning behavior once and for all. >
It's fine to break this for 8.x, but I think 7.4 already has an added API for this? Can't remember the name, but something about unmangled vars.
> 4. ReflectionProperty::getValue() does not trigger a guard clause if you > call setAccessible(true) first, so I don't think the objection that this is > the only way to access private data is true here. Correct me if I forgot an > edge case that prevents this. >
Guards are triggered for unset properties, even with reflection.
> 5. Arguments of API design that toSomethingString() is somehow "better" > than using __toString() and therefore the addition of __toArray() is bad > pit one paradigm (explicit OOP vs magic methods) and should be avoided in > an RFC discussion. We are also not discussing to remove functions in PHP > because we now have classes and methods and this falls into the same > category. Its a subject design decision and shouldn't influence the > expansion of an existing feature/paradigm that one doesn't use themselves.. >
I'd say that the paradigm is at the core of the discussion: magic calls are some of the worst traps in the language, leading to monstrous amounts of complexity due to them always being active (rather than being explicitly declared via interface).