From 47d6424c989377133cfb178d0a008b26ca872716 Mon Sep 17 00:00:00 2001
From: alWerewolf <alwerewolf@gmail.com>
Date: Thu, 30 Nov 2017 18:02:34 +0300
Subject: [PATCH 1/3] Tolerate $this usage in anonymous functions

Alternative solution for issue 536.
Gives warning "$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check." for $this var usage.
And hints developer to check class of $this before usage.
Class check can be added in form "if ($this instanceof [class])" {/*use $this*/}.
Note: if class check added no type checking is done its in language server it just hides the warning.
---
 .../this_in_anonymous_function_check.php      |  7 ++
 .../this_in_method_in_anonymous_function.php  | 11 +++
 ..._in_method_in_anonymous_function_check.php | 13 +++
 ...tic_method_in_anonymous_function_check.php | 13 +++
 .../this_in_static_anonymous_function.php     |  5 +
 .../warnings/this_in_anonymous_function.php   |  5 +
 .../this_in_anonymous_function_no_check.php   |  5 +
 ...in_static_method_in_anonymous_function.php | 11 +++
 src/TreeAnalyzer.php                          | 61 ++++++++++--
 tests/Diagnostics/InvalidThisUsageTest.php    | 96 ++++++++++++++++++-
 10 files changed, 217 insertions(+), 10 deletions(-)
 create mode 100644 fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
 create mode 100644 fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
 create mode 100644 fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
 create mode 100644 fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
 create mode 100644 fixtures/diagnostics/errors/this_in_static_anonymous_function.php
 create mode 100644 fixtures/diagnostics/warnings/this_in_anonymous_function.php
 create mode 100644 fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
 create mode 100644 fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php

diff --git a/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
new file mode 100644
index 0000000..6f3cd18
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
@@ -0,0 +1,7 @@
+<?php
+
+$dummy = function(){
+    if($this instanceof StdClass){
+        return $this;
+    }
+};
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
new file mode 100644
index 0000000..32f990c
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
@@ -0,0 +1,11 @@
+<?php
+
+class Foo
+{
+    public function bar()
+    {
+        return function(){
+            return $this;
+        };
+    }
+}
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
new file mode 100644
index 0000000..936af99
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
@@ -0,0 +1,13 @@
+<?php
+
+class Foo
+{
+    public function bar()
+    {
+        return function(){
+            if($this instanceof StdClass){
+                return $this;
+            }
+        };
+    }
+}
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
new file mode 100644
index 0000000..2574420
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
@@ -0,0 +1,13 @@
+<?php
+
+class Foo
+{
+    public static function bar()
+    {
+        return function(){
+            if($this instanceof StdClass){
+                return $this;
+            }
+        };
+    }
+}
diff --git a/fixtures/diagnostics/errors/this_in_static_anonymous_function.php b/fixtures/diagnostics/errors/this_in_static_anonymous_function.php
new file mode 100644
index 0000000..7812435
--- /dev/null
+++ b/fixtures/diagnostics/errors/this_in_static_anonymous_function.php
@@ -0,0 +1,5 @@
+<?php
+
+$dummy = static function(){
+    return $this;
+};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_anonymous_function.php b/fixtures/diagnostics/warnings/this_in_anonymous_function.php
new file mode 100644
index 0000000..bc37003
--- /dev/null
+++ b/fixtures/diagnostics/warnings/this_in_anonymous_function.php
@@ -0,0 +1,5 @@
+<?php
+
+$dummy = function(){
+    return $this;
+};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php b/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
new file mode 100644
index 0000000..bc37003
--- /dev/null
+++ b/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
@@ -0,0 +1,5 @@
+<?php
+
+$dummy = function(){
+    return $this;
+};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php b/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php
new file mode 100644
index 0000000..cf529dd
--- /dev/null
+++ b/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php
@@ -0,0 +1,11 @@
+<?php
+
+class Foo
+{
+    public static function bar()
+    {
+        return function(){
+            return $this;
+        };
+    }
+}
diff --git a/src/TreeAnalyzer.php b/src/TreeAnalyzer.php
index 37e2c16..759949c 100644
--- a/src/TreeAnalyzer.php
+++ b/src/TreeAnalyzer.php
@@ -96,19 +96,66 @@ class TreeAnalyzer
 
         // Check for invalid usage of $this.
         if ($node instanceof Node\Expression\Variable && $node->getName() === 'this') {
-            // Find the first ancestor that's a class method. Return an error
-            // if there is none, or if the method is static.
-            $method = $node->getFirstAncestor(Node\MethodDeclaration::class);
-            if ($method === null || $method->isStatic()) {
+            // Find the first ancestor that's a class method or anonymous function.
+            // Return an error if there is none, or if the method or anonymous function is static
+            // Return a warning if there is no class check, for ex. if ($this instanceof [class]) { /*use $this*/}
+            $methodOrAnnonFunc = $node->getFirstAncestor(Node\MethodDeclaration::class, Node\Expression\AnonymousFunctionCreationExpression::class);
+            if ($methodOrAnnonFunc === null){
                 $this->diagnostics[] = new Diagnostic(
-                    $method === null
-                        ? "\$this can only be used in an object context."
-                        : "\$this can not be used in static methods.",
+                    "\$this can only be used in an object context or non-static anonymous functions.",
                     Range::fromNode($node),
                     null,
                     DiagnosticSeverity::ERROR,
                     'php'
                 );
+            } else if ($methodOrAnnonFunc instanceof Node\MethodDeclaration){
+                $method = $methodOrAnnonFunc;
+                if ($method->isStatic()) {
+                    $this->diagnostics[] = new Diagnostic(
+                        "\$this can not be used in static methods.",
+                        Range::fromNode($node),
+                        null,
+                        DiagnosticSeverity::ERROR,
+                        'php'
+                    );
+                }
+            } else if ($methodOrAnnonFunc instanceof Node\Expression\AnonymousFunctionCreationExpression){
+                $anonFunc = $methodOrAnnonFunc;
+                if ($anonFunc->staticModifier){
+                    $this->diagnostics[] = new Diagnostic(
+                        "\$this can not be used in static anonymous functions.",
+                        Range::fromNode($node),
+                        null,
+                        DiagnosticSeverity::ERROR,
+                        'php'
+                    );
+                } else {
+                    // IfStatementNode must be in AnonymousFunctionCreationExpression so get what was first
+                    $ifStatement = $node->getFirstAncestor(Node\Statement\IfStatementNode::class, Node\Expression\AnonymousFunctionCreationExpression::class);
+                    // naive check for Node\Expression\BinaryExpression $ifStatement->expression "$this instanceof [class]":
+                    // checks
+                    //     leftOperand Node\Expression\Variable is current node ($this node) or is any $this node in ifStatement
+                    //     operator kind PhpParser\TokenKind $ifStatement->expression->operator->kind equal to InstanceOfKeyword
+                    // for information: class is in PhpParser\Node\QualifiedName $ifStatement->expression->rightOperand
+                    if (!($ifStatement 
+                        && $ifStatement instanceof Node\Statement\IfStatementNode
+                        && ($expression = $ifStatement->expression)
+                        && $expression instanceof Node\Expression\BinaryExpression
+                        && ($expression->leftOperand = $node 
+                            || $expression->leftOperand instanceof Node\Expression\Variable && $expression->leftOperand->getName() === 'this')
+                        && $expression->operator->kind === PhpParser\TokenKind::InstanceOfKeyword
+                        )){
+                            if (($method = $node->getFirstAncestor(Node\MethodDeclaration::class))===null || $method->isStatic()){
+                                $this->diagnostics[] = new Diagnostic(
+                                        "\$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.",
+                                        Range::fromNode($node),
+                                        null,
+                                        DiagnosticSeverity::WARNING,
+                                        'php'
+                                );
+                            }//else we should warn here that invoker can bind $this to object of any class. Should we?
+                    }
+                }
             }
         }
     }
diff --git a/tests/Diagnostics/InvalidThisUsageTest.php b/tests/Diagnostics/InvalidThisUsageTest.php
index 7542918..e8f7249 100644
--- a/tests/Diagnostics/InvalidThisUsageTest.php
+++ b/tests/Diagnostics/InvalidThisUsageTest.php
@@ -76,11 +76,11 @@ class InvalidThisUsageTest extends TestCase
         $diagnostics = $this->collectDiagnostics(
             __DIR__ . '/../../fixtures/diagnostics/errors/this_in_function.php'
         );
-
+        
         $this->assertCount(1, $diagnostics);
         $this->assertDiagnostic(
             $diagnostics[0],
-            '$this can only be used in an object context.',
+            '$this can only be used in an object context or non-static anonymous functions.',
             DiagnosticSeverity::ERROR,
             new Range(
                 new Position(4, 11),
@@ -98,7 +98,7 @@ class InvalidThisUsageTest extends TestCase
         $this->assertCount(1, $diagnostics);
         $this->assertDiagnostic(
             $diagnostics[0],
-            '$this can only be used in an object context.',
+            '$this can only be used in an object context or non-static anonymous functions.',
             DiagnosticSeverity::ERROR,
             new Range(
                 new Position(2, 5),
@@ -115,4 +115,94 @@ class InvalidThisUsageTest extends TestCase
 
         $this->assertCount(0, $diagnostics);
     }
+
+    public function testThisInMethodInAnonymousFunctionWithNoCheckProducesNoError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+
+    public function testThisInMethodInAnonymousFunctionWithCheckProducesNoError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+
+    public function testThisInAnonymousFunctionWithCheckProducesNoError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_anonymous_function_check.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+
+    public function testThisInStaticMethodInAnonymousFunctionWithCheckProducesWarning()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+
+    public function testThisInStaticAnonymousFunctionProducesError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/errors/this_in_static_anonymous_function.php'
+        );
+
+        $this->assertCount(1, $diagnostics);
+        $this->assertDiagnostic(
+            $diagnostics[0],
+            '$this can not be used in static anonymous functions.',
+            DiagnosticSeverity::ERROR,
+            new Range(
+                new Position(3, 11),
+                new Position(3, 16)
+            )
+        );
+    }
+
+    public function testThisInAnonymousFunctionWithNoCheckProducesWarning()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/warnings/this_in_anonymous_function.php'
+        );
+
+        $this->assertCount(1, $diagnostics);
+        $this->assertDiagnostic(
+            $diagnostics[0],
+            '$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.',
+            DiagnosticSeverity::WARNING,
+            new Range(
+                new Position(3, 11),
+                new Position(3, 16)
+            )
+        );
+    }
+
+    public function testThisInStaticMethodInAnonymousFunctionWithNoCheckProducesWarning()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php'
+        );
+
+        $this->assertCount(1, $diagnostics);
+        $this->assertDiagnostic(
+            $diagnostics[0],
+            '$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.',
+            DiagnosticSeverity::WARNING,
+            new Range(
+                new Position(7, 19),
+                new Position(7, 24)
+            )
+        );
+    }
 }

From ed32ac765929f39f2028d907bd4c2b90186e3234 Mon Sep 17 00:00:00 2001
From: alWerewolf <alwerewolf@gmail.com>
Date: Thu, 30 Nov 2017 18:10:38 +0300
Subject: [PATCH 2/3] Revert "Tolerate $this usage in anonymous functions"

This reverts commit 47d6424c989377133cfb178d0a008b26ca872716.
---
 .../this_in_anonymous_function_check.php      |  7 --
 .../this_in_method_in_anonymous_function.php  | 11 ---
 ..._in_method_in_anonymous_function_check.php | 13 ---
 ...tic_method_in_anonymous_function_check.php | 13 ---
 .../this_in_static_anonymous_function.php     |  5 -
 .../warnings/this_in_anonymous_function.php   |  5 -
 .../this_in_anonymous_function_no_check.php   |  5 -
 ...in_static_method_in_anonymous_function.php | 11 ---
 src/TreeAnalyzer.php                          | 61 ++----------
 tests/Diagnostics/InvalidThisUsageTest.php    | 96 +------------------
 10 files changed, 10 insertions(+), 217 deletions(-)
 delete mode 100644 fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
 delete mode 100644 fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
 delete mode 100644 fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
 delete mode 100644 fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
 delete mode 100644 fixtures/diagnostics/errors/this_in_static_anonymous_function.php
 delete mode 100644 fixtures/diagnostics/warnings/this_in_anonymous_function.php
 delete mode 100644 fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
 delete mode 100644 fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php

diff --git a/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
deleted file mode 100644
index 6f3cd18..0000000
--- a/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
+++ /dev/null
@@ -1,7 +0,0 @@
-<?php
-
-$dummy = function(){
-    if($this instanceof StdClass){
-        return $this;
-    }
-};
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
deleted file mode 100644
index 32f990c..0000000
--- a/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
+++ /dev/null
@@ -1,11 +0,0 @@
-<?php
-
-class Foo
-{
-    public function bar()
-    {
-        return function(){
-            return $this;
-        };
-    }
-}
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
deleted file mode 100644
index 936af99..0000000
--- a/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
+++ /dev/null
@@ -1,13 +0,0 @@
-<?php
-
-class Foo
-{
-    public function bar()
-    {
-        return function(){
-            if($this instanceof StdClass){
-                return $this;
-            }
-        };
-    }
-}
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
deleted file mode 100644
index 2574420..0000000
--- a/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
+++ /dev/null
@@ -1,13 +0,0 @@
-<?php
-
-class Foo
-{
-    public static function bar()
-    {
-        return function(){
-            if($this instanceof StdClass){
-                return $this;
-            }
-        };
-    }
-}
diff --git a/fixtures/diagnostics/errors/this_in_static_anonymous_function.php b/fixtures/diagnostics/errors/this_in_static_anonymous_function.php
deleted file mode 100644
index 7812435..0000000
--- a/fixtures/diagnostics/errors/this_in_static_anonymous_function.php
+++ /dev/null
@@ -1,5 +0,0 @@
-<?php
-
-$dummy = static function(){
-    return $this;
-};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_anonymous_function.php b/fixtures/diagnostics/warnings/this_in_anonymous_function.php
deleted file mode 100644
index bc37003..0000000
--- a/fixtures/diagnostics/warnings/this_in_anonymous_function.php
+++ /dev/null
@@ -1,5 +0,0 @@
-<?php
-
-$dummy = function(){
-    return $this;
-};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php b/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
deleted file mode 100644
index bc37003..0000000
--- a/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
+++ /dev/null
@@ -1,5 +0,0 @@
-<?php
-
-$dummy = function(){
-    return $this;
-};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php b/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php
deleted file mode 100644
index cf529dd..0000000
--- a/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php
+++ /dev/null
@@ -1,11 +0,0 @@
-<?php
-
-class Foo
-{
-    public static function bar()
-    {
-        return function(){
-            return $this;
-        };
-    }
-}
diff --git a/src/TreeAnalyzer.php b/src/TreeAnalyzer.php
index 759949c..37e2c16 100644
--- a/src/TreeAnalyzer.php
+++ b/src/TreeAnalyzer.php
@@ -96,66 +96,19 @@ class TreeAnalyzer
 
         // Check for invalid usage of $this.
         if ($node instanceof Node\Expression\Variable && $node->getName() === 'this') {
-            // Find the first ancestor that's a class method or anonymous function.
-            // Return an error if there is none, or if the method or anonymous function is static
-            // Return a warning if there is no class check, for ex. if ($this instanceof [class]) { /*use $this*/}
-            $methodOrAnnonFunc = $node->getFirstAncestor(Node\MethodDeclaration::class, Node\Expression\AnonymousFunctionCreationExpression::class);
-            if ($methodOrAnnonFunc === null){
+            // Find the first ancestor that's a class method. Return an error
+            // if there is none, or if the method is static.
+            $method = $node->getFirstAncestor(Node\MethodDeclaration::class);
+            if ($method === null || $method->isStatic()) {
                 $this->diagnostics[] = new Diagnostic(
-                    "\$this can only be used in an object context or non-static anonymous functions.",
+                    $method === null
+                        ? "\$this can only be used in an object context."
+                        : "\$this can not be used in static methods.",
                     Range::fromNode($node),
                     null,
                     DiagnosticSeverity::ERROR,
                     'php'
                 );
-            } else if ($methodOrAnnonFunc instanceof Node\MethodDeclaration){
-                $method = $methodOrAnnonFunc;
-                if ($method->isStatic()) {
-                    $this->diagnostics[] = new Diagnostic(
-                        "\$this can not be used in static methods.",
-                        Range::fromNode($node),
-                        null,
-                        DiagnosticSeverity::ERROR,
-                        'php'
-                    );
-                }
-            } else if ($methodOrAnnonFunc instanceof Node\Expression\AnonymousFunctionCreationExpression){
-                $anonFunc = $methodOrAnnonFunc;
-                if ($anonFunc->staticModifier){
-                    $this->diagnostics[] = new Diagnostic(
-                        "\$this can not be used in static anonymous functions.",
-                        Range::fromNode($node),
-                        null,
-                        DiagnosticSeverity::ERROR,
-                        'php'
-                    );
-                } else {
-                    // IfStatementNode must be in AnonymousFunctionCreationExpression so get what was first
-                    $ifStatement = $node->getFirstAncestor(Node\Statement\IfStatementNode::class, Node\Expression\AnonymousFunctionCreationExpression::class);
-                    // naive check for Node\Expression\BinaryExpression $ifStatement->expression "$this instanceof [class]":
-                    // checks
-                    //     leftOperand Node\Expression\Variable is current node ($this node) or is any $this node in ifStatement
-                    //     operator kind PhpParser\TokenKind $ifStatement->expression->operator->kind equal to InstanceOfKeyword
-                    // for information: class is in PhpParser\Node\QualifiedName $ifStatement->expression->rightOperand
-                    if (!($ifStatement 
-                        && $ifStatement instanceof Node\Statement\IfStatementNode
-                        && ($expression = $ifStatement->expression)
-                        && $expression instanceof Node\Expression\BinaryExpression
-                        && ($expression->leftOperand = $node 
-                            || $expression->leftOperand instanceof Node\Expression\Variable && $expression->leftOperand->getName() === 'this')
-                        && $expression->operator->kind === PhpParser\TokenKind::InstanceOfKeyword
-                        )){
-                            if (($method = $node->getFirstAncestor(Node\MethodDeclaration::class))===null || $method->isStatic()){
-                                $this->diagnostics[] = new Diagnostic(
-                                        "\$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.",
-                                        Range::fromNode($node),
-                                        null,
-                                        DiagnosticSeverity::WARNING,
-                                        'php'
-                                );
-                            }//else we should warn here that invoker can bind $this to object of any class. Should we?
-                    }
-                }
             }
         }
     }
diff --git a/tests/Diagnostics/InvalidThisUsageTest.php b/tests/Diagnostics/InvalidThisUsageTest.php
index e8f7249..7542918 100644
--- a/tests/Diagnostics/InvalidThisUsageTest.php
+++ b/tests/Diagnostics/InvalidThisUsageTest.php
@@ -76,11 +76,11 @@ class InvalidThisUsageTest extends TestCase
         $diagnostics = $this->collectDiagnostics(
             __DIR__ . '/../../fixtures/diagnostics/errors/this_in_function.php'
         );
-        
+
         $this->assertCount(1, $diagnostics);
         $this->assertDiagnostic(
             $diagnostics[0],
-            '$this can only be used in an object context or non-static anonymous functions.',
+            '$this can only be used in an object context.',
             DiagnosticSeverity::ERROR,
             new Range(
                 new Position(4, 11),
@@ -98,7 +98,7 @@ class InvalidThisUsageTest extends TestCase
         $this->assertCount(1, $diagnostics);
         $this->assertDiagnostic(
             $diagnostics[0],
-            '$this can only be used in an object context or non-static anonymous functions.',
+            '$this can only be used in an object context.',
             DiagnosticSeverity::ERROR,
             new Range(
                 new Position(2, 5),
@@ -115,94 +115,4 @@ class InvalidThisUsageTest extends TestCase
 
         $this->assertCount(0, $diagnostics);
     }
-
-    public function testThisInMethodInAnonymousFunctionWithNoCheckProducesNoError()
-    {
-        $diagnostics = $this->collectDiagnostics(
-            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php'
-        );
-
-        $this->assertCount(0, $diagnostics);
-    }
-
-    public function testThisInMethodInAnonymousFunctionWithCheckProducesNoError()
-    {
-        $diagnostics = $this->collectDiagnostics(
-            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php'
-        );
-
-        $this->assertCount(0, $diagnostics);
-    }
-
-    public function testThisInAnonymousFunctionWithCheckProducesNoError()
-    {
-        $diagnostics = $this->collectDiagnostics(
-            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_anonymous_function_check.php'
-        );
-
-        $this->assertCount(0, $diagnostics);
-    }
-
-    public function testThisInStaticMethodInAnonymousFunctionWithCheckProducesWarning()
-    {
-        $diagnostics = $this->collectDiagnostics(
-            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php'
-        );
-
-        $this->assertCount(0, $diagnostics);
-    }
-
-    public function testThisInStaticAnonymousFunctionProducesError()
-    {
-        $diagnostics = $this->collectDiagnostics(
-            __DIR__ . '/../../fixtures/diagnostics/errors/this_in_static_anonymous_function.php'
-        );
-
-        $this->assertCount(1, $diagnostics);
-        $this->assertDiagnostic(
-            $diagnostics[0],
-            '$this can not be used in static anonymous functions.',
-            DiagnosticSeverity::ERROR,
-            new Range(
-                new Position(3, 11),
-                new Position(3, 16)
-            )
-        );
-    }
-
-    public function testThisInAnonymousFunctionWithNoCheckProducesWarning()
-    {
-        $diagnostics = $this->collectDiagnostics(
-            __DIR__ . '/../../fixtures/diagnostics/warnings/this_in_anonymous_function.php'
-        );
-
-        $this->assertCount(1, $diagnostics);
-        $this->assertDiagnostic(
-            $diagnostics[0],
-            '$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.',
-            DiagnosticSeverity::WARNING,
-            new Range(
-                new Position(3, 11),
-                new Position(3, 16)
-            )
-        );
-    }
-
-    public function testThisInStaticMethodInAnonymousFunctionWithNoCheckProducesWarning()
-    {
-        $diagnostics = $this->collectDiagnostics(
-            __DIR__ . '/../../fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php'
-        );
-
-        $this->assertCount(1, $diagnostics);
-        $this->assertDiagnostic(
-            $diagnostics[0],
-            '$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.',
-            DiagnosticSeverity::WARNING,
-            new Range(
-                new Position(7, 19),
-                new Position(7, 24)
-            )
-        );
-    }
 }

From 631be91b17e5cd1889ae27bdaaf669b3ca0065bd Mon Sep 17 00:00:00 2001
From: alWerewolf <alwerewolf@gmail.com>
Date: Thu, 30 Nov 2017 18:10:46 +0300
Subject: [PATCH 3/3] Revert "Revert "Tolerate $this usage in anonymous
 functions""

This reverts commit ed32ac765929f39f2028d907bd4c2b90186e3234.
---
 .../this_in_anonymous_function_check.php      |  7 ++
 .../this_in_method_in_anonymous_function.php  | 11 +++
 ..._in_method_in_anonymous_function_check.php | 13 +++
 ...tic_method_in_anonymous_function_check.php | 13 +++
 .../this_in_static_anonymous_function.php     |  5 +
 .../warnings/this_in_anonymous_function.php   |  5 +
 .../this_in_anonymous_function_no_check.php   |  5 +
 ...in_static_method_in_anonymous_function.php | 11 +++
 src/TreeAnalyzer.php                          | 61 ++++++++++--
 tests/Diagnostics/InvalidThisUsageTest.php    | 96 ++++++++++++++++++-
 10 files changed, 217 insertions(+), 10 deletions(-)
 create mode 100644 fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
 create mode 100644 fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
 create mode 100644 fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
 create mode 100644 fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
 create mode 100644 fixtures/diagnostics/errors/this_in_static_anonymous_function.php
 create mode 100644 fixtures/diagnostics/warnings/this_in_anonymous_function.php
 create mode 100644 fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
 create mode 100644 fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php

diff --git a/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
new file mode 100644
index 0000000..6f3cd18
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_anonymous_function_check.php
@@ -0,0 +1,7 @@
+<?php
+
+$dummy = function(){
+    if($this instanceof StdClass){
+        return $this;
+    }
+};
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
new file mode 100644
index 0000000..32f990c
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php
@@ -0,0 +1,11 @@
+<?php
+
+class Foo
+{
+    public function bar()
+    {
+        return function(){
+            return $this;
+        };
+    }
+}
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
new file mode 100644
index 0000000..936af99
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php
@@ -0,0 +1,13 @@
+<?php
+
+class Foo
+{
+    public function bar()
+    {
+        return function(){
+            if($this instanceof StdClass){
+                return $this;
+            }
+        };
+    }
+}
\ No newline at end of file
diff --git a/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php b/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
new file mode 100644
index 0000000..2574420
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php
@@ -0,0 +1,13 @@
+<?php
+
+class Foo
+{
+    public static function bar()
+    {
+        return function(){
+            if($this instanceof StdClass){
+                return $this;
+            }
+        };
+    }
+}
diff --git a/fixtures/diagnostics/errors/this_in_static_anonymous_function.php b/fixtures/diagnostics/errors/this_in_static_anonymous_function.php
new file mode 100644
index 0000000..7812435
--- /dev/null
+++ b/fixtures/diagnostics/errors/this_in_static_anonymous_function.php
@@ -0,0 +1,5 @@
+<?php
+
+$dummy = static function(){
+    return $this;
+};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_anonymous_function.php b/fixtures/diagnostics/warnings/this_in_anonymous_function.php
new file mode 100644
index 0000000..bc37003
--- /dev/null
+++ b/fixtures/diagnostics/warnings/this_in_anonymous_function.php
@@ -0,0 +1,5 @@
+<?php
+
+$dummy = function(){
+    return $this;
+};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php b/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
new file mode 100644
index 0000000..bc37003
--- /dev/null
+++ b/fixtures/diagnostics/warnings/this_in_anonymous_function_no_check.php
@@ -0,0 +1,5 @@
+<?php
+
+$dummy = function(){
+    return $this;
+};
\ No newline at end of file
diff --git a/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php b/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php
new file mode 100644
index 0000000..cf529dd
--- /dev/null
+++ b/fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php
@@ -0,0 +1,11 @@
+<?php
+
+class Foo
+{
+    public static function bar()
+    {
+        return function(){
+            return $this;
+        };
+    }
+}
diff --git a/src/TreeAnalyzer.php b/src/TreeAnalyzer.php
index 37e2c16..759949c 100644
--- a/src/TreeAnalyzer.php
+++ b/src/TreeAnalyzer.php
@@ -96,19 +96,66 @@ class TreeAnalyzer
 
         // Check for invalid usage of $this.
         if ($node instanceof Node\Expression\Variable && $node->getName() === 'this') {
-            // Find the first ancestor that's a class method. Return an error
-            // if there is none, or if the method is static.
-            $method = $node->getFirstAncestor(Node\MethodDeclaration::class);
-            if ($method === null || $method->isStatic()) {
+            // Find the first ancestor that's a class method or anonymous function.
+            // Return an error if there is none, or if the method or anonymous function is static
+            // Return a warning if there is no class check, for ex. if ($this instanceof [class]) { /*use $this*/}
+            $methodOrAnnonFunc = $node->getFirstAncestor(Node\MethodDeclaration::class, Node\Expression\AnonymousFunctionCreationExpression::class);
+            if ($methodOrAnnonFunc === null){
                 $this->diagnostics[] = new Diagnostic(
-                    $method === null
-                        ? "\$this can only be used in an object context."
-                        : "\$this can not be used in static methods.",
+                    "\$this can only be used in an object context or non-static anonymous functions.",
                     Range::fromNode($node),
                     null,
                     DiagnosticSeverity::ERROR,
                     'php'
                 );
+            } else if ($methodOrAnnonFunc instanceof Node\MethodDeclaration){
+                $method = $methodOrAnnonFunc;
+                if ($method->isStatic()) {
+                    $this->diagnostics[] = new Diagnostic(
+                        "\$this can not be used in static methods.",
+                        Range::fromNode($node),
+                        null,
+                        DiagnosticSeverity::ERROR,
+                        'php'
+                    );
+                }
+            } else if ($methodOrAnnonFunc instanceof Node\Expression\AnonymousFunctionCreationExpression){
+                $anonFunc = $methodOrAnnonFunc;
+                if ($anonFunc->staticModifier){
+                    $this->diagnostics[] = new Diagnostic(
+                        "\$this can not be used in static anonymous functions.",
+                        Range::fromNode($node),
+                        null,
+                        DiagnosticSeverity::ERROR,
+                        'php'
+                    );
+                } else {
+                    // IfStatementNode must be in AnonymousFunctionCreationExpression so get what was first
+                    $ifStatement = $node->getFirstAncestor(Node\Statement\IfStatementNode::class, Node\Expression\AnonymousFunctionCreationExpression::class);
+                    // naive check for Node\Expression\BinaryExpression $ifStatement->expression "$this instanceof [class]":
+                    // checks
+                    //     leftOperand Node\Expression\Variable is current node ($this node) or is any $this node in ifStatement
+                    //     operator kind PhpParser\TokenKind $ifStatement->expression->operator->kind equal to InstanceOfKeyword
+                    // for information: class is in PhpParser\Node\QualifiedName $ifStatement->expression->rightOperand
+                    if (!($ifStatement 
+                        && $ifStatement instanceof Node\Statement\IfStatementNode
+                        && ($expression = $ifStatement->expression)
+                        && $expression instanceof Node\Expression\BinaryExpression
+                        && ($expression->leftOperand = $node 
+                            || $expression->leftOperand instanceof Node\Expression\Variable && $expression->leftOperand->getName() === 'this')
+                        && $expression->operator->kind === PhpParser\TokenKind::InstanceOfKeyword
+                        )){
+                            if (($method = $node->getFirstAncestor(Node\MethodDeclaration::class))===null || $method->isStatic()){
+                                $this->diagnostics[] = new Diagnostic(
+                                        "\$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.",
+                                        Range::fromNode($node),
+                                        null,
+                                        DiagnosticSeverity::WARNING,
+                                        'php'
+                                );
+                            }//else we should warn here that invoker can bind $this to object of any class. Should we?
+                    }
+                }
             }
         }
     }
diff --git a/tests/Diagnostics/InvalidThisUsageTest.php b/tests/Diagnostics/InvalidThisUsageTest.php
index 7542918..e8f7249 100644
--- a/tests/Diagnostics/InvalidThisUsageTest.php
+++ b/tests/Diagnostics/InvalidThisUsageTest.php
@@ -76,11 +76,11 @@ class InvalidThisUsageTest extends TestCase
         $diagnostics = $this->collectDiagnostics(
             __DIR__ . '/../../fixtures/diagnostics/errors/this_in_function.php'
         );
-
+        
         $this->assertCount(1, $diagnostics);
         $this->assertDiagnostic(
             $diagnostics[0],
-            '$this can only be used in an object context.',
+            '$this can only be used in an object context or non-static anonymous functions.',
             DiagnosticSeverity::ERROR,
             new Range(
                 new Position(4, 11),
@@ -98,7 +98,7 @@ class InvalidThisUsageTest extends TestCase
         $this->assertCount(1, $diagnostics);
         $this->assertDiagnostic(
             $diagnostics[0],
-            '$this can only be used in an object context.',
+            '$this can only be used in an object context or non-static anonymous functions.',
             DiagnosticSeverity::ERROR,
             new Range(
                 new Position(2, 5),
@@ -115,4 +115,94 @@ class InvalidThisUsageTest extends TestCase
 
         $this->assertCount(0, $diagnostics);
     }
+
+    public function testThisInMethodInAnonymousFunctionWithNoCheckProducesNoError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method_in_anonymous_function.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+
+    public function testThisInMethodInAnonymousFunctionWithCheckProducesNoError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method_in_anonymous_function_check.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+
+    public function testThisInAnonymousFunctionWithCheckProducesNoError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_anonymous_function_check.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+
+    public function testThisInStaticMethodInAnonymousFunctionWithCheckProducesWarning()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_static_method_in_anonymous_function_check.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+
+    public function testThisInStaticAnonymousFunctionProducesError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/errors/this_in_static_anonymous_function.php'
+        );
+
+        $this->assertCount(1, $diagnostics);
+        $this->assertDiagnostic(
+            $diagnostics[0],
+            '$this can not be used in static anonymous functions.',
+            DiagnosticSeverity::ERROR,
+            new Range(
+                new Position(3, 11),
+                new Position(3, 16)
+            )
+        );
+    }
+
+    public function testThisInAnonymousFunctionWithNoCheckProducesWarning()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/warnings/this_in_anonymous_function.php'
+        );
+
+        $this->assertCount(1, $diagnostics);
+        $this->assertDiagnostic(
+            $diagnostics[0],
+            '$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.',
+            DiagnosticSeverity::WARNING,
+            new Range(
+                new Position(3, 11),
+                new Position(3, 16)
+            )
+        );
+    }
+
+    public function testThisInStaticMethodInAnonymousFunctionWithNoCheckProducesWarning()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/warnings/this_in_static_method_in_anonymous_function.php'
+        );
+
+        $this->assertCount(1, $diagnostics);
+        $this->assertDiagnostic(
+            $diagnostics[0],
+            '$this might not be bound by invoker of callable or might be bound to object of any class. Consider adding instance class check.',
+            DiagnosticSeverity::WARNING,
+            new Range(
+                new Position(7, 19),
+                new Position(7, 24)
+            )
+        );
+    }
 }