Enable allowUnusedForeachVariables and allowUnusedCaughtExceptions (#73)

* Tests: change allowUnusedForeachVariables to true

* Tests: remove unused foreach vars from test

* Change allowUnusedForeachVariables to true

* Tests: change allowUnusedCaughtExceptions to true

* Change allowUnusedCaughtExceptions to true

Author: sirbrillig

README.md

@@ -63,12 +63,12 @@ There's a variety of options to customize the behaviour of VariableAnalysis, tak
 The available options are as follows:
 
 - `allowUnusedFunctionParameters` (bool, default `false`): if set to true, function arguments will never be marked as unused.
-- `allowUnusedCaughtExceptions` (bool, default `false`): if set to true, caught Exception variables will never be marked as unused.
+- `allowUnusedCaughtExceptions` (bool, default `true`): if set to true, caught Exception variables will never be marked as unused.
 - `allowUnusedParametersBeforeUsed` (bool, default `true`): if set to true, unused function arguments will be ignored if they are followed by used function arguments.
 - `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.
+- `allowUnusedForeachVariables` (bool, default `true`): if set to true, unused keys or values created by the `as` statement in a `foreach` loop will never be marked as unused.
 - `sitePassByRefFunctions` (string, default `null`): a list of custom functions which pass in variables to be initialized by reference (eg `preg_match()`) and therefore should not require those variables to be defined ahead of time. The list is space separated and each entry is of the form `functionName:1,2`. The function name comes first followed by a colon and a comma-separated list of argument numbers (starting from 1) which should be considered variable definitions. The special value `...` in the arguments list will cause all arguments after the last number to be considered variable definitions.
 - `allowWordPressPassByRefFunctions` (bool, default `false`): if set to true, a list of common WordPress pass-by-reference functions will be added to the list of PHP ones so that passing undefined variables to these functions (to be initialized by reference) will be allowed.
 

VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php

@@ -39,10 +39,9 @@ class VariableAnalysisSniff implements Sniff {
   public $allowWordPressPassByRefFunctions = false;
 
   /**
-   *  Allows exceptions in a catch block to be unused without provoking unused-var warning.
-   *  Set generic.codeanalysis.variableanalysis.allowUnusedCaughtExceptions to a true value.
+   *  Allow exceptions in a catch block to be unused without warning.
    */
-  public $allowUnusedCaughtExceptions = false;
+  public $allowUnusedCaughtExceptions = true;
 
   /**
    *  Allow function parameters to be unused without provoking unused-var warning.
@@ -81,7 +80,7 @@ class VariableAnalysisSniff implements Sniff {
    * 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 $allowUnusedForeachVariables = true;
 
   public function register() {
     return [

VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php

@@ -123,10 +123,6 @@ public function testFunctionWithForeachWarnings() {
       22,
       24,
       26,
-      48,
-      50,
-      52,
-      54,
       // FIXME: this is an unused variable that needs to be fixed but for now
       // we will ignore it. See
       // https://github.com/sirbrillig/phpcs-variable-analysis/pull/36
@@ -560,7 +556,6 @@ public function testUnusedParamsAreReported() {
       16,
       27,
       39,
-      66,
       72,
       73,
     ];
@@ -586,7 +581,6 @@ public function testValidUnusedVariableNamesIgnoresUnusedVariables() {
       4,
       16,
       39,
-      66,
       72,
       73,
     ];
@@ -608,13 +602,37 @@ public function testAllowUnusedFunctionParametersIgnoresUnusedVariables() {
     );
     $phpcsFile->process();
     $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+
+  public function testAllowUnusedCaughtExceptionsIgnoresUnusedVariablesIfSet() {
+    $fixtureFile = $this->getFixture('FunctionWithUnusedParamsFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
+    $phpcsFile->ruleset->setSniffProperty(
+      'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
+      'allowUnusedParametersBeforeUsed',
+      'false'
+    );
+    $phpcsFile->ruleset->setSniffProperty(
+      'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
+      'allowUnusedCaughtExceptions',
+      'true'
+    );
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
     $expectedWarnings = [
-      66,
+      4,
+      16,
+      27,
+      39,
+      72,
+      73,
     ];
     $this->assertEquals($expectedWarnings, $lines);
   }
 
-  public function testAllowUnusedCaughtExceptionsIgnoresUnusedVariables() {
+  public function testAllowUnusedCaughtExceptionsDoesNotIgnoreUnusedVariablesIfFalse() {
     $fixtureFile = $this->getFixture('FunctionWithUnusedParamsFixture.php');
     $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
     $phpcsFile->ruleset->setSniffProperty(
@@ -625,7 +643,7 @@ public function testAllowUnusedCaughtExceptionsIgnoresUnusedVariables() {
     $phpcsFile->ruleset->setSniffProperty(
       'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
       'allowUnusedCaughtExceptions',
-      'true'
+      'false'
     );
     $phpcsFile->process();
     $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
@@ -634,6 +652,7 @@ public function testAllowUnusedCaughtExceptionsIgnoresUnusedVariables() {
       16,
       27,
       39,
+      66,
       72,
       73,
     ];
@@ -759,7 +778,25 @@ public function testPregReplaceIgnoresNumericVariables() {
     $this->assertEquals($expectedWarnings, $lines);
   }
 
-  public function testUnusedForeachVariablesAreNotIgnoredByDefault() {
+  public function testUnusedForeachVariablesAreIgnoredByDefault() {
+    $fixtureFile = $this->getFixture('UnusedForeachFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+      7,
+      8,
+      16,
+      17,
+      25,
+      26,
+      33,
+      34,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+
+  public function testUnusedForeachVariablesAreNotIgnoredIfDisabled() {
     $fixtureFile = $this->getFixture('UnusedForeachFixture.php');
     $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile);
     $phpcsFile->ruleset->setSniffProperty(