fix(indexing): properly resolve self, static and parent keywords (#532)

Previously we would dump static, self and parent as literal FQNs into the index.
2017-11-18 16:59:57 -08:00 · 2017-11-18 16:59:57 -08:00 · 80ef8ff503
parent b1a1875070
commit 80ef8ff503
21 changed files with 172 additions and 108 deletions
--- a/.travis.yml
+++ b/.travis.yml
@ -23,7 +23,7 @@ install:
  - composer install --prefer-dist --no-interaction
 script:
  - vendor/bin/phpcs -n
-  - vendor/bin/phpunit --coverage-clover=coverage.xml
+  - vendor/bin/phpunit --coverage-clover=coverage.xml --colors=always
  - bash <(curl -s https://codecov.io/bash)

 jobs:
--- a/.vscode/launch.json
+++ b/.vscode/launch.json
@ -1,29 +1,27 @@
 {
-    "version": "0.2.0",
-    "configurations": [
-        {
-            "name": "Listen for XDebug",
-            "type": "php",
-            "request": "launch",
-            "port": 9000
-        },
-        {
-            "name": "Launch currently open script",
-            "type": "php",
-            "request": "launch",
-            "program": "${file}",
-            "cwd": "${fileDirname}",
-            "port": 9000
-        },
-        {
-            "name": "PHPUnit",
-            "type": "php",
-            "request": "launch",
-            "program": "${workspaceRoot}/vendor/phpunit/phpunit/phpunit",
-            "cwd": "${workspaceRoot}",
-            "args": [
-                // "--filter", "CompletionTest"
-            ]
-        }
-    ]
+  "version": "0.2.0",
+  "configurations": [
+    {
+      "name": "PHPUnit",
+      "type": "php",
+      "request": "launch",
+      "program": "${workspaceRoot}/vendor/phpunit/phpunit/phpunit",
+      // "args": ["--filter", "testDefinitionForSelfKeyword"],
+      "cwd": "${workspaceRoot}"
+    },
+    {
+      "name": "Listen for XDebug",
+      "type": "php",
+      "request": "launch",
+      "port": 9000
+    },
+    {
+      "name": "Launch currently open script",
+      "type": "php",
+      "request": "launch",
+      "program": "${file}",
+      "cwd": "${fileDirname}",
+      "port": 9000
+    }
+  ]
 }
--- a/src/DefinitionResolver.php
+++ b/src/DefinitionResolver.php
@ -264,13 +264,38 @@ class DefinitionResolver
        // Other references are references to a global symbol that have an FQN
        // Find out the FQN
        $fqn = $this->resolveReferenceNodeToFqn($node);
-        if ($fqn === null) {
+        if (!$fqn) {
            return null;
        }
+
+        if ($fqn === 'self' || $fqn === 'static') {
+            // Resolve self and static keywords to the containing class
+            // (This is not 100% correct for static but better than nothing)
+            $classNode = $node->getFirstAncestor(Node\Statement\ClassDeclaration::class);
+            if (!$classNode) {
+                return;
+            }
+            $fqn = (string)$classNode->getNamespacedName();
+            if (!$fqn) {
+                return;
+            }
+        } else if ($fqn === 'parent') {
+            // Resolve parent keyword to the base class FQN
+            $classNode = $node->getFirstAncestor(Node\Statement\ClassDeclaration::class);
+            if (!$classNode || !$classNode->classBaseClause || !$classNode->classBaseClause->baseClass) {
+                return;
+            }
+            $fqn = (string)$classNode->classBaseClause->baseClass->getResolvedName();
+            if (!$fqn) {
+                return;
+            }
+        }
+
        // If the node is a function or constant, it could be namespaced, but PHP falls back to global
        // http://php.net/manual/en/language.namespaces.fallback.php
        // TODO - verify that this is not a method
        $globalFallback = ParserHelpers\isConstantFetch($node) || $parent instanceof Node\Expression\CallExpression;
+
        // Return the Definition object from the index index
        return $this->index->getDefinition($fqn, $globalFallback);
    }
@ -278,6 +303,7 @@ class DefinitionResolver
    /**
     * Given any node, returns the FQN of the symbol that is referenced
     * Returns null if the FQN could not be resolved or the reference node references a variable
+     * May also return "static", "self" or "parent"
     *
     * @param Node $node
     * @return string|null
--- a/src/TreeAnalyzer.php
+++ b/src/TreeAnalyzer.php
@ -140,8 +140,9 @@ class TreeAnalyzer
            $this->definitionNodes[$fqn] = $node;
            $this->definitions[$fqn] = $this->definitionResolver->createDefinitionFromNode($node, $fqn);
        } else {
+
            $parent = $node->parent;
-            if (!(
+            if (
                (
                    // $node->parent instanceof Node\Expression\ScopedPropertyAccessExpression ||
                    ($node instanceof Node\Expression\ScopedPropertyAccessExpression ||
@ -150,41 +151,68 @@ class TreeAnalyzer
                        $node->parent instanceof Node\Expression\CallExpression ||
                        $node->memberName instanceof PhpParser\Token
                    ))
-                || ($parent instanceof Node\Statement\NamespaceDefinition && $parent->name !== null && $parent->name->getStart() === $node->getStart()))
+                || ($parent instanceof Node\Statement\NamespaceDefinition && $parent->name !== null && $parent->name->getStart() === $node->getStart())
            ) {
-                $fqn = $this->definitionResolver->resolveReferenceNodeToFqn($node);
-                if ($fqn !== null) {
-                    $this->addReference($fqn, $node);
+                return;
+            }

-                    if (
-                        $node instanceof Node\QualifiedName
-                        && ($node->isQualifiedName() || $node->parent instanceof Node\NamespaceUseClause)
-                        && !($parent instanceof Node\Statement\NamespaceDefinition && $parent->name->getStart() === $node->getStart()
-                        )
-                    ) {
-                        // Add references for each referenced namespace
-                        $ns = $fqn;
-                        while (($pos = strrpos($ns, '\\')) !== false) {
-                            $ns = substr($ns, 0, $pos);
-                            $this->addReference($ns, $node);
-                        }
-                    }
+            $fqn = $this->definitionResolver->resolveReferenceNodeToFqn($node);
+            if (!$fqn) {
+                return;
+            }

-                    // Namespaced constant access and function calls also need to register a reference
-                    // to the global version because PHP falls back to global at runtime
-                    // http://php.net/manual/en/language.namespaces.fallback.php
-                    if (ParserHelpers\isConstantFetch($node) ||
-                        ($parent instanceof Node\Expression\CallExpression
-                            && !(
-                                $node instanceof Node\Expression\ScopedPropertyAccessExpression ||
-                                $node instanceof Node\Expression\MemberAccessExpression
-                            ))) {
-                        $parts = explode('\\', $fqn);
-                        if (count($parts) > 1) {
-                            $globalFqn = end($parts);
-                            $this->addReference($globalFqn, $node);
-                        }
-                    }
+            if ($fqn === 'self' || $fqn === 'static') {
+                // Resolve self and static keywords to the containing class
+                // (This is not 100% correct for static but better than nothing)
+                $classNode = $node->getFirstAncestor(Node\Statement\ClassDeclaration::class);
+                if (!$classNode) {
+                    return;
+                }
+                $fqn = (string)$classNode->getNamespacedName();
+                if (!$fqn) {
+                    return;
+                }
+            } else if ($fqn === 'parent') {
+                // Resolve parent keyword to the base class FQN
+                $classNode = $node->getFirstAncestor(Node\Statement\ClassDeclaration::class);
+                if (!$classNode || !$classNode->classBaseClause || !$classNode->classBaseClause->baseClass) {
+                    return;
+                }
+                $fqn = (string)$classNode->classBaseClause->baseClass->getResolvedName();
+                if (!$fqn) {
+                    return;
+                }
+            }
+
+            $this->addReference($fqn, $node);
+
+            if (
+                $node instanceof Node\QualifiedName
+                && ($node->isQualifiedName() || $node->parent instanceof Node\NamespaceUseClause)
+                && !($parent instanceof Node\Statement\NamespaceDefinition && $parent->name->getStart() === $node->getStart()
+                )
+            ) {
+                // Add references for each referenced namespace
+                $ns = $fqn;
+                while (($pos = strrpos($ns, '\\')) !== false) {
+                    $ns = substr($ns, 0, $pos);
+                    $this->addReference($ns, $node);
+                }
+            }
+
+            // Namespaced constant access and function calls also need to register a reference
+            // to the global version because PHP falls back to global at runtime
+            // http://php.net/manual/en/language.namespaces.fallback.php
+            if (ParserHelpers\isConstantFetch($node) ||
+                ($parent instanceof Node\Expression\CallExpression
+                    && !(
+                        $node instanceof Node\Expression\ScopedPropertyAccessExpression ||
+                        $node instanceof Node\Expression\MemberAccessExpression
+                    ))) {
+                $parts = explode('\\', $fqn);
+                if (count($parts) > 1) {
+                    $globalFqn = end($parts);
+                    $this->addReference($globalFqn, $node);
                }
            }
        }
--- a/tests/Server/ServerTestCase.php
+++ b/tests/Server/ServerTestCase.php
@ -122,15 +122,16 @@ abstract class ServerTestCase extends TestCase
                0 => new Location($referencesUri, new Range(new Position(29,  5), new Position(29, 15)))
            ],
            'TestNamespace\\TestClass' => [
-                0 => new Location($symbolsUri   , new Range(new Position(99, 25), new Position(99, 34))), // class ChildClass extends TestClass {}
-                1 => new Location($referencesUri, new Range(new Position( 4, 11), new Position( 4, 20))), // $obj = new TestClass();
-                2 => new Location($referencesUri, new Range(new Position( 7,  0), new Position( 7,  9))), // TestClass::staticTestMethod();
-                3 => new Location($referencesUri, new Range(new Position( 8,  5), new Position( 8, 14))), // echo TestClass::$staticTestProperty;
-                4 => new Location($referencesUri, new Range(new Position( 9,  5), new Position( 9, 14))), // TestClass::TEST_CLASS_CONST;
-                5 => new Location($referencesUri, new Range(new Position(21, 18), new Position(21, 27))), // function whatever(TestClass $param)
-                6 => new Location($referencesUri, new Range(new Position(21, 37), new Position(21, 46))), // function whatever(TestClass $param): TestClass
-                7 => new Location($referencesUri, new Range(new Position(39,  0), new Position(39,  9))), // TestClass::$staticTestProperty[123]->testProperty;
-                8 => new Location($useUri,        new Range(new Position( 4,  4), new Position( 4, 27))), // use TestNamespace\TestClass;
+                0 => new Location($symbolsUri,    new Range(new Position(48, 13), new Position(48, 17))), // echo self::TEST_CLASS_CONST;
+                1 => new Location($symbolsUri   , new Range(new Position(99, 25), new Position(99, 34))), // class ChildClass extends TestClass {}
+                2 => new Location($referencesUri, new Range(new Position( 4, 11), new Position( 4, 20))), // $obj = new TestClass();
+                3 => new Location($referencesUri, new Range(new Position( 7,  0), new Position( 7,  9))), // TestClass::staticTestMethod();
+                4 => new Location($referencesUri, new Range(new Position( 8,  5), new Position( 8, 14))), // echo TestClass::$staticTestProperty;
+                5 => new Location($referencesUri, new Range(new Position( 9,  5), new Position( 9, 14))), // TestClass::TEST_CLASS_CONST;
+                6 => new Location($referencesUri, new Range(new Position(21, 18), new Position(21, 27))), // function whatever(TestClass $param)
+                7 => new Location($referencesUri, new Range(new Position(21, 37), new Position(21, 46))), // function whatever(TestClass $param): TestClass
+                8 => new Location($referencesUri, new Range(new Position(39,  0), new Position(39,  9))), // TestClass::$staticTestProperty[123]->testProperty;
+                9 => new Location($useUri,        new Range(new Position( 4,  4), new Position( 4, 27))), // use TestNamespace\TestClass;
            ],
            'TestNamespace\\TestChild' => [
                0 => new Location($referencesUri, new Range(new Position(42,  5), new Position(42, 25))), // echo $child->testProperty;
@ -176,14 +177,15 @@ abstract class ServerTestCase extends TestCase
                1 => new Location($globalReferencesUri, new Range(new Position(29,  5), new Position(29, 15)))
            ],
            'TestClass' => [
-                0 => new Location($globalSymbolsUri,    new Range(new Position(99, 25), new Position(99, 34))), // class ChildClass extends TestClass {}
-                1 => new Location($globalReferencesUri, new Range(new Position( 4, 11), new Position( 4, 20))), // $obj = new TestClass();
-                2 => new Location($globalReferencesUri, new Range(new Position( 7,  0), new Position( 7,  9))), // TestClass::staticTestMethod();
-                3 => new Location($globalReferencesUri, new Range(new Position( 8,  5), new Position( 8, 14))), // echo TestClass::$staticTestProperty;
-                4 => new Location($globalReferencesUri, new Range(new Position( 9,  5), new Position( 9, 14))), // TestClass::TEST_CLASS_CONST;
-                5 => new Location($globalReferencesUri, new Range(new Position(21, 18), new Position(21, 27))), // function whatever(TestClass $param)
-                6 => new Location($globalReferencesUri, new Range(new Position(21, 37), new Position(21, 46))), // function whatever(TestClass $param): TestClass
-                7 => new Location($globalReferencesUri, new Range(new Position(39,  0), new Position(39,  9))), // TestClass::$staticTestProperty[123]->testProperty;
+                0 => new Location($globalSymbolsUri,    new Range(new Position(48, 13), new Position(48, 17))), // echo self::TEST_CLASS_CONST;
+                1 => new Location($globalSymbolsUri,    new Range(new Position(99, 25), new Position(99, 34))), // class ChildClass extends TestClass {}
+                2 => new Location($globalReferencesUri, new Range(new Position( 4, 11), new Position( 4, 20))), // $obj = new TestClass();
+                3 => new Location($globalReferencesUri, new Range(new Position( 7,  0), new Position( 7,  9))), // TestClass::staticTestMethod();
+                4 => new Location($globalReferencesUri, new Range(new Position( 8,  5), new Position( 8, 14))), // echo TestClass::$staticTestProperty;
+                5 => new Location($globalReferencesUri, new Range(new Position( 9,  5), new Position( 9, 14))), // TestClass::TEST_CLASS_CONST;
+                6 => new Location($globalReferencesUri, new Range(new Position(21, 18), new Position(21, 27))), // function whatever(TestClass $param)
+                7 => new Location($globalReferencesUri, new Range(new Position(21, 37), new Position(21, 46))), // function whatever(TestClass $param): TestClass
+                8 => new Location($globalReferencesUri, new Range(new Position(39,  0), new Position(39,  9))), // TestClass::$staticTestProperty[123]->testProperty;
            ],
            'TestChild' => [
                0 => new Location($globalReferencesUri, new Range(new Position(42,  5), new Position(42, 25))), // echo $child->testProperty;
--- a/tests/Server/TextDocument/Definition/GlobalTest.php
+++ b/tests/Server/TextDocument/Definition/GlobalTest.php
@ -29,11 +29,23 @@ class GlobalTest extends ServerTestCase
        $this->assertEquals([], $result);
    }

+    public function testDefinitionForSelfKeyword()
+    {
+        // echo self::TEST_CLASS_CONST;
+        // Get definition for self
+        $reference = $this->getReferenceLocations('TestClass')[0];
+        $result = $this->textDocument->definition(
+            new TextDocumentIdentifier($reference->uri),
+            $reference->range->start
+        )->wait();
+        $this->assertEquals($this->getDefinitionLocation('TestClass'), $result);
+    }
+
    public function testDefinitionForClassLike()
    {
        // $obj = new TestClass();
        // Get definition for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[0];
+        $reference = $this->getReferenceLocations('TestClass')[1];
        $result = $this->textDocument->definition(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
@ -45,7 +57,7 @@ class GlobalTest extends ServerTestCase
    {
        // TestClass::staticTestMethod();
        // Get definition for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[1];
+        $reference = $this->getReferenceLocations('TestClass')[2];
        $result = $this->textDocument->definition(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
@ -57,7 +69,7 @@ class GlobalTest extends ServerTestCase
    {
        // echo TestClass::$staticTestProperty;
        // Get definition for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[2];
+        $reference = $this->getReferenceLocations('TestClass')[3];
        $result = $this->textDocument->definition(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
@ -69,7 +81,7 @@ class GlobalTest extends ServerTestCase
    {
        // TestClass::TEST_CLASS_CONST;
        // Get definition for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[3];
+        $reference = $this->getReferenceLocations('TestClass')[4];
        $result = $this->textDocument->definition(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
@ -213,7 +225,7 @@ class GlobalTest extends ServerTestCase
    {
        // function whatever(TestClass $param) {
        // Get definition for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[4];
+        $reference = $this->getReferenceLocations('TestClass')[5];
        $result = $this->textDocument->definition(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
@ -225,7 +237,7 @@ class GlobalTest extends ServerTestCase
    {
        // function whatever(TestClass $param): TestClass {
        // Get definition for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[5];
+        $reference = $this->getReferenceLocations('TestClass')[6];
        $result = $this->textDocument->definition(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
--- a/tests/Server/TextDocument/Definition/NamespacedTest.php
+++ b/tests/Server/TextDocument/Definition/NamespacedTest.php
@ -34,7 +34,7 @@ class NamespacedTest extends GlobalTest
    {
        // use TestNamespace\TestClass;
        // Get definition for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[6];
+        $reference = $this->getReferenceLocations('TestClass')[7];
        $result = $this->textDocument->definition(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
@ -46,7 +46,7 @@ class NamespacedTest extends GlobalTest
    {
        // use TestNamespace\{TestTrait, TestInterface};
        // Get definition for TestInterface
-        $reference = $this->getReferenceLocations('TestClass')[0];
+        $reference = $this->getReferenceLocations('TestClass')[1];
        $result = $this->textDocument->definition(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
--- a/tests/Server/TextDocument/HoverTest.php
+++ b/tests/Server/TextDocument/HoverTest.php
@ -15,7 +15,7 @@ class HoverTest extends ServerTestCase
    {
        // $obj = new TestClass();
        // Get hover for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[0];
+        $reference = $this->getReferenceLocations('TestClass')[1];
        $result = $this->textDocument->hover(
            new TextDocumentIdentifier($reference->uri),
            $reference->range->start
--- a/tests/Server/TextDocument/References/GlobalTest.php
+++ b/tests/Server/TextDocument/References/GlobalTest.php
@ -151,7 +151,7 @@ class GlobalTest extends ServerTestCase
    {
        // $obj = new TestClass();
        // Get references for TestClass
-        $reference = $this->getReferenceLocations('TestClass')[0];
+        $reference = $this->getReferenceLocations('TestClass')[1];
        $result = $this->textDocument->references(
            new ReferenceContext,
            new TextDocumentIdentifier($reference->uri),
--- a/tests/Validation/cases/WithReturnTypehints.php.expected.json
+++ b/tests/Validation/cases/WithReturnTypehints.php.expected.json
@ -3,7 +3,7 @@
        "Fixtures\\Prophecy\\EmptyClass": [
            "./WithReturnTypehints.php"
        ],
-        "self": [
+        "Fixtures\\Prophecy\\WithReturnTypehints": [
            "./WithReturnTypehints.php"
        ],
        "Fixtures\\Prophecy\\__CLASS__": [
@ -11,9 +11,6 @@
        ],
        "__CLASS__": [
            "./WithReturnTypehints.php"
-        ],
-        "parent": [
-            "./WithReturnTypehints.php"
        ]
    },
    "definitions": {
--- a/tests/Validation/cases/nameToken.php.expected.json
+++ b/tests/Validation/cases/nameToken.php.expected.json
@ -1,5 +1,9 @@
 {
-    "references": [],
+    "references": {
+        "A": [
+            "./nameToken.php"
+        ]
+    },
    "definitions": {
        "A": {
            "fqn": "A",
--- a/tests/Validation/cases/parent1.php.expected.json
+++ b/tests/Validation/cases/parent1.php.expected.json
@ -2,9 +2,6 @@
    "references": {
        "MyNamespace\\B": [
            "./parent1.php"
-        ],
-        "parent": [
-            "./parent1.php"
        ]
    },
    "definitions": {
--- a/tests/Validation/cases/parent3.php.expected.json
+++ b/tests/Validation/cases/parent3.php.expected.json
@ -5,9 +5,6 @@
        ],
        "MyNamespace\\B->b()": [
            "./parent3.php"
-        ],
-        "parent": [
-            "./parent3.php"
        ]
    },
    "definitions": {
--- a/tests/Validation/cases/self1.php.expected.json
+++ b/tests/Validation/cases/self1.php.expected.json
@ -6,7 +6,7 @@
        "MyNamespace\\A::b()": [
            "./self1.php"
        ],
-        "self": [
+        "MyNamespace\\A": [
            "./self1.php"
        ]
    },
--- a/tests/Validation/cases/self2.php.expected.json
+++ b/tests/Validation/cases/self2.php.expected.json
@ -6,7 +6,7 @@
        "MyNamespace\\A::b()": [
            "./self2.php"
        ],
-        "self": [
+        "MyNamespace\\A": [
            "./self2.php"
        ]
    },
--- a/tests/Validation/cases/self3.php.expected.json
+++ b/tests/Validation/cases/self3.php.expected.json
@ -6,7 +6,7 @@
        "MyNamespace\\A->b()": [
            "./self3.php"
        ],
-        "self": [
+        "MyNamespace\\A": [
            "./self3.php"
        ]
    },
--- a/tests/Validation/cases/self4.php.expected.json
+++ b/tests/Validation/cases/self4.php.expected.json
@ -1,6 +1,6 @@
 {
    "references": {
-        "self": [
+        "MyNamespace\\A": [
            "./self4.php"
        ],
        "MyNamespace\\A->addTestFile()": [
--- a/tests/Validation/cases/static1.php.expected.json
+++ b/tests/Validation/cases/static1.php.expected.json
@ -6,7 +6,7 @@
        "MyNamespace\\A::b()": [
            "./static1.php"
        ],
-        "static": [
+        "MyNamespace\\A": [
            "./static1.php"
        ]
    },
--- a/tests/Validation/cases/static2.php.expected.json
+++ b/tests/Validation/cases/static2.php.expected.json
@ -6,7 +6,7 @@
        "MyNamespace\\A::b()": [
            "./static2.php"
        ],
-        "static": [
+        "MyNamespace\\A": [
            "./static2.php"
        ]
    },
--- a/tests/Validation/cases/static3.php.expected.json
+++ b/tests/Validation/cases/static3.php.expected.json
@ -3,10 +3,10 @@
        "MyNamespace\\B": [
            "./static3.php"
        ],
-        "MyNamespace\\b()": [
+        "static->b()": [
            "./static3.php"
        ],
-        "b()": [
+        "MyNamespace\\A": [
            "./static3.php"
        ]
    },
--- a/tests/Validation/cases/static4.php.expected.json
+++ b/tests/Validation/cases/static4.php.expected.json
@ -2,6 +2,9 @@
    "references": {
        "MyNamespace\\B": [
            "./static4.php"
+        ],
+        "MyNamespace\\A": [
+            "./static4.php"
        ]
    },
    "definitions": {