Fix #8232 - always reference classes in `var_export()` via their FQCN #8233

Ocramius · 2022-03-21T13:16:30Z

This fix corrects a behavior of var_export() that was mostly "hidden" until PHP 8.1 introduced:

properties with object initializers
constants containing object references
default values of class properties containing enums

Since var_export(..., true) is mostly used in conjunction with code generation,
and we cannot make assumptions about the generated code being placed in the root
namespace, we must always provide the FQCN of a class in exported code.

For example:

<?php

namespace MyNamespace { class Foo {} }

namespace { echo "<?php\n\nnamespace Example;\n\n" . var_export(new \MyNamespace\Foo(), true) . ';'; }

produces:

<?php

namespace Example;

MyNamespace\Foo::__set_state(array(
));

This code snippet is invalid, because Example\MyNamespace\Foo::__set_state() (which
does not exist) is called.

With this patch applied, the code looks like following (valid):

<?php

namespace Example;

\MyNamespace\Foo::__set_state(array(
));

Fixes #8232
Ref: Ocramius/ProxyManager#754

Zend/tests/bug73350.phpt

ext/intl/tests/dateformat_format.phpt

ext/intl/tests/dateformat_format_variant2.phpt

ext/intl/tests/dateformat_format_variant3.phpt

ext/spl/tests/fixedarray_023.phpt

ext/standard/tests/array/007.phpt

ext/standard/tests/array/array_intersect_1.phpt

ext/standard/tests/general_functions/var_export-locale.phpt

ext/standard/tests/general_functions/var_export-locale_32.phpt

main/php_version.h

iluuu1994 · 2022-03-21T13:37:58Z

Related. doctrine/orm#9371 IMO this change is too controversial for a patch for sure.

EDIT: Just adding the \ for enums would be less controversial and OK from my side.

EDIT 2: IN any case, you might want to see what the mailing list thinks about this change.

Ocramius · 2022-03-21T13:40:30Z

Nothing controversial about it: var_export() exports values for usage somewhere else, so IMO this is fine as-is. If other maintainers want to bring this up to RFC level, that's up to them, but IMO this is a clear bugfix.

Ocramius · 2022-03-21T13:42:06Z

Note: a userland fix is not viable, because values need to be referenced via FQCN even in nested structures like: [[Foo::BAR]] being var_export()-ed.

Ocramius · 2022-03-21T13:49:40Z

Well, poop. @JetBrains seems to drop all whitespace on commit (even after I disabled all the tooling around it).

Guess editing from plaintext editor it is :D

iluuu1994 · 2022-03-21T13:51:43Z

@Ocramius You can revert all test changes and run the tests with --bless.

Ocramius · 2022-03-21T13:54:03Z

Can't run tests locally due to https://bugs.php.net/bug.php?id=80585 - manual editing required for now.

IonBazan · 2022-03-21T14:17:52Z

@Ocramius to me it also looks quite controversial. Especially that serialize(), $object::class and get_class() output it without the leading slash so it seems a bit inconsistent from there. I understand this is the easiest solution but I am a bit worried about existing code that relies on current behaviour.

Also found another related issue: doctrine/orm#9371
See also some ancient bug: https://bugs.php.net/bug.php?id=64554

Ocramius · 2022-03-21T14:23:25Z

serialize() produces a string that is designed to be used with unserialize()
var_export() produces a code snippet of executable PHP code, to be used with include, require or eval()

Different use-cases, no intended consistency across the two.

…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

Ocramius · 2022-03-21T14:41:51Z

Since I never (ever) read PHP docs, I went there and checked for the ancient bug found by @IonBazan, and went to actually fix the problem at its root there too.

Docs PR is therefore at php/doc-en#1472

Also: holy crap, people post-processing var_export() output with regular expressions are totally nuts - some comments in there really need to be deleted, before they lay eggs :|

cmb69 · 2022-03-21T15:26:40Z

IN any case, you might want to see what the mailing list thinks about this change.

👍

iluuu1994 · 2022-03-31T14:10:35Z

@Ocramius Are you planning on starting a discussion on the mailing list?

Ocramius · 2022-03-31T14:29:56Z

Nay, already got enough to do 😁

iluuu1994 · 2022-03-31T15:06:11Z

Ok, I'll send one (but proposing for master, not PHP-8.1).

Ocramius · 2022-04-19T15:38:09Z

As mentioned in https://externals.io/message/117466#117532, this will go to PHP 8.2.

This fix corrects a behavior of `var_export()` that was mostly "hidden" until PHP 8.1 introduced: * properties with object initializers * constants containing object references * default values of class properties containing `enum`s Since `var_export(..., true)` is mostly used in conjunction with code generation, and we cannot make assumptions about the generated code being placed in the root namespace, we must always provide the FQCN of a class in exported code. For example: ```php <?php namespace MyNamespace { class Foo {} } namespace { echo "<?php\n\nnamespace Example;\n\n" . var_export(new \MyNamespace\Foo(), true) . ';'; } ``` produces: ```php <?php namespace Example; MyNamespace\Foo::__set_state(array( )); ``` This code snippet is invalid, because `Example\MyNamespace\Foo::__set_state()` (which does not exist) is called. With this patch applied, the code looks like following (valid): ```php <?php namespace Example; \MyNamespace\Foo::__set_state(array( )); ``` Ref: php#8232 Ref: Ocramius/ProxyManager#754

Ocramius · 2022-04-23T09:10:34Z

Thanks @iluuu1994!

This PR was merged into the 4.4 branch. Discussion ---------- Fix dumping enums on PHP 8.2 | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | - | License | MIT | Doc PR | - Follows php/php-src#8233 Makes the CI green again on PHP 8.2. Replaces #46160 Commits ------- 4244aa8 Fix dumping enums on PHP 8.2

Ocramius changed the base branch from master to PHP-8.1 March 21, 2022 13:19

Ocramius commented Mar 21, 2022

View reviewed changes

Ocramius force-pushed the fix/#8232-ensure-var-export-always-produces-namespaced-code branch from 1e45b50 to 8e0a680 Compare March 21, 2022 13:39

Ocramius force-pushed the fix/#8232-ensure-var-export-always-produces-namespaced-code branch from 8e0a680 to a53ad90 Compare March 21, 2022 13:47

Ocramius force-pushed the fix/#8232-ensure-var-export-always-produces-namespaced-code branch from a53ad90 to 8b12af4 Compare March 21, 2022 14:18

Ocramius force-pushed the fix/#8232-ensure-var-export-always-produces-namespaced-code branch from 8b12af4 to bca7f7f Compare March 21, 2022 14:24

Ocramius mentioned this pull request Mar 21, 2022

Setting property default value to enum points to invalid namespace Ocramius/ProxyManager#754

Open

Ocramius mentioned this pull request Mar 21, 2022

Removed warning about missing namespace leading backslash in var_export() docs php/doc-en#1472

Closed

Ocramius force-pushed the fix/#8232-ensure-var-export-always-produces-namespaced-code branch 3 times, most recently from 52d28ad to 4f5a3b0 Compare March 21, 2022 15:07

cmb69 linked an issue Mar 21, 2022 that may be closed by this pull request

var_export() combined with enum produces code unsuitable for inclusion in namespaces #8232

Closed

iluuu1994 added the Waiting on Author label Mar 31, 2022

iluuu1994 removed the Waiting on Author label Mar 31, 2022

iluuu1994 force-pushed the fix/#8232-ensure-var-export-always-produces-namespaced-code branch from 4f5a3b0 to 09db6c8 Compare April 22, 2022 14:03

iluuu1994 changed the base branch from PHP-8.1 to master April 22, 2022 14:03

iluuu1994 force-pushed the fix/#8232-ensure-var-export-always-produces-namespaced-code branch 2 times, most recently from 89ecffd to 46d9157 Compare April 22, 2022 14:11

iluuu1994 force-pushed the fix/#8232-ensure-var-export-always-produces-namespaced-code branch from 46d9157 to fd6228e Compare April 22, 2022 15:15

iluuu1994 closed this in 25cb9cd Apr 23, 2022

Ocramius deleted the fix/#8232-ensure-var-export-always-produces-namespaced-code branch April 23, 2022 09:10

nicolas-grekas mentioned this pull request Apr 25, 2022

Fix dumping enums on PHP 8.2 symfony/symfony#46170

Merged

IonBazan mentioned this pull request Oct 13, 2022

fixExport() produces invalid FQCN on PHP 8.2 FriendsOfPHP/proxy-manager-lts#28

Closed

qkdreyer mentioned this pull request Jan 4, 2023

feat: add php 8.2 compatibility rgsystemes/php-driver#5

Merged

cmb69 mentioned this pull request Jan 5, 2023

var_export() added leading backslashes in PHP 8.2 php/doc-en#2145

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #8232 - always reference classes in `var_export()` via their FQCN #8233

Fix #8232 - always reference classes in `var_export()` via their FQCN #8233

Ocramius commented Mar 21, 2022 •

edited

Loading

iluuu1994 commented Mar 21, 2022 •

edited

Loading

Ocramius commented Mar 21, 2022

Ocramius commented Mar 21, 2022

Ocramius commented Mar 21, 2022

iluuu1994 commented Mar 21, 2022

Ocramius commented Mar 21, 2022

IonBazan commented Mar 21, 2022 •

edited

Loading

Ocramius commented Mar 21, 2022 •

edited

Loading

Ocramius commented Mar 21, 2022

cmb69 commented Mar 21, 2022

iluuu1994 commented Mar 31, 2022

Ocramius commented Mar 31, 2022 •

edited

Loading

iluuu1994 commented Mar 31, 2022

Ocramius commented Apr 19, 2022

Ocramius commented Apr 23, 2022

Fix #8232 - always reference classes in var_export() via their FQCN #8233

Fix #8232 - always reference classes in var_export() via their FQCN #8233

Conversation

Ocramius commented Mar 21, 2022 • edited Loading

iluuu1994 commented Mar 21, 2022 • edited Loading

Ocramius commented Mar 21, 2022

Ocramius commented Mar 21, 2022

Ocramius commented Mar 21, 2022

iluuu1994 commented Mar 21, 2022

Ocramius commented Mar 21, 2022

IonBazan commented Mar 21, 2022 • edited Loading

Ocramius commented Mar 21, 2022 • edited Loading

Ocramius commented Mar 21, 2022

cmb69 commented Mar 21, 2022

iluuu1994 commented Mar 31, 2022

Ocramius commented Mar 31, 2022 • edited Loading

iluuu1994 commented Mar 31, 2022

Ocramius commented Apr 19, 2022

Ocramius commented Apr 23, 2022

Fix #8232 - always reference classes in `var_export()` via their FQCN #8233

Fix #8232 - always reference classes in `var_export()` via their FQCN #8233

Ocramius commented Mar 21, 2022 •

edited

Loading

iluuu1994 commented Mar 21, 2022 •

edited

Loading

IonBazan commented Mar 21, 2022 •

edited

Loading

Ocramius commented Mar 21, 2022 •

edited

Loading

Ocramius commented Mar 31, 2022 •

edited

Loading