PHP7 Error: Indirect access to variable

Looking at cleaning up my php7 codesniffs as well. Any idea if there is a reproducible error as a result of this on php7 or is this just an improved compliance patch? I've poked around a bit but can't seem to create any unexpected behaviour without this patch. Either way I'll be running with the patch and report back if I see any problems.

Hm. I'm not sure this patch is right.

See:

http://php.net/manual/en/migration70.incompatible.php#migration70.incomp...

It gives this example:

// Expression
$foo->$bar['baz']

// PHP 5 interpretation
$foo->{$bar['baz']}

// PHP 7 interpretation
($foo->$bar)['baz']

I think the PHP 7 interpretation is actually what that code is expecting! So, this actually should doing the wrong thing on PHP 5...

But also, the patch is using curly braces, but I don't think actually affecting the order that things are evaluated because they are only wrapping one part, instead of two.

I think the code we want is:

{$entity->$field_name}[$langcode] = array();

Right? Or, am misunderstanding the PHP5/PHP7 difference here?

@dsnopek you are right that the patch likely isn't doing anything but I'm not sure about what the code is expecting. Per the documentation if we want to maintain the php5 interpretation in php7 then we want to use the php5 interpretation as our example:

$foo->{$bar['baz']}

so we need:

$entity->{$field_name[$langcode]} = array();

Again I can't reproduce a problem in php5 or 7 so it's difficult to know if this is the correct solution. All I can say for sure is that this will cause this line to be interpreted the same in php7 as it previously was in php5. Thanks for calling attention to this as we clearly didn't have a working solution yet.

Yeah, the thing is that PHP5 interpretation (at least per those docs) would be wrong, so I'm actually not sure how this code is working now :-)

$entity->{$field_name[$langcode]} = array();

This means "get the $langcode index from the $field_name array, and use it as the property on the $entity object" whereas what we want to say is "get the $langcode index from the array identified by the $field_name property on the $entity object" (the PHP7 interpretation)

Maybe PHP5 does things differently when it's all variables rather than a literal for the array index?

@dsnopek I see what you're saying and logically it makes sense. However your suggestion won't compile since you can't start a line with { so I'm not sure where we go from here. If I can figure out a simple way to get this code to fire I'll run through some scenarios and see what actually happens.

I don't think patch #8 works at all. $field_name is a string, $entity->$field_name is an array.

After the patch, first $field_name[$lang_code] will be evaluated, which doesn't exist, and after that $entity->null is evaluated, which also doesn't exist. The right order is $entity->$field_name to get the field contents and then $field_contents[$langcode] to get the contents. The current code works fine in PHP 7, maybe it doesn't work in PHP 5.

@Jorrit the more I look at this the more I think you and @dsnopek are right that this is actually broken in php5. I think #4 gives us exactly what we want by fixing the behaviour in php5 and maintaining the correct behaviour in php7. This is a pretty weird way to find this bug but I'm quite comfortable running with #4.

If I understand everything correctly here are two samples to show the output with and without the brackets.

Current code
PHP 7 - works
PHP 5 -

Warning: Illegal string offset 'en' in /in/HZ9OR on line 10
Notice: Undefined property: entity::$b in /in/HZ9OR on line 10

With brackets like #4
PHP 7 - works
PHP 5 - works

Tentatively moving #4 to RTBC as I think we have this sorted out.

Interesting! Thanks for all the testing :-)

The patch on #4 looked to me like it wouldn't do anything and that we'd need something more like:

($entity->$field_name)[$langcode] = array();

Would it be possible for you to do your tests again with the patch from #4 but also capture what the $entity object looks like after that line runs on both PHP5 and PHP7?

As a random idea (there might be an easier way), you could do watchdog('debug', print_r($entity, TRUE)); and then copy-paste it from the "Recent log messages" report into a text file and attach here.

If that field ends up the way we're expecting on both versions, I'll commit that patch!

Thanks again :-)

I can not for the life of me get this line of code to fire. I tried to follow the process in the issue that lead to this line of code but I can't get inside the if statement at line 628:
if ($field['type'] == 'field_collection') {

When I do dpm($field); on the line preceding that if statement there is never a $field[type] property. Here is the output of that dpm($field);

translatable (String, 1 characters ) 0
entity_types (Array, 0 elements)
settings (Array, 1 element)
storage (Array, 5 elements)
foreign keys (Array, 1 element)
indexes (Array, 1 element)
id (String, 2 characters ) 24
field_name (String, 10 characters ) field_make
type (String, 4 characters ) text
module (String, 4 characters ) text
active (String, 1 characters ) 1
locked (String, 1 characters ) 0
cardinality (String, 1 characters ) 1
deleted (String, 1 characters ) 0
columns (Array, 2 elements)
bundles (Array, 1 element)

I get one for each field within the collection instead of one field for the entire collection. I have tried doing a regular content field collection and a display field collection and the results are the same. Attached is a screenshot of the live preview using IPE to edit this page. It just works all the time and never gets into the if statement so none of the extra field_collection code ever runs. Not sure where to go from here.

Duh. I mixed up field_group and field_collection. When using field collection in php5 I get exactly the expected error in watchdog when previewing a panel page with a field collection:

Warning: Illegal string offset 'und' in _panopoly_magic_process_field_collection_fields() (line 655 of /Users/James/Sites/devdesktop/panopoly/profiles/panopoly/modules/panopoly/panopoly_magic/panopoly_magic.module).

Line 655 is $entity->$field_name[$langcode] = array();

If I add a watchdog('debug', print_r($entity->$field_name[$langcode], TRUE)); it comes back as array() so of course assigning it to an empty array has no effect. If I change the print_r to use the brackets in the patch watchdog('debug', print_r($entity->{$field_name}[$langcode], TRUE)); we get what we intended, the field_collection array and it's values.

Array ( [0] => Array ( [field_fc_make] => Array ( [und] => Array ( [0] => Array ( [value] => Ford ) ) ) [field_fc_model] => Array ( [und] => Array ( [0] => Array ( [value] => Fusion ) ) ) ) )

If I adjust line 655 to match then we actually clear out those values instead of just assigning an empty array to an empty array.

If I switch to php7 I get the same results with or without the brackets. I get the expected values in the array:

Array ( [0] => Array ( [field_fc_make] => Array ( [und] => Array ( [0] => Array ( [value] => Ford ) ) ) [field_fc_model] => Array ( [und] => Array ( [0] => Array ( [value] => Fusion ) ) ) ) )

and they are blanked out by line 655 as intended. This code definitely never did anything in php5 and would work as intended in php7. Patch #4 get is identical behaviour in both.

My only lingering question is if this line of code is actually accomplishing anything. I don't see any negative effects beyond the watchdog warnings in php5 so it's difficult to say if something is actually broken.

You asked for print_r of the entire entity before and after the patched line so here it is using the patched code to actually show some effect:

 watchdog('debug', print_r($entity, TRUE));
 $entity->{$field_name}[$langcode] = array()
 watchdog('debug', print_r($entity, TRUE));

Before $entity->{$field_name}[$langcode] = array()

stdClass Object ( [vid] => 18 [timestamp] => 1511312092 [uid] => 1 [title] => My car [log] => [vuuid] => d15fa426-928a-42bc-9f65-624c46d4cf6f [fpid] => 7 [bundle] => car [link] => 0 [path] => [reusable] => 0 [admin_title] => [admin_description] => [category] => Reusable Content [view_access] => [edit_access] => [created] => 1511311300 [changed] => 1511312092 [uuid] => 86f11446-e93f-425e-b948-7da2197f7b59 [language] => [current_vid] => 18 [field_info] => Array ( [und] => Array ( [0] => Array ( [field_fc_make] => Array ( [und] => Array ( [0] => Array ( [value] => Ford ) ) ) [field_fc_model] => Array ( [und] => Array ( [0] => Array ( [value] => Fusion ) ) ) ) ) ) [revision] => 1 [f] => Array ( ) )

After $entity->{$field_name}[$langcode] = array()

stdClass Object ( [vid] => 18 [timestamp] => 1511312092 [uid] => 1 [title] => My car [log] => [vuuid] => d15fa426-928a-42bc-9f65-624c46d4cf6f [fpid] => 7 [bundle] => car [link] => 0 [path] => [reusable] => 0 [admin_title] => [admin_description] => [category] => Reusable Content [view_access] => [edit_access] => [created] => 1511311300 [changed] => 1511312092 [uuid] => 86f11446-e93f-425e-b948-7da2197f7b59 [language] => [current_vid] => 18 [field_info] => Array ( [und] => Array ( ) ) [revision] => 1 [f] => Array ( ) )

If I do it in php5 without the patch the before and after are identical.

Awesome! Thanks again for all the testing :-) Based on the results in #16, I've merged the patch from #4