refactor(completion): make completion of global symbols use Index more efficiently

src/CompletionProvider.php

@@ -127,8 +127,11 @@ class CompletionProvider
      * @param CompletionContext $context The completion context
      * @return CompletionList
      */
-    public function provideCompletion(PhpDocument $doc, Position $pos, CompletionContext $context = null): CompletionList
+    public function provideCompletion(
-    {
+        PhpDocument $doc,
+        Position $pos,
+        CompletionContext $context = null
+    ): CompletionList {
         // This can be made much more performant if the tree follows specific invariants.
         $node = $doc->getNodeAtPosition($pos);
 
@@ -282,7 +285,6 @@ class CompletionProvider
             $prefix = $nameNode instanceof Node\QualifiedName
                 ? (string)PhpParser\ResolvedName::buildName($nameNode->nameParts, $nameNode->getFileContents())
                 : $nameNode->getText($node->getFileContents());
-            $prefixLen = strlen($prefix);
 
             /** Whether the prefix is qualified (contains at least one backslash) */
             $isQualified = $nameNode instanceof Node\QualifiedName && $nameNode->isQualifiedName();
@@ -293,84 +295,120 @@ class CompletionProvider
             /** The closest NamespaceDefinition Node */
             $namespaceNode = $node->getNamespaceDefinition();
 
-            /** @var string The name of the namespace */
+            if ($nameNode instanceof Node\QualifiedName) {
-            $namespacedPrefix = null;
+                /** @var array For Psr\Http\Mess this will be ['Psr', 'Http'] */
-            if ($namespaceNode) {
+                $namePartsWithoutLast = $nameNode->nameParts;
-                $namespacedPrefix = (string)PhpParser\ResolvedName::buildName($namespaceNode->name->nameParts, $node->getFileContents()) . '\\' . $prefix;
+                array_pop($namePartsWithoutLast);
-                $namespacedPrefixLen = strlen($namespacedPrefix);
+                /** @var string When typing \Foo\Bar\Fooba, this will be Foo\Bar */
+                $prefixParentNamespace = (string)PhpParser\ResolvedName::buildName(
+                    $namePartsWithoutLast,
+                    $node->getFileContents()
+                );
+            } else {
+                // Not qualified, parent namespace is root.
+                $prefixParentNamespace = '';
             }
 
+            /** @var string[] Namespaces to search completions in. */
+            $namespacesToSearch = [];
+            if ($namespaceNode && !$isFullyQualified) {
+                /** @var string Declared namespace of the file (or section) */
+                $currentNamespace = (string)PhpParser\ResolvedName::buildName(
+                    $namespaceNode->name->nameParts,
+                    $namespaceNode->getFileContents()
+                );
+
+                if ($prefixParentNamespace === '') {
+                    $namespacesToSearch[] = $currentNamespace;
+                } else {
+                    // Partially qualified, concatenate with current namespace.
+                    $namespacesToSearch[] = $currentNamespace . '\\' . $prefixParentNamespace;
+                }
+                /** @var string Prefix with namespace inferred. */
+                $namespacedPrefix = $currentNamespace . '\\' . $prefix;
+            } else {
+                // In the global namespace, prefix parent refers to global namespace,
+                // OR completing a fully qualified name, prefix parent starts from the global namespace.
+                $namespacesToSearch[] = $prefixParentNamespace;
+                $namespacedPrefix = $prefix;
+            }
+
+            if (!$isQualified && $namespacesToSearch[0] !== '' && ($prefix === '' || !isset($creation))) {
+                // Also search the global namespace for non-qualified completions, as roamed
+                // definitions may be found. Also, without a prefix, suggest completions from the global namespace.
+                // Since only functions and constants can be roamed, don't search the global namespace for creation
+                // with a prefix.
+                $namespacesToSearch[] = '';
+            }
+
+            /** @var int Length of $namespacedPrefix */
+            $namespacedPrefixLen = strlen($namespacedPrefix);
+            /** @var int Length of $prefix */
+            $prefixLen = strlen($prefix);
+
             // Get the namespace use statements
             // TODO: use function statements, use const statements
 
             /** @var string[] $aliases A map from local alias to fully qualified name */
             list($aliases,,) = $node->getImportTablesForCurrentScope();
 
-            foreach ($aliases as $alias => $name) {
-                $aliases[$alias] = (string)$name;
-            }
-
             // If there is a prefix that does not start with a slash, suggest `use`d symbols
-            if ($prefix && !$isFullyQualified) {
+            if (!$isQualified) {
                 foreach ($aliases as $alias => $fqn) {
                     // Suggest symbols that have been `use`d and match the prefix
-                    if (substr($alias, 0, $prefixLen) === $prefix && ($def = $this->index->getDefinition($fqn))) {
+                    if (substr($alias, 0, $prefixLen) === $prefix
-                        $list->items[] = CompletionItem::fromDefinition($def);
+                        && ($def = $this->index->getDefinition((string)$fqn))) {
-                    }
+                        // TODO: complete even when getDefinition($fqn) fails, e.g. complete definitions that are were
-                }
+                        // not found in the files parsed.
-            }
+                        $item = CompletionItem::fromDefinition($def);
-
-            // Suggest global (ie non member) symbols that either
-            //  - start with the current namespace + prefix, if the Name node is not fully qualified
-            //  - start with just the prefix, if the Name node is fully qualified
-            foreach ($this->index->getDefinitions() as $fqn => $def) {
-
-                $fqnStartsWithPrefix = substr($fqn, 0, $prefixLen) === $prefix;
-
-                if (
-                    // Exclude methods, properties etc.
-                    !$def->isMember
-                    && (
-                        !$prefix
-                        || (
-                            // Either not qualified, but a matching prefix with global fallback
-                            ($def->roamed && !$isQualified && $fqnStartsWithPrefix)
-                            // Or not in a namespace or a fully qualified name or AND matching the prefix
-                            || ((!$namespaceNode || $isFullyQualified) && $fqnStartsWithPrefix)
-                            // Or in a namespace, not fully qualified and matching the prefix + current namespace
-                            || (
-                                $namespaceNode
-                                && !$isFullyQualified
-                                && substr($fqn, 0, $namespacedPrefixLen) === $namespacedPrefix
-                            )
-                        )
-                    )
-                    // Only suggest classes for `new`
-                    && (!isset($creation) || $def->canBeInstantiated)
-                ) {
-                    $item = CompletionItem::fromDefinition($def);
-                    // Find the shortest name to reference the symbol
-                    if ($namespaceNode && ($alias = array_search($fqn, $aliases, true)) !== false) {
-                        // $alias is the name under which this definition is aliased in the current namespace
                         $item->insertText = $alias;
-                    } else if ($namespaceNode && !($prefix && $isFullyQualified)) {
+                        $list->items[] = $item;
-                        // Insert the global FQN with leading backslash
-                        $item->insertText = '\\' . $fqn;
-                    } else {
-                        // Insert the FQN without leading backlash
-                        $item->insertText = $fqn;
                     }
-                    // Don't insert the parenthesis for functions
-                    // TODO return a snippet and put the cursor inside
-                    if (substr($item->insertText, -2) === '()') {
-                        $item->insertText = substr($item->insertText, 0, -2);
-                    }
-                    $list->items[] = $item;
                 }
             }
 
-            // If not a class instantiation, also suggest keywords
+            foreach ($namespacesToSearch as $namespaceToSearch) {
-            if (!isset($creation)) {
+                foreach ($this->index->getChildDefinitionsForFqn($namespaceToSearch) as $fqn => $def) {
+                    if (isset($creation) && !$def->canBeInstantiated) {
+                        // Only suggest classes for `new`
+                        continue;
+                    }
+
+                    $fqnStartsWithPrefix = substr($fqn, 0, $prefixLen) === $prefix;
+                    $fqnStartsWithNamespacedPrefix = substr($fqn, 0, $namespacedPrefixLen) === $namespacedPrefix;
+
+                    if (
+                        // No prefix - return all,
+                        $prefix === ''
+                        // or FQN starts with namespaced prefix,
+                        || $fqnStartsWithNamespacedPrefix
+                        // or a roamed definition (i.e. global fallback to a constant or a function) matches prefix.
+                        || ($def->roamed && $fqnStartsWithPrefix)
+                    ) {
+                        $item = CompletionItem::fromDefinition($def);
+                        // Find the shortest name to reference the symbol
+                        if ($namespaceNode && ($alias = array_search($fqn, $aliases, true)) !== false) {
+                            // $alias is the name under which this definition is aliased in the current namespace
+                            $item->insertText = $alias;
+                        } else if ($namespaceNode && !($prefix && $isFullyQualified)) {
+                            // Insert the global FQN with a leading backslash
+                            $item->insertText = '\\' . $fqn;
+                        } else {
+                            // Insert the FQN without a leading backslash
+                            $item->insertText = $fqn;
+                        }
+                        // Don't insert the parenthesis for functions
+                        // TODO return a snippet and put the cursor inside
+                        if (substr($item->insertText, -2) === '()') {
+                            $item->insertText = substr($item->insertText, 0, -2);
+                        }
+                        $list->items[] = $item;
+                    }
+                }
+            }
+
+            // Suggest keywords
+            if (!$isQualified && !isset($creation)) {
                 foreach (self::KEYWORDS as $keyword) {
                     if (substr($keyword, 0, $prefixLen) === $prefix) {
                         $item = new CompletionItem($keyword, CompletionItemKind::KEYWORD);
@@ -450,8 +488,9 @@ class CompletionProvider
                 }
             }
 
-            if ($level instanceof Node\Expression\AnonymousFunctionCreationExpression && $level->anonymousFunctionUseClause !== null &&
+            if ($level instanceof Node\Expression\AnonymousFunctionCreationExpression
-                $level->anonymousFunctionUseClause->useVariableNameList !== null) {
+                && $level->anonymousFunctionUseClause !== null
+                && $level->anonymousFunctionUseClause->useVariableNameList !== null) {
                 foreach ($level->anonymousFunctionUseClause->useVariableNameList->getValues() as $use) {
                     $useName = $use->getName();
                     if (empty($namePrefix) || strpos($useName, $namePrefix) !== false) {

src/Index/Index.php

@@ -22,7 +22,7 @@ class Index implements ReadableIndex, \Serializable
      *         '\Log' => [
      *             '\LoggerInterface' => [
      *                 ''        => $def1, // definition for 'Psr\Log\LoggerInterface' which is non-member
-     *                 '->log()' => $def2, // definition for 'Psr\Log\LoggerInterface->log()' which is a member definition
+     *                 '->log()' => $def2, // definition for 'Psr\Log\LoggerInterface->log()' which is a member
      *             ],
      *         ],
      *     ],
@@ -130,7 +130,11 @@ class Index implements ReadableIndex, \Serializable
             if ($name === '') {
                 continue;
             }
-            yield $fqn.$name => $item;
+            if ($item instanceof Definition) {
+                yield $fqn.$name => $item;
+            } elseif (is_array($item) && isset($item[''])) {
+                yield $fqn.$name => $item[''];
+            }
         }
     }
 
@@ -317,12 +321,9 @@ class Index implements ReadableIndex, \Serializable
         // split fqn at backslashes
         $parts = explode('\\', $fqn);
 
-        // write back the backslach prefix to the first part if it was present
+        // write back the backslash prefix to the first part if it was present
-        if ('' === $parts[0]) {
+        if ('' === $parts[0] && count($parts) > 1) {
-            if (count($parts) > 1) {
+            $parts = array_slice($parts, 1);
-                $parts = array_slice($parts, 1);
-            }
-
             $parts[0] = '\\' . $parts[0];
         }
 
@@ -346,7 +347,8 @@ class Index implements ReadableIndex, \Serializable
             }
         }
 
-        if (!$hasOperator) {
+        // The end($parts) === '' holds for the root namespace.
+        if (!$hasOperator && end($parts) !== '') {
             // add an empty part to store the non-member definition to avoid
             // definition collisions in the index array, eg
             // 'Psr\Log\LoggerInterface' will be stored at
@@ -364,25 +366,24 @@ class Index implements ReadableIndex, \Serializable
      * It can be an index node or a Definition if the $parts are precise
      * enough. Returns null when nothing is found.
      *
-     * @param string[] $parts         The splitted FQN
+     * @param string[] $path              The splitted FQN
-     * @param array &$storage         The array in which to store the $definition
+     * @param array|Definition &$storage  The current level to look for $path.
      * @return array|Definition|null
      */
-    private function getIndexValue(array $parts, array &$storage)
+    private function getIndexValue(array $path, &$storage)
     {
-        $part = $parts[0];
+        // Empty path returns the object itself.
+        if (empty($path)) {
+            return $storage;
+        }
+
+        $part = array_shift($path);
 
         if (!isset($storage[$part])) {
             return null;
         }
 
-        $parts = array_slice($parts, 1);
+        return $this->getIndexValue($path, $storage[$part]);
-        // we've reached the last provided part
-        if (empty($parts)) {
-            return $storage[$part];
-        }
-
-        return $this->getIndexValue($parts, $storage[$part]);
     }
 
     /**

tests/Server/TextDocument/CompletionTest.php

@@ -218,24 +218,6 @@ class CompletionTest extends TestCase
                 null,
                 'TestClass'
             ),
-            new CompletionItem(
-                'ChildClass',
-                CompletionItemKind::CLASS_,
-                'TestNamespace',
-                null,
-                null,
-                null,
-                '\TestNamespace\ChildClass'
-            ),
-            new CompletionItem(
-                'Example',
-                CompletionItemKind::CLASS_,
-                'TestNamespace',
-                null,
-                null,
-                null,
-                '\TestNamespace\Example'
-            )
         ], true), $items);
     }
 
@@ -257,7 +239,10 @@ class CompletionTest extends TestCase
                     'laboris commodo ad commodo velit mollit qui non officia id. Nulla duis veniam' . "\n" .
                     'veniam officia deserunt et non dolore mollit ea quis eiusmod sit non. Occaecat' . "\n" .
                     'consequat sunt culpa exercitation pariatur id reprehenderit nisi incididunt Lorem' . "\n" .
-                    'sint. Officia culpa pariatur laborum nostrud cupidatat consequat mollit.'
+                    'sint. Officia culpa pariatur laborum nostrud cupidatat consequat mollit.',
+                null,
+                null,
+                'TestClass'
             )
         ], true), $items);
     }