Add tests for global constant/function fallback

composer.json

@@ -24,7 +24,7 @@
     "bin": ["bin/php-language-server.php"],
     "require": {
         "php": ">=7.0",
-        "nikic/php-parser": "^3.0.0beta1",
+        "nikic/php-parser": "dev-master",
         "phpdocumentor/reflection-docblock": "^3.0",
         "sabre/event": "^4.0",
         "felixfbecker/advanced-json-rpc": "^1.2",

fixtures/global_fallback.php

@@ -0,0 +1,10 @@
+<?php
+
+namespace GlobalFallback;
+
+// Should fall back to global_symbols.php
+test_function();
+echo TEST_CONST;
+
+// Should not fall back
+$obj = new TestClass();

src/NodeVisitor/ReferencesCollector.php

@@ -16,28 +16,26 @@ class ReferencesCollector extends NodeVisitorAbstract
      *
      * @var Node[][]
      */
-    public $references;
+    public $references = [];
-
-    /**
-     * @var Node[]
-     */
-    private $definitions;
-
-    /**
-     * @param Node[] $definitions The definitions that references should be tracked for
-     */
-    public function __construct(array $definitions)
-    {
-        $this->definitions = $definitions;
-        $this->references = array_fill_keys(array_keys($definitions), []);
-    }
 
     public function enterNode(Node $node)
     {
         // Check if the node references any global symbol
         $fqn = $node->getAttribute('ownerDocument')->getReferencedFqn($node);
         if ($fqn) {
-            $this->references[$fqn][] = $node;
+            $this->addReference($fqn, $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
+            $parent = $node->getAttribute('parentNode');
+            if ($parent instanceof Node\Expr\ConstFetch || $parent instanceof Node\Expr\FuncCall) {
+                $parts = explode('\\', $fqn);
+                if (count($parts) > 1) {
+                    $globalFqn = end($parts);
+                    $this->addReference($globalFqn, $node);
+                }
+            }
+            // Namespaced constant references and function calls also need to register a reference to the global
             // Static method calls, constant and property fetches also need to register a reference to the class
             // A reference like TestNamespace\TestClass::myStaticMethod() registers a reference for
             //  - TestNamespace\TestClass
@@ -48,8 +46,16 @@ class ReferencesCollector extends NodeVisitorAbstract
                 || $node instanceof Node\Expr\ClassConstFetch)
                 && $node->class instanceof Node\Name
             ) {
-                $this->references[(string)$node->class][] = $node->class;
+                $this->addReference((string)$node->class, $node->class);
             }
         }
     }
+
+    private function addReference(string $fqn, Node $node)
+    {
+        if (!isset($this->references[$fqn])) {
+            $this->references[$fqn] = [];
+        }
+        $this->references[$fqn][] = $node;
+    }
 }

src/PhpDocument.php

@@ -154,28 +154,29 @@ class PhpDocument
             // Add column attributes to nodes
             $traverser->addVisitor(new ColumnCalculator($content));
 
+            $traverser->traverse($stmts);
+            $traverser = new NodeTraverser;
+
             // Collect all definitions
             $definitionCollector = new DefinitionCollector;
             $traverser->addVisitor($definitionCollector);
 
+            // Collect all references
+            $referencesCollector = new ReferencesCollector($this->definitions);
+            $traverser->addVisitor($referencesCollector);
+
             $traverser->traverse($stmts);
 
             // Register this document on the project for all the symbols defined in it
+            $this->definitions = $definitionCollector->definitions;
             foreach ($definitionCollector->definitions as $fqn => $node) {
                 $this->project->setDefinitionUri($fqn, $this->uri);
             }
 
-            $this->definitions = $definitionCollector->definitions;
-
-            // Collect all references
-            $traverser = new NodeTraverser;
-            $referencesCollector = new ReferencesCollector($this->definitions);
-            $traverser->addVisitor($referencesCollector);
-            $traverser->traverse($stmts);
-            $this->references = $referencesCollector->references;
             // Register this document on the project for references
+            $this->references = $referencesCollector->references;
             foreach ($referencesCollector->references as $fqn => $nodes) {
-                $this->project->addReferenceDocument($fqn, $this);
+                $this->project->addReferenceUri($fqn, $this->uri);
             }
 
             $this->stmts = $stmts;
@@ -396,9 +397,9 @@ class PhpDocument
             if ($parent->name instanceof Node\Expr) {
                 return null;
             }
-            $name = (string)$parent->name;
+            $name = (string)($node->getAttribute('namespacedName') ?? $parent->name);
         } else if ($parent instanceof Node\Expr\ConstFetch) {
-            $name = (string)$parent->name;
+            $name = (string)($node->getAttribute('namespacedName') ?? $parent->name);
         } else if (
             $node instanceof Node\Expr\ClassConstFetch
             || $node instanceof Node\Expr\StaticPropertyFetch
@@ -422,20 +423,6 @@ class PhpDocument
         if (!isset($name)) {
             return null;
         }
-        // If the node is a function or constant, it could be namespaced, but PHP falls back to global
-        // The NameResolver therefor does not currently resolve these to namespaced names
-        // https://github.com/nikic/PHP-Parser/issues/236
-        // http://php.net/manual/en/language.namespaces.fallback.php
-        if ($parent instanceof Node\Expr\FuncCall || $parent instanceof Node\Expr\ConstFetch) {
-            // Find and try with namespace
-            $n = $parent;
-            while (isset($n)) {
-                $n = $n->getAttribute('parentNode');
-                if ($n instanceof Node\Stmt\Namespace_) {
-                    return (string)$n->name . '\\' . $name;
-                }
-            }
-        }
         return $name;
     }
 
@@ -458,6 +445,16 @@ class PhpDocument
             return null;
         }
         $document = $this->project->getDefinitionDocument($fqn);
+        if (!isset($document)) {
+            // 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
+            $parent = $node->getAttribute('parentNode');
+            if ($parent instanceof Node\Expr\ConstFetch || $parent instanceof Node\Expr\FuncCall) {
+                $parts = explode('\\', $fqn);
+                $fqn = end($parts);
+                $document = $this->project->getDefinitionDocument($fqn);
+            }
+        }
         if (!isset($document)) {
             return null;
         }
@@ -521,7 +518,7 @@ class PhpDocument
     {
         $n = $var;
         // Traverse the AST up
-        while (isset($n) && $n = $n->getAttribute('parentNode')) {
+        do {
             // If a function is met, check the parameters and use statements
             if ($n instanceof Node\FunctionLike) {
                 foreach ($n->getParams() as $param) {
@@ -545,7 +542,7 @@ class PhpDocument
                     return $n;
                 }
             }
-        }
+        } while (isset($n) && $n = $n->getAttribute('parentNode'));
         // Return null if nothing was found
         return null;
     }

src/Project.php

@@ -151,19 +151,19 @@ class Project
     }
 
     /**
-     * Adds a document as a referencee of a specific symbol
+     * Adds a document URI as a referencee of a specific symbol
      *
      * @param string $fqn The fully qualified name of the symbol
      * @return void
      */
-    public function addReferenceDocument(string $fqn, PhpDocument $document)
+    public function addReferenceUri(string $fqn, string $uri)
     {
         if (!isset($this->references[$fqn])) {
             $this->references[$fqn] = [];
         }
         // TODO: use DS\Set instead of searching array
-        if (array_search($document, $this->references[$fqn], true) === false) {
+        if (array_search($uri, $this->references[$fqn], true) === false) {
-            $this->references[$fqn][] = $document;
+            $this->references[$fqn][] = $uri;
         }
     }
 
@@ -175,7 +175,10 @@ class Project
      */
     public function getReferenceDocuments(string $fqn)
     {
-        return $this->references[$fqn] ?? [];
+        if (!isset($this->references[$fqn])) {
+            return [];
+        }
+        return array_map([$this, 'getDocument'], $this->references[$fqn]);
     }
 
     /**

tests/Server/TextDocument/Definition/GlobalFallbackTest.php

@@ -0,0 +1,74 @@
+<?php
+declare(strict_types = 1);
+
+namespace LanguageServer\Tests\Server\TextDocument\Definition;
+
+use PHPUnit\Framework\TestCase;
+use LanguageServer\Tests\MockProtocolStream;
+use LanguageServer\{Server, LanguageClient, Project};
+use LanguageServer\Protocol\{TextDocumentIdentifier, Position};
+
+class GlobalFallbackTest extends TestCase
+{
+    /**
+     * @var Server\TextDocument
+     */
+    private $textDocument;
+
+    public function setUp()
+    {
+        $client = new LanguageClient(new MockProtocolStream());
+        $project = new Project($client);
+        $this->textDocument = new Server\TextDocument($project, $client);
+        $project->openDocument('global_fallback', file_get_contents(__DIR__ . '/../../../../fixtures/global_fallback.php'));
+        $project->openDocument('global_symbols', file_get_contents(__DIR__ . '/../../../../fixtures/global_symbols.php'));
+    }
+
+    public function testClassDoesNotFallback()
+    {
+        // $obj = new TestClass();
+        // Get definition for TestClass should not fall back to global
+        $result = $this->textDocument->definition(new TextDocumentIdentifier('global_fallback'), new Position(9, 16));
+        $this->assertEquals([], $result);
+    }
+
+    public function testFallsBackForConstants()
+    {
+        // echo TEST_CONST;
+        // Get definition for TEST_CONST
+        $result = $this->textDocument->definition(new TextDocumentIdentifier('global_fallback'), new Position(6, 10));
+        $this->assertEquals([
+            'uri' => 'global_symbols',
+            'range' => [
+                'start' => [
+                    'line' => 4,
+                    'character' => 6
+                ],
+                'end' => [
+                    'line' => 4,
+                    'character' => 22
+                ]
+            ]
+        ], json_decode(json_encode($result), true));
+    }
+
+    public function testFallsBackForFunctions()
+    {
+        // test_function();
+        // Get definition for test_function
+        $result = $this->textDocument->definition(new TextDocumentIdentifier('global_fallback'), new Position(5, 6));
+        $this->assertEquals([
+            'uri' => 'global_symbols',
+            'range' => [
+                'start' => [
+                    'line' => 33,
+                    'character' => 0
+                ],
+                'end' => [
+                    'line' => 36,
+                    'character' => 1
+                ]
+            ]
+        ], json_decode(json_encode($result), true));
+    }
+}

tests/Server/TextDocument/Definition/NamespacedTest.php

@@ -24,9 +24,6 @@ class NamespacedTest extends TestCase
         $project->openDocument('references', file_get_contents(__DIR__ . '/../../../../fixtures/references.php'));
         $project->openDocument('symbols', file_get_contents(__DIR__ . '/../../../../fixtures/symbols.php'));
         $project->openDocument('use', file_get_contents(__DIR__ . '/../../../../fixtures/use.php'));
-        // Load this to check that there are no conflicts
-        $project->loadDocument(pathToUri(realpath(__DIR__ . '/../../../../fixtures/global_symbols.php')));
-        $project->loadDocument(pathToUri(realpath(__DIR__ . '/../../../../fixtures/global_references.php')));
     }
 
     public function testDefinitionFileBeginning() {

tests/Server/TextDocument/References/GlobalFallbackTest.php

@@ -0,0 +1,78 @@
+<?php
+declare(strict_types = 1);
+
+namespace LanguageServer\Tests\Server\TextDocument\References;
+
+use PHPUnit\Framework\TestCase;
+use LanguageServer\Tests\MockProtocolStream;
+use LanguageServer\{Server, LanguageClient, Project};
+use LanguageServer\Protocol\{TextDocumentIdentifier, Position, ReferenceContext};
+
+class GlobalFallbackTest extends TestCase
+{
+    /**
+     * @var Server\TextDocument
+     */
+    private $textDocument;
+
+    public function setUp()
+    {
+        $client = new LanguageClient(new MockProtocolStream());
+        $project = new Project($client);
+        $this->textDocument = new Server\TextDocument($project, $client);
+        $project->openDocument('global_fallback', file_get_contents(__DIR__ . '/../../../../fixtures/global_fallback.php'));
+        $project->openDocument('global_symbols', file_get_contents(__DIR__ . '/../../../../fixtures/global_symbols.php'));
+    }
+
+    public function testClassDoesNotFallback()
+    {
+        // class TestClass implements TestInterface
+        // Get references for TestClass
+        $result = $this->textDocument->references(new ReferenceContext, new TextDocumentIdentifier('global_symbols'), new Position(6, 9));
+        $this->assertEquals([], $result);
+    }
+
+    public function testFallsBackForConstants()
+    {
+        // const TEST_CONST = 123;
+        // Get references for TEST_CONST
+        $result = $this->textDocument->references(new ReferenceContext, new TextDocumentIdentifier('global_symbols'), new Position(4, 13));
+        $this->assertEquals([
+            [
+                'uri' => 'global_fallback',
+                'range' => [
+                    'start' => [
+                        'line' => 6,
+                        'character' => 5
+                    ],
+                    'end' => [
+                        'line' => 6,
+                        'character' => 15
+                    ]
+                ]
+            ]
+        ], json_decode(json_encode($result), true));
+    }
+
+    public function testFallsBackForFunctions()
+    {
+        // function test_function()
+        // Get references for test_function
+        $result = $this->textDocument->references(new ReferenceContext, new TextDocumentIdentifier('global_symbols'), new Position(33, 16));
+        $this->assertEquals([
+            [
+                'uri' => 'global_fallback',
+                'range' => [
+                    'start' => [
+                        'line' => 5,
+                        'character' => 0
+                    ],
+                    'end' => [
+                        'line' => 5,
+                        'character' => 13
+                    ]
+                ]
+            ]
+        ], json_decode(json_encode($result), true));
+    }
+}

tests/Server/TextDocument/References/GlobalTest.php

@@ -28,10 +28,6 @@ class GlobalTest extends TestCase
         $this->referencesUri = pathToUri(realpath(__DIR__ . '/../../../../fixtures/global_references.php'));
         $project->loadDocument($this->referencesUri);
         $project->loadDocument($this->symbolsUri);
-        // Load this to check that there are no conflicts
-        $project->loadDocument(pathToUri(realpath(__DIR__ . '/../../../../fixtures/symbols.php')));
-        $project->loadDocument(pathToUri(realpath(__DIR__ . '/../../../../fixtures/references.php')));
-        $project->loadDocument(pathToUri(realpath(__DIR__ . '/../../../../fixtures/use.php')));
     }
 
     public function testReferencesForClassLike()

tests/Server/TextDocument/References/NamespacedTest.php

@@ -31,9 +31,6 @@ class NamespacedTest extends TestCase
         $project->loadDocument($this->referencesUri);
         $project->loadDocument($this->symbolsUri);
         $project->loadDocument($this->useUri);
-        // Load this to check that there are no conflicts
-        $project->loadDocument(pathToUri(realpath(__DIR__ . '/../../../../fixtures/global_symbols.php')));
-        $project->loadDocument(pathToUri(realpath(__DIR__ . '/../../../../fixtures/global_references.php')));
     }
 
     public function testReferencesForClassLike()