Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] Add __toArray() method to objects that would be called on cast to array

Posted by Benoît Burnichon 
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
On Wed, Mar 15, 2017 at 11:49 AM, Benoît Burnichon <[email protected]>
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 )


> Regards,
>
> Benoît Burnichon
>
> 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.
Hi,

On Wed, Mar 15, 2017 at 8:31 PM, Benoît Burnichon <[email protected]> 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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
>
> > 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.
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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" <[email protected]> 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
>
Hi

2017-03-15 21:41 GMT+01:00 Marco Pivetta <[email protected]>:
> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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" <[email protected]> wrote:

> Hi
>
> 2017-03-15 21:41 GMT+01:00 Marco Pivetta <[email protected]>:
> > 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
>
On Wed, Mar 15, 2017 at 4:33 PM, Marco Pivetta <[email protected]> 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.


> On 15 Mar 2017 11:27 p.m., "Kalle Sommer Nielsen" <[email protected]> wrote:
>
> > Hi
> >
> > 2017-03-15 21:41 GMT+01:00 Marco Pivetta <[email protected]>:
> > > 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
> >
>
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" <[email protected]> wrote:

>
>
> On Wed, Mar 15, 2017 at 4:33 PM, Marco Pivetta <[email protected]> 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.
>
>
>> On 15 Mar 2017 11:27 p.m., "Kalle Sommer Nielsen" <[email protected]> wrote:
>>
>> > Hi
>> >
>> > 2017-03-15 21:41 GMT+01:00 Marco Pivetta <[email protected]>:
>> > > 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
>> >
>>
>
>
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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 <[email protected]> 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.
>
On Mar 15, 2017 16:40, "Marco Pivetta" <[email protected]> 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" <[email protected]> wrote:

>
>
> On Wed, Mar 15, 2017 at 4:33 PM, Marco Pivetta <[email protected]> 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.
>
>
>> On 15 Mar 2017 11:27 p.m., "Kalle Sommer Nielsen" <[email protected]> wrote:
>>
>> > Hi
>> >
>> > 2017-03-15 21:41 GMT+01:00 Marco Pivetta <[email protected]>:
>> > > 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
>> >
>>
>
>
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" <[email protected]> wrote:



On Mar 15, 2017 16:40, "Marco Pivetta" <[email protected]> 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" <[email protected]> wrote:

>
>
> On Wed, Mar 15, 2017 at 4:33 PM, Marco Pivetta <[email protected]> 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.
>
>
>> On 15 Mar 2017 11:27 p.m., "Kalle Sommer Nielsen" <[email protected]> wrote:
>>
>> > Hi
>> >
>> > 2017-03-15 21:41 GMT+01:00 Marco Pivetta <[email protected]>:
>> > > 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
>> >
>>
>
>
On Mar 15, 2017 5:03 PM, "Marco Pivetta" <[email protected]> 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" <[email protected]> wrote:



On Mar 15, 2017 16:40, "Marco Pivetta" <[email protected]> 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" <[email protected]> wrote:

>
>
> On Wed, Mar 15, 2017 at 4:33 PM, Marco Pivetta <[email protected]> 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.
>
>
>> On 15 Mar 2017 11:27 p.m., "Kalle Sommer Nielsen" <[email protected]> wrote:
>>
>> > Hi
>> >
>> > 2017-03-15 21:41 GMT+01:00 Marco Pivetta <[email protected]>:
>> > > 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.
>>
> Does it not through get properties?

>
>> >
>> > --
>> > regards,
>> >
>> > Kalle Sommer Nielsen
>> > kalle@php.net
>> >
>>
>
>
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" <[email protected]> wrote:

>
>
> On Mar 15, 2017 5:03 PM, "Marco Pivetta" <[email protected]> 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" <[email protected]> wrote:
>
>
>
> On Mar 15, 2017 16:40, "Marco Pivetta" <[email protected]> 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" <[email protected]> wrote:
>
>>
>>
>> On Wed, Mar 15, 2017 at 4:33 PM, Marco Pivetta <[email protected]>
>> 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.
>>
>>
>>> On 15 Mar 2017 11:27 p.m., "Kalle Sommer Nielsen" <[email protected]> wrote:
>>>
>>> > Hi
>>> >
>>> > 2017-03-15 21:41 GMT+01:00 Marco Pivetta <[email protected]>:
>>> > > 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.
>>>
>> Does it not through get properties?
>
> >
>>> >
>>> > --
>>> > regards,
>>> >
>>> > Kalle Sommer Nielsen
>>> > kalle@php.net
>>> >
>>>
>>
>>
>
>
>
Hi,

On Thu, Mar 16, 2017 at 1:33 AM, Marco Pivetta <[email protected]> wrote:
> 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.
>

I'm not particularly interested in the idea anyway, but if you change
code and *then* something changes - that's not a BC break.

Also, I have to agree with Fleshgrinder on that we don't need more
magic methods and using an interface would be much cleaner.

Cheers,
Andrey.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
What changes is the interface of the `(array)` operator.

On 16 Mar 2017 12:53 a.m., "Andrey Andreev" <[email protected]> wrote:

> Hi,
>
> On Thu, Mar 16, 2017 at 1:33 AM, Marco Pivetta <[email protected]> wrote:
> > 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.
> >
>
> I'm not particularly interested in the idea anyway, but if you change
> code and *then* something changes - that's not a BC break.
>
> Also, I have to agree with Fleshgrinder on that we don't need more
> magic methods and using an interface would be much cleaner.
>
> Cheers,
> Andrey.
>
On Mar 16, 2017 2:01 AM, "Marco Pivetta" <[email protected]> wrote:

What changes is the interface of the `(array)` operator.


I understand what you mean, I just disagree that it constitutes a BC break
in the sense that no existing code would break/misbehave by simply updating
to a PHP version including the feature.

That's just the only sane criteria by which to label changes as BC breaks,
because if we don't stick to that, there'd be an argument to be made that
literally every change is. And that would mean the term loses its meaning
and becomes useless.

Cheers,
Andrey.
As just told: existing code hinting against generic `object` breaks. This
is a change in semantics in an existing operator. It is just sane to
consider it a BC break, since the operator cannot be relied upon for a
certain family of problems: invent a new operator.

That's basically the path to follow anytime something could be overloaded,
but shouldn't due to BC constraints.



On 16 Mar 2017 1:22 a.m., "Andrey Andreev" <[email protected]> wrote:

>
>
> On Mar 16, 2017 2:01 AM, "Marco Pivetta" <[email protected]> wrote:
>
> What changes is the interface of the `(array)` operator.
>
>
> I understand what you mean, I just disagree that it constitutes a BC break
> in the sense that no existing code would break/misbehave by simply updating
> to a PHP version including the feature.
>
> That's just the only sane criteria by which to label changes as BC breaks,
> because if we don't stick to that, there'd be an argument to be made that
> literally every change is. And that would mean the term loses its meaning
> and becomes useless.
>
> Cheers,
> Andrey.
>
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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
FWIW I do prefer having a specific interface to do such conversions. I was
just trying to find a way to have list-like or Traversable objects ability
to be converted to plain array. This brings more problems than actually
solving. I will just stop thinking about this and can give this thread as
reference of why this feature would be a bad idea.
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/

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
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/
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/
2017-03-16 11:01 GMT-03:00 Marco Pivetta <[email protected]>:

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

Thanks,
Márcio.
On Thu, Mar 16, 2017 at 3:34 PM, Marcio Almada <[email protected]>
wrote:

> 2017-03-16 11:01 GMT-03:00 Marco Pivetta <[email protected]>:
>
>> 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/
Sorry, only registered users may post in this forum.

Click here to login