diff --git a/composer.json b/composer.json index 24a9d32..5ef4623 100644 --- a/composer.json +++ b/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", diff --git a/fixtures/global_fallback.php b/fixtures/global_fallback.php new file mode 100644 index 0000000..90917bb --- /dev/null +++ b/fixtures/global_fallback.php @@ -0,0 +1,10 @@ +definitions = $definitions; - $this->references = array_fill_keys(array_keys($definitions), []); - } + public $references = []; 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; + } } diff --git a/src/PhpDocument.php b/src/PhpDocument.php index eaf50d6..9a716de 100644 --- a/src/PhpDocument.php +++ b/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; } diff --git a/src/Project.php b/src/Project.php index b416524..06b3a08 100644 --- a/src/Project.php +++ b/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) { - $this->references[$fqn][] = $document; + if (array_search($uri, $this->references[$fqn], true) === false) { + $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]); } /** diff --git a/tests/Server/TextDocument/Definition/GlobalFallbackTest.php b/tests/Server/TextDocument/Definition/GlobalFallbackTest.php new file mode 100644 index 0000000..d734d9e --- /dev/null +++ b/tests/Server/TextDocument/Definition/GlobalFallbackTest.php @@ -0,0 +1,74 @@ +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)); + } +} diff --git a/tests/Server/TextDocument/Definition/NamespacedTest.php b/tests/Server/TextDocument/Definition/NamespacedTest.php index a5124a2..1d5f1c9 100644 --- a/tests/Server/TextDocument/Definition/NamespacedTest.php +++ b/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() { diff --git a/tests/Server/TextDocument/References/GlobalFallbackTest.php b/tests/Server/TextDocument/References/GlobalFallbackTest.php new file mode 100644 index 0000000..a501613 --- /dev/null +++ b/tests/Server/TextDocument/References/GlobalFallbackTest.php @@ -0,0 +1,78 @@ +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)); + } +} diff --git a/tests/Server/TextDocument/References/GlobalTest.php b/tests/Server/TextDocument/References/GlobalTest.php index 940bb42..5e0e177 100644 --- a/tests/Server/TextDocument/References/GlobalTest.php +++ b/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() diff --git a/tests/Server/TextDocument/References/NamespacedTest.php b/tests/Server/TextDocument/References/NamespacedTest.php index 9c6f125..eed82ee 100644 --- a/tests/Server/TextDocument/References/NamespacedTest.php +++ b/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()