Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removed warning about missing namespace leading backslash in var_export() docs #1472

Closed
wants to merge 1 commit into from

Conversation

Ocramius
Copy link

var_export() needs to prefix classes it references with \, because its code could be transplanted in
any source location/namespace, so the assumption of it being used only in the context of the root
namespace is not sufficient.

For example, in a code snippet like following ( https://3v4l.org/4mONc ):

<?php

class SomeObject {}
var_export([new SomeObject]);

This should produce:

array (
  0 => 
  \SomeObject::__set_state(array(
  )),
)

Userland should not concern itself with the contents of the var_export()-produced code
snippets, and use them as-is instead.

Ref: php/php-src#8232
Ref: php/php-src#8233
Ref: Ocramius/ProxyManager#754

…ort()` docs

`var_export()` needs to prefix classes it references with `\`, because its code could be transplanted in
any source location/namespace, so the assumption of it being used only in the context of the root
namespace is not sufficient.

For example, in a code snippet like following ( https://3v4l.org/4mONc ):

```php
<?php

class SomeObject {}
var_export([new SomeObject]);
```

This should produce:

```
array (
  0 => 
  \SomeObject::__set_state(array(
  )),
)
```

Userland should not concern itself with the contents of the `var_export()`-produced code
snippets, and use them as-is instead.

Ref: php/php-src#8232
Ref: php/php-src#8233
Ref: Ocramius/ProxyManager#754
@cmb69
Copy link
Member

cmb69 commented Mar 21, 2022

Hmm, but see https://externals.io/message/67008#67010. ;)

@Ocramius
Copy link
Author

Aye, mistakes were made :|

Indeed confusing scope on my end:

  • serialize() provides a value inside a string, hence always global namespace
  • var_export() provides a code snippet, hence contextual to execution scope

Sorry for my previous mistake on that: my idea of var_export() was indeed broken.

See also php/php-src#8233 (comment)

@guilliamxavier
Copy link
Contributor

Should probably also add a row in the Changelog, and update Examples, as was done in 68601c39b9543a3d848e01f90f1fbb5512f39929^...9ddc8b6?

@Ocramius
Copy link
Author

Thanks @guilliamxavier: will try emulating that patch here, once php/php-src#8233 is merged

@TimWolla TimWolla requested a review from Girgias December 13, 2022 19:36
Copy link
Member

@TimWolla TimWolla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, especially with the change in php/php-src#8233. Users should not make any assumptions about the output, except that the return value is a valid PHP expression.

@TimWolla TimWolla requested a review from cmb69 December 13, 2022 19:37
@Girgias
Copy link
Member

Girgias commented Dec 15, 2022

This feels like it needs a changelog entry for 8.2 to explain the change (and also added to the migration guide/UPGRADING file, as that somehow didn't get done, so the change got missed).

Otherwise, I don't really mind dropping it, but I think it may be useful to have it somewhere that details the nuance and pitfall.

@cmb69
Copy link
Member

cmb69 commented Jan 11, 2023

I just landed 0f27fad, so this ticket can be closed. Thanks everybody!

@cmb69 cmb69 closed this Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants