diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php index d0f60fb8..51724827 100644 --- a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php +++ b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php @@ -605,15 +605,24 @@ protected function checkForThisWithinClass(File $phpcsFile, $stackPtr, $varName) return false; } + $inFunction = false; foreach (array_reverse($token['conditions'], true) as $scopePtr => $scopeCode) { // $this within a closure is valid - // Note: have to fetch code from $tokens, T_CLOSURE isn't set for conditions codes. - if ($tokens[$scopePtr]['code'] === T_CLOSURE) { + if ($scopeCode === T_CLOSURE && $inFunction === false) { return true; } if ($scopeCode === T_CLASS || $scopeCode === T_ANON_CLASS || $scopeCode === T_TRAIT) { return true; } + + // Handle nested function declarations. + if ($scopeCode === T_FUNCTION) { + if ($inFunction === true) { + break; + } + + $inFunction = true; + } } return false; diff --git a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php index 5ac9bb5f..369029a7 100644 --- a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php +++ b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php @@ -907,4 +907,20 @@ public function testGetDefinedVarsCountsAsRead() { ]; $this->assertEquals($expectedWarnings, $lines); } + + public function testThisWithinNestedClosedScope() { + $fixtureFile = $this->getFixture('ThisWithinNestedClosedScopeFixture.php'); + $phpcsFile = $this->prepareLocalFileForSniffs($fixtureFile); + $phpcsFile->process(); + $lines = $this->getWarningLineNumbersFromFile($phpcsFile); + $expectedWarnings = [ + 5, + 8, + 20, + 33, + 47, + 61, + ]; + $this->assertEquals($expectedWarnings, $lines); + } } diff --git a/VariableAnalysis/Tests/CodeAnalysis/fixtures/ThisWithinNestedClosedScopeFixture.php b/VariableAnalysis/Tests/CodeAnalysis/fixtures/ThisWithinNestedClosedScopeFixture.php new file mode 100644 index 00000000..a55ab07d --- /dev/null +++ b/VariableAnalysis/Tests/CodeAnalysis/fixtures/ThisWithinNestedClosedScopeFixture.php @@ -0,0 +1,67 @@ +something) { + function nestedFunctionDeclaration() { + // Using $this here will also not work as the nested function is a closed scope in the global namespace. + if ($this->something) { + // Do something. + } + } + } +}; + +$closure = function() { + // Using $this here is fine. + if ($this->something) { + function nestedFunctionDeclaration() { + // Using $this here is not ok as the nested function is a closed scope in the global namespace. + if ($this->something) { + // Do something. + } + } + } +}; + +class Foo { + public function bar() { + // Using $this here is fine. + if ($this->something) { + function nestedFunctionDeclaration() { + // Using $this here is not ok as the nested function is a closed scope in the global namespace. + if ($this->something) { + // Do something. + } + } + } + } +} + +$anonClass = class() { + public function bar() { + // Using $this here is fine. + if ($this->something) { + function nestedFunctionDeclaration() { + // Using $this here is not ok as the nested function is a closed scope in the global namespace. + if ($this->something) { + // Do something. + } + } + } + } +} + +trait FooTrait { + public function bar() { + // Using $this here is fine. + if ($this->something) { + function nestedFunctionDeclaration() { + // Using $this here is not ok as the nested function is a closed scope in the global namespace. + if ($this->something) { + // Do something. + } + } + } + } +}