Treat writing to a by-reference foreach loop variable as a read (#221)

* Add test for foreach with pass-by-reference

* Add counterexample of unused reference variable in foreach

* Add isDynamicReference to VariableInfo

* Set isDynamicReference for by-reference foreach loop vars

* Treat writing to a by-reference foreach loopvar as a read

Author: sirbrillig

Tests/VariableAnalysisSniff/VariableAnalysisTest.php

@@ -286,6 +286,7 @@ public function testFunctionWithReferenceWarnings() {
       59,
       60,
       64,
+      81,
     ];
     $this->assertEquals($expectedWarnings, $lines);
   }
@@ -312,6 +313,7 @@ public function testFunctionWithReferenceWarningsAllowsCustomFunctions() {
       40,
       46,
       64,
+      81,
     ];
     $this->assertEquals($expectedWarnings, $lines);
   }
@@ -339,6 +341,7 @@ public function testFunctionWithReferenceWarningsAllowsWordPressFunctionsIfSet()
       46,
       59,
       60,
+      81,
     ];
     $this->assertEquals($expectedWarnings, $lines);
   }

Tests/VariableAnalysisSniff/fixtures/FunctionWithReferenceFixture.php

@@ -73,3 +73,13 @@ function function_with_array_walk($userNameParts) {
     $value = ucfirst($value);
   });
 }
+
+function function_with_foreach_with_reference($derivatives, $base_plugin_definition) {
+  foreach ($derivatives as &$entry) {
+    $entry .= $base_plugin_definition;
+  }
+  foreach ($derivatives as &$unused) { // unused variable
+    $base_plugin_definition .= '1';
+  }
+  return $derivatives;
+}

VariableAnalysis/Lib/VariableInfo.php

@@ -30,6 +30,13 @@ class VariableInfo {
    */
   public $referencedVariableScope;
 
+  /**
+   * True if the variable is a reference but one created at runtime
+   *
+   * @var bool
+   */
+  public $isDynamicReference = false;
+
   /**
    * Stack pointer of first declaration
    *

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -418,6 +418,7 @@ protected function markVariableAssignment($varName, $stackPtr, $currScope) {
       return;
     }
     $varInfo->firstInitialized = $stackPtr;
+    Helpers::debug('markVariableAssignment: marked as initialized', $varName);
   }
 
   /**
@@ -944,6 +945,14 @@ protected function processVariableAsAssignment(File $phpcsFile, $stackPtr, $varN
 
     Helpers::debug('processVariableAsAssignment: marking as assignment in scope', $currScope);
     $this->markVariableAssignment($varName, $stackPtr, $currScope);
+
+    // If the left-hand-side of the assignment (the variable we are examining)
+    // is itself a reference, then that counts as a read as well as a write.
+    $varInfo = $this->getOrCreateVariableInfo($varName, $currScope);
+    if ($varInfo->isDynamicReference) {
+      Helpers::debug('processVariableAsAssignment: also marking as a use because variable is a reference');
+      $this->markVariableRead($varName, $stackPtr, $currScope);
+    }
   }
 
   /**
@@ -1164,6 +1173,13 @@ protected function processVariableAsForeachLoopVar(File $phpcsFile, $stackPtr, $
       $varInfo->isForeachLoopAssociativeValue = true;
     }
 
+    // Are we pass-by-reference?
+    $referencePtr = $phpcsFile->findPrevious(Tokens::$emptyTokens, $stackPtr - 1, null, true, null, true);
+    if (($referencePtr !== false) && ($tokens[$referencePtr]['code'] === T_BITWISE_AND)) {
+      Helpers::debug("processVariableAsForeachLoopVar: found foreach loop variable assigned by reference");
+      $varInfo->isDynamicReference = true;
+    }
+
     return true;
   }