Use DefinitionCollector for symbol requests

src/NodeVisitors/SymbolFinder.php

@@ -1,121 +0,0 @@
-<?php
-declare(strict_types = 1);
-
-namespace LanguageServer\NodeVisitors;
-
-use PhpParser\{NodeVisitorAbstract, Node};
-
-use LanguageServer\Protocol\{SymbolInformation, SymbolKind, Range, Position, Location};
-
-class SymbolFinder extends NodeVisitorAbstract
-{
-    const NODE_SYMBOL_KIND_MAP = [
-        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,
-        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--;
-        }
-    }
-}

src/PhpDocument.php

@@ -3,8 +3,8 @@ declare(strict_types = 1);
 
 namespace LanguageServer;
 
-use LanguageServer\Protocol\{Diagnostic, DiagnosticSeverity, Range, Position, SymbolKind, TextEdit};
+use LanguageServer\Protocol\{Diagnostic, DiagnosticSeverity, Range, Position, SymbolInformation, SymbolKind, TextEdit, Location};
-use LanguageServer\NodeVisitors\{NodeAtPositionFinder, ReferencesAdder, DefinitionCollector, SymbolFinder, ColumnCalculator};
+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) {

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;
     }

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,

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,

tests/PhpDocumentTest.php

@@ -30,44 +30,7 @@ class PhpDocumentTest extends TestCase
 
         $symbols = $document->getSymbols();
 
-        $this->assertEquals([
+        $this->assertEquals([], json_decode(json_encode($symbols), true));
-            [
-                '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));
     }
 
     public function testGetNodeAtPosition()

tests/ProjectTest.php

@@ -41,6 +41,7 @@ class ProjectTest extends TestCase
     {
         $this->project->getDocument('file:///document1.php')->updateContent("<?php\nfunction foo() {}\nfunction bar() {}\n");
         $this->project->getDocument('file:///document2.php')->updateContent("<?php\nfunction baz() {}\nfunction frob() {}\n");
+        $this->project->getDocument('invalid_file')->updateContent(file_get_contents(__DIR__ . '/../fixtures/invalid_file.php'));
 
         $symbols = $this->project->findSymbols('ba');
 

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,