Allow self within class method closures (#70)

* Test: self can be used inside class closures * Allow self within closures * Test: more tests for self/static/this in closures
sirbrillig · Feb 8, 2019 · 6d66956 · 6d66956
1 parent 30e1278
commit 6d66956
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 10 deletions.
diff --git a/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php b/VariableAnalysis/Sniffs/CodeAnalysis/VariableAnalysisSniff.php
@@ -450,14 +450,8 @@ protected function checkForStaticOutsideClass(File $phpcsFile, $stackPtr, $varNa
     }
     $errorClass = $code === T_SELF ? 'SelfOutsideClass' : 'StaticOutsideClass';
     $staticRefType = $code === T_SELF ? 'self::' : 'static::';
-    if (!empty($token['conditions'])) {
-      if (Helpers::areAnyConditionsAClosure($phpcsFile, $token['conditions'])) {
-        $phpcsFile->addError("Use of {$staticRefType}%s inside closure.", $stackPtr, $errorClass, ["\${$varName}"]);
-        return true;
-      }
-      if (Helpers::areAnyConditionsAClass($token['conditions'])) {
-        return false;
-      }
+    if (!empty($token['conditions']) && Helpers::areAnyConditionsAClass($token['conditions'])) {
+      return false;
     }
     $phpcsFile->addError(
       "Use of {$staticRefType}%s outside class definition.",
@@ -760,7 +754,7 @@ protected function processVariable(File $phpcsFile, $stackPtr) {
     //   Is an optional function/closure parameter with non-null value
     //   Is closure use declaration of a variable defined within containing scope
     //   catch (...) block start
-    //   $this within a class (but not within a closure).
+    //   $this within a class.
     //   $GLOBALS, $_REQUEST, etc superglobals.
     //   $var part of class::$var static member
     //   Assignment via =

diff --git a/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php b/VariableAnalysis/Tests/CodeAnalysis/VariableAnalysisTest.php
@@ -200,7 +200,8 @@ public function testFunctionWithClosureErrors() {
     $phpcsFile->process();
     $lines = $this->getErrorLineNumbersFromFile($phpcsFile);
     $expectedErrors = [
-      50,
+      58,
+      70,
     ];
     $this->assertEquals($expectedErrors, $lines);
   }
@@ -229,6 +230,7 @@ public function testFunctionWithClosureWarnings() {
       27,
       28,
       35,
+      64,
     ];
     $this->assertEquals($expectedWarnings, $lines);
   }

diff --git a/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithClosureFixture.php b/VariableAnalysis/Tests/CodeAnalysis/fixtures/FunctionWithClosureFixture.php
@@ -52,3 +52,33 @@ function method_with_self_inside_closure() {
         echo self::$static_member;
     }
 }
+
+function function_with_self_in_closure() {
+    return function() {
+        return self::$foobar; // should be an error
+    }
+}
+
+function function_with_this_in_closure() {
+    return function() {
+        return $this->$foobar; // should be an error
+    }
+}
+
+function function_with_static_in_closure() {
+    return function() {
+        return static::$foobar; // should be an error
+    }
+}
+
+class ClassWithStaticInsideClosure {
+    static $static_member;
+
+    function method_with_self_inside_closure() {
+        echo static::$static_member;
+        array_map(function () {
+                echo static::$static_member;
+            }, array());
+        echo static::$static_member;
+    }
+}