Add __toArray() method to objects that would be called on cast to array

php.internals

Benoît Burnichon

9 years ago
Hi all, Looking at code of PHPUnit, I stumbled upon an inconsistent array conversion: ------ /** * @param ArrayAccess|array $other */ function evaluate($other) { // type cast $other as an array to allow //support in standard array functions. if ($other instanceof ArrayAccess) { $data = (array) $data; } $patched = array_replace_recursive($other, $this->subset); // ... } ----- This would work only for `ArrayAccess` implementations extending `ArrayObject` as shown by https://3v4l.org/ti4aY Looking at the manual http://php.net/manual/en/language.types.array.php#language.types.array.casting , it seems `ArrayObject` class does not comply to array casting because integer public properties could also be retrieved. Some tests showed that regular class always have string keys even when a `$key = 0; $this->{$key} = 'avalue';` is called. In this case, `var_export((array)$object);` returns `array('0' => 'avalue')` (Notice the quote around key 0 - https://3v4l.org/6QW70) What do you think of adding an optional `__toArray()` method to classes which would default to current behavior but would allow specifying behavior. The way of internal `__toString()` method and could explain inconsistency of the `ArrayObject` class? Regards, Benoît Burnichon

Ryan Pallas

9 years ago
On Wed, Mar 15, 2017 at 11:49 AM, Benoît Burnichon <bburnichon@gmail.com> wrote:
> Hi all, > > Looking at code of PHPUnit, I stumbled upon an inconsistent array > conversion: > > ------ > /** > * @param ArrayAccess|array $other > */ > function evaluate($other) > { > // type cast $other as an array to allow > //support in standard array functions. > if ($other instanceof ArrayAccess) { > $data = (array) $data; > } > > $patched = array_replace_recursive($other, $this->subset); > > // ... > } > ----- > > This would work only for `ArrayAccess` implementations extending > `ArrayObject` as shown by https://3v4l.org/ti4aY > > Looking at the manual > http://php.net/manual/en/language.types.array.php# > language.types.array.casting > , > it seems `ArrayObject` class does not comply to array casting because > integer public properties could also be retrieved. Some tests showed that > regular class always have string keys even when a `$key = 0; $this->{$key} > = 'avalue';` is called. In this case, `var_export((array)$object);` returns > `array('0' => 'avalue')` (Notice the quote around key 0 - > https://3v4l.org/6QW70) > > What do you think of adding an optional `__toArray()` method to classes > which would default to current behavior but would allow specifying > behavior. The way of internal `__toString()` method and could explain > inconsistency of the `ArrayObject` class? >
I like the idea kind of, but would this remove the ability to cast to array all classes not implementing __toArray, as is the case with __toString? This would be a HUGE BC if so: $ php -r 'class Foo {public $foo = "foobar";} var_dump((array) (new Foo));' array(1) { ["foo"]=> string(6) "foobar" } $ php -r 'class Foo {public $foo = "foobar";} var_dump((string) (new Foo));' PHP Recoverable fatal error: Object of class Foo could not be converted to string in Command line code on line 1 $ php -v PHP 7.1.2 (cli) (built: Feb 27 2017 00:02:44) ( ZTS )

Benoît Burnichon

9 years ago
> I like the idea kind of, but would this remove the ability to cast to > array all classes not implementing __toArray, as is the case with > __toString? This would be a HUGE BC if so: > > $ php -r 'class Foo {public $foo = "foobar";} var_dump((array) (new Foo));' > array(1) { > ["foo"]=> > string(6) "foobar" > } > $ php -r 'class Foo {public $foo = "foobar";} var_dump((string) (new > Foo));' > PHP Recoverable fatal error: Object of class Foo could not be converted > to string in Command line code on line 1 > $ php -v > PHP 7.1.2 (cli) (built: Feb 27 2017 00:02:44) ( ZTS ) >
No. For me, classes not implementing __toArray() should keep current behavior. That was what I had in mind when I wrote that __toArray() should have a default standard implementation. Same restrictions could be applied to this magic method: http://us3.php.net/manual/en/language.oop5.magic.php#language.oop5.magic.tostring By this, I mean, no exceptions should be thrown in this method and return value MUST be an array.

Andrey Andreev

9 years ago
Hi, On Wed, Mar 15, 2017 at 8:31 PM, Benoît Burnichon <bburnichon@gmail.com> wrote:
>> I like the idea kind of, but would this remove the ability to cast to >> array all classes not implementing __toArray, as is the case with >> __toString? This would be a HUGE BC if so: >> >> $ php -r 'class Foo {public $foo = "foobar";} var_dump((array) (new Foo));' >> array(1) { >> ["foo"]=> >> string(6) "foobar" >> } >> $ php -r 'class Foo {public $foo = "foobar";} var_dump((string) (new >> Foo));' >> PHP Recoverable fatal error: Object of class Foo could not be converted >> to string in Command line code on line 1 >> $ php -v >> PHP 7.1.2 (cli) (built: Feb 27 2017 00:02:44) ( ZTS ) >> > > No. For me, classes not implementing __toArray() should keep current > behavior. That was what I had in mind when I wrote that __toArray() should > have a default standard implementation. > > Same restrictions could be applied to this magic method: > http://us3.php.net/manual/en/language.oop5.magic.php#language.oop5.magic.tostring > > By this, I mean, no exceptions should be thrown in this method and return > value MUST be an array.
Exceptions cannot be thrown from inside __toString() because that's hard to implement; it's not a "feature" that anybody wanted. Cheers, Andrey.

Benoît Burnichon

9 years ago
> > > Same restrictions could be applied to this magic method: > > > http://us3.php.net/manual/en/language.oop5.magic.php#language.oop5.magic.tostring > > > > By this, I mean, no exceptions should be thrown in this method and return > > value MUST be an array. > > Exceptions cannot be thrown from inside __toString() because that's > hard to implement; it's not a "feature" that anybody wanted. >
That is why I said 'could be applied'. I don't currently know whether this will be hard to implement. Maybe these restrictions are unneeded but I prefer to be stricter than necessary as it generally make it easier to implement. But thanks for the clarification.

Fleshgrinder

9 years ago
On 3/15/2017 6:49 PM, Benoît Burnichon wrote:
> Hi all, > > Looking at code of PHPUnit, I stumbled upon an inconsistent array > conversion: > > ------ > /** > * @param ArrayAccess|array $other > */ > function evaluate($other) > { > // type cast $other as an array to allow > //support in standard array functions. > if ($other instanceof ArrayAccess) { > $data = (array) $data; > } > > $patched = array_replace_recursive($other, $this->subset); > > // ... > } > ----- > > This would work only for `ArrayAccess` implementations extending > `ArrayObject` as shown by https://3v4l.org/ti4aY > > Looking at the manual > http://php.net/manual/en/language.types.array.php#language.types.array.casting > , > it seems `ArrayObject` class does not comply to array casting because > integer public properties could also be retrieved. Some tests showed that > regular class always have string keys even when a `$key = 0; $this->{$key} > = 'avalue';` is called. In this case, `var_export((array)$object);` returns > `array('0' => 'avalue')` (Notice the quote around key 0 - > https://3v4l.org/6QW70) > > What do you think of adding an optional `__toArray()` method to classes > which would default to current behavior but would allow specifying > behavior. The way of internal `__toString()` method and could explain > inconsistency of the `ArrayObject` class? > > Regards, > > Benoît Burnichon >
Wouldn't it also be possible to add a `Arrayable` interface with a totally normal `toArray` method? This would avoid littering code bases even more with feature detection code like `method_exists($x, '__toString')`. There would be nothing special to do here, other than implementing that interface for all core classes for which it makes sense. The only question that remains (regardless of the strategy) would be: is it iterable? I guess the answer "yes" is obvious here. Hence, `Arrayable` would need to extend at least `Traversable`.
-- Richard "Fleshgrinder" Fussenegger

Marco Pivetta

9 years ago
This is a BC break due to the fact that the `(array)` cast is used to extract property information from private properties in library code. On 15 Mar 2017 6:50 p.m., "Benoît Burnichon" <bburnichon@gmail.com> wrote:

Kalle Sommer Nielsen

9 years ago
Hi 2017-03-15 21:41 GMT+01:00 Marco Pivetta <ocramius@gmail.com>:
> This is a BC break due to the fact that the `(array)` cast is used to > extract property information from private properties in library code.
Yep, but then again that is more of an undocumented-not-really-supported case afair, if anything then Reflection should have the APIs to officially allow that, although I am still skeptic of this.
-- regards, Kalle Sommer Nielsen kalle@php.net

Marco Pivetta

9 years ago
It's the only way to distinguish between set and unset properties. Also the only way to get all properties from an instance of an inheritance tree. Also, it's covered by tests that were explicitly added to prevent regressions on this. Same as all similar discussions before this one: need an alternative way to do things before proposing a BC break. On 15 Mar 2017 11:27 p.m., "Kalle Sommer Nielsen" <kalle@php.net> wrote:

Ryan Pallas

9 years ago
On Wed, Mar 15, 2017 at 4:33 PM, Marco Pivetta <ocramius@gmail.com> wrote:
> It's the only way to distinguish between set and unset properties. Also the > only way to get all properties from an instance of an inheritance tree. > Also, it's covered by tests that were explicitly added to prevent > regressions on this. > > Same as all similar discussions before this one: need an alternative way to > do things before proposing a BC break. > >
As mentioned in previous mails - the intent isn't to change existing behaviour, but to provide a way for a class to override the default behaviour. As long as those classes you are casting to array don't implement __toArray they will behave exactly as they always have. The only concern then, is that you might be relying on a library to not implement that function on a class you are casting.

Marco Pivetta

9 years ago
Which is precisely the BC break: such a library would now throw an exception "unsupported class X" when `__toString` is implemented. Also, such a library would break silently when this feature is implemented. Much like the already very broken `__debugInfo()`, this automatic type-cast is a giga-Joule footgun. On 15 Mar 2017 11:36 p.m., "Ryan Pallas" <derokorian@gmail.com> wrote:

Fleshgrinder

9 years ago
On 3/15/2017 11:40 PM, Marco Pivetta wrote:
> Which is precisely the BC break: such a library would now throw an > exception "unsupported class X" when `__toString` is implemented. Also, > such a library would break silently when this feature is implemented. > > Much like the already very broken `__debugInfo()`, this automatic type-cast > is a giga-Joule footgun.
I have to backup Ocramius here. As I already wrote, let's go for a totally normal implementation without any kind of magic. This is the only way to be safe, and the only way without adding more weirdness. All this special casing and magic only creates problems in short and long term.
-- Richard "Fleshgrinder" Fussenegger

Benoît Burnichon

9 years ago
I was not aware of all that being the only way to distinguish set/unset properties. I do agree with Marco on this point that it would be a BC break. It was not my intention at all and did not foresee that. Up to now, I thought array casting objects was doing the same as for scalar values. I was wrong and I think many other developers around too. On Wed, 15 Mar 2017 at 23:40 Marco Pivetta <ocramius@gmail.com> wrote:

Ryan Pallas

9 years ago
On Mar 15, 2017 16:40, "Marco Pivetta" <ocramius@gmail.com> wrote: Which is precisely the BC break: such a library would now throw an exception "unsupported class X" when `__toString` is implemented. Also, such a library would break silently when this feature is implemented. Much like the already very broken `__debugInfo()`, this automatic type-cast is a giga-Joule footgun. First please stop top posting. Second bc means that if no code changes no functionality is changed. That's exactly what we're talking about. If the class doesn't change neither does the functionality. So unless classes already have __toArray they will not change in behaviour. As pointed out in answer to my question earlier a class with no code change would see no change in casting behaviour. Only if new method is implemented will behaviour change. How does that not maintain bc? On 15 Mar 2017 11:36 p.m., "Ryan Pallas" <derokorian@gmail.com> wrote:

Marco Pivetta

9 years ago
Hi Ryan, I'm top-posting because I'm writing from a phone. I always do and I also stopped caring for top-posters myself because it's fairly normal, plus modern email clients deal with it. If I can write a damn mail from a phone keyboard because I don't have any better right now, then you can probably use the scroll wheel once on your pc too. The BC break applies to API that accepts `object` (any object). Such API is common in library code in frameworks, data-mappers, etc. Such code would not work anymore for cases where the magic method is implemented, adding either exceptions (forcing a library-level BC break) or simply by causing the existing stable versions of these libs to be incompatible with newer php versions. On 15 Mar 2017 11:57 p.m., "Ryan Pallas" <derokorian@gmail.com> wrote: On Mar 15, 2017 16:40, "Marco Pivetta" <ocramius@gmail.com> wrote: Which is precisely the BC break: such a library would now throw an exception "unsupported class X" when `__toString` is implemented. Also, such a library would break silently when this feature is implemented. Much like the already very broken `__debugInfo()`, this automatic type-cast is a giga-Joule footgun. First please stop top posting. Second bc means that if no code changes no functionality is changed. That's exactly what we're talking about. If the class doesn't change neither does the functionality. So unless classes already have __toArray they will not change in behaviour. As pointed out in answer to my question earlier a class with no code change would see no change in casting behaviour. Only if new method is implemented will behaviour change. How does that not maintain bc? On 15 Mar 2017 11:36 p.m., "Ryan Pallas" <derokorian@gmail.com> wrote:

Ryan Pallas

9 years ago
On Mar 15, 2017 5:03 PM, "Marco Pivetta" <ocramius@gmail.com> wrote: Hi Ryan, I'm top-posting because I'm writing from a phone. I always do and I also stopped caring for top-posters myself because it's fairly normal, plus modern email clients deal with it. If I can write a damn mail from a phone keyboard because I don't have any better right now, then you can probably use the scroll wheel once on your pc too. I'll just say this: I'm also on a mobile device. Don't make assumptions. The BC break applies to API that accepts `object` (any object). Such API is common in library code in frameworks, data-mappers, etc. Such code would not work anymore for cases where the magic method is implemented, adding either exceptions (forcing a library-level BC break) or simply by causing the existing stable versions of these libs to be incompatible with newer php versions. I must misunderstand what constitutes an BC break. I thought a BC break was when identical code produced different results. But you're saying its a break because someone could change their code to use a new feature and break their use of your library? On 15 Mar 2017 11:57 p.m., "Ryan Pallas" <derokorian@gmail.com> wrote: On Mar 15, 2017 16:40, "Marco Pivetta" <ocramius@gmail.com> wrote: Which is precisely the BC break: such a library would now throw an exception "unsupported class X" when `__toString` is implemented. Also, such a library would break silently when this feature is implemented. Much like the already very broken `__debugInfo()`, this automatic type-cast is a giga-Joule footgun. First please stop top posting. Second bc means that if no code changes no functionality is changed. That's exactly what we're talking about. If the class doesn't change neither does the functionality. So unless classes already have __toArray they will not change in behaviour. As pointed out in answer to my question earlier a class with no code change would see no change in casting behaviour. Only if new method is implemented will behaviour change. How does that not maintain bc? On 15 Mar 2017 11:36 p.m., "Ryan Pallas" <derokorian@gmail.com> wrote:

Marco Pivetta

9 years ago
Correct: passing an object that implements `__toArray()` to an API that uses an `(array)` cast internally will break or misbehave, if this feature is added to the language. On 16 Mar 2017 12:27 a.m., "Ryan Pallas" <derokorian@gmail.com> wrote:

Sebastian Bergmann

9 years ago
Am 15.03.2017 um 23:33 schrieb Marco Pivetta:
> Also the only way to get all properties from an instance of an inheritance tree.
I use the Reflection API in https://github.com/sebastianbergmann/object-reflector/blob/master/src/ObjectReflector.php#L24 for retrieving inherited non-public attributes from an object by traversing the inheritance chain using while ($reflector = $reflector->getParentClass()). I wish the ReflectionObject would allow access to all attributes of an object no matter whether an attribute is declared in the class of the object or in a parent class.

Andrew Faulds

9 years ago
Hi Benoît, Benoît Burnichon wrote:
> Looking at the manual > http://php.net/manual/en/language.types.array.php#language.types.array.casting > , > it seems `ArrayObject` class does not comply to array casting because > integer public properties could also be retrieved. Some tests showed that > regular class always have string keys even when a `$key = 0; $this->{$key} > = 'avalue';` is called. In this case, `var_export((array)$object);` returns > `array('0' => 'avalue')` (Notice the quote around key 0 - > https://3v4l.org/6QW70)
I'm not sure what specifically you see to be the problem, but I think it might be worth directing you to my RFC which relates to this: https://wiki.php.net/rfc/convert_numeric_keys_in_object_array_casts
-- Andrea Faulds https://ajf.me/