Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add allowUnusedVariablesBeforeRequire option #196

Merged
merged 6 commits into from
Aug 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `'/^_/'`.
Expand Down
74 changes: 74 additions & 0 deletions Tests/VariableAnalysisSniff/UnusedFollowedByRequireTest.php
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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';
}
59 changes: 59 additions & 0 deletions VariableAnalysis/Lib/Helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
}
40 changes: 13 additions & 27 deletions VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)[]
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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);
}

Expand Down