On Thu, Mar 16, 2017 at 3:34 PM, Marcio Almada <marcio.w...@gmail.com> wrote:
> 2017-03-16 11:01 GMT-03:00 Marco Pivetta <ocram...@gmail.com>: > >> Since some folks keep banging on "it's not a BC break", I propose a >> challenge in fixing this particular BC break example (reads: find a >> different way to make it work, and no warnings/notices allowed): >> >> I made a very simplistic example of where `__toArray` will break existing >> API that currently literally accepts any kind of object: >> https://3v4l.org/Tj0GH >> >> For reference, here's the code copy: >> >> <?php >> >> class Foo >> { >> public $bar; >> public $baz; >> public $taz; >> } >> >> // please note that this API has a signature of `object $object` - any >> object allowed. >> function unsetProperties($object) { >> // we ignore inheritance and private properties for simplicity >> return array_diff( >> array_map( >> function (\ReflectionProperty $p) : string { >> return $p->getName(); >> }, >> (new ReflectionClass($object))->getProperties() >> ), >> array_keys((array) $object) >> ); >> } >> >> var_dump(unsetProperties(new Foo)); >> >> $foo = new Foo; >> >> unset($foo->baz); >> >> var_dump(unsetProperties($foo)); >> >> There are dozens of simpler examples that break (typically, data-mapper >> layers): I picked this particular one because there is no other way to >> detect unset properties without causing side-effects. >> >> If `__toArray` is implemented on an object, the API defined above will >> either report all properties to be unset, or in general produce unreliable >> results. >> >> The current API specification of the `(array)` operator is as following: >> >> * for every SET property produce an array value, with the key being: >> * "\0" . $className . "\0" . $propertyName for private properties >> * "\0*\0" . $propertyName for protected properties >> * $propertyName for public properties >> * $propertyName for dynamically defined properties >> >> This is a **VERY SPECIFIC** behavior that the operator currently >> guarantees. Break that, and you can go on github hunting for `(array)` >> cast >> usages. >> >> The behavior was frozen in the current suite in >> https://github.com/php/php- >> src/blob/8015daeddb7bb4951c808fa253b97753787fb0ea/tests/classes/ >> <https://github.com/php/php-%0Asrc/blob/8015daeddb7bb4951c808fa253b97753787fb0ea/tests/classes/> >> array_conversion_keys.phpt (although, now that I notice, doesn't cover >> dynamically defined properties - should add that). >> >> Marco Pivetta >> >> http://twitter.com/Ocramius >> >> http://ocramius.github.com/ >> > > I kinda agree that (array) being aware of toArray() is not a very good > idea at this point, but... > > It's still not a BC break. In order for the output of "(array) $obj" to > change, one would need to change the implementation of the subject $obj > class. Lawyering this as a BC break will just hurt any possibility of > advancing the conversation in any direction. > > IMMO, if anyone would like to proceed with this toArray() proposal, a > better object reflection/destructuring API could be provided first. > That's pretty much the point: we need to have a way to keep API that works with `object` to keep working with `object` (BC). A different API for extracting all properties is sufficient, as long as it stays stable and survives the `__toArray` change. Marco Pivetta http://twitter.com/Ocramius http://ocramius.github.com/
