Add support for variable suggestions

Refactor logic into CompletionProvider class

fixtures/completion/variable.php

@@ -0,0 +1,10 @@
+<?php
+
+/**
+ * @param string|null $param A parameter
+ */
+function test(string $param = null)
+{
+    $var = 123;
+    $
+}

src/CompletionProvider.php

@@ -0,0 +1,198 @@
+<?php
+declare(strict_types = 1);
+
+namespace LanguageServer;
+
+use PhpParser\Node;
+use phpDocumentor\Reflection\Types;
+use LanguageServer\Protocol\{
+    Position,
+    SymbolKind,
+    CompletionItem,
+    CompletionItemKind
+};
+
+class CompletionProvider
+{
+    /**
+     * @var DefinitionResolver
+     */
+    private $definitionResolver;
+
+    /**
+     * @var Project
+     */
+    private $project;
+
+    /**
+     * @param DefinitionResolver $definitionResolver
+     * @param Project $project
+     */
+    public function __construct(DefinitionResolver $definitionResolver, Project $project)
+    {
+        $this->definitionResolver = $definitionResolver;
+        $this->project = $project;
+    }
+
+    /**
+     * Returns suggestions for a specific cursor position in a document
+     *
+     * @param PhpDocument $document The opened document
+     * @param Position $position The cursor position
+     * @return CompletionItem[]
+     */
+    public function provideCompletion(PhpDocument $document, Position $position): array
+    {
+        $node = $document->getNodeAtPosition($position);
+
+        /** @var CompletionItem[] */
+        $items = [];
+
+        if ($node instanceof Node\Expr\Error) {
+            $node = $node->getAttribute('parentNode');
+        }
+
+        // If we get a property fetch node, resolve items of the class
+        if ($node instanceof Node\Expr\PropertyFetch) {
+            $objType = $this->definitionResolver->resolveExpressionNodeToType($node->var);
+            if ($objType instanceof Types\Object_ && $objType->getFqsen() !== null) {
+                $prefix = substr((string)$objType->getFqsen(), 1) . '::';
+                if (is_string($node->name)) {
+                    $prefix .= $node->name;
+                }
+                $prefixLen = strlen($prefix);
+                foreach ($this->project->getDefinitions() as $fqn => $def) {
+                    if (substr($fqn, 0, $prefixLen) === $prefix) {
+                        $item = new CompletionItem;
+                        $item->label = $def->symbolInformation->name;
+                        if ($def->type) {
+                            $item->detail = (string)$def->type;
+                        }
+                        if ($def->documentation) {
+                            $item->documentation = $def->documentation;
+                        }
+                        if ($def->symbolInformation->kind === SymbolKind::PROPERTY) {
+                            $item->kind = CompletionItemKind::PROPERTY;
+                        } else if ($def->symbolInformation->kind === SymbolKind::METHOD) {
+                            $item->kind = CompletionItemKind::METHOD;
+                        }
+                        $items[] = $item;
+                    }
+                }
+            }
+        } else {
+            // Find variables, parameters and use statements in the scope
+            foreach ($this->suggestVariablesAtNode($node) as $var) {
+                $item = new CompletionItem;
+                $item->kind = CompletionItemKind::VARIABLE;
+                $item->documentation = $this->definitionResolver->getDocumentationFromNode($var);
+                if ($var instanceof Node\Param) {
+                    $item->label = '$' . $var->name;
+                    $item->detail = (string)$this->definitionResolver->getTypeFromNode($var); // TODO make it handle variables as well. Makes sense because needs to handle @var tag too!
+                } else if ($var instanceof Node\Expr\Variable || $var instanceof Node\Expr\ClosureUse) {
+                    $item->label = '$' . ($var instanceof Node\Expr\ClosureUse ? $var->var : $var->name);
+                    $item->detail = (string)$this->definitionResolver->resolveExpressionNodeToType($var->getAttribute('parentNode'));
+                } else {
+                    throw new \LogicException;
+                }
+                $items[] = $item;
+            }
+        }
+
+        return $items;
+    }
+
+    /**
+     * Will walk the AST upwards until a function-like node is met
+     * and at each level walk all previous siblings and their children to search for definitions
+     * of that variable
+     *
+     * @param Node $node
+     * @return array <Node\Expr\Variable|Node\Param|Node\Expr\ClosureUse>
+     */
+    private function suggestVariablesAtNode(Node $node): array
+    {
+        $vars = [];
+
+        // Find variables in the node itself
+        // When getting completion in the middle of a function, $node will be the function node
+        // so we need to search it
+        foreach ($this->findVariableDefinitionsInNode($node) as $var) {
+            // Only use the first definition
+            if (!isset($vars[$var->name])) {
+                $vars[$var->name] = $var;
+            }
+        }
+
+        // Walk the AST upwards until a scope boundary is met
+        $level = $node;
+        while ($level && !($level instanceof Node\FunctionLike)) {
+            // Walk siblings before the node
+            $sibling = $level;
+            while ($sibling = $sibling->getAttribute('previousSibling')) {
+                // Collect all variables inside the sibling node
+                foreach ($this->findVariableDefinitionsInNode($sibling) as $var) {
+                    $vars[$var->name] = $var;
+                }
+            }
+            $level = $level->getAttribute('parentNode');
+        }
+
+        // If the traversal ended because a function was met,
+        // also add its parameters and closure uses to the result list
+        if ($level instanceof Node\FunctionLike) {
+            foreach ($level->params as $param) {
+                if (!isset($vars[$param->name])) {
+                    $vars[$param->name] = $param;
+                }
+            }
+            if ($level instanceof Node\Expr\Closure) {
+                foreach ($level->uses as $use) {
+                    if (!isset($vars[$param->name])) {
+                        $vars[$use->var] = $use;
+                    }
+                }
+            }
+        }
+
+        return array_values($vars);
+    }
+
+    /**
+     * Searches the subnodes of a node for variable assignments
+     *
+     * @param Node $node
+     * @return Node\Expr\Variable[]
+     */
+    private function findVariableDefinitionsInNode(Node $node): array
+    {
+        $vars = [];
+        // If the child node is a variable assignment, save it
+        $parent = $node->getAttribute('parentNode');
+        if (
+            $node instanceof Node\Expr\Variable
+            && ($parent instanceof Node\Expr\Assign || $parent instanceof Node\Expr\AssignOp)
+            && is_string($node->name) // Variable variables are of no use
+        ) {
+            $vars[] = $node;
+        }
+        // Iterate over subnodes
+        foreach ($node->getSubNodeNames() as $attr) {
+            if (!isset($node->$attr)) {
+                continue;
+            }
+            $children = is_array($node->$attr) ? $node->$attr : [$node->$attr];
+            foreach ($children as $child) {
+                // Dont try to traverse scalars
+                // Dont traverse functions, the contained variables are in a different scope
+                if (!($child instanceof Node) || $child instanceof Node\FunctionLike) {
+                    continue;
+                }
+                foreach ($this->findVariableDefinitionsInNode($child) as $var) {
+                    $vars[] = $var;
+                }
+            }
+        }
+        return $vars;
+    }
+}

src/DefinitionResolver.php

@@ -281,25 +281,34 @@ class DefinitionResolver
     /**
      * Returns the assignment or parameter node where a variable was defined
      *
-     * @param Node\Expr\Variable $n The variable access
+     * @param Node\Expr\Variable|Node\Expr\ClosureUse $var The variable access
      * @return Node\Expr\Assign|Node\Param|Node\Expr\ClosureUse|null
      */
-    public static function resolveVariableToNode(Node\Expr\Variable $var)
+    public static function resolveVariableToNode(Node\Expr $var)
     {
         $n = $var;
+        // When a use is passed, start outside the closure to not return immediatly
+        if ($var instanceof Node\Expr\ClosureUse) {
+            $n = $var->getAttribute('parentNode')->getAttribute('parentNode');
+            $name = $var->var;
+        } else if ($var instanceof Node\Expr\Variable || $var instanceof Node\Param) {
+            $name = $var->name;
+        } else {
+            throw new \InvalidArgumentException('$var must be Variable, Param or ClosureUse, not ' . get_class($var));
+        }
         // Traverse the AST up
         do {
             // 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) {
+                    if ($param->name === $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) {
+                        if ($use->var === $name) {
                             return $use;
                         }
                     }
@@ -310,7 +319,7 @@ class DefinitionResolver
             while ($n->getAttribute('previousSibling') && $n = $n->getAttribute('previousSibling')) {
                 if (
                     ($n instanceof Node\Expr\Assign || $n instanceof Node\Expr\AssignOp)
-                    && $n->var instanceof Node\Expr\Variable && $n->var->name === $var->name
+                    && $n->var instanceof Node\Expr\Variable && $n->var->name === $name
                 ) {
                     return $n;
                 }
@@ -329,8 +338,8 @@ class DefinitionResolver
      */
     public function resolveExpressionNodeToType(Node\Expr $expr): Type
     {
-        if ($expr instanceof Node\Expr\Variable) {
+        if ($expr instanceof Node\Expr\Variable || $expr instanceof Node\Expr\ClosureUse) {
-            if ($expr->name === 'this') {
+            if ($expr instanceof Node\Expr\Variable && $expr->name === 'this') {
                 return new Types\This;
             }
             // Find variable definition

src/Server/TextDocument.php

@@ -3,7 +3,7 @@ declare(strict_types = 1);
 
 namespace LanguageServer\Server;
 
-use LanguageServer\{LanguageClient, Project, PhpDocument, DefinitionResolver};
+use LanguageServer\{LanguageClient, Project, PhpDocument, DefinitionResolver, CompletionProvider};
 use PhpParser\PrettyPrinter\Standard as PrettyPrinter;
 use PhpParser\Node;
 use LanguageServer\Protocol\{
@@ -23,7 +23,6 @@ use LanguageServer\Protocol\{
     CompletionItem,
     CompletionItemKind
 };
-use phpDocumentor\Reflection\Types;
 use Sabre\Event\Promise;
 use function Sabre\Event\coroutine;
 
@@ -54,12 +53,18 @@ class TextDocument
      */
     private $definitionResolver;
 
+    /**
+     * @var CompletionProvider
+     */
+    private $completionProvider;
+
     public function __construct(Project $project, LanguageClient $client)
     {
         $this->project = $project;
         $this->client = $client;
         $this->prettyPrinter = new PrettyPrinter();
         $this->definitionResolver = new DefinitionResolver($project);
+        $this->completionProvider = new CompletionProvider($this->definitionResolver, $project);
     }
 
     /**
@@ -233,45 +238,7 @@ class TextDocument
     {
         return coroutine(function () use ($textDocument, $position) {
             $document = yield $this->project->getOrLoadDocument($textDocument->uri);
-            $node = $document->getNodeAtPosition($position);
+            return $this->completionProvider->provideCompletion($document, $position);
-            if ($node === null) {
-                return [];
-            }
-            if ($node instanceof Node\Expr\Error) {
-                $node = $node->getAttribute('parentNode');
-            }
-            if ($node instanceof Node\Expr\PropertyFetch) {
-                // Resolve object
-                $objType = $this->definitionResolver->resolveExpressionNodeToType($node->var);
-                if ($objType instanceof Types\Object_ && $objType->getFqsen() !== null) {
-                    $prefix = substr((string)$objType->getFqsen(), 1) . '::';
-                    if (is_string($node->name)) {
-                        $prefix .= $node->name;
-                    }
-                    $prefixLen = strlen($prefix);
-                    $items = [];
-                    foreach ($this->project->getDefinitions() as $fqn => $def) {
-                        if (substr($fqn, 0, $prefixLen) === $prefix) {
-                            $item = new CompletionItem;
-                            $item->label = $def->symbolInformation->name;
-                            if ($def->type) {
-                                $item->detail = (string)$def->type;
-                            }
-                            if ($def->documentation) {
-                                $item->documentation = $def->documentation;
-                            }
-                            if ($def->symbolInformation->kind === SymbolKind::PROPERTY) {
-                                $item->kind = CompletionItemKind::PROPERTY;
-                            } else if ($def->symbolInformation->kind === SymbolKind::METHOD) {
-                                $item->kind = CompletionItemKind::METHOD;
-                            }
-                            $items[] = $item;
-                        }
-                    }
-                    return $items;
-                }
-            }
-            return [];
         });
     }
 }

tests/LanguageServerTest.php

@@ -64,7 +64,7 @@ class LanguageServerTest extends TestCase
             if ($msg->body->method === 'window/logMessage' && $promise->state === Promise::PENDING) {
                 if ($msg->body->params->type === MessageType::ERROR) {
                     $promise->reject(new Exception($msg->body->params->message));
-                } else if (strpos($msg->body->params->message, 'All 11 PHP files parsed') !== false) {
+                } else if (strpos($msg->body->params->message, 'All 12 PHP files parsed') !== false) {
                     $promise->fulfill();
                 }
             }
@@ -109,7 +109,7 @@ class LanguageServerTest extends TestCase
                     if ($promise->state === Promise::PENDING) {
                         $promise->reject(new Exception($msg->body->params->message));
                     }
-                } else if (strpos($msg->body->params->message, 'All 11 PHP files parsed') !== false) {
+                } else if (strpos($msg->body->params->message, 'All 12 PHP files parsed') !== false) {
                     // Indexing finished
                     $promise->fulfill();
                 }

tests/Server/TextDocument/CompletionTest.php

@@ -52,4 +52,18 @@ class CompletionTest extends TestCase
             )
         ], $items);
     }
+
+    public function testForVariables()
+    {
+        $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/variable.php');
+        $this->project->openDocument($completionUri, file_get_contents($completionUri));
+        $items = $this->textDocument->completion(
+            new TextDocumentIdentifier($completionUri),
+            new Position(8, 5)
+        )->wait();
+        $this->assertEquals([
+            new CompletionItem('$var', CompletionItemKind::VARIABLE, 'int'),
+            new CompletionItem('$param', CompletionItemKind::VARIABLE, 'string|null', 'A parameter')
+        ], $items);
+    }
 }