[RFC][DISCUSSION] Change var_export() array syntax to use short hand arrays View as Markdown

On Sun, Mar 29, 2020 at 3:43 PM Benjamin Morel benjamin.morel@gmail.com
wrote:

This is the syntax I'm using in brick/varexporter
https://github.com/brick/varexporter, and supporting it natively in
var_export() makes sense.

Ditto Danack's question, adding: "Then why not use brick/varexporter?"

-Sara

Hi again,

Andrea Faulds wrote:

Hi Sherif,

Sherif Ramadan wrote:

I'm proposing a new RFC to change var_export()'s array syntax to use the
short hand array syntax instead.

The RFC is available here
https://wiki.php.net/rfc/var-export-array-syntax

This is a forwards-compatible change proposed for and targeting PHP 8.0.

I'd like to open up discussion.

What about stdClass? That is currently exported using an object cast
applied to an array literal, and that array literal uses long array
syntax for consistency. I think this should be mentioned in the RFC,
whatever choice you make.

Oops, I forgot about classes with ::__set_state! (I only thought about
stdClass because it was me that added support for it, unlike for
__set_state.) That also uses the old array syntax.

Thanks and sorry for spam,
Andrea

@Dan Ackroyd danack@basereality.com

That's an aesthetic choice, which not everyone will agree with.

Yes, I freely state that in the RFC. Everyone is free to vote how they like.

"There shouldn't be any backwards incompatible changes"

It apparently breaks 25 tests that compare the output of var_export
with an expected version. Userland users who have similar tests will
experience similar breaks.

A test that compares var_export output is a pretty broken test, in my
opinion. This is similar to a test that would use the value of a constant
to test that the constant works. You're testing PHP code you shouldn't care
if it uses the long form array construct or short form. Just test that it
gives you an array.

With that said, I will update the RFC to make note of these tests and I
will fix them should the RFC be voted in.

Is there any reason to do this change other than aesthetic choice?

Just because it is aesthetic is not actually an argument against. Why
wouldn't you want to improve the aesthetics of PHP? Is it only an aesthetic
choice, however? Not entirely. I also state that it is easier to read and
type (saves space) and is more inline with JSON notation which is a lot
more ubiquitous these days. These are all pros. I see no cons in that
change.

VAR_EXPORT_NO_INDEX

Note that I wouldn't consider a BC break to change var_export()'s behaviour
to always discard indexes when they're numeric, I'm not sure this should
even be an option!

• Ben

So to recap on what's been discussed so far, because I plan to bring this
to a vote next week...

I am updating the RFC to reflect that the change will no longer break
backwards compatibility. I will be adding an flags argument with the
following 3 optional bitwise flags for var_export:

• VAR_EXPORT_SHORT_ARRAY [Triggers short-array syntax]
• VAR_EXPORT_NO_INDEX [Discards numbered indexes in arrays (only if they
  are sequential from 0)]
• VAR_EXPORT_NO_WHITESPACE [For one-liners]

This complicates the implementation slightly, but I think it brings a
greater net-benefit.

Hello,

First, thank you for proposing this RFC. I have no strong opinion
either way, but anyway it cannot work as is, because var_export
already has a second optional parameter (bool $return = false). You
could add the flags as a third optional parameter (int $options = 0),
or maybe make it replace (extend) the existing one and add an extra
constant, say VAR_EXPORT_RETURN = 1, so that it can be combined
(although I'm not sure that changing the type from bool to int (or
maybe bool|int) would be BC-compliant...)

Moreover the RFC currently says that var_export([1, 2, 3]) produces

array(1, 2, 3)

but it actually produces

array (
  0 => 1,
  1 => 2,
  2 => 3,
)

Regards,

--
Guilliam Xavier