Add ignore unused foreach option (#66)

* Tests: add tests for ignoreUnusedForeachVariables

* README: Add ignoreUnusedForeachVariables

* Tests: add normal warnings just to be sure

* Add ignoreUnusedForeachVariables option

* Add tags to gitignore

* Add isForeachLoopVar to VariableInfo

* Rename variable to allowUnusedForeachVariables

This better matches the other bool options.

* Tests: Add foreach loop without key

Author: sirbrillig

.gitignore

@@ -3,3 +3,4 @@
 phpunit.xml
 .phpcs.xml
 phpcs.xml
+tags

README.md

@@ -68,6 +68,7 @@ The available options are as follows:
 - `validUnusedVariableNames` (string, default `null`): a space-separated list of names of placeholder variables that you want to ignore from unused variable warnings. For example, to ignore the variables `$junk` and `$unused`, this could be set to `'junk unused'`.
 - `ignoreUnusedRegexp` (string, default `null`): a PHP regexp string (note that this requires explicit delimiters) for variables that you want to ignore from unused variable warnings. For example, to ignore the variables `$_junk` and `$_unused`, this could be set to `'/^_/'`.
 - `validUndefinedVariableNames` (string, default `null`): a space-separated list of names of placeholder variables that you want to ignore from undefined variable warnings. For example, to ignore the variables `$post` and `$undefined`, this could be set to `'post undefined'`.
+- `allowUnusedForeachVariables` (bool, default `false`): if set to true, unused keys or values created by the `as` statement in a `foreach` loop will never be marked as unused.
 
 To set these these options, you must use XML in your ruleset. For details, see the [phpcs customizable sniff properties page](https://github.com/squizlabs/PHP_CodeSniffer/wiki/Customisable-Sniff-Properties). Here is an example that ignores all variables that start with an underscore:
 

VariableAnalysis/Lib/VariableInfo.php

@@ -18,6 +18,7 @@ class VariableInfo {
   public $firstRead;
   public $ignoreUnused = false;
   public $ignoreUndefined = false;
+  public $isForeachLoopVar = false;
 
   public static $scopeTypeDescriptions = array(
     'local'  => 'variable',

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -67,6 +67,12 @@ class VariableAnalysisSniff implements Sniff {
    */
   public $allowUnusedParametersBeforeUsed = true;
 
+  /**
+   * If set to true, unused keys or values created by the `as` statement
+   * in a `foreach` loop will never be marked as unused.
+   */
+  public $allowUnusedForeachVariables = false;
+
   public function register() {
     return [
       T_VARIABLE,
@@ -644,8 +650,10 @@ protected function checkForForeachLoopVar(File $phpcsFile, $stackPtr, $varName,
     if ($phpcsFile->findPrevious(T_AS, $stackPtr - 1, $openParenPtr) === false) {
       return false;
     }
-
     $this->markVariableAssignment($varName, $stackPtr, $currScope);
+    $varInfo = $this->getOrCreateVariableInfo($varName, $currScope);
+    $varInfo->isForeachLoopVar = true;
+
     return true;
   }
 
@@ -974,6 +982,9 @@ protected function processScopeCloseForVariable($phpcsFile, $varInfo, $scopeInfo
     if ($this->allowUnusedParametersBeforeUsed && $varInfo->scopeType === 'param' && $this->areFollowingArgumentsUsed($varInfo, $scopeInfo)) {
       return;
     }
+    if ($this->allowUnusedForeachVariables && $varInfo->isForeachLoopVar) {
+      return;
+    }
     if ($varInfo->passByReference && isset($varInfo->firstInitialized)) {
       // If we're pass-by-reference then it's a common pattern to
       // use the variable to return data to the caller, so any

VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php

@@ -700,4 +700,54 @@ public function testPregReplaceIgnoresNumericVariables() {
     ];
     $this->assertEquals($expectedWarnings, $lines);
   }
+
+  public function testUnusedForeachVariablesAreNotIgnoredByDefault() {
+    $fixtureFile = $this->getFixture('UnusedForeachFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
+    $phpcsFile->ruleset->setSniffProperty(
+      'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
+      'allowUnusedForeachVariables',
+      'false'
+    );
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+      5,
+      7,
+      8,
+      14,
+      16,
+      17,
+      23,
+      25,
+      26,
+      32,
+      33,
+      34,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+
+  public function testUnusedForeachVariablesAreIgnoredIfSet() {
+    $fixtureFile = $this->getFixture('UnusedForeachFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
+    $phpcsFile->ruleset->setSniffProperty(
+      'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
+      'allowUnusedForeachVariables',
+      'true'
+    );
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+      7,
+      8,
+      16,
+      17,
+      25,
+      26,
+      33,
+      34,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
 }

VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php

@@ -0,0 +1,36 @@
+<?php
+
+function loopWithUnusedKey() {
+    $array = [];
+    foreach ( $array as $key => $value ) { // maybe marked as unused
+        echo $value;
+        $unused = 'foobar'; // should always be marked as unused
+        echo $undefined; // should always be marked as undefined
+    }
+}
+
+function loopWithUnusedValue() {
+    $array = [];
+    foreach ( $array as $key => $value ) { // maybe marked as unused
+        echo $key;
+        $unused = 'foobar'; // should always be marked as unused
+        echo $undefined; // should always be marked as undefined
+    }
+}
+
+function loopWithUnusedKeyAndValue() {
+    $array = [];
+    foreach ( $array as $key => $value ) { // maybe marked as unused
+        echo 'hello';
+        $unused = 'foobar'; // should always be marked as unused
+        echo $undefined; // should always be marked as undefined
+    }
+}
+
+function loopWithUnusedValueOnly() {
+    $array = [];
+    foreach ( $array as $value ) { // maybe marked as unused
+        $unused = 'foobar'; // should always be marked as unused
+        echo $undefined; // should always be marked as undefined
+    }
+}