Bug fix: $this in nested function declaration (#119)

While `$this` can be used freely within classes, traits and closures, if a nested closed scope is defined within any of these, `$this` is not inherited and should be considered undefined.

As things were, the sniff would not throw a warning for this (false negative).
This commit fixes this.

Includes unit tests.

Includes removing an outdated comment. `T_CLOSURE` works perfectly fine with the conditions array (and has for quite a few years).

Fixes 109

Author: jrfnl

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;

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);
+  }
 }

VariableAnalysis/Tests/CodeAnalysis/fixtures/ThisWithinNestedClosedScopeFixture.php

@@ -0,0 +1,67 @@
+<?php
+
+function foo() {
+    // Using $this will not work.
+    if ($this->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.
+                }
+            }
+        }
+    }
+}