From 59670af7bdf25c6d75ccabdf0fe6bd84e67ee0d2 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sun, 20 Nov 2016 18:53:03 +0100 Subject: [PATCH] Add support for variable suggestions Refactor logic into CompletionProvider class --- fixtures/completion/variable.php | 10 + src/CompletionProvider.php | 198 +++++++++++++++++++ src/DefinitionResolver.php | 23 ++- src/Server/TextDocument.php | 49 +---- tests/LanguageServerTest.php | 4 +- tests/Server/TextDocument/CompletionTest.php | 14 ++ 6 files changed, 248 insertions(+), 50 deletions(-) create mode 100644 fixtures/completion/variable.php create mode 100644 src/CompletionProvider.php diff --git a/fixtures/completion/variable.php b/fixtures/completion/variable.php new file mode 100644 index 0000000..483f5dc --- /dev/null +++ b/fixtures/completion/variable.php @@ -0,0 +1,10 @@ +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 + */ + 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; + } +} diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index 64ae247..677cf89 100644 --- a/src/DefinitionResolver.php +++ b/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->name === 'this') { + if ($expr instanceof Node\Expr\Variable || $expr instanceof Node\Expr\ClosureUse) { + if ($expr instanceof Node\Expr\Variable && $expr->name === 'this') { return new Types\This; } // Find variable definition diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index a04407d..5998acc 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, 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); - 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 []; + return $this->completionProvider->provideCompletion($document, $position); }); } } diff --git a/tests/LanguageServerTest.php b/tests/LanguageServerTest.php index 98d5d36..6892f8f 100644 --- a/tests/LanguageServerTest.php +++ b/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(); } diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index 800d88c..e493488 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/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); + } }