From e6ea78062ea06611e6a8089658266d04c7efd88d Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Tue, 15 Sep 2020 11:54:06 -0400 Subject: [PATCH 1/3] Add test for UndefinedUnsetVariable sniff code --- Tests/VariableAnalysisSniff/UnsetTest.php | 29 +++++++++++++++++++ .../fixtures/UnsetFixture.php | 12 ++++++++ 2 files changed, 41 insertions(+) create mode 100644 Tests/VariableAnalysisSniff/UnsetTest.php create mode 100644 Tests/VariableAnalysisSniff/fixtures/UnsetFixture.php diff --git a/Tests/VariableAnalysisSniff/UnsetTest.php b/Tests/VariableAnalysisSniff/UnsetTest.php new file mode 100644 index 00000000..d34d01a2 --- /dev/null +++ b/Tests/VariableAnalysisSniff/UnsetTest.php @@ -0,0 +1,29 @@ +getFixture('UnsetFixture.php'); + $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile); + $phpcsFile->process(); + $lines = $this->getWarningLineNumbersFromFile($phpcsFile); + // Technically, these are not illegal, but they may be typos. See https://github.com/sirbrillig/phpcs-variable-analysis/issues/174 + $expectedWarnings = [ + 6, + 11, + ]; + $this->assertEquals($expectedWarnings, $lines); + } + + public function testUnsetHasCorrectSniffCodes() { + $fixtureFile = $this->getFixture('UnsetFixture.php'); + $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile); + $phpcsFile->process(); + + $warnings = $phpcsFile->getWarnings(); + $this->assertEquals('VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedUnsetVariable', $warnings[6][7][0]['source']); + $this->assertEquals('VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedUnsetVariable', $warnings[11][9][0]['source']); + } +} diff --git a/Tests/VariableAnalysisSniff/fixtures/UnsetFixture.php b/Tests/VariableAnalysisSniff/fixtures/UnsetFixture.php new file mode 100644 index 00000000..b4f794d6 --- /dev/null +++ b/Tests/VariableAnalysisSniff/fixtures/UnsetFixture.php @@ -0,0 +1,12 @@ + Date: Tue, 15 Sep 2020 12:00:08 -0400 Subject: [PATCH 2/3] Add UndefinedUnsetVariable sniff code --- VariableAnalysis/Lib/Helpers.php | 21 ++++++++++++++++ .../CodeAnalysis/VariableAnalysisSniff.php | 24 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index 14a13403..8c8d7047 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -830,4 +830,25 @@ public static function isVariableArrayPushShortcut(File $phpcsFile, $stackPtr) { return true; } + + /** + * @param File $phpcsFile + * @param int $stackPtr + * + * @return bool + */ + public static function isVariableInsideUnset(File $phpcsFile, $stackPtr) { + $functionIndex = self::getFunctionIndexForFunctionCallArgument($phpcsFile, $stackPtr); + if (! is_int($functionIndex)) { + return false; + } + $tokens = $phpcsFile->getTokens(); + if (! isset($tokens[$functionIndex])) { + return false; + } + if ($tokens[$functionIndex]['content'] === 'unset') { + return true; + } + return false; + } } diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 042aa978..a3a2c148 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -560,12 +560,19 @@ protected function markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $sta $this->markVariableRead($varName, $stackPtr, $currScope); if ($this->isVariableUndefined($varName, $stackPtr, $currScope) === true) { Helpers::debug("variable $varName looks undefined"); + if (Helpers::isVariableArrayPushShortcut($phpcsFile, $stackPtr)) { $this->warnAboutUndefinedArrayPushShortcut($phpcsFile, $varName, $stackPtr); // Mark the variable as defined if it's of the form `$x[] = 1;` $this->markVariableAssignment($varName, $stackPtr, $currScope); return; } + + if (Helpers::isVariableInsideUnset($phpcsFile, $stackPtr)) { + $this->warnAboutUndefinedUnset($phpcsFile, $varName, $stackPtr); + return; + } + $this->warnAboutUndefinedVariable($phpcsFile, $varName, $stackPtr); } } @@ -1773,6 +1780,7 @@ protected function warnAboutUndefinedVariable(File $phpcsFile, $varName, $stackP ["\${$varName}"] ); } + /** * @param File $phpcsFile * @param string $varName @@ -1788,4 +1796,20 @@ protected function warnAboutUndefinedArrayPushShortcut(File $phpcsFile, $varName ["\${$varName}"] ); } + + /** + * @param File $phpcsFile + * @param string $varName + * @param int $stackPtr + * + * @return void + */ + protected function warnAboutUndefinedUnset(File $phpcsFile, $varName, $stackPtr) { + $phpcsFile->addWarning( + "Variable %s inside unset call is undefined.", + $stackPtr, + 'UndefinedUnsetVariable', + ["\${$varName}"] + ); + } } From 4c68e53665592d2eb532b66a037651dc09bb9387 Mon Sep 17 00:00:00 2001 From: Payton Swick Date: Tue, 15 Sep 2020 12:01:14 -0400 Subject: [PATCH 3/3] Add UndefinedUnsetVariable to README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 7f4ec4d7..263932cd 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,7 @@ Please note that this README is for VariableAnalysis v3. For documentation about - Warns if variables are used without being defined. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable`) - Warns if variables are used for an array push shortcut without being defined. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedArrayVariable`) +- Warns if variables are used inside `unset()` without being defined. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedUnsetVariable`) - Warns if variables are set or declared but never used. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedVariable`) - Warns if function parameters are declared but never used. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedParameter`) - Warns if function parameters are declared but never used before other parameters that are used. (Sniff code: `VariableAnalysis.CodeAnalysis.VariableAnalysis.UnusedParameterBeforeUsed`)