Incorrect behavior of WrapVariableVariableNameInCurlyBracesRector

[corentin-larose]

Bug Report

Subject	Details
Rector version	last dev-main
Installed as	composer dependency

Minimal PHP Code Causing Issue

See https://getrector.com/demo/7ebfdb2e-88ec-4c6e-9dbe-2faa74a30977

<?php
$$foo['bar']['baz'];

Responsible rules

• WrapVariableVariableNameInCurlyBracesRector

Expected Behavior

According to https://www.php.net/manual/en/migration70.incompatible.php:

Expression	PHP 5 interpretation
$$foo['bar']['baz']	${$foo['bar']['baz']}

[samsonasik]

The result seems valid, see https://3v4l.org/D3TDJ , the rule is on php 7.0 migration, not php 5.x and it show valid and equal result, could you show me which one that it cause invalid result at https://3v4l.org ? Thank you.

[corentin-larose]

Your test hides the versions which actually interest us: PHP5.6 and PHP7.0.

Take a close look at those tests, you will see that the code won't do the same think in PHP5.6 and PHP7.0 despite the Rector modification, proof is you have an error in PHP5.6, I will take some time tonight in order to create better examples to explain my point:

https://3v4l.org/oCUdQ#v5.6.40 https://3v4l.org/gh24o#v7.0.33

[samsonasik]

The rule is for php7.0 upgrade, not php 5.6, so the invalid php 5.6 code is fixed on php 7.0 rule.

We don't change to 2 kind of versions, php 5.6 vs php 7.0, instead: use syntax that works for both :), see https://3v4l.org/gh24o#v5.6.40

[corentin-larose]

Thanks @samsonasik, let's put it another way, I understand I have not been clear in my previous message.

Can you please check each proposition and check it if you agree:

• [ ] Purpose of the SetList::PHP70 rule is to modify PHP 5.6 code in order to make it work with the PHP 7.0 interpreter, obviously NOT changing the result of the code.
• [ ] Rector\Php70\Rector\Variable\WrapVariableVariableNameInCurlyBracesRector belongs to SetList::PHP70.
• [ ] In PHP 7.0, you can no longer write $$foo->bar, you MUST write ${$foo->bar} instead.
• [ ] WrapVariableVariableNameInCurlyBracesRector rule has been written in order to perform this very part of the PHP 5.6 to PHP 7.0 migration process.
• [ ] When you apply WrapVariableVariableNameInCurlyBracesRector rule on the PHP 5.0 code $$foo['bar'] it modifies it as ${$foo}['bar'].
• Now please, run the following code:

// Run this on PHP 5.6 (https://3v4l.org/XB19f#v5.6.40)
<?php
$baz = 'bat';
$foo = array('bar' => 'baz');

echo $$foo['bar'];
echo PHP_EOL;
echo ${$foo['bar']};
echo PHP_EOL;

// PHP 5.6 output:
bat
bat

// Run this on PHP 7.0 (https://3v4l.org/jBLbv#v7.0.0)
<?php
$baz = 'bat';
$foo = array('bar' => 'baz');

echo ${$foo}['bar']; // This is the way `WrapVariableVariableNameInCurlyBracesRector` currently fixes the code.
echo PHP_EOL;
echo ${$foo['bar']}; // This is the way `WrapVariableVariableNameInCurlyBracesRector` SHOULD fix the code. 
echo PHP_EOL;

// PHP7.0 output
Notice: Array to string conversion in /in/jBLbv on line 5

Notice: Undefined variable: Array in /in/jBLbv on line 5

bat

• [ ] After applying rule Rector\Php70\Rector\Variable\WrapVariableVariableNameInCurlyBracesRector the behavior of the code changes, WHICH IS A BUG.
• As I wrote before, this is absolutely NO SUPRISE, since this is explained in the "Changes to the handling of indirect variables, properties, and methods" of the PHP 5.6 to PHP 7.0 migration documentation.

I DO make mistakes, but I take a lot of time before filing bugs, so please take your time to read me carefully :)

[samsonasik]

Re-open, it seems cause invalid on valid code:

        $foo = 'a';
        $a = [
            'bar' => [
                'baz' => 'test',
            ],
        ];

        echo $$foo['bar']['baz'];

[samsonasik]

It seems it always depends of how variable is fetched, see it https://3v4l.org/eip64 vs https://3v4l.org/tYFTb

[samsonasik]

@corentin-larose after some try, it always depends of what variable and its value, so I am reverting to original functionality.

Thank you for understanding.

[github-actions[bot]]

This issue has been automatically locked because it has been closed for 150 days. Please open a new issue if you have a similar problem.