Re: Proposal for a RFC

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 <nicolas.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).