Assign-by-reference false positive for call-time pass-by-reference sniff

[bishopb]

Assignment by reference is only deprecated for object construction since PHP 5.3. Consider:

$o =& new StdClass(); // deprecated since 5.3

$a = array (1, 2, 3);
$b =& $a[0]; // non-deprecated in any version of PHP

However, this scenario isn't covered by Tests/sniff-examples/call_time_pass_by_reference.php and produces a false positive for code such as the above.

[aik099]

$o =& new StdClass();

Isn't this a Fatal Error since PHP 5.4?

[bishopb]

$o =& new StdClass() is fatal in 7+, a deprecation error in 5.3-5.6, and a strict error in 5.0-5.2.

See also the PHP reference.

$b =& $a[0] and all similar assignments-by-reference other than the result of new are valid in all versions of PHP.

[aik099]

In fact this code ends up with Fatal Error:

<?php

function testMe(&$param) {

}

$var = 1;
testMe(&$var);

http://3v4l.org/ErN2s

[bishopb]

Yes, that code gives a fatal. But that's not the code I'm pointing out in this issue. There are three kinds of "by reference" in PHP: pass-by-reference, assign-by-reference, and return-by-reference. The sniff handles only pass-by-reference, gives a false positive for assign-by-reference, and may also give a false positive for return-by-reference (I haven't checked that scenario).

[pixelchutes]

Something like this will throw a PHP Strict Standard error in 5.4+:

$array = array('a' => 1, 'b' => 2, 'c' => 3);
$value = array_shift(array_values($array)); // Only variables should be passed by reference
echo $value;

It would be great if PHPCompatibility could also identify this scenario.

[octalmage]

I'm getting false positives from this for a few popular WordPress plugins like Jetpack. The code looks like this:

$attr  = $this->doExtraAttributes("h$level", $dummy =& $matches[3]);

You can find the code here:

https://github.com/Automattic/jetpack/blob/master/_inc/lib/markdown/extra.php#L2505

And the logs here:

FILE: /jetpack/_inc/lib/markdown/extra.php
---------------------------------------------------------------------------------------
FOUND 5 ERRORS AFFECTING 5 LINES
---------------------------------------------------------------------------------------
1739 | ERROR | Using a call-time pass-by-reference is prohibited since php 5.4
2329 | ERROR | Using a call-time pass-by-reference is prohibited since php 5.4
2439 | ERROR | Using a call-time pass-by-reference is prohibited since php 5.4
2499 | ERROR | Using a call-time pass-by-reference is prohibited since php 5.4
2505 | ERROR | Using a call-time pass-by-reference is prohibited since php 5.4
---------------------------------------------------------------------------------------

How difficult would this be to resolve?

[wimg]

This issue was resolved a few weeks ago. Anyone still having issues with it ?

[wimg]

No new information was received, so closing for now.

[octalmage]

I just got another report of this:

This is bogus, a false-positive. The scanner is detecting this incorrectly. The Comet Cache use of =& is in a write context and not in a pass-by-reference context. Here is an example of what the scanner is detecting incorrectly: /src/includes/traits/Plugin/WcpUtils.php#L298 This is inside a function call, and while it might appear to be a pass-by-reference to a dumb robot, it is in fact not a pass-by-reference. The value is being assigned.

The code looks like this:

        if (!is_null($done = &$this->cacheKey('autoPurgeCache'))) {

It's actually assigning the value, then passing it.

Full report here:

https://wordpress.org/support/topic/false-positive-comet-cache/