CallTimePassByReferenceSniff ignores functions with return value

[jan-svab]

A line 75 in file CodeSniffer / Standards / Generic / Sniffs / Functions / CallTimePassByReferenceSniff.php contains condition to ignore call time pass by references in functions calls, which have a return value:
|| isset(PHP_CodeSniffer_Tokens::$assignmentTokens[$prevCode]) === true
This is obviously intentional, because it is commented on line 71 as "Also skip if the return value is being assigned to a variable."

However, this causes for example this code not to produce an error:

<?PHP
$foo = Bar(&$fooBar);
?>

Commenting out line 75 makes sniffer correctly alert this code.

This behavior seems wrong to me and eventhough it is obviously intentional, I cannot see any reason why this line is present in source code of the sniffer. Is there any such reason? Or is it some kind of mistake? I'm using this sniffer with line 75 commentend out and it works fine for me.

[gsherwood]

I went back and had a look at the commit: e4b7736

I installed 2.3.3 and confirmed the error it was trying to fix. Installed 2.3.4 and confirmed it was fixed. Then commented out line 75 in 2.3.4 and the fix was still there and that the unit tests were still passing.

So it looks like that code was added during development but later not needed. It can probably just be removed, but I'll do a bit more testing first.

[gsherwood]

Probably also worth noting that the code would produce a warning in PHP 5.3, a fatal error in PHP 5.4+, and a syntax error in PHP 7, so we want the error to be reported.

[gsherwood]

I couldn't find any reason for that code there. The assignment scenario is checked down further, so it looks like it was a mistake. Code has been removed and all tests are still passing.

Thanks for finding and reporting this.