From 4f672c24d8c127d88d51eefb32973668e0cc54de Mon Sep 17 00:00:00 2001
From: Maarten Staa <maartenstaa@gmail.com>
Date: Sun, 19 Nov 2017 02:41:37 +0100
Subject: [PATCH] feat(diagnostics): report error when $this is used in a
 static method or outside a class method (#528)

---
 .../diagnostics/baselines/this_in_method.php  |   9 ++
 .../diagnostics/errors/this_in_function.php   |   6 +
 fixtures/diagnostics/errors/this_in_root.php  |   3 +
 .../errors/this_in_static_method.php          |   9 ++
 src/TreeAnalyzer.php                          |  19 +++
 tests/Diagnostics/InvalidThisUsageTest.php    | 118 ++++++++++++++++++
 6 files changed, 164 insertions(+)
 create mode 100644 fixtures/diagnostics/baselines/this_in_method.php
 create mode 100644 fixtures/diagnostics/errors/this_in_function.php
 create mode 100644 fixtures/diagnostics/errors/this_in_root.php
 create mode 100644 fixtures/diagnostics/errors/this_in_static_method.php
 create mode 100644 tests/Diagnostics/InvalidThisUsageTest.php

diff --git a/fixtures/diagnostics/baselines/this_in_method.php b/fixtures/diagnostics/baselines/this_in_method.php
new file mode 100644
index 0000000..0463b79
--- /dev/null
+++ b/fixtures/diagnostics/baselines/this_in_method.php
@@ -0,0 +1,9 @@
+<?php
+
+class Foo
+{
+    public function bar()
+    {
+        return $this;
+    }
+}
diff --git a/fixtures/diagnostics/errors/this_in_function.php b/fixtures/diagnostics/errors/this_in_function.php
new file mode 100644
index 0000000..db6139d
--- /dev/null
+++ b/fixtures/diagnostics/errors/this_in_function.php
@@ -0,0 +1,6 @@
+<?php
+
+function foo()
+{
+    return $this;
+}
diff --git a/fixtures/diagnostics/errors/this_in_root.php b/fixtures/diagnostics/errors/this_in_root.php
new file mode 100644
index 0000000..95f5ed7
--- /dev/null
+++ b/fixtures/diagnostics/errors/this_in_root.php
@@ -0,0 +1,3 @@
+<?php
+
+echo $this;
diff --git a/fixtures/diagnostics/errors/this_in_static_method.php b/fixtures/diagnostics/errors/this_in_static_method.php
new file mode 100644
index 0000000..e29b23d
--- /dev/null
+++ b/fixtures/diagnostics/errors/this_in_static_method.php
@@ -0,0 +1,9 @@
+<?php
+
+class Foo
+{
+    public static function bar()
+    {
+        return $this;
+    }
+}
diff --git a/src/TreeAnalyzer.php b/src/TreeAnalyzer.php
index 1daabe7..37e2c16 100644
--- a/src/TreeAnalyzer.php
+++ b/src/TreeAnalyzer.php
@@ -68,6 +68,7 @@ class TreeAnalyzer
      */
     private function collectDiagnostics($node)
     {
+        // Get errors from the parser.
         if (($error = PhpParser\DiagnosticsProvider::checkDiagnostics($node)) !== null) {
             $range = PhpParser\PositionUtilities::getRangeFromPosition($error->start, $error->length, $this->sourceFileNode->fileContents);
 
@@ -92,6 +93,24 @@ class TreeAnalyzer
                 'php'
             );
         }
+
+        // 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()) {
+                $this->diagnostics[] = new Diagnostic(
+                    $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'
+                );
+            }
+        }
     }
 
     /**
diff --git a/tests/Diagnostics/InvalidThisUsageTest.php b/tests/Diagnostics/InvalidThisUsageTest.php
new file mode 100644
index 0000000..7542918
--- /dev/null
+++ b/tests/Diagnostics/InvalidThisUsageTest.php
@@ -0,0 +1,118 @@
+<?php
+declare(strict_types = 1);
+
+namespace LanguageServer\Tests\Diagnostics;
+
+use PHPUnit\Framework\TestCase;
+use phpDocumentor\Reflection\DocBlockFactory;
+use LanguageServer\{
+    DefinitionResolver, TreeAnalyzer
+};
+use LanguageServer\Index\{Index};
+use LanguageServer\Protocol\{
+    Diagnostic, DiagnosticSeverity, Position, Range
+};
+use function LanguageServer\pathToUri;
+use Microsoft\PhpParser\Parser;
+
+class InvalidThisUsageTest extends TestCase
+{
+    /**
+     * Parse the given file and return diagnostics.
+     *
+     * @param string $path
+     * @return Diagnostic[]
+     */
+    private function collectDiagnostics(string $path): array
+    {
+        $uri = pathToUri($path);
+        $parser = new Parser();
+
+        $docBlockFactory = DocBlockFactory::createInstance();
+        $index = new Index;
+        $definitionResolver = new DefinitionResolver($index);
+        $content = file_get_contents($path);
+
+        $treeAnalyzer = new TreeAnalyzer($parser, $content, $docBlockFactory, $definitionResolver, $uri);
+        return $treeAnalyzer->getDiagnostics();
+    }
+
+    /**
+     * Assertions about a diagnostic.
+     *
+     * @param Diagnostic|null $diagnostic
+     * @param int $message
+     * @param string $severity
+     * @param Range $range
+     */
+    private function assertDiagnostic($diagnostic, $message, $severity, $range)
+    {
+        $this->assertInstanceOf(Diagnostic::class, $diagnostic);
+        $this->assertEquals($message, $diagnostic->message);
+        $this->assertEquals($severity, $diagnostic->severity);
+        $this->assertEquals($range, $diagnostic->range);
+    }
+
+    public function testThisInStaticMethodProducesError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/errors/this_in_static_method.php'
+        );
+
+        $this->assertCount(1, $diagnostics);
+        $this->assertDiagnostic(
+            $diagnostics[0],
+            '$this can not be used in static methods.',
+            DiagnosticSeverity::ERROR,
+            new Range(
+                new Position(6, 15),
+                new Position(6, 20)
+            )
+        );
+    }
+
+    public function testThisInFunctionProducesError()
+    {
+        $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.',
+            DiagnosticSeverity::ERROR,
+            new Range(
+                new Position(4, 11),
+                new Position(4, 16)
+            )
+        );
+    }
+
+    public function testThisInRoot()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/errors/this_in_root.php'
+        );
+
+        $this->assertCount(1, $diagnostics);
+        $this->assertDiagnostic(
+            $diagnostics[0],
+            '$this can only be used in an object context.',
+            DiagnosticSeverity::ERROR,
+            new Range(
+                new Position(2, 5),
+                new Position(2, 10)
+            )
+        );
+    }
+
+    public function testThisInMethodProducesNoError()
+    {
+        $diagnostics = $this->collectDiagnostics(
+            __DIR__ . '/../../fixtures/diagnostics/baselines/this_in_method.php'
+        );
+
+        $this->assertCount(0, $diagnostics);
+    }
+}