diff --git a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php index 098ee41e..8f5e5e48 100644 --- a/Tests/VariableAnalysisSniff/VariableAnalysisTest.php +++ b/Tests/VariableAnalysisSniff/VariableAnalysisTest.php @@ -97,6 +97,7 @@ public function testFunctionWithGlobalVarWarnings() { 28, 29, 39, + 54, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -762,6 +763,7 @@ public function testValidUndefinedVariableNamesIgnoresVarsInGlobalScope() { 7, 23, 39, + 54, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -802,6 +804,8 @@ public function testUnusedArgumentsBeforeUsedArgumentsAreFoundIfFalse() { 8, 16, 19, + 28, + 34, ]; $this->assertEquals($expectedWarnings, $lines); } @@ -814,6 +818,8 @@ public function testUnusedArgumentsBeforeUsedArgumentsAreIgnoredByDefault() { $expectedWarnings = [ 8, 19, + 28, + 34, ]; $this->assertEquals($expectedWarnings, $lines); } diff --git a/Tests/VariableAnalysisSniff/fixtures/FunctionWithGlobalVarFixture.php b/Tests/VariableAnalysisSniff/fixtures/FunctionWithGlobalVarFixture.php index 1f6d93a3..36e0f253 100644 --- a/Tests/VariableAnalysisSniff/fixtures/FunctionWithGlobalVarFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/FunctionWithGlobalVarFixture.php @@ -50,3 +50,8 @@ function updateGlobalWithListShorthand($newVal) { [ $myGlobal, $otherVar ] = my_function($newVal); echo $otherVar; } + +function globalWithUnusedFunctionArg($user_type, $text, $testvar) { // should warn that testvar is unused + global $config; + return $config . $text . $user_type; +} diff --git a/Tests/VariableAnalysisSniff/fixtures/UnusedAfterUsedFixture.php b/Tests/VariableAnalysisSniff/fixtures/UnusedAfterUsedFixture.php index 5c57520c..b532c3a8 100644 --- a/Tests/VariableAnalysisSniff/fixtures/UnusedAfterUsedFixture.php +++ b/Tests/VariableAnalysisSniff/fixtures/UnusedAfterUsedFixture.php @@ -23,3 +23,17 @@ function inner_function() { }; $foo(); } + +// The following line should report an unused variable (unused after used) +function function_with_one_unused_param($used, $used_two, $unused_three) { + echo $used; + echo $used_two; +} + +// The following line should report an unused variable (unused after used) +function function_with_local_and_unused_params($used, $used_two, $unused_three) { + $foobar = 'hello'; + echo $used; + echo $foobar; + echo $used_two; +} \ No newline at end of file diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index a30f20b6..bfe3bed6 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -283,6 +283,9 @@ protected function areFollowingArgumentsUsed($varInfo, $scopeInfo) { if (! $foundVarPosition) { continue; } + if ($variable->scopeType !== 'param') { + continue; + } if ($variable->firstRead) { return true; } @@ -522,12 +525,10 @@ protected function checkForFunctionPrototype(File $phpcsFile, $stackPtr, $varNam /** * @param File $phpcsFile * @param int $stackPtr - * @param string $varName - * @param int $currScope * * @return bool */ - protected function checkForClassProperty(File $phpcsFile, $stackPtr, $varName, $currScope) { + protected function checkForClassProperty(File $phpcsFile, $stackPtr) { $propertyDeclarationKeywords = [ T_PUBLIC, T_PRIVATE, @@ -660,12 +661,10 @@ protected function checkForSuperGlobal(File $phpcsFile, $stackPtr, $varName) { /** * @param File $phpcsFile * @param int $stackPtr - * @param string $varName - * @param int $currScope * * @return bool */ - protected function checkForStaticMember(File $phpcsFile, $stackPtr, $varName, $currScope) { + protected function checkForStaticMember(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); $doubleColonPtr = $phpcsFile->findPrevious(Tokens::$emptyTokens, $stackPtr - 1, null, true); @@ -696,11 +695,10 @@ protected function checkForStaticMember(File $phpcsFile, $stackPtr, $varName, $c * @param File $phpcsFile * @param int $stackPtr * @param string $varName - * @param int $currScope * * @return bool */ - protected function checkForStaticOutsideClass(File $phpcsFile, $stackPtr, $varName, $currScope) { + protected function checkForStaticOutsideClass(File $phpcsFile, $stackPtr, $varName) { // Are we refering to self:: outside a class? $tokens = $phpcsFile->getTokens(); @@ -752,7 +750,7 @@ protected function checkForAssignment(File $phpcsFile, $stackPtr, $varName, $cur } // Is this a variable variable? If so, it's not an assignment to the current variable. - if ($this->checkForVariableVariable($phpcsFile, $stackPtr, $varName, $currScope)) { + if ($this->checkForVariableVariable($phpcsFile, $stackPtr)) { Helpers::debug('found variable variable'); return false; } @@ -779,12 +777,10 @@ protected function checkForAssignment(File $phpcsFile, $stackPtr, $varName, $cur /** * @param File $phpcsFile * @param int $stackPtr - * @param string $varName - * @param int $currScope * * @return bool */ - protected function checkForVariableVariable(File $phpcsFile, $stackPtr, $varName, $currScope) { + protected function checkForVariableVariable(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true); @@ -1184,18 +1180,18 @@ protected function processVariable(File $phpcsFile, $stackPtr) { } // Check for static members used outside a class - if ($this->checkForStaticOutsideClass($phpcsFile, $stackPtr, $varName, $currScope)) { + if ($this->checkForStaticOutsideClass($phpcsFile, $stackPtr, $varName)) { Helpers::debug('found static usage outside of class'); return; } // $var part of class::$var static member - if ($this->checkForStaticMember($phpcsFile, $stackPtr, $varName, $currScope)) { + if ($this->checkForStaticMember($phpcsFile, $stackPtr)) { Helpers::debug('found static member'); return; } - if ($this->checkForClassProperty($phpcsFile, $stackPtr, $varName, $currScope)) { + if ($this->checkForClassProperty($phpcsFile, $stackPtr)) { Helpers::debug('found class property definition'); return; } @@ -1403,6 +1399,7 @@ protected function processScopeCloseForVariable(File $phpcsFile, VariableInfo $v return; } if ($this->allowUnusedParametersBeforeUsed && $varInfo->scopeType === 'param' && $this->areFollowingArgumentsUsed($varInfo, $scopeInfo)) { + Helpers::debug("variable {$varInfo->name} at end of scope has unused following args"); return; } if ($this->allowUnusedForeachVariables && $varInfo->isForeachLoopAssociativeValue) {