Everything

php

[PATCH] Make var_export() output "(object)array(..." instead of "stdClass::__set_state(..." for stdClass

php.internals

Andrew Faulds

9 years ago

Hi everyone,

Since stdClass has no __set_state method, var_export() produces unusable
output if given an object of that class. I wrote a patch that would make
var_export() produce a cast to object instead, which could be evaluated
to get back a stdClass:

https://github.com/php/php-src/pull/2420

Any thoughts/comments?

If you're wondering about whether a __set_state method should be added
to stdClass, I posted some thoughts in the pull request discussion already.

Thanks!

--
Andrea Faulds
https://ajf.me/

Fleshgrinder

9 years ago

On 3/14/2017 7:57 PM, Andrea Faulds wrote:

> Hi everyone,
>
> Since stdClass has no __set_state method, var_export() produces unusable
> output if given an object of that class. I wrote a patch that would make
> var_export() produce a cast to object instead, which could be evaluated
> to get back a stdClass:
>
> https://github.com/php/php-src/pull/2420
>
> Any thoughts/comments?
>
> If you're wondering about whether a __set_state method should be added
> to stdClass, I posted some thoughts in the pull request discussion already.
>
> Thanks!

I actually already used __set_state occasionally for userland classes.
Adding __set_state to stdClass seems to be more consistent with the rest
of PHP, rather than special casing it in one method. I cannot see any
issues with this to be honest.

--
Richard "Fleshgrinder" Fussenegger

Marc Bennewitz

9 years ago

Am 14.03.2017 um 20:26 schrieb Fleshgrinder:

> On 3/14/2017 7:57 PM, Andrea Faulds wrote:
>> Hi everyone,
>>
>> Since stdClass has no __set_state method, var_export() produces unusable
>> output if given an object of that class. I wrote a patch that would make
>> var_export() produce a cast to object instead, which could be evaluated
>> to get back a stdClass:
>>
>> https://github.com/php/php-src/pull/2420
>>
>> Any thoughts/comments?
>>
>> If you're wondering about whether a __set_state method should be added
>> to stdClass, I posted some thoughts in the pull request discussion already.
>>
>> Thanks!
>
> I actually already used __set_state occasionally for userland classes.
> Adding __set_state to stdClass seems to be more consistent with the rest
> of PHP, rather than special casing it in one method. I cannot see any
> issues with this to be honest.
>

Personally I would like to have a more reasonable way in general. No
special case and no magic method.

So my proposal then would be to try to add a class cast operator (needs
an own RFC) and later to with this approach on var_export.

Example:

class Foo {}

$std = new stdClass;
$foo = new Foo;
$arr = ['arr' => 'arr'];

var_dump((object)$std);   // no change
var_dump((array)$std);    // no change
var_dump((Foo)$std);      // object of class Foo
var_dump((object)$foo);   // no change
var_dump((stdClass)$foo); // object of class stdClass
var_dump((array)$foo);    // no change
var_dump((array)$arr);    // no change
var_dump((object)$arr);   // no change
var_dump((stdClass)$arr); // object of class stdClass
var_dump((Foo)$arr);      // object of class Foo

Thoughts?

Marc

Andrew Faulds

9 years ago

Hi Marc,

Marc Bennewitz wrote:

>
> Personally I would like to have a more reasonable way in general. No
> special case and no magic method.
>
> So my proposal then would be to try to add a class cast operator (needs
> an own RFC) and later to with this approach on var_export.

I've wondered if this would make sense before myself. But I think any
such mechanism would need explicit support from the class being casted
to, if the resulting internal object state is to make sense. stdClass
wouldn't have that support.

Thanks.

--
Andrea Faulds
https://ajf.me/

Derick Rethans

9 years ago

On Tue, 14 Mar 2017, Andrea Faulds wrote:

> Hi everyone,
>
> Since stdClass has no __set_state method, var_export() produces unusable
> output if given an object of that class. I wrote a patch that would make
> var_export() produce a cast to object instead, which could be evaluated to get
> back a stdClass:
>
> https://github.com/php/php-src/pull/2420
>
> Any thoughts/comments?

As original author of var_export, I approve of this — including not
adding __set_state().

cheers,
Derick

Nikita Popov

9 years ago

On Tue, Mar 14, 2017 at 11:20 PM, Derick Rethans <derick@php.net> wrote:

> On Tue, 14 Mar 2017, Andrea Faulds wrote:
>
> > Hi everyone,
> >
> > Since stdClass has no __set_state method, var_export() produces unusable
> > output if given an object of that class. I wrote a patch that would make
> > var_export() produce a cast to object instead, which could be evaluated
> to get
> > back a stdClass:
> >
> > https://github.com/php/php-src/pull/2420
> >
> > Any thoughts/comments?
>
> As original author of var_export, I approve of this — including not
> adding __set_state().
>
> cheers,
> Derick
>

I'm also +1 on this, and also prefer not having __set_state(). An object
cast is both more idiomatic and more performant.

Additionally I feel that adding methods to stdClass will muddy the waters
-- for example, this means that extending stdClass is no longer entirely
unreasonable, as you might want to do it to reuse the __set_state()
implementation. Not something I want to see happening.

Nikita

Stas Malyshev

9 years ago

Hi!

> Additionally I feel that adding methods to stdClass will muddy the waters
> -- for example, this means that extending stdClass is no longer entirely
> unreasonable, as you might want to do it to reuse the __set_state()
> implementation. Not something I want to see happening.

I'm afraid that ship has sailed long ago:
https://github.com/search?q=%22extends+stdclass%22&type=Code&utf8=%E2%9C%93

--
Stas Malyshev
smalyshev@gmail.com

Christoph Becker

8 years ago

On 14.03.2017 at 19:57, Andrea Faulds wrote:

> Since stdClass has no __set_state method, var_export() produces unusable
> output if given an object of that class. I wrote a patch that would make
> var_export() produce a cast to object instead, which could be evaluated
> to get back a stdClass:
>
> https://github.com/php/php-src/pull/2420
>
> Any thoughts/comments?
>
> If you're wondering about whether a __set_state method should be added
> to stdClass, I posted some thoughts in the pull request discussion already.

FTR: If nobody objects, I'll merge this PR into master on 2018-07-12, so
that it goes into PHP 7.3.

See <https://github.com/php/php-src/pull/2420#issuecomment-402561913>.

--
Christoph M. Becker

Andreas Hennings

8 years ago

I do not disagree, just want to make an observation.

If multiple properties or array keys reference the same instance of
\stdClass, there will be multiple instances with identical values after a
round-trip with var_export() + eval().
This is not necessarily a problem, just something to be aware of.

On Wed, 4 Jul 2018 at 22:33, Christoph M. Becker <cmbecker69@gmx.de> wrote:

Andrew Faulds

8 years ago

Christoph M. Becker wrote:

> On 14.03.2017 at 19:57, Andrea Faulds wrote:
>
>> Since stdClass has no __set_state method, var_export() produces unusable
>> output if given an object of that class. I wrote a patch that would make
>> var_export() produce a cast to object instead, which could be evaluated
>> to get back a stdClass:
>>
>> https://github.com/php/php-src/pull/2420
>>
>> Any thoughts/comments?
>>
>> If you're wondering about whether a __set_state method should be added
>> to stdClass, I posted some thoughts in the pull request discussion already.
>
> FTR: If nobody objects, I'll merge this PR into master on 2018-07-12, so
> that it goes into PHP 7.3.
>
> See <https://github.com/php/php-src/pull/2420#issuecomment-402561913>.
>

And so it was done:
https://github.com/php/php-src/commit/e4e9cd835550990a6b8df7c61d59b6cc0da9b5b2

Thanks, Cristoph! :)

--
Andrea Faulds
https://ajf.me/