From 6b85d33f5d8646878cf6d0003223bbe1392cda6d Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 28 Jan 2019 16:54:01 -0500 Subject: [PATCH 1/8] Tests: add tests for ignoreUnusedForeachVariables --- .../CodeAnalysis/VariableAnalysisTest.php | 32 +++++++++++++++++++ .../fixtures/UnusedForeachFixture.php | 22 +++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php diff --git a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php index c795ceb9..76935406 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php +++ b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php @@ -700,4 +700,36 @@ 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', + 'ignoreUnusedForeachVariables', + 'false' + ); + $phpcsFile->process(); + $lines = $this->getWarningLineNumbersFromFile($phpcsFile); + $expectedWarnings = [ + 5, + 12, + 19, + ]; + $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', + 'ignoreUnusedForeachVariables', + 'true' + ); + $phpcsFile->process(); + $lines = $this->getWarningLineNumbersFromFile($phpcsFile); + $expectedWarnings = []; + $this->assertEquals($expectedWarnings, $lines); + } } diff --git a/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php b/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php new file mode 100644 index 00000000..a8bbb8a6 --- /dev/null +++ b/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php @@ -0,0 +1,22 @@ + $value ) { + echo $value; + } +} + +function loopWithUnusedValue() { + $array = []; + foreach ( $array as $key => $value ) { + echo $key; + } +} + +function loopWithUnusedKeyAndValue() { + $array = []; + foreach ( $array as $key => $value ) { + echo 'hello'; + } +} From 70a1d1e31a842feb5ca754a382b92d185778f21a Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 28 Jan 2019 16:55:31 -0500 Subject: [PATCH 2/8] README: Add ignoreUnusedForeachVariables --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index d22e3dc3..cf3dba09 100644 --- a/README.md +++ b/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'`. +- `ignoreUnusedForeachVariables` (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: From 2deb13fe28463911ab9f9a564ec06e6d374fcd9c Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 28 Jan 2019 16:59:18 -0500 Subject: [PATCH 3/8] Tests: add normal warnings just to be sure --- .../CodeAnalysis/VariableAnalysisTest.php | 19 ++++++++++++++++--- .../fixtures/UnusedForeachFixture.php | 12 +++++++++--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php index 76935406..3a10f12f 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php +++ b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php @@ -713,8 +713,14 @@ public function testUnusedForeachVariablesAreNotIgnoredByDefault() { $lines = $this->getWarningLineNumbersFromFile($phpcsFile); $expectedWarnings = [ 5, - 12, - 19, + 7, + 8, + 14, + 16, + 17, + 23, + 25, + 26, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -729,7 +735,14 @@ public function testUnusedForeachVariablesAreIgnoredIfSet() { ); $phpcsFile->process(); $lines = $this->getWarningLineNumbersFromFile($phpcsFile); - $expectedWarnings = []; + $expectedWarnings = [ + 7, + 8, + 16, + 17, + 25, + 26, + ]; $this->assertEquals($expectedWarnings, $lines); } } diff --git a/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php b/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php index a8bbb8a6..a479191e 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php +++ b/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php @@ -2,21 +2,27 @@ function loopWithUnusedKey() { $array = []; - foreach ( $array as $key => $value ) { + 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 ) { + 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 ) { + 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 } } From 67170dc75873d283f728ec89b3a6b37b50fa8695 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 28 Jan 2019 17:00:35 -0500 Subject: [PATCH 4/8] Add ignoreUnusedForeachVariables option --- .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index dc387a2c..3b4d3ce5 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/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 $ignoreUnusedForeachVariables = false; + public function register() { return [ T_VARIABLE, From 806d77cb0d7df3e25431635b5e1f85e86102e80d Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 28 Jan 2019 17:01:25 -0500 Subject: [PATCH 5/8] Add tags to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 244dd4fc..d3ce4ec3 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ phpunit.xml .phpcs.xml phpcs.xml +tags From 742b36c3c6ad3e7751036316498c2a7032162848 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 28 Jan 2019 17:17:27 -0500 Subject: [PATCH 6/8] Add isForeachLoopVar to VariableInfo --- VariableAnalysis/Lib/VariableInfo.php | 1 + .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/VariableAnalysis/Lib/VariableInfo.php b/VariableAnalysis/Lib/VariableInfo.php index e271337c..4fe914cc 100644 --- a/VariableAnalysis/Lib/VariableInfo.php +++ b/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', diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 3b4d3ce5..377b00a6 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -650,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; } @@ -980,6 +982,9 @@ protected function processScopeCloseForVariable($phpcsFile, $varInfo, $scopeInfo if ($this->allowUnusedParametersBeforeUsed && $varInfo->scopeType === 'param' && $this->areFollowingArgumentsUsed($varInfo, $scopeInfo)) { return; } + if ($this->ignoreUnusedForeachVariables && $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 From 5ac344b3f134ffda01c8165321d0bdd279479784 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 28 Jan 2019 18:20:35 -0500 Subject: [PATCH 7/8] Rename variable to allowUnusedForeachVariables This better matches the other bool options. --- README.md | 2 +- .../Sniffs/CodeAnalysis/VariableAnalysisSniff.php | 4 ++-- VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index cf3dba09..f555fa3a 100644 --- a/README.md +++ b/README.md @@ -68,7 +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'`. -- `ignoreUnusedForeachVariables` (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 `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: diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 377b00a6..471013cc 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -71,7 +71,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 $ignoreUnusedForeachVariables = false; + public $allowUnusedForeachVariables = false; public function register() { return [ @@ -982,7 +982,7 @@ protected function processScopeCloseForVariable($phpcsFile, $varInfo, $scopeInfo if ($this->allowUnusedParametersBeforeUsed && $varInfo->scopeType === 'param' && $this->areFollowingArgumentsUsed($varInfo, $scopeInfo)) { return; } - if ($this->ignoreUnusedForeachVariables && $varInfo->isForeachLoopVar) { + if ($this->allowUnusedForeachVariables && $varInfo->isForeachLoopVar) { return; } if ($varInfo->passByReference && isset($varInfo->firstInitialized)) { diff --git a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php index 3a10f12f..55147ca3 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php +++ b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php @@ -706,7 +706,7 @@ public function testUnusedForeachVariablesAreNotIgnoredByDefault() { $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile); $phpcsFile->ruleset->setSniffProperty( 'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff', - 'ignoreUnusedForeachVariables', + 'allowUnusedForeachVariables', 'false' ); $phpcsFile->process(); @@ -730,7 +730,7 @@ public function testUnusedForeachVariablesAreIgnoredIfSet() { $phpcsFile = $this->prepareLocalFileForSniffs($this->getSniffFiles(), $fixtureFile); $phpcsFile->ruleset->setSniffProperty( 'VariableAnalysis\Sniffs\CodeAnalysis\VariableAnalysisSniff', - 'ignoreUnusedForeachVariables', + 'allowUnusedForeachVariables', 'true' ); $phpcsFile->process(); From 21c20907d6c049020a3c1b2e3f513764b15af50f Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Mon, 28 Jan 2019 18:24:22 -0500 Subject: [PATCH 8/8] Tests: Add foreach loop without key --- .../Tests/CodeAnalysis/VariableAnalysisTest.php | 5 +++++ .../Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php index 55147ca3..b5e554cd 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php +++ b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php @@ -721,6 +721,9 @@ public function testUnusedForeachVariablesAreNotIgnoredByDefault() { 23, 25, 26, + 32, + 33, + 34, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -742,6 +745,8 @@ public function testUnusedForeachVariablesAreIgnoredIfSet() { 17, 25, 26, + 33, + 34, ]; $this->assertEquals($expectedWarnings, $lines); } diff --git a/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php b/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php index a479191e..c87b1af8 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php +++ b/VariableAnalysis/Tests/CodeAnalysis/fixtures/UnusedForeachFixture.php @@ -26,3 +26,11 @@ function loopWithUnusedKeyAndValue() { 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 + } +}