From 063c7f9ad234b1ff871d97a585f25f3dbf586d7d Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 8 Oct 2016 12:59:22 +0200 Subject: [PATCH 01/13] Move NodeVisitors to own namespace --- src/{ => NodeVisitors}/ColumnCalculator.php | 2 +- src/{ => NodeVisitors}/SymbolFinder.php | 2 +- src/PhpDocument.php | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) rename src/{ => NodeVisitors}/ColumnCalculator.php (96%) rename src/{ => NodeVisitors}/SymbolFinder.php (98%) diff --git a/src/ColumnCalculator.php b/src/NodeVisitors/ColumnCalculator.php similarity index 96% rename from src/ColumnCalculator.php rename to src/NodeVisitors/ColumnCalculator.php index 9050264..102c37c 100644 --- a/src/ColumnCalculator.php +++ b/src/NodeVisitors/ColumnCalculator.php @@ -1,7 +1,7 @@ Date: Sat, 8 Oct 2016 13:14:42 +0200 Subject: [PATCH 02/13] Add more symbols to symbol test * constants * static properties * static methods --- fixtures/symbols.php | 9 +++ src/NodeVisitors/SymbolFinder.php | 2 +- tests/Server/TextDocumentTest.php | 92 +++++++++++++++++++++++++++---- 3 files changed, 92 insertions(+), 11 deletions(-) diff --git a/fixtures/symbols.php b/fixtures/symbols.php index 03281c5..77dff07 100644 --- a/fixtures/symbols.php +++ b/fixtures/symbols.php @@ -2,10 +2,19 @@ namespace TestNamespace; +const TEST_CONST = 123; + class TestClass { + const TEST_CLASS_CONST = 123; + public static $staticTestProperty; public $testProperty; + public static function staticTestMethod() + { + + } + public function testMethod($testParameter) { $testVariable = 123; diff --git a/src/NodeVisitors/SymbolFinder.php b/src/NodeVisitors/SymbolFinder.php index 31e7c1c..c262043 100644 --- a/src/NodeVisitors/SymbolFinder.php +++ b/src/NodeVisitors/SymbolFinder.php @@ -59,7 +59,7 @@ class SymbolFinder extends NodeVisitorAbstract public function enterNode(Node $node) { $this->nodeStack[] = $node; - $containerName = end($this->nameStack); + $containerName = empty($this->nameStack) ? null : end($this->nameStack); // If we enter a named node, push its name onto name stack. // Else push the current name onto stack. diff --git a/tests/Server/TextDocumentTest.php b/tests/Server/TextDocumentTest.php index 746ded6..17d8031 100644 --- a/tests/Server/TextDocumentTest.php +++ b/tests/Server/TextDocumentTest.php @@ -44,6 +44,24 @@ class TextDocumentTest extends TestCase ], 'containerName' => null ], + [ + 'name' => 'TEST_CONST', + 'kind' => SymbolKind::CONSTANT, + 'location' => [ + 'uri' => 'whatever', + 'range' => [ + 'start' => [ + 'line' => 4, + 'character' => 6 + ], + 'end' => [ + 'line' => 4, + 'character' => 22 + ] + ] + ], + 'containerName' => 'TestNamespace' + ], [ 'name' => 'TestClass', 'kind' => SymbolKind::CLASS_, @@ -51,17 +69,53 @@ class TextDocumentTest extends TestCase 'uri' => 'whatever', 'range' => [ 'start' => [ - 'line' => 4, + 'line' => 6, 'character' => 0 ], 'end' => [ - 'line' => 12, + 'line' => 21, 'character' => 1 ] ] ], 'containerName' => 'TestNamespace' ], + [ + 'name' => 'TEST_CLASS_CONST', + 'kind' => SymbolKind::CONSTANT, + 'location' => [ + 'uri' => 'whatever', + 'range' => [ + 'start' => [ + 'line' => 8, + 'character' => 10 + ], + 'end' => [ + 'line' => 8, + 'character' => 32 + ] + ] + ], + 'containerName' => 'TestNamespace\\TestClass' + ], + [ + 'name' => 'staticTestProperty', + 'kind' => SymbolKind::PROPERTY, + 'location' => [ + 'uri' => 'whatever', + 'range' => [ + 'start' => [ + 'line' => 9, + 'character' => 18 + ], + 'end' => [ + 'line' => 9, + 'character' => 37 + ] + ] + ], + 'containerName' => 'TestNamespace\\TestClass' + ], [ 'name' => 'testProperty', 'kind' => SymbolKind::PROPERTY, @@ -69,17 +123,35 @@ class TextDocumentTest extends TestCase 'uri' => 'whatever', 'range' => [ 'start' => [ - 'line' => 6, + 'line' => 10, 'character' => 11 ], 'end' => [ - 'line' => 6, + 'line' => 10, 'character' => 24 ] ] ], 'containerName' => 'TestNamespace\\TestClass' ], + [ + 'name' => 'staticTestMethod', + 'kind' => SymbolKind::METHOD, + 'location' => [ + 'uri' => 'whatever', + 'range' => [ + 'start' => [ + 'line' => 12, + 'character' => 4 + ], + 'end' => [ + 'line' => 15, + 'character' => 5 + ] + ] + ], + 'containerName' => 'TestNamespace\\TestClass' + ], [ 'name' => 'testMethod', 'kind' => SymbolKind::METHOD, @@ -87,11 +159,11 @@ class TextDocumentTest extends TestCase 'uri' => 'whatever', 'range' => [ 'start' => [ - 'line' => 8, + 'line' => 17, 'character' => 4 ], 'end' => [ - 'line' => 11, + 'line' => 20, 'character' => 5 ] ] @@ -105,11 +177,11 @@ class TextDocumentTest extends TestCase 'uri' => 'whatever', 'range' => [ 'start' => [ - 'line' => 14, + 'line' => 23, 'character' => 0 ], 'end' => [ - 'line' => 17, + 'line' => 26, 'character' => 1 ] ] @@ -123,11 +195,11 @@ class TextDocumentTest extends TestCase 'uri' => 'whatever', 'range' => [ 'start' => [ - 'line' => 19, + 'line' => 28, 'character' => 0 ], 'end' => [ - 'line' => 22, + 'line' => 31, 'character' => 1 ] ] From 4786fe173ce690eb176f847e89b0456df7d44596 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 8 Oct 2016 13:22:34 +0200 Subject: [PATCH 03/13] Decorate all nodes with parent, sibling references --- src/NodeVisitors/ReferencesAdder.php | 41 ++++++++++++++++++++++++++++ src/PhpDocument.php | 18 +++++++++--- 2 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 src/NodeVisitors/ReferencesAdder.php diff --git a/src/NodeVisitors/ReferencesAdder.php b/src/NodeVisitors/ReferencesAdder.php new file mode 100644 index 0000000..0d62a80 --- /dev/null +++ b/src/NodeVisitors/ReferencesAdder.php @@ -0,0 +1,41 @@ +stack)) { + $node->setAttribute('parentNode', end($this->stack)); + } + if (isset($this->previous) && $this->previous->getAttribute('parentNode') === $node->getAttribute('parentNode')) { + $node->setAttribute('previousSibling', $this->previous); + $this->previous->setAttribute('nextSibling', $node); + } + $this->stack[] = $node; + } + + public function leaveNode(Node $node) + { + $this->previous = $node; + array_pop($this->stack); + } +} diff --git a/src/PhpDocument.php b/src/PhpDocument.php index e394bc2..ac46a85 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -4,7 +4,7 @@ declare(strict_types = 1); namespace LanguageServer; use LanguageServer\Protocol\{Diagnostic, DiagnosticSeverity, Range, Position, SymbolKind, TextEdit}; -use LanguageServer\NodeVisitors\{SymbolFinder, ColumnCalculator}; +use LanguageServer\NodeVisitors\{ReferencesAdder, SymbolFinder, ColumnCalculator}; use PhpParser\{Error, Comment, Node, ParserFactory, NodeTraverser, Lexer, Parser}; use PhpParser\PrettyPrinter\Standard as PrettyPrinter; use PhpParser\NodeVisitor\NameResolver; @@ -138,13 +138,23 @@ class PhpDocument // $stmts can be null in case of a fatal parsing error if ($stmts) { $traverser = new NodeTraverser; - $finder = new SymbolFinder($this->uri); + + // Resolve aliased names to FQNs $traverser->addVisitor(new NameResolver); + + // Add parentNode, previousSibling, nextSibling attributes + $traverser->addVisitor(new ReferencesAdder); + + // Add column attributes to nodes $traverser->addVisitor(new ColumnCalculator($this->content)); - $traverser->addVisitor($finder); + + // Collect all symbols + $symbolFinder = new SymbolFinder($this->uri); + $traverser->addVisitor($symbolFinder); + $traverser->traverse($stmts); - $this->symbols = $finder->symbols; + $this->symbols = $symbolFinder->symbols; } return $stmts; From 48c71e5bc170c5522b8848303b99ffc04216545e Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 8 Oct 2016 13:34:49 +0200 Subject: [PATCH 04/13] Add method to find out node at position Keep AST in memory --- src/NodeVisitors/NodeAtPositionFinder.php | 47 +++++++++++++++++++++++ src/PhpDocument.php | 31 +++++++++++---- tests/PhpDocumentTest.php | 15 +++++++- 3 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 src/NodeVisitors/NodeAtPositionFinder.php diff --git a/src/NodeVisitors/NodeAtPositionFinder.php b/src/NodeVisitors/NodeAtPositionFinder.php new file mode 100644 index 0000000..35de06f --- /dev/null +++ b/src/NodeVisitors/NodeAtPositionFinder.php @@ -0,0 +1,47 @@ +position = $position; + } + + public function leaveNode(Node $node) + { + if ( + !isset($this->node) + && $node->getAttribute('startLine') <= $this->position->line + 1 + && $node->getAttribute('endLine') >= $this->position->line + 1 + && $node->getAttribute('startColumn') <= $this->position->character + 1 + && $node->getAttribute('endColumn') >= $this->position->character + 1 + ) { + $this->node = $node; + } + } +} diff --git a/src/PhpDocument.php b/src/PhpDocument.php index ac46a85..8a264c6 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -4,7 +4,7 @@ declare(strict_types = 1); namespace LanguageServer; use LanguageServer\Protocol\{Diagnostic, DiagnosticSeverity, Range, Position, SymbolKind, TextEdit}; -use LanguageServer\NodeVisitors\{ReferencesAdder, SymbolFinder, ColumnCalculator}; +use LanguageServer\NodeVisitors\{NodeAtPositionFinder, ReferencesAdder, SymbolFinder, ColumnCalculator}; use PhpParser\{Error, Comment, Node, ParserFactory, NodeTraverser, Lexer, Parser}; use PhpParser\PrettyPrinter\Standard as PrettyPrinter; use PhpParser\NodeVisitor\NameResolver; @@ -104,7 +104,7 @@ class PhpDocument * Re-parses a source file, updates symbols, reports parsing errors * that may have occured as diagnostics and returns parsed nodes. * - * @return \PhpParser\Node[] + * @return void */ public function parse() { @@ -155,9 +155,9 @@ class PhpDocument $traverser->traverse($stmts); $this->symbols = $symbolFinder->symbols; - } - return $stmts; + $this->stmts = $stmts; + } } /** @@ -167,14 +167,13 @@ class PhpDocument */ public function getFormattedText() { - $stmts = $this->parse(); - if (empty($stmts)) { + if (empty($this->stmts)) { return []; } $prettyPrinter = new PrettyPrinter(); $edit = new TextEdit(); $edit->range = new Range(new Position(0, 0), new Position(PHP_INT_MAX, PHP_INT_MAX)); - $edit->newText = $prettyPrinter->prettyPrintFile($stmts); + $edit->newText = $prettyPrinter->prettyPrintFile($this->stmts); return [$edit]; } @@ -187,4 +186,22 @@ class PhpDocument { return $this->content; } + + /** + * Returns the node at a specified position + * + * @param Position $position + * @return Node|null + */ + public function getNodeAtPosition(Position $position) + { + if ($this->stmts === null) { + return null; + } + $traverser = new NodeTraverser; + $finder = new NodeAtPositionFinder($position); + $traverser->addVisitor($finder); + $traverser->traverse($this->stmts); + return $finder->node; + } } diff --git a/tests/PhpDocumentTest.php b/tests/PhpDocumentTest.php index 465748a..40a5dea 100644 --- a/tests/PhpDocumentTest.php +++ b/tests/PhpDocumentTest.php @@ -6,7 +6,9 @@ namespace LanguageServer\Tests\Server; use PHPUnit\Framework\TestCase; use LanguageServer\Tests\MockProtocolStream; use LanguageServer\{LanguageClient, Project}; -use LanguageServer\Protocol\SymbolKind; +use LanguageServer\NodeVisitors\NodeAtPositionFinder; +use LanguageServer\Protocol\{SymbolKind, Position}; +use PhpParser\Node; class PhpDocumentTest extends TestCase { @@ -23,7 +25,7 @@ class PhpDocumentTest extends TestCase public function testParsesVariableVariables() { $document = $this->project->getDocument('whatever'); - + $document->updateContent("getSymbols(); @@ -67,4 +69,13 @@ class PhpDocumentTest extends TestCase ] ], json_decode(json_encode($symbols), true)); } + + public function testGetNodeAtPosition() + { + $document = $this->project->getDocument('whatever'); + $document->updateContent("getNodeAtPosition(new Position(1, 13)); + $this->assertInstanceOf(Node\Name\FullyQualified::class, $node); + $this->assertEquals('SomeClass', (string)$node); + } } From 987308fc0a417bf8c2dcad0461da85c50e9110ae Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 8 Oct 2016 14:51:49 +0200 Subject: [PATCH 05/13] Refactor TextDocument tests into separate classes --- tests/Server/TextDocument/DidChangeTest.php | 38 +++++ .../DocumentSymbolTest.php} | 154 +++--------------- tests/Server/TextDocument/FormattingTest.php | 57 +++++++ tests/Server/TextDocument/ParseErrorsTest.php | 73 +++++++++ 4 files changed, 195 insertions(+), 127 deletions(-) create mode 100644 tests/Server/TextDocument/DidChangeTest.php rename tests/Server/{TextDocumentTest.php => TextDocument/DocumentSymbolTest.php} (53%) create mode 100644 tests/Server/TextDocument/FormattingTest.php create mode 100644 tests/Server/TextDocument/ParseErrorsTest.php diff --git a/tests/Server/TextDocument/DidChangeTest.php b/tests/Server/TextDocument/DidChangeTest.php new file mode 100644 index 0000000..081978f --- /dev/null +++ b/tests/Server/TextDocument/DidChangeTest.php @@ -0,0 +1,38 @@ +getDocument('whatever'); + $phpDocument->updateContent("range = new Range(new Position(0, 0), new Position(9999, 9999)); + $changeEvent->rangeLength = 9999; + $changeEvent->text = "didChange($identifier, [$changeEvent]); + + $this->assertEquals("getContent()); + } +} diff --git a/tests/Server/TextDocumentTest.php b/tests/Server/TextDocument/DocumentSymbolTest.php similarity index 53% rename from tests/Server/TextDocumentTest.php rename to tests/Server/TextDocument/DocumentSymbolTest.php index 17d8031..b46648b 100644 --- a/tests/Server/TextDocumentTest.php +++ b/tests/Server/TextDocument/DocumentSymbolTest.php @@ -1,36 +1,38 @@ uri = 'whatever'; - $textDocumentItem->languageId = 'php'; - $textDocumentItem->version = 1; - $textDocumentItem->text = file_get_contents(__DIR__ . '/../../fixtures/symbols.php'); - $textDocument->didOpen($textDocumentItem); + $this->textDocument = new Server\TextDocument($project, $client); + $project->getDocument('symbols')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/symbols.php')); + } + + public function test() + { // Request symbols - $result = $textDocument->documentSymbol(new TextDocumentIdentifier('whatever')); + $result = $this->textDocument->documentSymbol(new TextDocumentIdentifier('symbols')); $this->assertEquals([ [ 'name' => 'TestNamespace', 'kind' => SymbolKind::NAMESPACE, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 2, @@ -48,7 +50,7 @@ class TextDocumentTest extends TestCase 'name' => 'TEST_CONST', 'kind' => SymbolKind::CONSTANT, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 4, @@ -66,7 +68,7 @@ class TextDocumentTest extends TestCase 'name' => 'TestClass', 'kind' => SymbolKind::CLASS_, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 6, @@ -84,7 +86,7 @@ class TextDocumentTest extends TestCase 'name' => 'TEST_CLASS_CONST', 'kind' => SymbolKind::CONSTANT, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 8, @@ -102,7 +104,7 @@ class TextDocumentTest extends TestCase 'name' => 'staticTestProperty', 'kind' => SymbolKind::PROPERTY, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 9, @@ -120,7 +122,7 @@ class TextDocumentTest extends TestCase 'name' => 'testProperty', 'kind' => SymbolKind::PROPERTY, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 10, @@ -138,7 +140,7 @@ class TextDocumentTest extends TestCase 'name' => 'staticTestMethod', 'kind' => SymbolKind::METHOD, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 12, @@ -156,7 +158,7 @@ class TextDocumentTest extends TestCase 'name' => 'testMethod', 'kind' => SymbolKind::METHOD, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 17, @@ -174,7 +176,7 @@ class TextDocumentTest extends TestCase 'name' => 'TestTrait', 'kind' => SymbolKind::CLASS_, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 23, @@ -192,7 +194,7 @@ class TextDocumentTest extends TestCase 'name' => 'TestInterface', 'kind' => SymbolKind::INTERFACE, 'location' => [ - 'uri' => 'whatever', + 'uri' => 'symbols', 'range' => [ 'start' => [ 'line' => 28, @@ -208,106 +210,4 @@ class TextDocumentTest extends TestCase ] ], json_decode(json_encode($result), true)); } - - public function testParseErrorsArePublishedAsDiagnostics() - { - $args = null; - $client = new LanguageClient(new MockProtocolStream()); - $client->textDocument = new class($args) extends Client\TextDocument { - private $args; - public function __construct(&$args) - { - parent::__construct(new MockProtocolStream()); - $this->args = &$args; - } - public function publishDiagnostics(string $uri, array $diagnostics) - { - $this->args = func_get_args(); - } - }; - - $project = new Project($client); - - $textDocument = new Server\TextDocument($project, $client); - - // Trigger parsing of source - $textDocumentItem = new TextDocumentItem(); - $textDocumentItem->uri = 'whatever'; - $textDocumentItem->languageId = 'php'; - $textDocumentItem->version = 1; - $textDocumentItem->text = file_get_contents(__DIR__ . '/../../fixtures/invalid_file.php'); - $textDocument->didOpen($textDocumentItem); - $this->assertEquals([ - 'whatever', - [[ - 'range' => [ - 'start' => [ - 'line' => 2, - 'character' => 10 - ], - 'end' => [ - 'line' => 2, - 'character' => 15 - ] - ], - 'severity' => DiagnosticSeverity::ERROR, - 'code' => null, - 'source' => 'php', - 'message' => "Syntax error, unexpected T_CLASS, expecting T_STRING" - ]] - ], json_decode(json_encode($args), true)); - } - - public function testFormatting() - { - $client = new LanguageClient(new MockProtocolStream()); - $project = new Project($client); - $textDocument = new Server\TextDocument($project, $client); - - // Trigger parsing of source - $textDocumentItem = new TextDocumentItem(); - $textDocumentItem->uri = 'whatever'; - $textDocumentItem->languageId = 'php'; - $textDocumentItem->version = 1; - $textDocumentItem->text = file_get_contents(__DIR__ . '/../../fixtures/format.php'); - $textDocument->didOpen($textDocumentItem); - - // how code should look after formatting - $expected = file_get_contents(__DIR__ . '/../../fixtures/format_expected.php'); - // Request formatting - $result = $textDocument->formatting(new TextDocumentIdentifier('whatever'), new FormattingOptions()); - $this->assertEquals([0 => [ - 'range' => [ - 'start' => [ - 'line' => 0, - 'character' => 0 - ], - 'end' => [ - 'line' => PHP_INT_MAX, - 'character' => PHP_INT_MAX - ] - ], - 'newText' => $expected - ]], json_decode(json_encode($result), true)); - } - - public function testDidChange() - { - $client = new LanguageClient(new MockProtocolStream()); - $project = new Project($client); - $textDocument = new Server\TextDocument($project, $client); - - $phpDocument = $project->getDocument('whatever'); - $phpDocument->updateContent("range = new Range(new Position(0,0), new Position(9999,9999)); - $changeEvent->rangeLength = 9999; - $changeEvent->text = "didChange($identifier, [$changeEvent]); - - $this->assertEquals("getContent()); - } } diff --git a/tests/Server/TextDocument/FormattingTest.php b/tests/Server/TextDocument/FormattingTest.php new file mode 100644 index 0000000..2dcc46e --- /dev/null +++ b/tests/Server/TextDocument/FormattingTest.php @@ -0,0 +1,57 @@ +textDocument = new Server\TextDocument($project, $client); + } + + public function test() + { + $client = new LanguageClient(new MockProtocolStream()); + $project = new Project($client); + $textDocument = new Server\TextDocument($project, $client); + + // Trigger parsing of source + $textDocumentItem = new TextDocumentItem(); + $textDocumentItem->uri = 'whatever'; + $textDocumentItem->languageId = 'php'; + $textDocumentItem->version = 1; + $textDocumentItem->text = file_get_contents(__DIR__ . '/../../../fixtures/format.php'); + $textDocument->didOpen($textDocumentItem); + + // how code should look after formatting + $expected = file_get_contents(__DIR__ . '/../../../fixtures/format_expected.php'); + // Request formatting + $result = $textDocument->formatting(new TextDocumentIdentifier('whatever'), new FormattingOptions()); + $this->assertEquals([0 => [ + 'range' => [ + 'start' => [ + 'line' => 0, + 'character' => 0 + ], + 'end' => [ + 'line' => PHP_INT_MAX, + 'character' => PHP_INT_MAX + ] + ], + 'newText' => $expected + ]], json_decode(json_encode($result), true)); + } +} diff --git a/tests/Server/TextDocument/ParseErrorsTest.php b/tests/Server/TextDocument/ParseErrorsTest.php new file mode 100644 index 0000000..24d155c --- /dev/null +++ b/tests/Server/TextDocument/ParseErrorsTest.php @@ -0,0 +1,73 @@ +textDocument = new Server\TextDocument($project, $client); + } + + public function testParseErrorsArePublishedAsDiagnostics() + { + $args = null; + $client = new LanguageClient(new MockProtocolStream()); + $client->textDocument = new class($args) extends Client\TextDocument { + private $args; + public function __construct(&$args) + { + parent::__construct(new MockProtocolStream()); + $this->args = &$args; + } + public function publishDiagnostics(string $uri, array $diagnostics) + { + $this->args = func_get_args(); + } + }; + + $project = new Project($client); + + $textDocument = new Server\TextDocument($project, $client); + + // Trigger parsing of source + $textDocumentItem = new TextDocumentItem(); + $textDocumentItem->uri = 'whatever'; + $textDocumentItem->languageId = 'php'; + $textDocumentItem->version = 1; + $textDocumentItem->text = file_get_contents(__DIR__ . '/../../../fixtures/invalid_file.php'); + $textDocument->didOpen($textDocumentItem); + $this->assertEquals([ + 'whatever', + [[ + 'range' => [ + 'start' => [ + 'line' => 2, + 'character' => 10 + ], + 'end' => [ + 'line' => 2, + 'character' => 15 + ] + ], + 'severity' => DiagnosticSeverity::ERROR, + 'code' => null, + 'source' => 'php', + 'message' => "Syntax error, unexpected T_CLASS, expecting T_STRING" + ]] + ], json_decode(json_encode($args), true)); + } +} From fbdf1aa4146d5e19050512e29d2738c6294430b4 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 8 Oct 2016 14:53:13 +0200 Subject: [PATCH 06/13] Add ownerDocument attribute to nodes --- src/NodeVisitors/ReferencesAdder.php | 14 ++++++++++++++ src/PhpDocument.php | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/NodeVisitors/ReferencesAdder.php b/src/NodeVisitors/ReferencesAdder.php index 0d62a80..96059b3 100644 --- a/src/NodeVisitors/ReferencesAdder.php +++ b/src/NodeVisitors/ReferencesAdder.php @@ -21,8 +21,22 @@ class ReferencesAdder extends NodeVisitorAbstract */ private $previous; + /** + * @var mixed + */ + private $document; + + /** + * @param mixed $document The value for the ownerDocument attribute + */ + public function __construct($document = null) + { + $this->document = $document; + } + public function enterNode(Node $node) { + $node->setAttribute('ownerDocument', $this->document); if (!empty($this->stack)) { $node->setAttribute('parentNode', end($this->stack)); } diff --git a/src/PhpDocument.php b/src/PhpDocument.php index 8a264c6..b0f9ede 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -143,7 +143,7 @@ class PhpDocument $traverser->addVisitor(new NameResolver); // Add parentNode, previousSibling, nextSibling attributes - $traverser->addVisitor(new ReferencesAdder); + $traverser->addVisitor(new ReferencesAdder($this)); // Add column attributes to nodes $traverser->addVisitor(new ColumnCalculator($this->content)); From 827ab4c8425e3ca276b3449788b865ea38f694db Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 8 Oct 2016 17:14:52 +0200 Subject: [PATCH 07/13] Add Position::compare() and Range::includes() --- src/NodeVisitors/NodeAtPositionFinder.php | 14 ++++++-------- src/Protocol/Position.php | 23 +++++++++++++++++++++++ src/Protocol/Range.php | 11 +++++++++++ 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/NodeVisitors/NodeAtPositionFinder.php b/src/NodeVisitors/NodeAtPositionFinder.php index 35de06f..8f38eec 100644 --- a/src/NodeVisitors/NodeAtPositionFinder.php +++ b/src/NodeVisitors/NodeAtPositionFinder.php @@ -4,7 +4,7 @@ declare(strict_types = 1); namespace LanguageServer\NodeVisitors; use PhpParser\{NodeVisitorAbstract, Node}; -use LanguageServer\Protocol\Position; +use LanguageServer\Protocol\{Position, Range}; /** * Finds the Node at a specified position @@ -34,13 +34,11 @@ class NodeAtPositionFinder extends NodeVisitorAbstract public function leaveNode(Node $node) { - if ( - !isset($this->node) - && $node->getAttribute('startLine') <= $this->position->line + 1 - && $node->getAttribute('endLine') >= $this->position->line + 1 - && $node->getAttribute('startColumn') <= $this->position->character + 1 - && $node->getAttribute('endColumn') >= $this->position->character + 1 - ) { + $range = new Range( + new Position($node->getAttribute('startLine') - 1, $node->getAttribute('startColumn') - 1), + new Position($node->getAttribute('endLine') - 1, $node->getAttribute('endColumn') - 1) + ); + if (!isset($this->node) && $range->includes($this->position)) { $this->node = $node; } } diff --git a/src/Protocol/Position.php b/src/Protocol/Position.php index 7831268..01cff0b 100644 --- a/src/Protocol/Position.php +++ b/src/Protocol/Position.php @@ -26,4 +26,27 @@ class Position $this->line = $line; $this->character = $character; } + + /** + * Compares this position to another position + * Returns + * - 0 if the positions match + * - a negative number if $this is before $position + * - a positive number otherwise + * + * @param Position $position + * @return int + */ + public function compare(Position $position): int + { + if ($this->line === $position->line && $this->character === $position->character) { + return 0; + } + + if ($this->line !== $position->line) { + return $this->line - $position->line; + } + + return $this->character - $position->character; + } } diff --git a/src/Protocol/Range.php b/src/Protocol/Range.php index c4a59ff..56dba0c 100644 --- a/src/Protocol/Range.php +++ b/src/Protocol/Range.php @@ -26,4 +26,15 @@ class Range $this->start = $start; $this->end = $end; } + + /** + * Checks if a position is within the range + * + * @param Position $position + * @return bool + */ + public function includes(Position $position): bool + { + return $this->start->compare($position) <= 0 && $this->end->compare($position) >= 0; + } } From d4757e0a24f2da0bf5e8f6f5a45479f73e640094 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 8 Oct 2016 14:59:08 +0200 Subject: [PATCH 08/13] Add textDocument/definition support --- fixtures/references.php | 24 ++ fixtures/symbols.php | 23 +- fixtures/use.php | 6 + src/LanguageServer.php | 3 + src/NodeVisitors/DefinitionCollector.php | 72 ++++ src/NodeVisitors/NodeAtPositionFinder.php | 5 + src/PhpDocument.php | 218 ++++++++++- src/Project.php | 30 ++ src/Server/TextDocument.php | 31 +- tests/LanguageServerTest.php | 2 +- .../NodeVisitors/DefinitionCollectorTest.php | 47 +++ tests/Server/TextDocument/DefinitionTest.php | 346 ++++++++++++++++++ .../TextDocument/DocumentSymbolTest.php | 18 + 13 files changed, 820 insertions(+), 5 deletions(-) create mode 100644 fixtures/references.php create mode 100644 fixtures/use.php create mode 100644 src/NodeVisitors/DefinitionCollector.php create mode 100644 tests/NodeVisitors/DefinitionCollectorTest.php create mode 100644 tests/Server/TextDocument/DefinitionTest.php diff --git a/fixtures/references.php b/fixtures/references.php new file mode 100644 index 0000000..6cb9190 --- /dev/null +++ b/fixtures/references.php @@ -0,0 +1,24 @@ +testMethod(); +echo $obj->testProperty; +TestClass::staticTestMethod(); +echo TestClass::$staticTestProperty; +echo TestClass::TEST_CLASS_CONST; +test_function(); + +$var = 123; +echo $var; + +function whatever(TestClass $param): TestClass { + echo $param; +} + +$fn = function() use ($var) { + echo $var; +}; + +echo TEST_CONST; diff --git a/fixtures/symbols.php b/fixtures/symbols.php index 77dff07..26379e4 100644 --- a/fixtures/symbols.php +++ b/fixtures/symbols.php @@ -4,7 +4,7 @@ namespace TestNamespace; const TEST_CONST = 123; -class TestClass +class TestClass implements TestInterface { const TEST_CLASS_CONST = 123; public static $staticTestProperty; @@ -30,3 +30,24 @@ interface TestInterface { } + +function test_function() +{ + +} + +new class { + const TEST_CLASS_CONST = 123; + public static $staticTestProperty; + public $testProperty; + + public static function staticTestMethod() + { + + } + + public function testMethod($testParameter) + { + $testVariable = 123; + } +}; diff --git a/fixtures/use.php b/fixtures/use.php new file mode 100644 index 0000000..28e3aec --- /dev/null +++ b/fixtures/use.php @@ -0,0 +1,6 @@ +workspaceSymbolProvider = true; // Support "Format Code" $serverCapabilities->documentFormattingProvider = true; + // Support "Go to definition" + $serverCapabilities->definitionProvider = true; + return new InitializeResult($serverCapabilities); } diff --git a/src/NodeVisitors/DefinitionCollector.php b/src/NodeVisitors/DefinitionCollector.php new file mode 100644 index 0000000..8ad80fd --- /dev/null +++ b/src/NodeVisitors/DefinitionCollector.php @@ -0,0 +1,72 @@ +name)) { + // Class, interface or trait declaration + $this->definitions[(string)$node->namespacedName] = $node; + } else if ($node instanceof Node\Stmt\Function_) { + // Function: use functioName() as the name + $name = (string)$node->namespacedName . '()'; + $this->definitions[$name] = $node; + } else if ($node instanceof Node\Stmt\ClassMethod) { + // Class method: use ClassName::methodName() as name + $class = $node->getAttribute('parentNode'); + if (!isset($class->name)) { + // Ignore anonymous classes + return; + } + $name = (string)$class->namespacedName . '::' . (string)$node->name . '()'; + $this->definitions[$name] = $node; + } else if ($node instanceof Node\Stmt\PropertyProperty) { + // Property: use ClassName::propertyName as name + $class = $node->getAttribute('parentNode')->getAttribute('parentNode'); + if (!isset($class->name)) { + // Ignore anonymous classes + return; + } + $name = (string)$class->namespacedName . '::' . (string)$node->name; + $this->definitions[$name] = $node; + } else if ($node instanceof Node\Const_) { + $parent = $node->getAttribute('parentNode'); + if ($parent instanceof Node\Stmt\Const_) { + // Basic constant: use CONSTANT_NAME as name + $name = (string)$node->namespacedName; + } else if ($parent instanceof Node\Stmt\ClassConst) { + // Class constant: use ClassName::CONSTANT_NAME as name + $class = $parent->getAttribute('parentNode'); + if (!isset($class->name) || $class->name instanceof Node\Expr) { + return; + } + $name = (string)$class->namespacedName . '::' . $node->name; + } + $this->definitions[$name] = $node; + } + } +} diff --git a/src/NodeVisitors/NodeAtPositionFinder.php b/src/NodeVisitors/NodeAtPositionFinder.php index 8f38eec..f68e39a 100644 --- a/src/NodeVisitors/NodeAtPositionFinder.php +++ b/src/NodeVisitors/NodeAtPositionFinder.php @@ -38,6 +38,11 @@ class NodeAtPositionFinder extends NodeVisitorAbstract new Position($node->getAttribute('startLine') - 1, $node->getAttribute('startColumn') - 1), new Position($node->getAttribute('endLine') - 1, $node->getAttribute('endColumn') - 1) ); + // Workaround for https://github.com/nikic/PHP-Parser/issues/311 + $parent = $node->getAttribute('parentNode'); + if (isset($parent) && $parent instanceof Node\Stmt\GroupUse && $parent->prefix === $node) { + return; + } if (!isset($this->node) && $range->includes($this->position)) { $this->node = $node; } diff --git a/src/PhpDocument.php b/src/PhpDocument.php index b0f9ede..1c37e87 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -4,7 +4,7 @@ declare(strict_types = 1); namespace LanguageServer; use LanguageServer\Protocol\{Diagnostic, DiagnosticSeverity, Range, Position, SymbolKind, TextEdit}; -use LanguageServer\NodeVisitors\{NodeAtPositionFinder, ReferencesAdder, SymbolFinder, ColumnCalculator}; +use LanguageServer\NodeVisitors\{NodeAtPositionFinder, ReferencesAdder, DefinitionCollector, SymbolFinder, ColumnCalculator}; use PhpParser\{Error, Comment, Node, ParserFactory, NodeTraverser, Lexer, Parser}; use PhpParser\PrettyPrinter\Standard as PrettyPrinter; use PhpParser\NodeVisitor\NameResolver; @@ -23,7 +23,9 @@ class PhpDocument * * @var Project */ - private $project; + public $project; + // for whatever reason I get "cannot access private property" error if $project is not public + // https://github.com/felixfbecker/php-language-server/pull/49#issuecomment-252427359 /** * The PHPParser instance @@ -46,6 +48,28 @@ class PhpDocument */ private $content; + /** + * The AST of the document + * + * @var Node[] + */ + private $stmts = []; + + /** + * Map from fully qualified name (FQN) to Node + * Examples of fully qualified names: + * - testFunction() + * - TestNamespace\TestClass + * - TestNamespace\TestClass::TEST_CONSTANT + * - TestNamespace\TestClass::staticTestProperty + * - TestNamespace\TestClass::testProperty + * - TestNamespace\TestClass::staticTestMethod() + * - TestNamespace\TestClass::testMethod() + * + * @var Node[] + */ + private $definitions = []; + /** * @var SymbolInformation[] */ @@ -149,13 +173,24 @@ class PhpDocument $traverser->addVisitor(new ColumnCalculator($this->content)); // Collect all symbols + // TODO: use DefinitionCollector for this $symbolFinder = new SymbolFinder($this->uri); $traverser->addVisitor($symbolFinder); + // Collect all definitions + $definitionCollector = new DefinitionCollector; + $traverser->addVisitor($definitionCollector); + $traverser->traverse($stmts); $this->symbols = $symbolFinder->symbols; + $this->definitions = $definitionCollector->definitions; + // Register this document on the project for all the symbols defined in it + foreach ($definitionCollector->definitions as $fqn => $node) { + $this->project->addDefinitionDocument($fqn, $this); + } + $this->stmts = $stmts; } } @@ -187,6 +222,16 @@ class PhpDocument return $this->content; } + /** + * Returns the URI of the document + * + * @return string + */ + public function getUri(): string + { + return $this->uri; + } + /** * Returns the node at a specified position * @@ -204,4 +249,173 @@ class PhpDocument $traverser->traverse($this->stmts); return $finder->node; } + + /** + * Returns the definition node for a fully qualified name + * + * @param string $fqn + * @return Node|null + */ + public function getDefinitionByFqn(string $fqn) + { + return $this->definitions[$fqn] ?? null; + } + + /** + * Returns the definition node for any node + * The definition node MAY be in another document, check the ownerDocument attribute + * + * @param Node $node + * @return Node|null + */ + public function getDefinitionByNode(Node $node) + { + if ($node instanceof Node\Name) { + $nameNode = $node; + $node = $node->getAttribute('parentNode'); + } + // Variables always stay in the boundary of the file and need to be searched inside their function scope + // by traversing the AST + if ($node instanceof Node\Expr\Variable) { + return $this->getVariableDefinition($node); + } + + if ( + ($node instanceof Node\Stmt\ClassLike + || $node instanceof Node\Param + || $node instanceof Node\Stmt\Function_) + && isset($nameNode) + ) { + // For extends, implements and type hints use the name directly + $name = (string)$nameNode; + } else if ($node instanceof Node\Stmt\UseUse) { + $name = (string)$node->name; + $parent = $node->getAttribute('parentNode'); + if ($parent instanceof Node\Stmt\GroupUse) { + $name = $parent->prefix . '\\' . $name; + } + } else if ($node instanceof Node\Expr\New_) { + if (!($node->class instanceof Node\Name)) { + // Cannot get definition of dynamic calls + return null; + } + $name = (string)$node->class; + } else if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\PropertyFetch) { + if ($node->name instanceof Node\Expr || !($node->var instanceof Node\Expr\Variable)) { + // Cannot get definition of dynamic calls + return null; + } + // Need to resolve variable to a class + $varDef = $this->getVariableDefinition($node->var); + if (!isset($varDef)) { + return null; + } + if ($varDef instanceof Node\Param) { + if (!isset($varDef->type)) { + // Cannot resolve to class without a type hint + // TODO: parse docblock + return null; + } + $name = (string)$varDef->type; + } else if ($varDef instanceof Node\Expr\Assign) { + if ($varDef->expr instanceof Node\Expr\New_) { + if (!($varDef->expr->class instanceof Node\Name)) { + // Cannot get definition of dynamic calls + return null; + } + $name = (string)$varDef->expr->class; + } else { + return null; + } + } else { + return null; + } + $name .= '::' . (string)$node->name; + } else if ($node instanceof Node\Expr\FuncCall) { + if ($node->name instanceof Node\Expr) { + return null; + } + $name = (string)$node->name; + } else if ($node instanceof Node\Expr\ConstFetch) { + $name = (string)$node->name; + } else if ( + $node instanceof Node\Expr\ClassConstFetch + || $node instanceof Node\Expr\StaticPropertyFetch + || $node instanceof Node\Expr\StaticCall + ) { + if ($node->class instanceof Node\Expr || $node->name instanceof Node\Expr) { + // Cannot get definition of dynamic names + return null; + } + $name = (string)$node->class . '::' . $node->name; + } + if ( + $node instanceof Node\Expr\MethodCall + || $node instanceof Node\Expr\FuncCall + || $node instanceof Node\Expr\StaticCall + ) { + $name .= '()'; + } + if (!isset($name)) { + return null; + } + // Search for the document where the class, interface, trait, function, method or property is defined + $document = $this->project->getDefinitionDocument($name); + if (!$document && $node instanceof Node\Expr\FuncCall) { + // Find and try with namespace + // Namespaces aren't added automatically by NameResolver because PHP falls back to global functions + $n = $node; + while (isset($n)) { + $n = $n->getAttribute('parentNode'); + if ($n instanceof Node\Stmt\Namespace_) { + $name = (string)$n->name . '\\' . $name; + $document = $this->project->getDefinitionDocument($name); + break; + } + } + } + if (!isset($document)) { + return null; + } + return $document->getDefinitionByFqn($name); + } + + /** + * Returns the assignment or parameter node where a variable was defined + * + * @param Node\Expr\Variable $n The variable access + * @return Node\Expr\Assign|Node\Param|Node\Expr\ClosureUse|null + */ + public function getVariableDefinition(Node\Expr\Variable $var) + { + $n = $var; + // Traverse the AST up + while (isset($n) && $n = $n->getAttribute('parentNode')) { + // If a function is met, check the parameters and use statements + if ($n instanceof Node\FunctionLike) { + foreach ($n->getParams() as $param) { + if ($param->name === $var->name) { + return $param; + } + } + // If it is a closure, also check use statements + if ($n instanceof Node\Expr\Closure) { + foreach ($n->uses as $use) { + if ($use->var === $var->name) { + return $use; + } + } + } + break; + } + // Check each previous sibling node for a variable assignment to that variable + while ($n->getAttribute('previousSibling') && $n = $n->getAttribute('previousSibling')) { + if ($n instanceof Node\Expr\Assign && $n->var->name === $var->name) { + return $n; + } + } + } + // Return null if nothing was found + return null; + } } diff --git a/src/Project.php b/src/Project.php index adf37f6..7dd3596 100644 --- a/src/Project.php +++ b/src/Project.php @@ -17,6 +17,14 @@ class Project */ private $documents; + /** + * An associative array [string => PhpDocument] + * that maps fully qualified symbol names to loaded PhpDocuments + * + * @var PhpDocument[] + */ + private $definitions; + /** * Instance of the PHP parser * @@ -54,6 +62,28 @@ class Project return $this->documents[$uri]; } + /** + * Adds a document as the container for a specific symbol + * + * @param string $fqn The fully qualified name of the symbol + * @return void + */ + public function addDefinitionDocument(string $fqn, PhpDocument $document) + { + $this->definitions[$fqn] = $document; + } + + /** + * Returns the document where a symbol is defined + * + * @param string $fqn The fully qualified name of the symbol + * @return PhpDocument|null + */ + public function getDefinitionDocument(string $fqn) + { + return $this->definitions[$fqn] ?? null; + } + /** * Finds symbols in all documents, filtered by query parameter. * diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index a6541c1..b13abd8 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -13,7 +13,8 @@ use LanguageServer\Protocol\{ Range, Position, FormattingOptions, - TextEdit + TextEdit, + Location }; /** @@ -88,4 +89,32 @@ class TextDocument { return $this->project->getDocument($textDocument->uri)->getFormattedText(); } + + /** + * The goto definition request is sent from the client to the server to resolve the definition location of a symbol + * at a given text document position. + * + * @param TextDocumentIdentifier $textDocument The text document + * @param Position $position The position inside the text document + * @return Location|Location[]|null + */ + public function definition(TextDocumentIdentifier $textDocument, Position $position) + { + $document = $this->project->getDocument($textDocument->uri); + $node = $document->getNodeAtPosition($position); + if ($node === null) { + return null; + } + $def = $document->getDefinitionByNode($node); + if ($def === null) { + return null; + } + return new Location( + $def->getAttribute('ownerDocument')->getUri(), + new Range( + new Position($def->getAttribute('startLine') - 1, $def->getAttribute('startColumn') - 1), + new Position($def->getAttribute('endLine') - 1, $def->getAttribute('endColumn') - 1) + ) + ); + } } diff --git a/tests/LanguageServerTest.php b/tests/LanguageServerTest.php index 7d82c23..521d602 100644 --- a/tests/LanguageServerTest.php +++ b/tests/LanguageServerTest.php @@ -34,7 +34,7 @@ class LanguageServerTest extends TestCase 'hoverProvider' => null, 'completionProvider' => null, 'signatureHelpProvider' => null, - 'definitionProvider' => null, + 'definitionProvider' => true, 'referencesProvider' => null, 'documentHighlightProvider' => null, 'workspaceSymbolProvider' => true, diff --git a/tests/NodeVisitors/DefinitionCollectorTest.php b/tests/NodeVisitors/DefinitionCollectorTest.php new file mode 100644 index 0000000..d22bd79 --- /dev/null +++ b/tests/NodeVisitors/DefinitionCollectorTest.php @@ -0,0 +1,47 @@ +addVisitor(new NameResolver); + $traverser->addVisitor(new ReferencesAdder); + $definitionCollector = new DefinitionCollector; + $traverser->addVisitor($definitionCollector); + $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); + $stmts = $parser->parse(file_get_contents(__DIR__ . '/../../fixtures/symbols.php')); + $traverser->traverse($stmts); + $defs = $definitionCollector->definitions; + $this->assertEquals([ + 'TestNamespace\\TEST_CONST', + 'TestNamespace\\TestClass', + 'TestNamespace\\TestClass::TEST_CLASS_CONST', + 'TestNamespace\\TestClass::staticTestProperty', + 'TestNamespace\\TestClass::testProperty', + 'TestNamespace\\TestClass::staticTestMethod()', + 'TestNamespace\\TestClass::testMethod()', + 'TestNamespace\\TestTrait', + 'TestNamespace\\TestInterface', + 'TestNamespace\\test_function()' + ], array_keys($defs)); + $this->assertInstanceOf(Node\Const_::class, $defs['TestNamespace\\TEST_CONST']); + $this->assertInstanceOf(Node\Stmt\Class_::class, $defs['TestNamespace\\TestClass']); + $this->assertInstanceOf(Node\Const_::class, $defs['TestNamespace\\TestClass::TEST_CLASS_CONST']); + $this->assertInstanceOf(Node\Stmt\PropertyProperty::class, $defs['TestNamespace\\TestClass::staticTestProperty']); + $this->assertInstanceOf(Node\Stmt\PropertyProperty::class, $defs['TestNamespace\\TestClass::testProperty']); + $this->assertInstanceOf(Node\Stmt\ClassMethod::class, $defs['TestNamespace\\TestClass::staticTestMethod()']); + $this->assertInstanceOf(Node\Stmt\ClassMethod::class, $defs['TestNamespace\\TestClass::testMethod()']); + $this->assertInstanceOf(Node\Stmt\Trait_::class, $defs['TestNamespace\\TestTrait']); + $this->assertInstanceOf(Node\Stmt\Interface_::class, $defs['TestNamespace\\TestInterface']); + $this->assertInstanceOf(Node\Stmt\Function_::class, $defs['TestNamespace\\test_function()']); + } +} diff --git a/tests/Server/TextDocument/DefinitionTest.php b/tests/Server/TextDocument/DefinitionTest.php new file mode 100644 index 0000000..eca8722 --- /dev/null +++ b/tests/Server/TextDocument/DefinitionTest.php @@ -0,0 +1,346 @@ +textDocument = new Server\TextDocument($project, $client); + $project->getDocument('references')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/references.php')); + $project->getDocument('symbols')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/symbols.php')); + $project->getDocument('use')->updateContent(file_get_contents(__DIR__ . '/../../../fixtures/use.php')); + } + + public function testDefinitionForClassLike() + { + // $obj = new TestClass(); + // Get definition for TestClass + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(4, 16)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 6, + 'character' => 0 + ], + 'end' => [ + 'line' => 21, + 'character' => 0 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForClassLikeUseStatement() + { + // use TestNamespace\TestClass; + // Get definition for TestClass + $result = $this->textDocument->definition(new TextDocumentIdentifier('use'), new Position(4, 22)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 6, + 'character' => 0 + ], + 'end' => [ + 'line' => 21, + 'character' => 0 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForClassLikeGroupUseStatement() + { + // use TestNamespace\{TestTrait, TestInterface}; + // Get definition for TestInterface + $result = $this->textDocument->definition(new TextDocumentIdentifier('use'), new Position(5, 37)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 28, + 'character' => 0 + ], + 'end' => [ + 'line' => 31, + 'character' => 0 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForImplements() + { + // class TestClass implements TestInterface + // Get definition for TestInterface + $result = $this->textDocument->definition(new TextDocumentIdentifier('symbols'), new Position(6, 33)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 28, + 'character' => 0 + ], + 'end' => [ + 'line' => 31, + 'character' => 0 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForClassConstants() + { + // echo TestClass::TEST_CLASS_CONST; + // Get definition for TEST_CLASS_CONST + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(9, 21)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 8, + 'character' => 10 + ], + 'end' => [ + 'line' => 8, + 'character' => 31 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForConstants() + { + // echo TEST_CONST; + // Get definition for TEST_CONST + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(23, 9)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 4, + 'character' => 6 + ], + 'end' => [ + 'line' => 4, + 'character' => 21 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForStaticMethods() + { + // TestClass::staticTestMethod(); + // Get definition for staticTestMethod + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(7, 20)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 12, + 'character' => 4 + ], + 'end' => [ + 'line' => 15, + 'character' => 4 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForStaticProperties() + { + // echo TestClass::$staticTestProperty; + // Get definition for staticTestProperty + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(8, 25)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 9, + 'character' => 18 + ], + 'end' => [ + 'line' => 9, + 'character' => 36 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForMethods() + { + // $obj->testMethod(); + // Get definition for testMethod + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(5, 11)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 17, + 'character' => 4 + ], + 'end' => [ + 'line' => 20, + 'character' => 4 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForProperties() + { + // echo $obj->testProperty; + // Get definition for testProperty + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(6, 18)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 10, + 'character' => 11 + ], + 'end' => [ + 'line' => 10, + 'character' => 23 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForVariables() + { + // echo $var; + // Get definition for $var + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(13, 7)); + $this->assertEquals([ + 'uri' => 'references', + 'range' => [ + 'start' => [ + 'line' => 12, + 'character' => 0 + ], + 'end' => [ + 'line' => 12, + 'character' => 9 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForParamTypeHints() + { + // function whatever(TestClass $param) { + // Get definition for TestClass + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(15, 23)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 6, + 'character' => 0 + ], + 'end' => [ + 'line' => 21, + 'character' => 0 + ] + ] + ], json_decode(json_encode($result), true)); + } + public function testDefinitionForReturnTypeHints() + { + // function whatever(TestClass $param) { + // Get definition for TestClass + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(15, 42)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 6, + 'character' => 0 + ], + 'end' => [ + 'line' => 21, + 'character' => 0 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForParams() + { + // echo $param; + // Get definition for $param + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(16, 13)); + $this->assertEquals([ + 'uri' => 'references', + 'range' => [ + 'start' => [ + 'line' => 15, + 'character' => 18 + ], + 'end' => [ + 'line' => 15, + 'character' => 33 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForUsedVariables() + { + // echo $var; + // Get definition for $var + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(20, 11)); + $this->assertEquals([ + 'uri' => 'references', + 'range' => [ + 'start' => [ + 'line' => 19, + 'character' => 22 + ], + 'end' => [ + 'line' => 19, + 'character' => 25 + ] + ] + ], json_decode(json_encode($result), true)); + } + + public function testDefinitionForFunctions() + { + // test_function(); + // Get definition for test_function + $result = $this->textDocument->definition(new TextDocumentIdentifier('references'), new Position(10, 4)); + $this->assertEquals([ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 33, + 'character' => 0 + ], + 'end' => [ + 'line' => 36, + 'character' => 0 + ] + ] + ], json_decode(json_encode($result), true)); + } +} diff --git a/tests/Server/TextDocument/DocumentSymbolTest.php b/tests/Server/TextDocument/DocumentSymbolTest.php index b46648b..307a1cb 100644 --- a/tests/Server/TextDocument/DocumentSymbolTest.php +++ b/tests/Server/TextDocument/DocumentSymbolTest.php @@ -207,6 +207,24 @@ class DocumentSymbolTest extends TestCase ] ], 'containerName' => 'TestNamespace' + ], + [ + 'name' => 'test_function', + 'kind' => SymbolKind::FUNCTION, + 'location' => [ + 'uri' => 'symbols', + 'range' => [ + 'start' => [ + 'line' => 33, + 'character' => 0 + ], + 'end' => [ + 'line' => 36, + 'character' => 1 + ] + ] + ], + 'containerName' => 'TestNamespace' ] ], json_decode(json_encode($result), true)); } From 6be53ad658d5b2f1893429b995ea3ea56a6b41a0 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 8 Oct 2016 19:08:44 +0200 Subject: [PATCH 09/13] Use DefinitionCollector for symbol requests --- src/NodeVisitors/SymbolFinder.php | 121 ------------------ src/PhpDocument.php | 58 ++++++--- src/Project.php | 5 +- src/Server/TextDocument.php | 2 +- src/Server/Workspace.php | 2 +- tests/PhpDocumentTest.php | 39 +----- tests/ProjectTest.php | 3 +- .../TextDocument/DocumentSymbolTest.php | 18 --- 8 files changed, 49 insertions(+), 199 deletions(-) delete mode 100644 src/NodeVisitors/SymbolFinder.php diff --git a/src/NodeVisitors/SymbolFinder.php b/src/NodeVisitors/SymbolFinder.php deleted file mode 100644 index c262043..0000000 --- a/src/NodeVisitors/SymbolFinder.php +++ /dev/null @@ -1,121 +0,0 @@ - SymbolKind::CLASS_, - Node\Stmt\Trait_::class => SymbolKind::CLASS_, - Node\Stmt\Interface_::class => SymbolKind::INTERFACE, - Node\Stmt\Namespace_::class => SymbolKind::NAMESPACE, - Node\Stmt\Function_::class => SymbolKind::FUNCTION, - Node\Stmt\ClassMethod::class => SymbolKind::METHOD, - Node\Stmt\PropertyProperty::class => SymbolKind::PROPERTY, - Node\Const_::class => SymbolKind::CONSTANT, - Node\Expr\Variable::class => SymbolKind::VARIABLE - ]; - - /** - * @var LanguageServer\Protocol\SymbolInformation[] - */ - public $symbols = []; - - /** - * @var string - */ - private $uri; - - /** - * @var string - */ - private $containerName; - - /** - * @var array - */ - private $nameStack = []; - - /** - * @var array - */ - private $nodeStack = []; - - /** - * @var int - */ - private $functionCount = 0; - - public function __construct(string $uri) - { - $this->uri = $uri; - } - - public function enterNode(Node $node) - { - $this->nodeStack[] = $node; - $containerName = empty($this->nameStack) ? null : end($this->nameStack); - - // If we enter a named node, push its name onto name stack. - // Else push the current name onto stack. - if (!empty($node->name) && (is_string($node->name) || method_exists($node->name, '__toString')) && !empty((string)$node->name)) { - if (empty($containerName)) { - $this->nameStack[] = (string)$node->name; - } else if ($node instanceof Node\Stmt\ClassMethod) { - $this->nameStack[] = $containerName . '::' . (string)$node->name; - } else { - $this->nameStack[] = $containerName . '\\' . (string)$node->name; - } - } else { - $this->nameStack[] = $containerName; - // We are not interested in unnamed nodes, return - return; - } - - $class = get_class($node); - if (!isset(self::NODE_SYMBOL_KIND_MAP[$class])) { - return; - } - - // if we enter a method or function, increase the function counter - if ($node instanceof Node\Stmt\Function_ || $node instanceof Node\Stmt\ClassMethod) { - $this->functionCount++; - } - - $kind = self::NODE_SYMBOL_KIND_MAP[$class]; - - // exclude non-global variable symbols. - if ($kind === SymbolKind::VARIABLE && $this->functionCount > 0) { - return; - } - - $symbol = new SymbolInformation(); - $symbol->kind = $kind; - $symbol->name = (string)$node->name; - $symbol->location = new Location( - $this->uri, - new Range( - new Position($node->getAttribute('startLine') - 1, $node->getAttribute('startColumn') - 1), - new Position($node->getAttribute('endLine') - 1, $node->getAttribute('endColumn')) - ) - ); - $symbol->containerName = $containerName; - $this->symbols[] = $symbol; - } - - public function leaveNode(Node $node) - { - array_pop($this->nodeStack); - array_pop($this->nameStack); - - // if we leave a method or function, decrease the function counter - if ($node instanceof Node\Stmt\Function_ || $node instanceof Node\Stmt\ClassMethod) { - $this->functionCount--; - } - } -} diff --git a/src/PhpDocument.php b/src/PhpDocument.php index 1c37e87..eb901b5 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -3,8 +3,8 @@ declare(strict_types = 1); namespace LanguageServer; -use LanguageServer\Protocol\{Diagnostic, DiagnosticSeverity, Range, Position, SymbolKind, TextEdit}; -use LanguageServer\NodeVisitors\{NodeAtPositionFinder, ReferencesAdder, DefinitionCollector, SymbolFinder, ColumnCalculator}; +use LanguageServer\Protocol\{Diagnostic, DiagnosticSeverity, Range, Position, SymbolInformation, SymbolKind, TextEdit, Location}; +use LanguageServer\NodeVisitors\{NodeAtPositionFinder, ReferencesAdder, DefinitionCollector, ColumnCalculator}; use PhpParser\{Error, Comment, Node, ParserFactory, NodeTraverser, Lexer, Parser}; use PhpParser\PrettyPrinter\Standard as PrettyPrinter; use PhpParser\NodeVisitor\NameResolver; @@ -70,11 +70,6 @@ class PhpDocument */ private $definitions = []; - /** - * @var SymbolInformation[] - */ - private $symbols = []; - /** * @param string $uri The URI of the document * @param Project $project The Project this document belongs to (to register definitions etc) @@ -92,22 +87,56 @@ class PhpDocument /** * Returns all symbols in this document. * - * @return SymbolInformation[] + * @return SymbolInformation[]|null */ public function getSymbols() { - return $this->symbols; + if (!isset($this->definitions)) { + return null; + } + $nodeSymbolKindMap = [ + Node\Stmt\Class_::class => SymbolKind::CLASS_, + Node\Stmt\Trait_::class => SymbolKind::CLASS_, + Node\Stmt\Interface_::class => SymbolKind::INTERFACE, + Node\Stmt\Namespace_::class => SymbolKind::NAMESPACE, + Node\Stmt\Function_::class => SymbolKind::FUNCTION, + Node\Stmt\ClassMethod::class => SymbolKind::METHOD, + Node\Stmt\PropertyProperty::class => SymbolKind::PROPERTY, + Node\Const_::class => SymbolKind::CONSTANT + ]; + $symbols = []; + foreach ($this->definitions as $fqn => $node) { + $symbol = new SymbolInformation(); + $symbol->kind = $nodeSymbolKindMap[get_class($node)]; + $symbol->name = (string)$node->name; + $symbol->location = new Location( + $this->getUri(), + new Range( + new Position($node->getAttribute('startLine') - 1, $node->getAttribute('startColumn') - 1), + new Position($node->getAttribute('endLine') - 1, $node->getAttribute('endColumn')) + ) + ); + $parts = preg_split('/(::|\\\\)/', $fqn); + array_pop($parts); + $symbol->containerName = implode('\\', $parts); + $symbols[] = $symbol; + } + return $symbols; } /** * Returns symbols in this document filtered by query string. * * @param string $query The search query - * @return SymbolInformation[] + * @return SymbolInformation[]|null */ public function findSymbols(string $query) { - return array_filter($this->symbols, function($symbol) use(&$query) { + $symbols = $this->getSymbols(); + if ($symbols === null) { + return null; + } + return array_filter($symbols, function($symbol) use ($query) { return stripos($symbol->name, $query) !== false; }); } @@ -172,19 +201,12 @@ class PhpDocument // Add column attributes to nodes $traverser->addVisitor(new ColumnCalculator($this->content)); - // Collect all symbols - // TODO: use DefinitionCollector for this - $symbolFinder = new SymbolFinder($this->uri); - $traverser->addVisitor($symbolFinder); - // Collect all definitions $definitionCollector = new DefinitionCollector; $traverser->addVisitor($definitionCollector); $traverser->traverse($stmts); - $this->symbols = $symbolFinder->symbols; - $this->definitions = $definitionCollector->definitions; // Register this document on the project for all the symbols defined in it foreach ($definitionCollector->definitions as $fqn => $node) { diff --git a/src/Project.php b/src/Project.php index 7dd3596..ff65d98 100644 --- a/src/Project.php +++ b/src/Project.php @@ -94,7 +94,10 @@ class Project { $queryResult = []; foreach ($this->documents as $uri => $document) { - $queryResult = array_merge($queryResult, $document->findSymbols($query)); + $documentQueryResult = $document->findSymbols($query); + if ($documentQueryResult !== null) { + $queryResult = array_merge($queryResult, $documentQueryResult); + } } return $queryResult; } diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index b13abd8..aaf3c25 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -3,7 +3,7 @@ declare(strict_types = 1); namespace LanguageServer\Server; -use LanguageServer\{LanguageClient, ColumnCalculator, SymbolFinder, Project}; +use LanguageServer\{LanguageClient, ColumnCalculator, Project}; use LanguageServer\Protocol\{ TextDocumentItem, TextDocumentIdentifier, diff --git a/src/Server/Workspace.php b/src/Server/Workspace.php index cbd0da4..c6d963a 100644 --- a/src/Server/Workspace.php +++ b/src/Server/Workspace.php @@ -6,7 +6,7 @@ namespace LanguageServer\Server; use PhpParser\{Error, Comment, Node, ParserFactory, NodeTraverser, Lexer}; use PhpParser\PrettyPrinter\Standard as PrettyPrinter; use PhpParser\NodeVisitor\NameResolver; -use LanguageServer\{LanguageClient, ColumnCalculator, SymbolFinder, Project}; +use LanguageServer\{LanguageClient, ColumnCalculator, Project}; use LanguageServer\Protocol\{ TextDocumentItem, TextDocumentIdentifier, diff --git a/tests/PhpDocumentTest.php b/tests/PhpDocumentTest.php index 40a5dea..5c4a618 100644 --- a/tests/PhpDocumentTest.php +++ b/tests/PhpDocumentTest.php @@ -30,44 +30,7 @@ class PhpDocumentTest extends TestCase $symbols = $document->getSymbols(); - $this->assertEquals([ - [ - 'name' => 'a', - 'kind' => SymbolKind::VARIABLE, - 'location' => [ - 'uri' => 'whatever', - 'range' => [ - 'start' => [ - 'line' => 1, - 'character' => 0 - ], - 'end' => [ - 'line' => 1, - 'character' => 3 - ] - ] - ], - 'containerName' => null - ], - [ - 'name' => 'bar', - 'kind' => SymbolKind::VARIABLE, - 'location' => [ - 'uri' => 'whatever', - 'range' => [ - 'start' => [ - 'line' => 2, - 'character' => 0 - ], - 'end' => [ - 'line' => 2, - 'character' => 4 - ] - ] - ], - 'containerName' => null - ] - ], json_decode(json_encode($symbols), true)); + $this->assertEquals([], json_decode(json_encode($symbols), true)); } public function testGetNodeAtPosition() diff --git a/tests/ProjectTest.php b/tests/ProjectTest.php index eb37fa7..5fb9afb 100644 --- a/tests/ProjectTest.php +++ b/tests/ProjectTest.php @@ -41,8 +41,9 @@ class ProjectTest extends TestCase { $this->project->getDocument('file:///document1.php')->updateContent("project->getDocument('file:///document2.php')->updateContent("project->getDocument('invalid_file')->updateContent(file_get_contents(__DIR__ . '/../fixtures/invalid_file.php')); - $symbols = $this->project->findSymbols('ba'); + $symbols = $this->project->findSymbols('ba'); $this->assertEquals([ [ diff --git a/tests/Server/TextDocument/DocumentSymbolTest.php b/tests/Server/TextDocument/DocumentSymbolTest.php index 307a1cb..e5edfe7 100644 --- a/tests/Server/TextDocument/DocumentSymbolTest.php +++ b/tests/Server/TextDocument/DocumentSymbolTest.php @@ -28,24 +28,6 @@ class DocumentSymbolTest extends TestCase // Request symbols $result = $this->textDocument->documentSymbol(new TextDocumentIdentifier('symbols')); $this->assertEquals([ - [ - 'name' => 'TestNamespace', - 'kind' => SymbolKind::NAMESPACE, - 'location' => [ - 'uri' => 'symbols', - 'range' => [ - 'start' => [ - 'line' => 2, - 'character' => 0 - ], - 'end' => [ - 'line' => 2, - 'character' => 24 - ] - ] - ], - 'containerName' => null - ], [ 'name' => 'TEST_CONST', 'kind' => SymbolKind::CONSTANT, From 3a880934e5c100df704d546cad7e77d37e0a80cd Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sun, 9 Oct 2016 10:09:09 +0200 Subject: [PATCH 10/13] Split up PhpDocument::getDefinitionByNode() --- src/PhpDocument.php | 77 ++++++++++++++++++++++++++++++++++----------- src/Project.php | 15 +++++++-- 2 files changed, 71 insertions(+), 21 deletions(-) diff --git a/src/PhpDocument.php b/src/PhpDocument.php index eb901b5..b378da4 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -70,6 +70,13 @@ class PhpDocument */ private $definitions = []; + /** + * Map from fully qualified name (FQN) to array of nodes that reference the symbol + * + * @var Node[][] + */ + private $references; + /** * @param string $uri The URI of the document * @param Project $project The Project this document belongs to (to register definitions etc) @@ -284,23 +291,28 @@ class PhpDocument } /** - * Returns the definition node for any node - * The definition node MAY be in another document, check the ownerDocument attribute + * Returns true if the given FQN is defined in this document + * + * @param string $fqn The fully qualified name of the symbol + * @return bool + */ + public function isDefined(string $fqn): bool + { + return isset($this->definitions[$fqn]); + } + + /** + * Returns the FQN that is referenced by a node * * @param Node $node - * @return Node|null + * @return string|null */ - public function getDefinitionByNode(Node $node) + public function getReferencedFqn(Node $node) { if ($node instanceof Node\Name) { $nameNode = $node; $node = $node->getAttribute('parentNode'); } - // Variables always stay in the boundary of the file and need to be searched inside their function scope - // by traversing the AST - if ($node instanceof Node\Expr\Variable) { - return $this->getVariableDefinition($node); - } if ( ($node instanceof Node\Stmt\ClassLike @@ -310,13 +322,15 @@ class PhpDocument ) { // For extends, implements and type hints use the name directly $name = (string)$nameNode; - } else if ($node instanceof Node\Stmt\UseUse) { + // Only the name node should be considered a reference, not the UseUse node itself + } else if ($node instanceof Node\Stmt\UseUse && isset($nameNode)) { $name = (string)$node->name; $parent = $node->getAttribute('parentNode'); if ($parent instanceof Node\Stmt\GroupUse) { $name = $parent->prefix . '\\' . $name; } - } else if ($node instanceof Node\Expr\New_) { + // Only the name node should be considered a reference, not the New_ node itself + } else if ($node instanceof Node\Expr\New_ && isset($nameNode)) { if (!($node->class instanceof Node\Name)) { // Cannot get definition of dynamic calls return null; @@ -381,25 +395,50 @@ class PhpDocument if (!isset($name)) { return null; } - // Search for the document where the class, interface, trait, function, method or property is defined - $document = $this->project->getDefinitionDocument($name); - if (!$document && $node instanceof Node\Expr\FuncCall) { + // If the node is a function or constant, it could be namespaced, but PHP falls back to global + // The NameResolver therefor does not resolve these to namespaced names + // http://php.net/manual/en/language.namespaces.fallback.php + if ($node instanceof Node\Expr\FuncCall || $node instanceof Node\Expr\ConstFetch) { // Find and try with namespace - // Namespaces aren't added automatically by NameResolver because PHP falls back to global functions $n = $node; while (isset($n)) { $n = $n->getAttribute('parentNode'); if ($n instanceof Node\Stmt\Namespace_) { - $name = (string)$n->name . '\\' . $name; - $document = $this->project->getDefinitionDocument($name); - break; + $namespacedName = (string)$n->name . '\\' . $name; + // If the namespaced version is defined, return that + // Otherwise fall back to global + if ($this->project->isDefined($namespacedName)) { + return $namespacedName; + } } } } + return $name; + } + + /** + * Returns the definition node for any node + * The definition node MAY be in another document, check the ownerDocument attribute + * + * @param Node $node + * @return Node|null + */ + public function getDefinitionByNode(Node $node) + { + // Variables always stay in the boundary of the file and need to be searched inside their function scope + // by traversing the AST + if ($node instanceof Node\Expr\Variable) { + return $this->getVariableDefinition($node); + } + $fqn = $this->getReferencedFqn($node); + if (!isset($fqn)) { + return null; + } + $document = $this->project->getDefinitionDocument($fqn); if (!isset($document)) { return null; } - return $document->getDefinitionByFqn($name); + return $document->getDefinitionByFqn($fqn); } /** diff --git a/src/Project.php b/src/Project.php index ff65d98..b889c8f 100644 --- a/src/Project.php +++ b/src/Project.php @@ -15,7 +15,7 @@ class Project * * @var PhpDocument[] */ - private $documents; + private $documents = []; /** * An associative array [string => PhpDocument] @@ -23,7 +23,7 @@ class Project * * @var PhpDocument[] */ - private $definitions; + private $definitions = []; /** * Instance of the PHP parser @@ -84,6 +84,17 @@ class Project return $this->definitions[$fqn] ?? null; } + /** + * Returns true if the given FQN is defined in the project + * + * @param string $fqn The fully qualified name of the symbol + * @return bool + */ + public function isDefined(string $fqn): bool + { + return isset($this->definitions[$fqn]); + } + /** * Finds symbols in all documents, filtered by query parameter. * From 7322a6c658639e408e359252780ac4a4b1c35b6c Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sun, 9 Oct 2016 14:34:30 +0200 Subject: [PATCH 11/13] Add fromNode() factories and correct columns --- src/NodeVisitors/NodeAtPositionFinder.php | 5 +-- src/PhpDocument.php | 14 ++++----- src/Protocol/Location.php | 13 ++++++++ src/Protocol/Range.php | 16 ++++++++++ src/Protocol/SymbolInformation.php | 2 ++ src/Server/TextDocument.php | 8 +---- tests/Server/TextDocument/DefinitionTest.php | 32 ++++++++++---------- 7 files changed, 55 insertions(+), 35 deletions(-) diff --git a/src/NodeVisitors/NodeAtPositionFinder.php b/src/NodeVisitors/NodeAtPositionFinder.php index f68e39a..33f074e 100644 --- a/src/NodeVisitors/NodeAtPositionFinder.php +++ b/src/NodeVisitors/NodeAtPositionFinder.php @@ -34,10 +34,7 @@ class NodeAtPositionFinder extends NodeVisitorAbstract public function leaveNode(Node $node) { - $range = new Range( - new Position($node->getAttribute('startLine') - 1, $node->getAttribute('startColumn') - 1), - new Position($node->getAttribute('endLine') - 1, $node->getAttribute('endColumn') - 1) - ); + $range = Range::fromNode($node); // Workaround for https://github.com/nikic/PHP-Parser/issues/311 $parent = $node->getAttribute('parentNode'); if (isset($parent) && $parent instanceof Node\Stmt\GroupUse && $parent->prefix === $node) { diff --git a/src/PhpDocument.php b/src/PhpDocument.php index b378da4..12bae1b 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -113,16 +113,14 @@ class PhpDocument ]; $symbols = []; foreach ($this->definitions as $fqn => $node) { + $class = get_class($node); + if (!isset($nodeSymbolKindMap[$class])) { + continue; + } $symbol = new SymbolInformation(); - $symbol->kind = $nodeSymbolKindMap[get_class($node)]; + $symbol->kind = $nodeSymbolKindMap[$class]; $symbol->name = (string)$node->name; - $symbol->location = new Location( - $this->getUri(), - new Range( - new Position($node->getAttribute('startLine') - 1, $node->getAttribute('startColumn') - 1), - new Position($node->getAttribute('endLine') - 1, $node->getAttribute('endColumn')) - ) - ); + $symbol->location = Location::fromNode($node); $parts = preg_split('/(::|\\\\)/', $fqn); array_pop($parts); $symbol->containerName = implode('\\', $parts); diff --git a/src/Protocol/Location.php b/src/Protocol/Location.php index d6bdc14..a1d9861 100644 --- a/src/Protocol/Location.php +++ b/src/Protocol/Location.php @@ -2,6 +2,8 @@ namespace LanguageServer\Protocol; +use PhpParser\Node; + /** * Represents a location inside a resource, such as a line inside a text file. */ @@ -17,6 +19,17 @@ class Location */ public $range; + /** + * Returns the location of the node + * + * @param Node $node + * @return self + */ + public static function fromNode(Node $node) + { + return new self($node->getAttribute('ownerDocument')->getUri(), Range::fromNode($node)); + } + public function __construct(string $uri = null, Range $range = null) { $this->uri = $uri; diff --git a/src/Protocol/Range.php b/src/Protocol/Range.php index 56dba0c..17f31da 100644 --- a/src/Protocol/Range.php +++ b/src/Protocol/Range.php @@ -2,6 +2,8 @@ namespace LanguageServer\Protocol; +use PhpParser\Node; + /** * A range in a text document expressed as (zero-based) start and end positions. */ @@ -21,6 +23,20 @@ class Range */ public $end; + /** + * Returns the range the node spans + * + * @param Node $node + * @return self + */ + public static function fromNode(Node $node) + { + return new self( + new Position($node->getAttribute('startLine') - 1, $node->getAttribute('startColumn') - 1), + new Position($node->getAttribute('endLine') - 1, $node->getAttribute('endColumn')) + ); + } + public function __construct(Position $start = null, Position $end = null) { $this->start = $start; diff --git a/src/Protocol/SymbolInformation.php b/src/Protocol/SymbolInformation.php index 12e9ccc..e02fd9b 100644 --- a/src/Protocol/SymbolInformation.php +++ b/src/Protocol/SymbolInformation.php @@ -2,6 +2,8 @@ namespace LanguageServer\Protocol; +use PhpParser\Node; + /** * Represents information about programming constructs like variables, classes, * interfaces etc. diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index aaf3c25..39a9699 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -109,12 +109,6 @@ class TextDocument if ($def === null) { return null; } - return new Location( - $def->getAttribute('ownerDocument')->getUri(), - new Range( - new Position($def->getAttribute('startLine') - 1, $def->getAttribute('startColumn') - 1), - new Position($def->getAttribute('endLine') - 1, $def->getAttribute('endColumn') - 1) - ) - ); + return Location::fromNode($def); } } diff --git a/tests/Server/TextDocument/DefinitionTest.php b/tests/Server/TextDocument/DefinitionTest.php index eca8722..948fb35 100644 --- a/tests/Server/TextDocument/DefinitionTest.php +++ b/tests/Server/TextDocument/DefinitionTest.php @@ -39,7 +39,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 21, - 'character' => 0 + 'character' => 1 ] ] ], json_decode(json_encode($result), true)); @@ -59,7 +59,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 21, - 'character' => 0 + 'character' => 1 ] ] ], json_decode(json_encode($result), true)); @@ -79,7 +79,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 31, - 'character' => 0 + 'character' => 1 ] ] ], json_decode(json_encode($result), true)); @@ -99,7 +99,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 31, - 'character' => 0 + 'character' => 1 ] ] ], json_decode(json_encode($result), true)); @@ -119,7 +119,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 8, - 'character' => 31 + 'character' => 32 ] ] ], json_decode(json_encode($result), true)); @@ -139,7 +139,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 4, - 'character' => 21 + 'character' => 22 ] ] ], json_decode(json_encode($result), true)); @@ -159,7 +159,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 15, - 'character' => 4 + 'character' => 5 ] ] ], json_decode(json_encode($result), true)); @@ -179,7 +179,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 9, - 'character' => 36 + 'character' => 37 ] ] ], json_decode(json_encode($result), true)); @@ -199,7 +199,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 20, - 'character' => 4 + 'character' => 5 ] ] ], json_decode(json_encode($result), true)); @@ -219,7 +219,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 10, - 'character' => 23 + 'character' => 24 ] ] ], json_decode(json_encode($result), true)); @@ -239,7 +239,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 12, - 'character' => 9 + 'character' => 10 ] ] ], json_decode(json_encode($result), true)); @@ -259,7 +259,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 21, - 'character' => 0 + 'character' => 1 ] ] ], json_decode(json_encode($result), true)); @@ -278,7 +278,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 21, - 'character' => 0 + 'character' => 1 ] ] ], json_decode(json_encode($result), true)); @@ -298,7 +298,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 15, - 'character' => 33 + 'character' => 34 ] ] ], json_decode(json_encode($result), true)); @@ -318,7 +318,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 19, - 'character' => 25 + 'character' => 26 ] ] ], json_decode(json_encode($result), true)); @@ -338,7 +338,7 @@ class DefinitionTest extends TestCase ], 'end' => [ 'line' => 36, - 'character' => 0 + 'character' => 1 ] ] ], json_decode(json_encode($result), true)); From 7f95b76cf86b8a12517b5908acfb6d816e71a45f Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sun, 9 Oct 2016 14:40:15 +0200 Subject: [PATCH 12/13] Refactor DefinitionResolver Move logic to PhpDocument::getDefininedFqn() for reusability Fix DefinitionResolverTest --- src/NodeVisitors/DefinitionCollector.php | 50 +------------- src/PhpDocument.php | 69 ++++++++++++++++--- .../NodeVisitors/DefinitionCollectorTest.php | 9 ++- 3 files changed, 71 insertions(+), 57 deletions(-) diff --git a/src/NodeVisitors/DefinitionCollector.php b/src/NodeVisitors/DefinitionCollector.php index 8ad80fd..54ce0e5 100644 --- a/src/NodeVisitors/DefinitionCollector.php +++ b/src/NodeVisitors/DefinitionCollector.php @@ -13,14 +13,6 @@ class DefinitionCollector extends NodeVisitorAbstract { /** * Map from fully qualified name (FQN) to Node - * Examples of fully qualified names: - * - testFunction() - * - TestNamespace\TestClass - * - TestNamespace\TestClass::TEST_CONSTANT - * - TestNamespace\TestClass::staticTestProperty - * - TestNamespace\TestClass::testProperty - * - TestNamespace\TestClass::staticTestMethod() - * - TestNamespace\TestClass::testMethod() * * @var Node[] */ @@ -28,45 +20,9 @@ class DefinitionCollector extends NodeVisitorAbstract public function enterNode(Node $node) { - if ($node instanceof Node\Stmt\ClassLike && isset($node->name)) { - // Class, interface or trait declaration - $this->definitions[(string)$node->namespacedName] = $node; - } else if ($node instanceof Node\Stmt\Function_) { - // Function: use functioName() as the name - $name = (string)$node->namespacedName . '()'; - $this->definitions[$name] = $node; - } else if ($node instanceof Node\Stmt\ClassMethod) { - // Class method: use ClassName::methodName() as name - $class = $node->getAttribute('parentNode'); - if (!isset($class->name)) { - // Ignore anonymous classes - return; - } - $name = (string)$class->namespacedName . '::' . (string)$node->name . '()'; - $this->definitions[$name] = $node; - } else if ($node instanceof Node\Stmt\PropertyProperty) { - // Property: use ClassName::propertyName as name - $class = $node->getAttribute('parentNode')->getAttribute('parentNode'); - if (!isset($class->name)) { - // Ignore anonymous classes - return; - } - $name = (string)$class->namespacedName . '::' . (string)$node->name; - $this->definitions[$name] = $node; - } else if ($node instanceof Node\Const_) { - $parent = $node->getAttribute('parentNode'); - if ($parent instanceof Node\Stmt\Const_) { - // Basic constant: use CONSTANT_NAME as name - $name = (string)$node->namespacedName; - } else if ($parent instanceof Node\Stmt\ClassConst) { - // Class constant: use ClassName::CONSTANT_NAME as name - $class = $parent->getAttribute('parentNode'); - if (!isset($class->name) || $class->name instanceof Node\Expr) { - return; - } - $name = (string)$class->namespacedName . '::' . $node->name; - } - $this->definitions[$name] = $node; + $fqn = $node->getAttribute('ownerDocument')->getDefinedFqn($node); + if ($fqn !== null) { + $this->definitions[$fqn] = $node; } } } diff --git a/src/PhpDocument.php b/src/PhpDocument.php index 12bae1b..d4edb1d 100644 --- a/src/PhpDocument.php +++ b/src/PhpDocument.php @@ -57,14 +57,6 @@ class PhpDocument /** * Map from fully qualified name (FQN) to Node - * Examples of fully qualified names: - * - testFunction() - * - TestNamespace\TestClass - * - TestNamespace\TestClass::TEST_CONSTANT - * - TestNamespace\TestClass::staticTestProperty - * - TestNamespace\TestClass::testProperty - * - TestNamespace\TestClass::staticTestMethod() - * - TestNamespace\TestClass::testMethod() * * @var Node[] */ @@ -299,6 +291,67 @@ class PhpDocument return isset($this->definitions[$fqn]); } + /** + * Returns the fully qualified name (FQN) that is defined by a node + * Examples of FQNs: + * - testFunction() + * - TestNamespace\TestClass + * - TestNamespace\TestClass::TEST_CONSTANT + * - TestNamespace\TestClass::staticTestProperty + * - TestNamespace\TestClass::testProperty + * - TestNamespace\TestClass::staticTestMethod() + * - TestNamespace\TestClass::testMethod() + * + * @param Node $node + * @return string|null + */ + public function getDefinedFqn(Node $node) + { + if ($node instanceof Node\Name) { + $nameNode = $node; + $node = $node->getAttribute('parentNode'); + } + // Only the class node should count as the definition, not the name node + // Anonymous classes don't count as a definition + if ($node instanceof Node\Stmt\ClassLike && !isset($nameNode) && isset($node->name)) { + // Class, interface or trait declaration + return (string)$node->namespacedName; + } else if ($node instanceof Node\Stmt\Function_) { + // Function: use functionName() as the name + return (string)$node->namespacedName . '()'; + } else if ($node instanceof Node\Stmt\ClassMethod) { + // Class method: use ClassName::methodName() as name + $class = $node->getAttribute('parentNode'); + if (!isset($class->name)) { + // Ignore anonymous classes + return null; + } + return (string)$class->namespacedName . '::' . (string)$node->name . '()'; + } else if ($node instanceof Node\Stmt\PropertyProperty) { + // Property: use ClassName::propertyName as name + $class = $node->getAttribute('parentNode')->getAttribute('parentNode'); + if (!isset($class->name)) { + // Ignore anonymous classes + return null; + } + return (string)$class->namespacedName . '::' . (string)$node->name; + } else if ($node instanceof Node\Const_) { + $parent = $node->getAttribute('parentNode'); + if ($parent instanceof Node\Stmt\Const_) { + // Basic constant: use CONSTANT_NAME as name + return (string)$node->namespacedName; + } + if ($parent instanceof Node\Stmt\ClassConst) { + // Class constant: use ClassName::CONSTANT_NAME as name + $class = $parent->getAttribute('parentNode'); + if (!isset($class->name) || $class->name instanceof Node\Expr) { + return null; + } + return (string)$class->namespacedName . '::' . $node->name; + } + } + } + /** * Returns the FQN that is referenced by a node * diff --git a/tests/NodeVisitors/DefinitionCollectorTest.php b/tests/NodeVisitors/DefinitionCollectorTest.php index d22bd79..2f18bec 100644 --- a/tests/NodeVisitors/DefinitionCollectorTest.php +++ b/tests/NodeVisitors/DefinitionCollectorTest.php @@ -6,18 +6,23 @@ namespace LanguageServer\Tests\Server\TextDocument; use PHPUnit\Framework\TestCase; use PhpParser\{ParserFactory, NodeTraverser, Node}; use PhpParser\NodeVisitor\NameResolver; +use LanguageServer\{LanguageClient, Project, PhpDocument}; +use LanguageServer\Tests\MockProtocolStream; use LanguageServer\NodeVisitors\{ReferencesAdder, DefinitionCollector}; class DefinitionCollectorTest extends TestCase { public function test() { + $client = new LanguageClient(new MockProtocolStream()); + $project = new Project($client); + $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); + $document = new PhpDocument('whatever', $project, $client, $parser); $traverser = new NodeTraverser; $traverser->addVisitor(new NameResolver); - $traverser->addVisitor(new ReferencesAdder); + $traverser->addVisitor(new ReferencesAdder($document)); $definitionCollector = new DefinitionCollector; $traverser->addVisitor($definitionCollector); - $parser = (new ParserFactory)->create(ParserFactory::PREFER_PHP7); $stmts = $parser->parse(file_get_contents(__DIR__ . '/../../fixtures/symbols.php')); $traverser->traverse($stmts); $defs = $definitionCollector->definitions; From 0387f287598c4dbc3d069b7fabdd63a7fbd602f1 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sun, 9 Oct 2016 16:03:56 +0200 Subject: [PATCH 13/13] Rename NodeVisitors NS to NodeVisitor --- src/{NodeVisitors => NodeVisitor}/ColumnCalculator.php | 2 +- src/{NodeVisitors => NodeVisitor}/DefinitionCollector.php | 2 +- src/{NodeVisitors => NodeVisitor}/NodeAtPositionFinder.php | 2 +- src/{NodeVisitors => NodeVisitor}/ReferencesAdder.php | 2 +- src/PhpDocument.php | 2 +- tests/{NodeVisitors => NodeVisitor}/DefinitionCollectorTest.php | 2 +- tests/PhpDocumentTest.php | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) rename src/{NodeVisitors => NodeVisitor}/ColumnCalculator.php (96%) rename src/{NodeVisitors => NodeVisitor}/DefinitionCollector.php (94%) rename src/{NodeVisitors => NodeVisitor}/NodeAtPositionFinder.php (96%) rename src/{NodeVisitors => NodeVisitor}/ReferencesAdder.php (97%) rename tests/{NodeVisitors => NodeVisitor}/DefinitionCollectorTest.php (97%) diff --git a/src/NodeVisitors/ColumnCalculator.php b/src/NodeVisitor/ColumnCalculator.php similarity index 96% rename from src/NodeVisitors/ColumnCalculator.php rename to src/NodeVisitor/ColumnCalculator.php index 102c37c..6f6b3bf 100644 --- a/src/NodeVisitors/ColumnCalculator.php +++ b/src/NodeVisitor/ColumnCalculator.php @@ -1,7 +1,7 @@