Add allowUnusedVariablesBeforeRequire option (#196)

* Add tests for unused before require * Move areFollowingArgumentsUsed to Helpers * Add test cases for all forms of require/include * Add isRequireInScopeAfter and use allowUnusedVariablesBeforeRequire * Add test to be sure option does not break other behavior * Add allowUnusedVariablesBeforeRequire to README
sirbrillig · Aug 18, 2020 · 3b19ddf · 3b19ddf
1 parent 26bf3d3
commit 3b19ddf
Show file tree

Hide file tree

Showing 5 changed files with 171 additions and 27 deletions.
diff --git a/README.md b/README.md
@@ -67,6 +67,7 @@ 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 `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.
+- `allowUnusedVariablesBeforeRequire` (bool, default `false`): if set to true, variables defined before a `require`, `require_once`, `include`, or `include_once` will not be marked as unused. They may be intended for the required file.
 - `allowUndefinedVariablesInFileScope` (bool, default `false`): if set to true, undefined variables in the file's top-level scope will never be marked as undefined.
 - `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 `'/^_/'`.

diff --git a/Tests/VariableAnalysisSniff/UnusedFollowedByRequireTest.php b/Tests/VariableAnalysisSniff/UnusedFollowedByRequireTest.php
@@ -0,0 +1,74 @@
+<?php
+namespace VariableAnalysis\Tests\VariableAnalysisSniff;
+
+use VariableAnalysis\Tests\BaseTestCase;
+
+class UnusedFollowedByRequire extends BaseTestCase {
+  public function testUnusedFollowedByRequireWarnsByDefault() {
+    $fixtureFile = $this->getFixture('UnusedFollowedByRequireFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+      2,
+      3,
+      4,
+      8,
+      9,
+      10,
+      14,
+      15,
+      16,
+      20,
+      21,
+      22,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+
+  public function testUnusedFollowedByRequireDoesNotWarnWhenSet() {
+    $fixtureFile = $this->getFixture('UnusedFollowedByRequireFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+    $phpcsFile->ruleset->setSniffProperty(
+      'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
+      'allowUnusedVariablesBeforeRequire',
+      'true'
+    );
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+      4,
+      10,
+      16,
+      22,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+
+  public function testUnusedFollowedByRequireDoesNotBreakOtherThingsWhenSet() {
+    $fixtureFile = $this->getFixture('FunctionWithoutParamFixture.php');
+    $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile);
+    $phpcsFile->ruleset->setSniffProperty(
+      'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff',
+      'allowUnusedVariablesBeforeRequire',
+      'true'
+    );
+    $phpcsFile->process();
+    $lines = $this->getWarningLineNumbersFromFile($phpcsFile);
+    $expectedWarnings = [
+      4,
+      5,
+      6,
+      7,
+      8,
+      9,
+      10,
+      11,
+      12,
+      13,
+      18,
+      19,
+    ];
+    $this->assertEquals($expectedWarnings, $lines);
+  }
+}
diff --git a/Tests/VariableAnalysisSniff/fixtures/UnusedFollowedByRequireFixture.php b/Tests/VariableAnalysisSniff/fixtures/UnusedFollowedByRequireFixture.php
@@ -0,0 +1,24 @@
+<?php
+function require_file_function($param) { // unused variable $param
+    $var = 'something'; // unused variable $var
+    activate_code($data); // undefined variable $data
+    require __DIR__ . '/views/my-view.php';
+}
+
+function require_once_file_function($param) { // unused variable $param
+    $var = 'something'; // unused variable $var
+    activate_code($data); // undefined variable $data
+    require_once __DIR__ . '/views/my-view.php';
+}
+
+function include_file_function($param) { // unused variable $param
+    $var = 'something'; // unused variable $var
+    activate_code($data); // undefined variable $data
+    include __DIR__ . '/views/my-view.php';
+}
+
+function include_once_file_function($param) { // unused variable $param
+    $var = 'something'; // unused variable $var
+    activate_code($data); // undefined variable $data
+    include_once __DIR__ . '/views/my-view.php';
+}
diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php
@@ -3,6 +3,9 @@
 namespace VariableAnalysis\Lib;
 
 use PHP_CodeSniffer\Files\File;
+use VariableAnalysis\Lib\ScopeInfo;
+use VariableAnalysis\Lib\ScopeType;
+use VariableAnalysis\Lib\VariableInfo;
 use PHP_CodeSniffer\Util\Tokens;
 use PHPCSUtils\Utils\FunctionDeclarations;
 
@@ -652,4 +655,60 @@ public static function getLastNonEmptyTokenIndexInFile(File $phpcsFile) {
     self::debug('no non-empty token found for end of file');
     return 0;
   }
+
+  /**
+   * @param VariableInfo $varInfo
+   * @param ScopeInfo $scopeInfo
+   *
+   * @return bool
+   */
+  public static function areFollowingArgumentsUsed(VariableInfo $varInfo, ScopeInfo $scopeInfo) {
+    $foundVarPosition = false;
+    foreach ($scopeInfo->variables as $variable) {
+      if ($variable === $varInfo) {
+        $foundVarPosition = true;
+        continue;
+      }
+      if (! $foundVarPosition) {
+        continue;
+      }
+      if ($variable->scopeType !== ScopeType::PARAM) {
+        continue;
+      }
+      if ($variable->firstRead) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  /**
+   * @param File $phpcsFile
+   * @param VariableInfo $varInfo
+   * @param ScopeInfo $scopeInfo
+   *
+   * @return bool
+   */
+  public static function isRequireInScopeAfter(File $phpcsFile, VariableInfo $varInfo, ScopeInfo $scopeInfo) {
+    $requireTokens = [
+      T_REQUIRE,
+      T_REQUIRE_ONCE,
+      T_INCLUDE,
+      T_INCLUDE_ONCE,
+    ];
+    $indexToStartSearch = $varInfo->firstDeclared;
+    if (! empty($varInfo->firstInitialized)) {
+      $indexToStartSearch = $varInfo->firstInitialized;
+    }
+    $tokens = $phpcsFile->getTokens();
+    $indexToStopSearch = isset($tokens[$scopeInfo->owner]['scope_closer']) ? $tokens[$scopeInfo->owner]['scope_closer'] : null;
+    if (! is_int($indexToStartSearch) || ! is_int($indexToStopSearch)) {
+      return false;
+    }
+    $requireTokenIndex = $phpcsFile->findNext($requireTokens, $indexToStartSearch + 1, $indexToStopSearch);
+    if (is_int($requireTokenIndex)) {
+      return true;
+    }
+    return false;
+  }
 }
diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
@@ -131,6 +131,15 @@ class VariableAnalysisSniff implements Sniff {
    */
   public $allowUnusedForeachVariables = true;
 
+  /**
+   * If set to true, unused variables in a function before a require or import
+   * statement will not be marked as unused because they may be used in the
+   * required file.
+   *
+   *  @var bool
+   */
+  public $allowUnusedVariablesBeforeRequire = false;
+
   /**
    * @return (int|string)[]
    */
@@ -337,32 +346,6 @@ protected function getOrCreateVariableInfo($varName, $currScope) {
     return $scopeInfo->variables[$varName];
   }
 
-  /**
-   * @param VariableInfo $varInfo
-   * @param ScopeInfo $scopeInfo
-   *
-   * @return bool
-   */
-  protected function areFollowingArgumentsUsed($varInfo, $scopeInfo) {
-    $foundVarPosition = false;
-    foreach ($scopeInfo->variables as $variable) {
-      if ($variable === $varInfo) {
-        $foundVarPosition = true;
-        continue;
-      }
-      if (! $foundVarPosition) {
-        continue;
-      }
-      if ($variable->scopeType !== ScopeType::PARAM) {
-        continue;
-      }
-      if ($variable->firstRead) {
-        return true;
-      }
-    }
-    return false;
-  }
-
   /**
    * @param string $varName
    * @param int $stackPtr
@@ -1601,7 +1584,7 @@ protected function processScopeCloseForVariable(File $phpcsFile, VariableInfo $v
     if ($this->allowUnusedFunctionParameters && $varInfo->scopeType === ScopeType::PARAM) {
       return;
     }
-    if ($this->allowUnusedParametersBeforeUsed && $varInfo->scopeType === ScopeType::PARAM && $this->areFollowingArgumentsUsed($varInfo, $scopeInfo)) {
+    if ($this->allowUnusedParametersBeforeUsed && $varInfo->scopeType === ScopeType::PARAM && Helpers::areFollowingArgumentsUsed($varInfo, $scopeInfo)) {
       Helpers::debug("variable {$varInfo->name} at end of scope has unused following args");
       return;
     }
@@ -1624,6 +1607,9 @@ protected function processScopeCloseForVariable(File $phpcsFile, VariableInfo $v
     if (empty($varInfo->firstDeclared) && empty($varInfo->firstInitialized)) {
       return;
     }
+    if ($this->allowUnusedVariablesBeforeRequire && Helpers::isRequireInScopeAfter($phpcsFile, $varInfo, $scopeInfo)) {
+      return;
+    }
     $this->warnAboutUnusedVariable($phpcsFile, $varInfo);
   }