Fixed bug #672 : Call-time pass-by-reference false positive

squizlabs · Aug 20, 2015 · e4b7736 · e4b7736
1 parent 646826c
commit e4b7736
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 29 deletions.
diff --git a/CodeSniffer/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php b/CodeSniffer/Standards/Generic/Sniffs/Functions/CallTimePassByReferenceSniff.php
@@ -57,32 +57,30 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
     {
         $tokens = $phpcsFile->getTokens();
 
-        // Skip tokens that are the names of functions or classes
-        // within their definitions. For example: function myFunction...
-        // "myFunction" is T_STRING but we should skip because it is not a
-        // function or method *call*.
-        $functionName = $stackPtr;
-        $findTokens   = array_merge(
+        $findTokens = array_merge(
             PHP_CodeSniffer_Tokens::$emptyTokens,
             array(T_BITWISE_AND)
         );
 
-        $functionKeyword = $phpcsFile->findPrevious(
-            $findTokens,
-            ($stackPtr - 1),
-            null,
-            true
-        );
+        $prev = $phpcsFile->findPrevious($findTokens, ($stackPtr - 1), null, true);
 
-        if ($tokens[$functionKeyword]['code'] === T_FUNCTION
-            || $tokens[$functionKeyword]['code'] === T_CLASS
+        // Skip tokens that are the names of functions or classes
+        // within their definitions. For example: function myFunction...
+        // "myFunction" is T_STRING but we should skip because it is not a
+        // function or method *call*.
+        // Also skip if the return value is being assigned to a variable.
+        $prevCode = $tokens[$prev]['code'];
+        if ($prevCode === T_FUNCTION
+            || $prevCode === T_CLASS
+            || isset(PHP_CodeSniffer_Tokens::$assignmentTokens[$prevCode]) === true
         ) {
             return;
         }
 
         // If the next non-whitespace token after the function or method call
         // is not an opening parenthesis then it cant really be a *call*.
-        $openBracket = $phpcsFile->findNext(
+        $functionName = $stackPtr;
+        $openBracket  = $phpcsFile->findNext(
             PHP_CodeSniffer_Tokens::$emptyTokens,
             ($functionName + 1),
             null,
@@ -133,21 +131,18 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
                 // We have to exclude all uses of T_BITWISE_AND that are not
                 // references. We use a blacklist approach as we prefer false
                 // positives to not identifying a pass-by-reference call at all.
-                // The blacklist may not yet be complete.
-                switch ($tokens[$tokenBefore]['code']) {
-                case T_VARIABLE:
-                case T_CLOSE_PARENTHESIS:
-                case T_LNUMBER:
-                    // In these cases T_BITWISE_AND represents
-                    // the bitwise and operator.
+                $tokenCode = $tokens[$tokenBefore]['code'];
+                if ($tokenCode === T_VARIABLE
+                    || $tokenCode === T_CLOSE_PARENTHESIS
+                    || $tokenCode === T_LNUMBER
+                    || isset(PHP_CodeSniffer_Tokens::$assignmentTokens[$tokenCode]) === true
+                ) {
                     continue;
-
-                default:
-                    // T_BITWISE_AND represents a pass-by-reference.
-                    $error = 'Call-time pass-by-reference calls are prohibited';
-                    $phpcsFile->addError($error, $tokenBefore, 'NotAllowed');
-                    break;
                 }
+
+                // T_BITWISE_AND represents a pass-by-reference.
+                $error = 'Call-time pass-by-reference calls are prohibited';
+                $phpcsFile->addError($error, $tokenBefore, 'NotAllowed');
             }//end if
         }//end while
 

diff --git a/CodeSniffer/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.inc b/CodeSniffer/Standards/Generic/Tests/Functions/CallTimePassByReferenceUnitTest.inc
@@ -21,4 +21,6 @@ while(testfunc($var1, &$var2, $var3, &$var4) === false) {
 sprintf("0%o", 0777 & $p);
 
 $foo(&$myvar);
-?>
+
+if (is_array($foo = &$this->bar())) {
+}
diff --git a/package.xml b/package.xml
@@ -51,6 +51,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
   - Fixed bug #667 : Scope indent check can go into infinite loop due to some parse errors
   - Fixed bug #670 : Endless loop in PSR1 SideEffects sniffer if no semicolon after last statement
     -- Thanks to Thomas Jarosch for the patch
+  - Fixed bug #672 : Call-time pass-by-reference false positive
  </notes>
  <contents>
   <dir name="/">