Skip to content

Commit

Permalink
Add ignore unused foreach option (#66)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
sirbrillig authored Feb 8, 2019
1 parent 6e022ca commit 30e1278
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 1 deletion.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@
phpunit.xml
.phpcs.xml
phpcs.xml
tags
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
1 change: 1 addition & 0 deletions VariableAnalysis/Lib/VariableInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class VariableInfo {
public $firstRead;
public $ignoreUnused = false;
public $ignoreUndefined = false;
public $isForeachLoopVar = false;

public static $scopeTypeDescriptions = array(
'local' => 'variable',
Expand Down
13 changes: 12 additions & 1 deletion VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
50 changes: 50 additions & 0 deletions VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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
}
}

0 comments on commit 30e1278

Please sign in to comment.