diff --git a/VariableAnalysis/Lib/Helpers.php b/VariableAnalysis/Lib/Helpers.php index 45c0873b..a1b12b0a 100644 --- a/VariableAnalysis/Lib/Helpers.php +++ b/VariableAnalysis/Lib/Helpers.php @@ -228,16 +228,6 @@ public static function findVariableScope(File $phpcsFile, $stackPtr) { return 0; } - public static function getStackPtrIfVariableIsUnused(VariableInfo $varInfo) { - if (isset($varInfo->firstDeclared)) { - return $varInfo->firstDeclared; - } - if (isset($varInfo->firstInitialized)) { - return $varInfo->firstInitialized; - } - return null; - } - public static function debug($message) { if (! defined('PHP_CODESNIFFER_VERBOSITY')) { return; diff --git a/VariableAnalysis/Lib/VariableInfo.php b/VariableAnalysis/Lib/VariableInfo.php index 4fe914cc..95f295b7 100644 --- a/VariableAnalysis/Lib/VariableInfo.php +++ b/VariableAnalysis/Lib/VariableInfo.php @@ -13,9 +13,9 @@ class VariableInfo { public $scopeType; public $typeHint; public $passByReference = false; - public $firstDeclared; - public $firstInitialized; - public $firstRead; + public $firstDeclared; // stack pointer of first declaration + public $firstInitialized; // stack pointer of first initialization + public $firstRead; // stack pointer of first read public $ignoreUnused = false; public $ignoreUndefined = false; public $isForeachLoopVar = false; diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index 16a5da63..54f1dc88 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -274,6 +274,7 @@ protected function isVariableUndefined($varName, $stackPtr, $currScope) { protected function markVariableReadAndWarnIfUndefined($phpcsFile, $varName, $stackPtr, $currScope) { $this->markVariableRead($varName, $stackPtr, $currScope); if ($this->isVariableUndefined($varName, $stackPtr, $currScope) === true) { + Helpers::debug("variable $varName looks undefined"); $phpcsFile->addWarning( "Variable %s is undefined.", $stackPtr, @@ -319,6 +320,7 @@ protected function checkForFunctionPrototype(File $phpcsFile, $stackPtr, $varNam $this->markVariableRead($varName, $stackPtr, $currScope); if ($this->isVariableUndefined($varName, $stackPtr, $currScope) === true) { // We haven't been defined by this point. + Helpers::debug("variable $varName in function prototype looks undefined"); $phpcsFile->addWarning("Variable %s is undefined.", $stackPtr, 'UndefinedVariable', ["\${$varName}"]); return true; } @@ -577,7 +579,6 @@ protected function checkForListAssignment(File $phpcsFile, $stackPtr, $varName, protected function checkForGlobalDeclaration(File $phpcsFile, $stackPtr, $varName, $currScope) { $tokens = $phpcsFile->getTokens(); - $token = $tokens[$stackPtr]; // Are we a global declaration? // Search backwards for first token that isn't whitespace, comma or variable. @@ -1052,8 +1053,15 @@ protected function processScopeCloseForVariable($phpcsFile, $varInfo, $scopeInfo // of "unused variable" warnings. return; } - $stackPtr = Helpers::getStackPtrIfVariableIsUnused($varInfo); + if ($varInfo->scopeType === 'global' && isset($varInfo->firstInitialized)) { + // If we imported this variable from the global scope, any further use of + // the variable, including assignment, should count as "variable use" for + // the purposes of "unused variable" warnings. + return; + } + $stackPtr = $varInfo->firstDeclared ?? $varInfo->firstInitialized ?? null; if ($stackPtr) { + Helpers::debug("variable {$varInfo->name} at end of scope looks undefined"); $phpcsFile->addWarning( "Unused %s %s.", $stackPtr, diff --git a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php index 53a44ee3..e426e7d1 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php +++ b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php @@ -95,7 +95,8 @@ public function testFunctionWithGlobalVarWarnings() { 8, 23, 28, - 29 + 29, + 39, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -734,6 +735,7 @@ public function testValidUndefinedVariableNamesIgnoresVarsInGlobalScope() { 4, 7, 23, + 39, ]; $this->assertEquals($expectedWarnings, $lines); } diff --git a/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithGlobalVarFixture.php b/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithGlobalVarFixture.php index 14c00e55..5ac38c22 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithGlobalVarFixture.php +++ b/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithGlobalVarFixture.php @@ -1,11 +1,11 @@