From 2d91ee8eff807bd4c48168cc0c6eb9f1bda13415 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Sun, 6 Aug 2017 16:48:41 +0200 Subject: [PATCH 01/35] speedup autocomplete with ReadableIndex::getDefinitionsForNamespace --- src/CompletionProvider.php | 35 +++++++++--- src/Index/AbstractAggregateIndex.php | 17 ++++++ src/Index/Index.php | 79 ++++++++++++++++++++++++++++ src/Index/ReadableIndex.php | 8 +++ 4 files changed, 133 insertions(+), 6 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 68c5a0cc..ae4f958b 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -201,21 +201,44 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi $this->definitionResolver->resolveExpressionNodeToType($node->dereferencableExpression) ); + $checksCount = 0; + $start = microtime(true); // Add the object access operator to only get members of all parents - $prefixes = []; - foreach ($this->expandParentFqns($fqns) as $prefix) { - $prefixes[] = $prefix . '->'; + $namespaces = []; + foreach ($this->expandParentFqns($fqns) as $namespace) { + $namespaces[] = $namespace; } // Collect all definitions that match any of the prefixes - foreach ($this->index->getDefinitions() as $fqn => $def) { - foreach ($prefixes as $prefix) { - if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember) { + foreach ($namespaces as $namespace) { + foreach ($this->index->getDefinitionsForNamespace($namespace) as $fqn => $def) { + ++$checksCount; + $prefix = $namespace . '->'; + if (substr($fqn, 0, strlen($prefix)) === $prefix && !$def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } } } + // $prefixes = []; + // foreach ($this->expandParentFqns($fqns) as $prefix) { + // $prefixes[] = $prefix . '->'; + // } + // foreach ($this->index->getDefinitions() as $fqn => $def) { + // foreach ($prefixes as $prefix) { + // ++$checksCount; + // if (substr($fqn, 0, strlen($prefix)) === $prefix && !$def->isGlobal) { + // $list->items[] = CompletionItem::fromDefinition($def); + // } + // } + // } + $duration = microtime(true) - $start; + file_put_contents( + '/home/nicolas/tmp/php_language-server.log', + sprintf("%d items found, %d checks, memory : %d bytes, %ss\n", sizeof($list->items), $checksCount, memory_get_usage(true), $duration), + FILE_APPEND + ); + } elseif ( ($scoped = $node->parent) instanceof Node\Expression\ScopedPropertyAccessExpression || ($scoped = $node) instanceof Node\Expression\ScopedPropertyAccessExpression diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index 5377c3a4..fa13d148 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -115,6 +115,23 @@ public function getDefinitions(): array return $defs; } + /** + * Returns the Definitions that are in the given namespace + * + * @param string $namespace + * @return Definitions[] + */ + public function getDefinitionsForNamespace(string $namespace): array + { + $defs = []; + foreach ($this->getIndexes() as $index) { + foreach ($index->getDefinitionsForNamespace($namespace) as $fqn => $def) { + $defs[$fqn] = $def; + } + } + return $defs; + } + /** * Returns the Definition object by a specific FQN * diff --git a/src/Index/Index.php b/src/Index/Index.php index 9cb975e5..8af0688a 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -21,6 +21,13 @@ class Index implements ReadableIndex, \Serializable */ private $definitions = []; + /** + * An associative array that maps namespaces to an associative array of FQN to Definitions + * + * @var Definition[] + */ + private $namespaceDefinitions = []; + /** * An associative array that maps fully qualified symbol names to arrays of document URIs that reference the symbol * @@ -94,6 +101,20 @@ public function getDefinitions(): array return $this->definitions; } + /** + * Returns the Definitions that are in the given namespace + * + * @param string $namespace + * @return Definitions[] + */ + public function getDefinitionsForNamespace(string $namespace): array + { + return isset($this->namespaceDefinitions[$namespace]) + ? $this->namespaceDefinitions[$namespace] + : [] + ; + } + /** * Returns the Definition object by a specific FQN * @@ -123,6 +144,7 @@ public function getDefinition(string $fqn, bool $globalFallback = false) public function setDefinition(string $fqn, Definition $definition) { $this->definitions[$fqn] = $definition; + $this->setNamespaceDefinition($fqn, $definition); $this->emit('definition-added'); } @@ -137,6 +159,7 @@ public function removeDefinition(string $fqn) { unset($this->definitions[$fqn]); unset($this->references[$fqn]); + $this->removeNamespaceDefinition($fqn); } /** @@ -207,6 +230,7 @@ public function unserialize($serialized) foreach ($data as $prop => $val) { $this->$prop = $val; } + $this->buildNamespaceDefinitionsIndex(); } /** @@ -222,4 +246,59 @@ public function serialize() 'staticComplete' => $this->staticComplete ]); } + + /** + * @return void + */ + private function buildNamespaceDefinitionsIndex() + { + foreach ($this->definitions as $fqn => $definition) { + $this->setNamespaceDefinition($fqn, $definition); + } + } + + /** + * Registers a definition to a namespace + * + * @param string $fqn The fully qualified name of the symbol + * @param Definition $definition The Definition object + * @return void + */ + private function setNamespaceDefinition(string $fqn, Definition $definition) + { + $namespace = $this->extractNamespace($fqn); + if (!isset($this->namespaceDefinitions[$namespace])) { + $this->namespaceDefinitions[$namespace] = []; + } + $this->namespaceDefinitions[$namespace][$fqn] = $definition; + } + + /** + * Removes a definition from a namespace + * + * @param string $fqn The fully qualified name of the symbol + * @return void + */ + private function removeNamespaceDefinition(string $fqn) + { + $namespace = $this->extractNamespace($fqn); + if (isset($this->namespaceDefinitions[$namespace])) { + unset($this->namespaceDefinitions[$namespace][$fqn]); + } + } + + /** + * @param string $fqn + * @return string The namespace extracted from the given FQN + */ + private function extractNamespace(string $fqn): string + { + foreach (['::', '->'] as $operator) { + if (false !== ($pos = strpos($fqn, $operator))) { + return substr($fqn, 0, $pos); + } + } + + return $fqn; + } } diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 67b20b63..c7d57321 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -37,6 +37,14 @@ public function isStaticComplete(): bool; */ public function getDefinitions(): array; + /** + * Returns the Definitions that are in the given namespace + * + * @param string $namespace + * @return Definitions[] + */ + public function getDefinitionsForNamespace(string $namespace): array; + /** * Returns the Definition object by a specific FQN * From b7c712842f514213137af25c8f8db6307c9e2373 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Sun, 6 Aug 2017 17:25:26 +0200 Subject: [PATCH 02/35] update tests --- tests/Server/TextDocument/CompletionTest.php | 40 ++++++++++---------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index 0d68ec3e..08b52a55 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -605,7 +605,7 @@ public function testThisWithoutPrefix() ) ], true), $items); } - + public function testThisWithPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/this_with_prefix.php'); @@ -615,18 +615,6 @@ public function testThisWithPrefix() new Position(12, 16) )->wait(); $this->assertEquals(new CompletionList([ - new CompletionItem( - 'testProperty', - CompletionItemKind::PROPERTY, - '\TestClass', // Type of the property - 'Reprehenderit magna velit mollit ipsum do.' - ), - new CompletionItem( - 'testMethod', - CompletionItemKind::METHOD, - '\TestClass', // Return type of the method - 'Non culpa nostrud mollit esse sunt laboris in irure ullamco cupidatat amet.' - ), new CompletionItem( 'foo', CompletionItemKind::PROPERTY, @@ -650,7 +638,19 @@ public function testThisWithPrefix() CompletionItemKind::METHOD, 'mixed', // Return type of the method null - ) + ), + new CompletionItem( + 'testProperty', + CompletionItemKind::PROPERTY, + '\TestClass', // Type of the property + 'Reprehenderit magna velit mollit ipsum do.' + ), + new CompletionItem( + 'testMethod', + CompletionItemKind::METHOD, + '\TestClass', // Return type of the method + 'Non culpa nostrud mollit esse sunt laboris in irure ullamco cupidatat amet.' + ), ], true), $items); } @@ -663,11 +663,6 @@ public function testThisReturnValue() new Position(17, 23) )->wait(); $this->assertEquals(new CompletionList([ - new CompletionItem( - 'foo', - CompletionItemKind::METHOD, - '$this' // Return type of the method - ), new CompletionItem( 'bar', CompletionItemKind::METHOD, @@ -677,7 +672,12 @@ public function testThisReturnValue() 'qux', CompletionItemKind::METHOD, 'mixed' // Return type of the method - ) + ), + new CompletionItem( + 'foo', + CompletionItemKind::METHOD, + '$this' // Return type of the method + ), ], true), $items); } } From 6d725a234c9f1c07edb862ef601335a32988b862 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Sun, 6 Aug 2017 17:48:06 +0200 Subject: [PATCH 03/35] speedup static access autocomplete --- src/CompletionProvider.php | 46 +++++++++++++++++++++++++++++--------- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index ae4f958b..07602e40 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -233,11 +233,11 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi // } // } $duration = microtime(true) - $start; - file_put_contents( - '/home/nicolas/tmp/php_language-server.log', - sprintf("%d items found, %d checks, memory : %d bytes, %ss\n", sizeof($list->items), $checksCount, memory_get_usage(true), $duration), - FILE_APPEND - ); + // file_put_contents( + // '/home/nicolas/tmp/php_language-server.log', + // sprintf("%d items found, %d checks, memory : %d bytes, %ss\n", sizeof($list->items), $checksCount, memory_get_usage(true), $duration), + // FILE_APPEND + // ); } elseif ( ($scoped = $node->parent) instanceof Node\Expression\ScopedPropertyAccessExpression || @@ -252,26 +252,50 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi // // TODO: $a::| + $checksCount = 0; + $start = microtime(true); // Resolve all possible types to FQNs $fqns = FqnUtilities\getFqnsFromType( $classType = $this->definitionResolver->resolveExpressionNodeToType($scoped->scopeResolutionQualifier) ); // Append :: operator to only get static members of all parents - $prefixes = []; - foreach ($this->expandParentFqns($fqns) as $prefix) { - $prefixes[] = $prefix . '::'; + $namespaces = []; + foreach ($this->expandParentFqns($fqns) as $namespace) { + $namespaces[] = $namespace; } // Collect all definitions that match any of the prefixes - foreach ($this->index->getDefinitions() as $fqn => $def) { - foreach ($prefixes as $prefix) { - if (substr(strtolower($fqn), 0, strlen($prefix)) === strtolower($prefix) && $def->isMember) { + foreach ($namespaces as $namespace) { + foreach ($this->index->getDefinitionsForNamespace($namespace) as $fqn => $def) { + ++$checksCount; + $prefix = strtolower($namespace . '::'); + if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && !$def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } } } + // $prefixes = []; + // foreach ($this->expandParentFqns($fqns) as $prefix) { + // $prefixes[] = $prefix . '::'; + // } + + // foreach ($this->index->getDefinitions() as $fqn => $def) { + // foreach ($prefixes as $prefix) { + // ++$checksCount; + // if (substr(strtolower($fqn), 0, strlen($prefix)) === strtolower($prefix) && !$def->isGlobal) { + // $list->items[] = CompletionItem::fromDefinition($def); + // } + // } + // } + $duration = microtime(true) - $start; + file_put_contents( + '/home/nicolas/tmp/php_language-server_static.log', + sprintf("%d items found, %d checks, memory : %d bytes, %ss\n", sizeof($list->items), $checksCount, memory_get_usage(true), $duration), + FILE_APPEND + ); + } elseif ( ParserHelpers\isConstantFetch($node) // Creation gets set in case of an instantiation (`new` expression) From 18f2f4c85c02edb9d2d5fd89642c53ef8e442959 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Sun, 6 Aug 2017 18:12:32 +0200 Subject: [PATCH 04/35] cleanup --- src/CompletionProvider.php | 55 +++++--------------------------------- 1 file changed, 6 insertions(+), 49 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 07602e40..4e313ad4 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -201,18 +201,16 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi $this->definitionResolver->resolveExpressionNodeToType($node->dereferencableExpression) ); - $checksCount = 0; - $start = microtime(true); - // Add the object access operator to only get members of all parents + // The namespaces of the symbol and its parents (eg the implemented interfaces) $namespaces = []; foreach ($this->expandParentFqns($fqns) as $namespace) { $namespaces[] = $namespace; } - // Collect all definitions that match any of the prefixes + // Collect namespaces definitions foreach ($namespaces as $namespace) { foreach ($this->index->getDefinitionsForNamespace($namespace) as $fqn => $def) { - ++$checksCount; + // Add the object access operator to only get members of all parents $prefix = $namespace . '->'; if (substr($fqn, 0, strlen($prefix)) === $prefix && !$def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); @@ -220,25 +218,6 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi } } - // $prefixes = []; - // foreach ($this->expandParentFqns($fqns) as $prefix) { - // $prefixes[] = $prefix . '->'; - // } - // foreach ($this->index->getDefinitions() as $fqn => $def) { - // foreach ($prefixes as $prefix) { - // ++$checksCount; - // if (substr($fqn, 0, strlen($prefix)) === $prefix && !$def->isGlobal) { - // $list->items[] = CompletionItem::fromDefinition($def); - // } - // } - // } - $duration = microtime(true) - $start; - // file_put_contents( - // '/home/nicolas/tmp/php_language-server.log', - // sprintf("%d items found, %d checks, memory : %d bytes, %ss\n", sizeof($list->items), $checksCount, memory_get_usage(true), $duration), - // FILE_APPEND - // ); - } elseif ( ($scoped = $node->parent) instanceof Node\Expression\ScopedPropertyAccessExpression || ($scoped = $node) instanceof Node\Expression\ScopedPropertyAccessExpression @@ -252,23 +231,21 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi // // TODO: $a::| - $checksCount = 0; - $start = microtime(true); // Resolve all possible types to FQNs $fqns = FqnUtilities\getFqnsFromType( $classType = $this->definitionResolver->resolveExpressionNodeToType($scoped->scopeResolutionQualifier) ); - // Append :: operator to only get static members of all parents + // The namespaces of the symbol and its parents (eg the implemented interfaces) $namespaces = []; foreach ($this->expandParentFqns($fqns) as $namespace) { $namespaces[] = $namespace; } - // Collect all definitions that match any of the prefixes + // Collect namespaces definitions foreach ($namespaces as $namespace) { foreach ($this->index->getDefinitionsForNamespace($namespace) as $fqn => $def) { - ++$checksCount; + // Append :: operator to only get static members of all parents $prefix = strtolower($namespace . '::'); if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && !$def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); @@ -276,26 +253,6 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi } } - // $prefixes = []; - // foreach ($this->expandParentFqns($fqns) as $prefix) { - // $prefixes[] = $prefix . '::'; - // } - - // foreach ($this->index->getDefinitions() as $fqn => $def) { - // foreach ($prefixes as $prefix) { - // ++$checksCount; - // if (substr(strtolower($fqn), 0, strlen($prefix)) === strtolower($prefix) && !$def->isGlobal) { - // $list->items[] = CompletionItem::fromDefinition($def); - // } - // } - // } - $duration = microtime(true) - $start; - file_put_contents( - '/home/nicolas/tmp/php_language-server_static.log', - sprintf("%d items found, %d checks, memory : %d bytes, %ss\n", sizeof($list->items), $checksCount, memory_get_usage(true), $duration), - FILE_APPEND - ); - } elseif ( ParserHelpers\isConstantFetch($node) // Creation gets set in case of an instantiation (`new` expression) From 17602aa7686a893547aa39641135e935bf52b0bc Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Sun, 6 Aug 2017 23:05:32 +0200 Subject: [PATCH 05/35] globalDefinitions cache to speedup autocomplete --- src/CompletionProvider.php | 26 ++++++++---------- src/Index/AbstractAggregateIndex.php | 19 ++++++++++++- src/Index/Index.php | 41 ++++++++++++++++++++++++---- src/Index/ReadableIndex.php | 10 ++++++- 4 files changed, 73 insertions(+), 23 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 4e313ad4..59348138 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -314,26 +314,22 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi // Suggest global 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) { + foreach ($this->index->getGlobalDefinitions() as $fqn => $def) { $fqnStartsWithPrefix = substr($fqn, 0, $prefixLen) === $prefix; if ( - // Exclude methods, properties etc. - !$def->isMember - && ( - !$prefix + !$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 || ( - // 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 - ) + $namespaceNode + && !$isFullyQualified + && substr($fqn, 0, $namespacedPrefixLen) === $namespacedPrefix ) ) // Only suggest classes for `new` diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index fa13d148..038117a9 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -100,7 +100,7 @@ public function isStaticComplete(): bool /** * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to Definitions + * to Definitions (global or not) * * @return Definition[] */ @@ -115,6 +115,23 @@ public function getDefinitions(): array return $defs; } + /** + * Returns an associative array [string => Definition] that maps fully qualified symbol names + * to global Definitions + * + * @return Definition[] + */ + public function getGlobalDefinitions(): array + { + $defs = []; + foreach ($this->getIndexes() as $index) { + foreach ($index->getGlobalDefinitions() as $fqn => $def) { + $defs[$fqn] = $def; + } + } + return $defs; + } + /** * Returns the Definitions that are in the given namespace * diff --git a/src/Index/Index.php b/src/Index/Index.php index 8af0688a..e33bc3c2 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -15,12 +15,19 @@ class Index implements ReadableIndex, \Serializable use EmitterTrait; /** - * An associative array that maps fully qualified symbol names to Definitions + * An associative array that maps fully qualified symbol names to Definitions (global or not) * * @var Definition[] */ private $definitions = []; + /** + * An associative array that maps fully qualified symbol names to global Definitions + * + * @var Definition[] + */ + private $globalDefinitions = []; + /** * An associative array that maps namespaces to an associative array of FQN to Definitions * @@ -92,7 +99,7 @@ public function isStaticComplete(): bool /** * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to Definitions + * to Definitions (global or not) * * @return Definition[] */ @@ -101,6 +108,17 @@ public function getDefinitions(): array return $this->definitions; } + /** + * Returns an associative array [string => Definition] that maps fully qualified symbol names + * to global Definitions + * + * @return Definition[] + */ + public function getGlobalDefinitions(): array + { + return $this->globalDefinitions; + } + /** * Returns the Definitions that are in the given namespace * @@ -144,6 +162,7 @@ public function getDefinition(string $fqn, bool $globalFallback = false) public function setDefinition(string $fqn, Definition $definition) { $this->definitions[$fqn] = $definition; + $this->setGlobalDefinition($fqn, $definition); $this->setNamespaceDefinition($fqn, $definition); $this->emit('definition-added'); } @@ -158,6 +177,7 @@ public function setDefinition(string $fqn, Definition $definition) public function removeDefinition(string $fqn) { unset($this->definitions[$fqn]); + unset($this->globalDefinitions[$fqn]); unset($this->references[$fqn]); $this->removeNamespaceDefinition($fqn); } @@ -227,10 +247,15 @@ public function removeReferenceUri(string $fqn, string $uri) public function unserialize($serialized) { $data = unserialize($serialized); + foreach ($data as $prop => $val) { $this->$prop = $val; } - $this->buildNamespaceDefinitionsIndex(); + + foreach ($this->definitions as $fqn => $definition) { + $this->setGlobalDefinition($fqn, $definition); + $this->setNamespaceDefinition($fqn, $definition); + } } /** @@ -248,12 +273,16 @@ public function serialize() } /** + * Registers a definition to the global definitions index if it is global + * + * @param string $fqn The fully qualified name of the symbol + * @param Definition $definition The Definition object * @return void */ - private function buildNamespaceDefinitionsIndex() + private function setGlobalDefinition(string $fqn, Definition $definition) { - foreach ($this->definitions as $fqn => $definition) { - $this->setNamespaceDefinition($fqn, $definition); + if ($definition->isGlobal) { + $this->globalDefinitions[$fqn] = $definition; } } diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index c7d57321..62b9c066 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -31,12 +31,20 @@ public function isStaticComplete(): bool; /** * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to Definitions + * to Definitions (global or not) * * @return Definitions[] */ public function getDefinitions(): array; + /** + * Returns an associative array [string => Definition] that maps fully qualified symbol names + * to global Definitions + * + * @return Definitions[] + */ + public function getGlobalDefinitions(): array; + /** * Returns the Definitions that are in the given namespace * From 6d30035c783c88b8e08306414fbcf78168fccc0d Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Tue, 8 Aug 2017 23:33:40 +0200 Subject: [PATCH 06/35] also remove empty namespace index --- src/Index/Index.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Index/Index.php b/src/Index/Index.php index e33bc3c2..bf036223 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -313,6 +313,10 @@ private function removeNamespaceDefinition(string $fqn) $namespace = $this->extractNamespace($fqn); if (isset($this->namespaceDefinitions[$namespace])) { unset($this->namespaceDefinitions[$namespace][$fqn]); + + if (0 === sizeof($this->namespaceDefinitions[$namespace])) { + unset($this->namespaceDefinitions[$namespace]); + } } } From ca0caf16785689c738fca8443f10f06223ccc4d1 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Wed, 9 Aug 2017 22:10:13 +0200 Subject: [PATCH 07/35] store definitions under the namespaceDefinitions cache key directly --- src/Index/AbstractAggregateIndex.php | 14 ++-- src/Index/Index.php | 98 +++++++++++----------------- src/Index/ReadableIndex.php | 8 +-- 3 files changed, 48 insertions(+), 72 deletions(-) diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index 038117a9..ca4b7d49 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -99,20 +99,16 @@ public function isStaticComplete(): bool } /** - * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to Definitions (global or not) + * Returns a Generator providing an associative array [string => Definition] + * that maps fully qualified symbol names to Definitions (global or not) * - * @return Definition[] + * @return \Generator providing Definition[] */ - public function getDefinitions(): array + public function getDefinitions(): \Generator { - $defs = []; foreach ($this->getIndexes() as $index) { - foreach ($index->getDefinitions() as $fqn => $def) { - $defs[$fqn] = $def; - } + yield $index->getDefinitions(); } - return $defs; } /** diff --git a/src/Index/Index.php b/src/Index/Index.php index bf036223..ffd59419 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -15,11 +15,12 @@ class Index implements ReadableIndex, \Serializable use EmitterTrait; /** - * An associative array that maps fully qualified symbol names to Definitions (global or not) + * An associative array that maps namespaces to + * an associative array that maps fully qualified symbol names to global Definitions * - * @var Definition[] + * @var array */ - private $definitions = []; + private $namespaceDefinitions = []; /** * An associative array that maps fully qualified symbol names to global Definitions @@ -28,13 +29,6 @@ class Index implements ReadableIndex, \Serializable */ private $globalDefinitions = []; - /** - * An associative array that maps namespaces to an associative array of FQN to Definitions - * - * @var Definition[] - */ - private $namespaceDefinitions = []; - /** * An associative array that maps fully qualified symbol names to arrays of document URIs that reference the symbol * @@ -98,14 +92,18 @@ public function isStaticComplete(): bool } /** - * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to Definitions (global or not) + * Returns a Generator providing an associative array [string => Definition] + * that maps fully qualified symbol names to Definitions (global or not) * - * @return Definition[] + * @return \Generator providing Definition[] */ - public function getDefinitions(): array + public function getDefinitions(): \Generator { - return $this->definitions; + foreach ($this->namespaceDefinitions as $namespaceDefinition) { + foreach ($namespaceDefinition as $fqn => $definition) { + yield $fqn => $definition; + } + } } /** @@ -142,9 +140,13 @@ public function getDefinitionsForNamespace(string $namespace): array */ public function getDefinition(string $fqn, bool $globalFallback = false) { - if (isset($this->definitions[$fqn])) { - return $this->definitions[$fqn]; + $namespace = $this->extractNamespace($fqn); + $definitions = $this->getDefinitionsForNamespace($namespace); + + if (isset($definitions[$fqn])) { + return $definitions[$fqn]; } + if ($globalFallback) { $parts = explode('\\', $fqn); $fqn = end($parts); @@ -161,9 +163,13 @@ public function getDefinition(string $fqn, bool $globalFallback = false) */ public function setDefinition(string $fqn, Definition $definition) { - $this->definitions[$fqn] = $definition; + $namespace = $this->extractNamespace($fqn); + if (!isset($this->namespaceDefinitions[$namespace])) { + $this->namespaceDefinitions[$namespace] = []; + } + + $this->namespaceDefinitions[$namespace][$fqn] = $definition; $this->setGlobalDefinition($fqn, $definition); - $this->setNamespaceDefinition($fqn, $definition); $this->emit('definition-added'); } @@ -176,10 +182,17 @@ public function setDefinition(string $fqn, Definition $definition) */ public function removeDefinition(string $fqn) { - unset($this->definitions[$fqn]); + $namespace = $this->extractNamespace($fqn); + if (isset($this->namespaceDefinitions[$namespace])) { + unset($this->namespaceDefinitions[$namespace][$fqn]); + + if (empty($this->namespaceDefinitions[$namespace])) { + unset($this->namespaceDefinitions[$namespace]); + } + } + unset($this->globalDefinitions[$fqn]); unset($this->references[$fqn]); - $this->removeNamespaceDefinition($fqn); } /** @@ -252,9 +265,10 @@ public function unserialize($serialized) $this->$prop = $val; } - foreach ($this->definitions as $fqn => $definition) { - $this->setGlobalDefinition($fqn, $definition); - $this->setNamespaceDefinition($fqn, $definition); + foreach ($this->namespaceDefinitions as $namespaceDefinition) { + foreach ($namespaceDefinition as $fqn => $definition) { + $this->setGlobalDefinition($fqn, $definition); + } } } @@ -265,7 +279,7 @@ public function unserialize($serialized) public function serialize() { return serialize([ - 'definitions' => $this->definitions, + 'namespaceDefinitions' => $this->namespaceDefinitions, 'references' => $this->references, 'complete' => $this->complete, 'staticComplete' => $this->staticComplete @@ -286,40 +300,6 @@ private function setGlobalDefinition(string $fqn, Definition $definition) } } - /** - * Registers a definition to a namespace - * - * @param string $fqn The fully qualified name of the symbol - * @param Definition $definition The Definition object - * @return void - */ - private function setNamespaceDefinition(string $fqn, Definition $definition) - { - $namespace = $this->extractNamespace($fqn); - if (!isset($this->namespaceDefinitions[$namespace])) { - $this->namespaceDefinitions[$namespace] = []; - } - $this->namespaceDefinitions[$namespace][$fqn] = $definition; - } - - /** - * Removes a definition from a namespace - * - * @param string $fqn The fully qualified name of the symbol - * @return void - */ - private function removeNamespaceDefinition(string $fqn) - { - $namespace = $this->extractNamespace($fqn); - if (isset($this->namespaceDefinitions[$namespace])) { - unset($this->namespaceDefinitions[$namespace][$fqn]); - - if (0 === sizeof($this->namespaceDefinitions[$namespace])) { - unset($this->namespaceDefinitions[$namespace]); - } - } - } - /** * @param string $fqn * @return string The namespace extracted from the given FQN diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 62b9c066..0c5efca5 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -30,12 +30,12 @@ public function isComplete(): bool; public function isStaticComplete(): bool; /** - * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to Definitions (global or not) + * Returns a Generator providing an associative array [string => Definition] + * that maps fully qualified symbol names to Definitions (global or not) * - * @return Definitions[] + * @return \Generator providing Definition[] */ - public function getDefinitions(): array; + public function getDefinitions(): \Generator; /** * Returns an associative array [string => Definition] that maps fully qualified symbol names From 8768b698d5ce011f67b930355b31c5c75a69ecbb Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Wed, 9 Aug 2017 22:51:42 +0200 Subject: [PATCH 08/35] use Generators to get definitions without wasting memory --- src/Index/AbstractAggregateIndex.php | 30 ++++++++++---------- src/Index/Index.php | 41 +++++++++++++++++++--------- src/Index/ReadableIndex.php | 14 +++++----- 3 files changed, 49 insertions(+), 36 deletions(-) diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index ca4b7d49..a5826abb 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -107,42 +107,40 @@ public function isStaticComplete(): bool public function getDefinitions(): \Generator { foreach ($this->getIndexes() as $index) { - yield $index->getDefinitions(); + foreach ($index->getDefinitions() as $fqn => $definitions) { + yield $fqn => $definition; + } } } /** - * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to global Definitions + * Returns a Generator providing an associative array [string => Definition] + * that maps fully qualified symbol names to global Definitions * - * @return Definition[] + * @return \Generator providing Definitions[] */ - public function getGlobalDefinitions(): array + public function getGlobalDefinitions(): \Generator { - $defs = []; foreach ($this->getIndexes() as $index) { - foreach ($index->getGlobalDefinitions() as $fqn => $def) { - $defs[$fqn] = $def; + foreach ($index->getGlobalDefinitions() as $fqn => $definition) { + yield $fqn => $definition; } } - return $defs; } /** - * Returns the Definitions that are in the given namespace + * Returns a Generator providing the Definitions that are in the given namespace * * @param string $namespace - * @return Definitions[] + * @return \Generator providing Definitions[] */ - public function getDefinitionsForNamespace(string $namespace): array + public function getDefinitionsForNamespace(string $namespace): \Generator { - $defs = []; foreach ($this->getIndexes() as $index) { - foreach ($index->getDefinitionsForNamespace($namespace) as $fqn => $def) { - $defs[$fqn] = $def; + foreach ($index->getDefinitionsForNamespace($namespace) as $fqn => $definition) { + yield $fqn => $definition; } } - return $defs; } /** diff --git a/src/Index/Index.php b/src/Index/Index.php index ffd59419..91302467 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -107,28 +107,29 @@ public function getDefinitions(): \Generator } /** - * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to global Definitions + * Returns a Generator providing an associative array [string => Definition] + * that maps fully qualified symbol names to global Definitions * - * @return Definition[] + * @return \Generator providing Definitions[] */ - public function getGlobalDefinitions(): array + public function getGlobalDefinitions(): \Generator { - return $this->globalDefinitions; + foreach ($this->globalDefinitions as $fqn => $definition) { + yield $fqn => $definition; + } } /** - * Returns the Definitions that are in the given namespace + * Returns a Generator providing the Definitions that are in the given namespace * * @param string $namespace - * @return Definitions[] + * @return \Generator providing Definitions[] */ - public function getDefinitionsForNamespace(string $namespace): array + public function getDefinitionsForNamespace(string $namespace): \Generator { - return isset($this->namespaceDefinitions[$namespace]) - ? $this->namespaceDefinitions[$namespace] - : [] - ; + foreach ($this->doGetDefinitionsForNamespace($namespace) as $fqn => $definition) { + yield $fqn => $definition; + } } /** @@ -141,7 +142,7 @@ public function getDefinitionsForNamespace(string $namespace): array public function getDefinition(string $fqn, bool $globalFallback = false) { $namespace = $this->extractNamespace($fqn); - $definitions = $this->getDefinitionsForNamespace($namespace); + $definitions = $this->doGetDefinitionsForNamespace($namespace); if (isset($definitions[$fqn])) { return $definitions[$fqn]; @@ -314,4 +315,18 @@ private function extractNamespace(string $fqn): string return $fqn; } + + /** + * Returns the Definitions that are in the given namespace + * + * @param string $namespace + * @return Definition[] + */ + private function doGetDefinitionsForNamespace(string $namespace): array + { + return isset($this->namespaceDefinitions[$namespace]) + ? $this->namespaceDefinitions[$namespace] + : [] + ; + } } diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 0c5efca5..8d4e1cad 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -38,20 +38,20 @@ public function isStaticComplete(): bool; public function getDefinitions(): \Generator; /** - * Returns an associative array [string => Definition] that maps fully qualified symbol names - * to global Definitions + * Returns a Generator providing an associative array [string => Definition] + * that maps fully qualified symbol names to global Definitions * - * @return Definitions[] + * @return \Generator providing Definitions[] */ - public function getGlobalDefinitions(): array; + public function getGlobalDefinitions(): \Generator; /** - * Returns the Definitions that are in the given namespace + * Returns a Generator providing the Definitions that are in the given namespace * * @param string $namespace - * @return Definitions[] + * @return \Generator providing Definitions[] */ - public function getDefinitionsForNamespace(string $namespace): array; + public function getDefinitionsForNamespace(string $namespace): \Generator; /** * Returns the Definition object by a specific FQN From 8801edb7a2e4f3bf9bcd9c6444ddbead43b0fbeb Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Thu, 10 Aug 2017 00:06:53 +0200 Subject: [PATCH 09/35] also yield URIs to save memory --- src/Index/AbstractAggregateIndex.php | 12 +++++------- src/Index/Index.php | 15 +++++++++++---- src/Index/ReadableIndex.php | 6 +++--- src/Server/TextDocument.php | 20 ++++++++++++++++---- 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index a5826abb..7ba38701 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -160,19 +160,17 @@ public function getDefinition(string $fqn, bool $globalFallback = false) } /** - * Returns all URIs in this index that reference a symbol + * Returns a Generator providing all URIs in this index that reference a symbol * * @param string $fqn The fully qualified name of the symbol - * @return string[] + * @return \Generator providing string[] */ - public function getReferenceUris(string $fqn): array + public function getReferenceUris(string $fqn): \Generator { - $refs = []; foreach ($this->getIndexes() as $index) { - foreach ($index->getReferenceUris($fqn) as $ref) { - $refs[] = $ref; + foreach ($index->getReferenceUris($fqn) as $uri) { + yield $uri; } } - return $refs; } } diff --git a/src/Index/Index.php b/src/Index/Index.php index 91302467..8d8675d3 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -197,14 +197,21 @@ public function removeDefinition(string $fqn) } /** - * Returns all URIs in this index that reference a symbol + * Returns a Generator providing all URIs in this index that reference a symbol * * @param string $fqn The fully qualified name of the symbol - * @return string[] + * @return \Generator providing string[] */ - public function getReferenceUris(string $fqn): array + public function getReferenceUris(string $fqn): \Generator { - return $this->references[$fqn] ?? []; + $uris = isset($this->references[$fqn]) + ? $this->references[$fqn] + : [] + ; + + foreach ($uris as $uri) { + yield $uri; + } } /** diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 8d4e1cad..6e2b8c61 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -63,10 +63,10 @@ public function getDefinitionsForNamespace(string $namespace): \Generator; public function getDefinition(string $fqn, bool $globalFallback = false); /** - * Returns all URIs in this index that reference a symbol + * Returns a Generator providing all URIs in this index that reference a symbol * * @param string $fqn The fully qualified name of the symbol - * @return string[] + * @return \Generator providing string[] */ - public function getReferenceUris(string $fqn): array; + public function getReferenceUris(string $fqn): \Generator; } diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index 58e7074a..46e822ac 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -219,10 +219,9 @@ public function references( return []; } } - $refDocuments = yield Promise\all(array_map( - [$this->documentLoader, 'getOrLoad'], - $this->index->getReferenceUris($fqn) - )); + $refDocuments = yield Promise\all(iterator_to_array( + $this->getOrLoadReferences($fqn)) + ); foreach ($refDocuments as $document) { $refs = $document->getReferenceNodesByFqn($fqn); if ($refs !== null) { @@ -397,4 +396,17 @@ public function xdefinition(TextDocumentIdentifier $textDocument, Position $posi return [new SymbolLocationInformation($descriptor, $def->symbolInformation->location)]; }); } + + /** + * Gets or loads the documents referencing the given FQN. + * + * @param string $fqn + * @return \Generator providing Promise + */ + private function getOrLoadReferences(string $fqn): \Generator + { + foreach ($this->index->getReferenceUris($fqn) as $ref) { + yield $this->documentLoader->getOrLoad($ref); + } + } } From 6a41a7f0dc95abf428989f31125c578e98968088 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Thu, 10 Aug 2017 10:01:18 +0200 Subject: [PATCH 10/35] add example of indexed definitions --- src/Index/Index.php | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Index/Index.php b/src/Index/Index.php index 8d8675d3..a33d9adb 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -16,21 +16,30 @@ class Index implements ReadableIndex, \Serializable /** * An associative array that maps namespaces to - * an associative array that maps fully qualified symbol names to global Definitions + * an associative array that maps fully qualified symbol names + * to global Definitions, e.g. : + * [ + * 'Psr\Log\LoggerInterface' => [ + * 'Psr\Log\LoggerInterface->log()' => $definition, + * 'Psr\Log\LoggerInterface->debug()' => $definition, + * ], + * ] * * @var array */ private $namespaceDefinitions = []; /** - * An associative array that maps fully qualified symbol names to global Definitions + * An associative array that maps fully qualified symbol names + * to global Definitions * * @var Definition[] */ private $globalDefinitions = []; /** - * An associative array that maps fully qualified symbol names to arrays of document URIs that reference the symbol + * An associative array that maps fully qualified symbol names + * to arrays of document URIs that reference the symbol * * @var string[][] */ From 6a368280691d0ba24a36ada8992398dc2fcf9f36 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Thu, 10 Aug 2017 20:42:08 +0200 Subject: [PATCH 11/35] avoid useless array --- src/CompletionProvider.php | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 59348138..0548e498 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -202,13 +202,8 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi ); // The namespaces of the symbol and its parents (eg the implemented interfaces) - $namespaces = []; foreach ($this->expandParentFqns($fqns) as $namespace) { - $namespaces[] = $namespace; - } - - // Collect namespaces definitions - foreach ($namespaces as $namespace) { + // Collect namespaces definitions foreach ($this->index->getDefinitionsForNamespace($namespace) as $fqn => $def) { // Add the object access operator to only get members of all parents $prefix = $namespace . '->'; @@ -237,13 +232,8 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi ); // The namespaces of the symbol and its parents (eg the implemented interfaces) - $namespaces = []; foreach ($this->expandParentFqns($fqns) as $namespace) { - $namespaces[] = $namespace; - } - - // Collect namespaces definitions - foreach ($namespaces as $namespace) { + // Collect namespaces definitions foreach ($this->index->getDefinitionsForNamespace($namespace) as $fqn => $def) { // Append :: operator to only get static members of all parents $prefix = strtolower($namespace . '::'); From e34d8e15dd780d96c85b74e5cb8c9c1c03d95b20 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Thu, 10 Aug 2017 22:03:16 +0200 Subject: [PATCH 12/35] use of null coalescing operator --- src/Index/Index.php | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/src/Index/Index.php b/src/Index/Index.php index a33d9adb..649ba297 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -136,7 +136,7 @@ public function getGlobalDefinitions(): \Generator */ public function getDefinitionsForNamespace(string $namespace): \Generator { - foreach ($this->doGetDefinitionsForNamespace($namespace) as $fqn => $definition) { + foreach ($this->namespaceDefinitions[$namespace] ?? [] as $fqn => $definition) { yield $fqn => $definition; } } @@ -151,7 +151,7 @@ public function getDefinitionsForNamespace(string $namespace): \Generator public function getDefinition(string $fqn, bool $globalFallback = false) { $namespace = $this->extractNamespace($fqn); - $definitions = $this->doGetDefinitionsForNamespace($namespace); + $definitions = $this->namespaceDefinitions[$namespace] ?? []; if (isset($definitions[$fqn])) { return $definitions[$fqn]; @@ -213,12 +213,7 @@ public function removeDefinition(string $fqn) */ public function getReferenceUris(string $fqn): \Generator { - $uris = isset($this->references[$fqn]) - ? $this->references[$fqn] - : [] - ; - - foreach ($uris as $uri) { + foreach ($this->references[$fqn] ?? [] as $uri) { yield $uri; } } @@ -331,18 +326,4 @@ private function extractNamespace(string $fqn): string return $fqn; } - - /** - * Returns the Definitions that are in the given namespace - * - * @param string $namespace - * @return Definition[] - */ - private function doGetDefinitionsForNamespace(string $namespace): array - { - return isset($this->namespaceDefinitions[$namespace]) - ? $this->namespaceDefinitions[$namespace] - : [] - ; - } } From 14f840bd2f5d9accd81b949d9926e18e880569bf Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Thu, 5 Oct 2017 20:45:57 +0200 Subject: [PATCH 13/35] use correct terminology --- src/CompletionProvider.php | 20 +++++------ src/Index/AbstractAggregateIndex.php | 10 +++--- src/Index/Index.php | 52 ++++++++++++++-------------- src/Index/ReadableIndex.php | 6 ++-- 4 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 0548e498..f058baca 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -201,12 +201,12 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi $this->definitionResolver->resolveExpressionNodeToType($node->dereferencableExpression) ); - // The namespaces of the symbol and its parents (eg the implemented interfaces) - foreach ($this->expandParentFqns($fqns) as $namespace) { - // Collect namespaces definitions - foreach ($this->index->getDefinitionsForNamespace($namespace) as $fqn => $def) { + // The FQNs of the symbol and its parents (eg the implemented interfaces) + foreach ($this->expandParentFqns($fqns) as $parentFqn) { + // Collect fqn definitions + foreach ($this->index->getDefinitionsForFqn($parentFqn) as $fqn => $def) { // Add the object access operator to only get members of all parents - $prefix = $namespace . '->'; + $prefix = $parentFqn . '->'; if (substr($fqn, 0, strlen($prefix)) === $prefix && !$def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } @@ -231,12 +231,12 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi $classType = $this->definitionResolver->resolveExpressionNodeToType($scoped->scopeResolutionQualifier) ); - // The namespaces of the symbol and its parents (eg the implemented interfaces) - foreach ($this->expandParentFqns($fqns) as $namespace) { - // Collect namespaces definitions - foreach ($this->index->getDefinitionsForNamespace($namespace) as $fqn => $def) { + // The FQNs of the symbol and its parents (eg the implemented interfaces) + foreach ($this->expandParentFqns($fqns) as $parentFqn) { + // Collect fqn definitions + foreach ($this->index->getDefinitionsForFqn($parentFqn) as $fqn => $def) { // Append :: operator to only get static members of all parents - $prefix = strtolower($namespace . '::'); + $prefix = strtolower($parentFqn . '::'); if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && !$def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index 7ba38701..e4e33b38 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -129,16 +129,16 @@ public function getGlobalDefinitions(): \Generator } /** - * Returns a Generator providing the Definitions that are in the given namespace + * Returns a Generator providing the Definitions that are in the given FQN * - * @param string $namespace + * @param string $fqn * @return \Generator providing Definitions[] */ - public function getDefinitionsForNamespace(string $namespace): \Generator + public function getDefinitionsForFqn(string $fqn): \Generator { foreach ($this->getIndexes() as $index) { - foreach ($index->getDefinitionsForNamespace($namespace) as $fqn => $definition) { - yield $fqn => $definition; + foreach ($index->getDefinitionsForFqn($fqn) as $symbolFqn => $definition) { + yield $symbolFqn => $definition; } } } diff --git a/src/Index/Index.php b/src/Index/Index.php index 649ba297..bb1e6b40 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -15,7 +15,7 @@ class Index implements ReadableIndex, \Serializable use EmitterTrait; /** - * An associative array that maps namespaces to + * An associative array that maps fully qualified names to * an associative array that maps fully qualified symbol names * to global Definitions, e.g. : * [ @@ -27,7 +27,7 @@ class Index implements ReadableIndex, \Serializable * * @var array */ - private $namespaceDefinitions = []; + private $fqnDefinitions = []; /** * An associative array that maps fully qualified symbol names @@ -108,8 +108,8 @@ public function isStaticComplete(): bool */ public function getDefinitions(): \Generator { - foreach ($this->namespaceDefinitions as $namespaceDefinition) { - foreach ($namespaceDefinition as $fqn => $definition) { + foreach ($this->fqnDefinitions as $fqnDefinition) { + foreach ($fqnDefinition as $fqn => $definition) { yield $fqn => $definition; } } @@ -129,15 +129,15 @@ public function getGlobalDefinitions(): \Generator } /** - * Returns a Generator providing the Definitions that are in the given namespace + * Returns a Generator providing the Definitions that are in the given FQN * - * @param string $namespace + * @param string $fqn * @return \Generator providing Definitions[] */ - public function getDefinitionsForNamespace(string $namespace): \Generator + public function getDefinitionsForFqn(string $fqn): \Generator { - foreach ($this->namespaceDefinitions[$namespace] ?? [] as $fqn => $definition) { - yield $fqn => $definition; + foreach ($this->fqnDefinitions[$fqn] ?? [] as $symbolFqn => $definition) { + yield $symbolFqn => $definition; } } @@ -150,8 +150,8 @@ public function getDefinitionsForNamespace(string $namespace): \Generator */ public function getDefinition(string $fqn, bool $globalFallback = false) { - $namespace = $this->extractNamespace($fqn); - $definitions = $this->namespaceDefinitions[$namespace] ?? []; + $namespacedFqn = $this->extractNamespacedFqn($fqn); + $definitions = $this->fqnDefinitions[$namespacedFqn] ?? []; if (isset($definitions[$fqn])) { return $definitions[$fqn]; @@ -173,12 +173,12 @@ public function getDefinition(string $fqn, bool $globalFallback = false) */ public function setDefinition(string $fqn, Definition $definition) { - $namespace = $this->extractNamespace($fqn); - if (!isset($this->namespaceDefinitions[$namespace])) { - $this->namespaceDefinitions[$namespace] = []; + $namespacedFqn = $this->extractNamespacedFqn($fqn); + if (!isset($this->fqnDefinitions[$namespacedFqn])) { + $this->fqnDefinitions[$namespacedFqn] = []; } - $this->namespaceDefinitions[$namespace][$fqn] = $definition; + $this->fqnDefinitions[$namespacedFqn][$fqn] = $definition; $this->setGlobalDefinition($fqn, $definition); $this->emit('definition-added'); } @@ -192,12 +192,12 @@ public function setDefinition(string $fqn, Definition $definition) */ public function removeDefinition(string $fqn) { - $namespace = $this->extractNamespace($fqn); - if (isset($this->namespaceDefinitions[$namespace])) { - unset($this->namespaceDefinitions[$namespace][$fqn]); + $namespacedFqn = $this->extractNamespacedFqn($fqn); + if (isset($this->fqnDefinitions[$namespacedFqn])) { + unset($this->fqnDefinitions[$namespacedFqn][$fqn]); - if (empty($this->namespaceDefinitions[$namespace])) { - unset($this->namespaceDefinitions[$namespace]); + if (empty($this->fqnDefinitions[$namespacedFqn])) { + unset($this->fqnDefinitions[$namespacedFqn]); } } @@ -277,8 +277,8 @@ public function unserialize($serialized) $this->$prop = $val; } - foreach ($this->namespaceDefinitions as $namespaceDefinition) { - foreach ($namespaceDefinition as $fqn => $definition) { + foreach ($this->fqnDefinitions as $fqnDefinition) { + foreach ($fqnDefinition as $fqn => $definition) { $this->setGlobalDefinition($fqn, $definition); } } @@ -291,7 +291,7 @@ public function unserialize($serialized) public function serialize() { return serialize([ - 'namespaceDefinitions' => $this->namespaceDefinitions, + 'fqnDefinitions' => $this->fqnDefinitions, 'references' => $this->references, 'complete' => $this->complete, 'staticComplete' => $this->staticComplete @@ -313,10 +313,10 @@ private function setGlobalDefinition(string $fqn, Definition $definition) } /** - * @param string $fqn - * @return string The namespace extracted from the given FQN + * @param string $fqn The symbol FQN + * @return string The namespaced FQN extracted from the given symbol FQN */ - private function extractNamespace(string $fqn): string + private function extractNamespacedFqn(string $fqn): string { foreach (['::', '->'] as $operator) { if (false !== ($pos = strpos($fqn, $operator))) { diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 6e2b8c61..7d3750ef 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -46,12 +46,12 @@ public function getDefinitions(): \Generator; public function getGlobalDefinitions(): \Generator; /** - * Returns a Generator providing the Definitions that are in the given namespace + * Returns a Generator providing the Definitions that are in the given FQN * - * @param string $namespace + * @param string $fqn * @return \Generator providing Definitions[] */ - public function getDefinitionsForNamespace(string $namespace): \Generator; + public function getDefinitionsForFqn(string $fqn): \Generator; /** * Returns the Definition object by a specific FQN From e9fd572a4978acb13e111d00c319361f505264a6 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Mon, 13 Nov 2017 20:57:22 +0100 Subject: [PATCH 14/35] consider the merge of #511 --- src/Index/Index.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Index/Index.php b/src/Index/Index.php index bb1e6b40..a5532b7e 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -17,7 +17,7 @@ class Index implements ReadableIndex, \Serializable /** * An associative array that maps fully qualified names to * an associative array that maps fully qualified symbol names - * to global Definitions, e.g. : + * to Definitions, e.g. : * [ * 'Psr\Log\LoggerInterface' => [ * 'Psr\Log\LoggerInterface->log()' => $definition, @@ -31,7 +31,7 @@ class Index implements ReadableIndex, \Serializable /** * An associative array that maps fully qualified symbol names - * to global Definitions + * to global (ie non member) Definitions * * @var Definition[] */ @@ -307,7 +307,7 @@ public function serialize() */ private function setGlobalDefinition(string $fqn, Definition $definition) { - if ($definition->isGlobal) { + if ($definition->isMember) { $this->globalDefinitions[$fqn] = $definition; } } From d1f85f15b6181b08ea37ea0ca3c9463865198a7b Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Mon, 13 Nov 2017 21:26:43 +0100 Subject: [PATCH 15/35] use tree representation index --- src/CompletionProvider.php | 32 +-- src/Index/AbstractAggregateIndex.php | 37 ++-- src/Index/Index.php | 292 +++++++++++++++++++-------- src/Index/ReadableIndex.php | 8 - 4 files changed, 244 insertions(+), 125 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index f058baca..bf107d5d 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -207,7 +207,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi foreach ($this->index->getDefinitionsForFqn($parentFqn) as $fqn => $def) { // Add the object access operator to only get members of all parents $prefix = $parentFqn . '->'; - if (substr($fqn, 0, strlen($prefix)) === $prefix && !$def->isMember) { + if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } } @@ -237,7 +237,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi foreach ($this->index->getDefinitionsForFqn($parentFqn) as $fqn => $def) { // Append :: operator to only get static members of all parents $prefix = strtolower($parentFqn . '::'); - if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && !$def->isMember) { + if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && $def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } } @@ -301,25 +301,29 @@ public function provideCompletion(PhpDocument $doc, Position $pos): CompletionLi } } - // Suggest global symbols that either + // 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->getGlobalDefinitions() as $fqn => $def) { + foreach ($this->index->getDefinitions() as $fqn => $def) { $fqnStartsWithPrefix = substr($fqn, 0, $prefixLen) === $prefix; if ( - !$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 + // Exclude methods, properties etc. + !$def->isMember + && ( + !$prefix || ( - $namespaceNode - && !$isFullyQualified - && substr($fqn, 0, $namespacedPrefixLen) === $namespacedPrefix + // 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` diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index e4e33b38..8c88659e 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -107,24 +107,11 @@ public function isStaticComplete(): bool public function getDefinitions(): \Generator { foreach ($this->getIndexes() as $index) { - foreach ($index->getDefinitions() as $fqn => $definitions) { - yield $fqn => $definition; - } - } - } + // foreach ($index->getDefinitions() as $fqn => $definition) { + // yield $fqn => $definition; + // } - /** - * Returns a Generator providing an associative array [string => Definition] - * that maps fully qualified symbol names to global Definitions - * - * @return \Generator providing Definitions[] - */ - public function getGlobalDefinitions(): \Generator - { - foreach ($this->getIndexes() as $index) { - foreach ($index->getGlobalDefinitions() as $fqn => $definition) { - yield $fqn => $definition; - } + yield from $index->getDefinitions(); } } @@ -137,9 +124,11 @@ public function getGlobalDefinitions(): \Generator public function getDefinitionsForFqn(string $fqn): \Generator { foreach ($this->getIndexes() as $index) { - foreach ($index->getDefinitionsForFqn($fqn) as $symbolFqn => $definition) { - yield $symbolFqn => $definition; - } + // foreach ($index->getDefinitionsForFqn($fqn) as $symbolFqn => $definition) { + // yield $symbolFqn => $definition; + // } + + yield from $index->getDefinitionsForFqn($fqn); } } @@ -168,9 +157,11 @@ public function getDefinition(string $fqn, bool $globalFallback = false) public function getReferenceUris(string $fqn): \Generator { foreach ($this->getIndexes() as $index) { - foreach ($index->getReferenceUris($fqn) as $uri) { - yield $uri; - } + // foreach ($index->getReferenceUris($fqn) as $uri) { + // yield $uri; + // } + + yield from $index->getReferenceUris($fqn); } } } diff --git a/src/Index/Index.php b/src/Index/Index.php index a5532b7e..af8eae03 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -15,27 +15,21 @@ class Index implements ReadableIndex, \Serializable use EmitterTrait; /** - * An associative array that maps fully qualified names to - * an associative array that maps fully qualified symbol names - * to Definitions, e.g. : - * [ - * 'Psr\Log\LoggerInterface' => [ - * 'Psr\Log\LoggerInterface->log()' => $definition, - * 'Psr\Log\LoggerInterface->debug()' => $definition, + * An associative array that maps splitted fully qualified symbol names + * to definitions, eg : + * [ + * 'Psr' => [ + * '\Log' => [ + * '\LoggerInterface' => [ + * '->log()' => $definition, + * ], * ], - * ] + * ], + * ] * * @var array */ - private $fqnDefinitions = []; - - /** - * An associative array that maps fully qualified symbol names - * to global (ie non member) Definitions - * - * @var Definition[] - */ - private $globalDefinitions = []; + private $definitions = []; /** * An associative array that maps fully qualified symbol names @@ -108,24 +102,13 @@ public function isStaticComplete(): bool */ public function getDefinitions(): \Generator { - foreach ($this->fqnDefinitions as $fqnDefinition) { - foreach ($fqnDefinition as $fqn => $definition) { - yield $fqn => $definition; - } - } - } + // foreach ($this->fqnDefinitions as $fqnDefinition) { + // foreach ($fqnDefinition as $fqn => $definition) { + // yield $fqn => $definition; + // } + // } - /** - * Returns a Generator providing an associative array [string => Definition] - * that maps fully qualified symbol names to global Definitions - * - * @return \Generator providing Definitions[] - */ - public function getGlobalDefinitions(): \Generator - { - foreach ($this->globalDefinitions as $fqn => $definition) { - yield $fqn => $definition; - } + yield from $this->generateDefinitionsRecursively($this->definitions); } /** @@ -136,8 +119,17 @@ public function getGlobalDefinitions(): \Generator */ public function getDefinitionsForFqn(string $fqn): \Generator { - foreach ($this->fqnDefinitions[$fqn] ?? [] as $symbolFqn => $definition) { - yield $symbolFqn => $definition; + // foreach ($this->fqnDefinitions[$fqn] ?? [] as $symbolFqn => $definition) { + // yield $symbolFqn => $definition; + // } + + $parts = $this->splitFqn($fqn); + $result = $this->getIndexValue($parts, $this->definitions); + + if ($result instanceof Definition) { + yield $fqn => $definition; + } elseif (is_array($result)) { + yield from $this->generateDefinitionsRecursively($result, $fqn); } } @@ -150,18 +142,23 @@ public function getDefinitionsForFqn(string $fqn): \Generator */ public function getDefinition(string $fqn, bool $globalFallback = false) { - $namespacedFqn = $this->extractNamespacedFqn($fqn); - $definitions = $this->fqnDefinitions[$namespacedFqn] ?? []; + // $namespacedFqn = $this->extractNamespacedFqn($fqn); + // $definitions = $this->fqnDefinitions[$namespacedFqn] ?? []; - if (isset($definitions[$fqn])) { - return $definitions[$fqn]; - } + // if (isset($definitions[$fqn])) { + // return $definitions[$fqn]; + // } - if ($globalFallback) { - $parts = explode('\\', $fqn); - $fqn = end($parts); - return $this->getDefinition($fqn); - } + // if ($globalFallback) { + // $parts = explode('\\', $fqn); + // $fqn = end($parts); + // return $this->getDefinition($fqn); + // } + + $parts = $this->splitFqn($fqn); + $result = $this->getIndexValue($parts, $this->definitions); + + return $result instanceof Definition ?? null; } /** @@ -173,13 +170,16 @@ public function getDefinition(string $fqn, bool $globalFallback = false) */ public function setDefinition(string $fqn, Definition $definition) { - $namespacedFqn = $this->extractNamespacedFqn($fqn); - if (!isset($this->fqnDefinitions[$namespacedFqn])) { - $this->fqnDefinitions[$namespacedFqn] = []; - } + // $namespacedFqn = $this->extractNamespacedFqn($fqn); + // if (!isset($this->fqnDefinitions[$namespacedFqn])) { + // $this->fqnDefinitions[$namespacedFqn] = []; + // } + + // $this->fqnDefinitions[$namespacedFqn][$fqn] = $definition; + + $parts = $this->splitFqn($fqn); + $this->indexDefinition(0, $parts, $this->definitions, $definition); - $this->fqnDefinitions[$namespacedFqn][$fqn] = $definition; - $this->setGlobalDefinition($fqn, $definition); $this->emit('definition-added'); } @@ -192,16 +192,18 @@ public function setDefinition(string $fqn, Definition $definition) */ public function removeDefinition(string $fqn) { - $namespacedFqn = $this->extractNamespacedFqn($fqn); - if (isset($this->fqnDefinitions[$namespacedFqn])) { - unset($this->fqnDefinitions[$namespacedFqn][$fqn]); + // $namespacedFqn = $this->extractNamespacedFqn($fqn); + // if (isset($this->fqnDefinitions[$namespacedFqn])) { + // unset($this->fqnDefinitions[$namespacedFqn][$fqn]); - if (empty($this->fqnDefinitions[$namespacedFqn])) { - unset($this->fqnDefinitions[$namespacedFqn]); - } - } + // if (empty($this->fqnDefinitions[$namespacedFqn])) { + // unset($this->fqnDefinitions[$namespacedFqn]); + // } + // } + + $parts = $this->splitFqn($fqn); + $this->removeIndexedDefinition(0, $parts, $this->definitions); - unset($this->globalDefinitions[$fqn]); unset($this->references[$fqn]); } @@ -276,12 +278,6 @@ public function unserialize($serialized) foreach ($data as $prop => $val) { $this->$prop = $val; } - - foreach ($this->fqnDefinitions as $fqnDefinition) { - foreach ($fqnDefinition as $fqn => $definition) { - $this->setGlobalDefinition($fqn, $definition); - } - } } /** @@ -291,7 +287,7 @@ public function unserialize($serialized) public function serialize() { return serialize([ - 'fqnDefinitions' => $this->fqnDefinitions, + 'definitions' => $this->definitions, 'references' => $this->references, 'complete' => $this->complete, 'staticComplete' => $this->staticComplete @@ -299,31 +295,167 @@ public function serialize() } /** - * Registers a definition to the global definitions index if it is global + * @param string $fqn The symbol FQN + * @return string The namespaced FQN extracted from the given symbol FQN + */ + // private function extractNamespacedFqn(string $fqn): string + // { + // foreach (['::', '->'] as $operator) { + // if (false !== ($pos = strpos($fqn, $operator))) { + // return substr($fqn, 0, $pos); + // } + // } + + // return $fqn; + // } + + /** + * Returns a Genrerator containing all the into the given $storage recursively. + * The generator yields key => value pairs, eg + * 'Psr\Log\LoggerInterface->log()' => $definition * - * @param string $fqn The fully qualified name of the symbol - * @param Definition $definition The Definition object - * @return void + * @param array &$storage + * @param string $prefix (optional) + * @return \Generator */ - private function setGlobalDefinition(string $fqn, Definition $definition) + private function generateDefinitionsRecursively(array &$storage, string $prefix = ''): \Generator { - if ($definition->isMember) { - $this->globalDefinitions[$fqn] = $definition; + foreach ($storage as $key => $value) { + $prefix .= $key; + if (!is_array($value)) { + yield $prefix => $value; + } else { + yield from generateDefinitionsRecursively($value, $prefix); + } } } /** - * @param string $fqn The symbol FQN - * @return string The namespaced FQN extracted from the given symbol FQN + * Splits the given FQN into an array, eg : + * 'Psr\Log\LoggerInterface->log' will be ['Psr', '\Log', '\LoggerInterface', '->log()'] + * '\Exception->getMessage()' will be ['\Exception', '->getMessage()'] + * 'PHP_VERSION' will be ['PHP_VERSION'] + * + * @param string $fqn + * @return array */ - private function extractNamespacedFqn(string $fqn): string + private function splitFqn(string $fqn): array { + // split fqn at backslashes + $parts = explode('\\', $fqn); + + // write back the backslach prefix to the first part if it was present + if ('' === $parts[0]) { + $parts = array_slice($parts, 1); + $parts[0] = sprintf('\\%s', $parts[0]); + } + + // write back the backslashes prefixes for the other parts + for ($i = 1; $i < count($parts); $i++) { + $parts[$i] = sprintf('\\%s', $parts[$i]); + } + + // split the last part in 2 parts at the operator + $lastPart = end($parts); foreach (['::', '->'] as $operator) { - if (false !== ($pos = strpos($fqn, $operator))) { - return substr($fqn, 0, $pos); + $endParts = explode($operator, $lastPart); + if (count($endParts) > 1) { + // replace the last part by its pieces + array_pop($parts); + $parts[] = $endParts[0]; + $parts[] = sprintf('%s%s', $operator, $endParts[1]); + break; } } - return $fqn; + return $parts; + } + + /** + * Return the values stored in this index under the given $parts array. + * It can be an index node or a Definition if the $parts are precise + * enough. Returns null when nothing is found. + * + * @param array $parts The splitted FQN + * @param array &$storage The array in which to store the $definition + * @return array|Definition|null + */ + private function getIndexValue(array $parts, array &$storage) + { + $part = $parts[0]; + + if (!isset($storage[$part])) { + return null; + } + + $parts = array_slice($parts, 1); + // we've reached the last provided part + if (0 === count($parts)) { + return $storage[$part]; + } + + return getIndexValue($parts, $storage[$part]); + } + + /** + * Recusrive function which store the given definition in the given $storage + * array represented as a tree matching the given $parts. + * + * @param int $level The current level of FQN part + * @param array $parts The splitted FQN + * @param array &$storage The array in which to store the $definition + * @param Definition $definition The Definition to store + */ + private function indexDefinition(int $level, array $parts, array &$storage, Definition $definition) + { + $part = $parts[$level]; + + if ($level + 1 === count($parts)) { + $storage[$part] = $definition; + + return; + } + + if (!isset($storage[$part])) { + $storage[$part] = []; + } + + $this->indexDefinition($level + 1, $parts, $storage[$part], $definition); + } + + /** + * Recusrive function which remove the definition matching the given $parts + * from the given $storage array. + * The function also looks up recursively to remove the parents of the + * definition which no longer has children to avoid to let empty arrays + * in the index. + * + * @param int $level The current level of FQN part + * @param array $parts The splitted FQN + * @param array &$storage The current array in which to remove data + * @param array &$rootStorage The root storage array + */ + private function removeIndexedDefinition(int $level, array $parts, array &$storage, &$rootStorage) + { + $part = $parts[$level]; + + if ($level + 1 === count($parts)) { + if (isset($storage[$part]) && count($storage[$part]) < 2) { + unset($storage[$part]); + + if (0 === $level) { + // we're at root level, no need to check for parents + // w/o children + return; + } + + array_pop($parts); + // parse again the definition tree to see if the parent + // can be removed too if it has no more children + removeIndexedDefinition(0, $parts, $rootStorage, $rootStorage); + } + } else { + removeIndexedDefinition($level + 1, $parts, $storage[$part], $rootStorage); + } } } diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 7d3750ef..5c54557d 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -37,14 +37,6 @@ public function isStaticComplete(): bool; */ public function getDefinitions(): \Generator; - /** - * Returns a Generator providing an associative array [string => Definition] - * that maps fully qualified symbol names to global Definitions - * - * @return \Generator providing Definitions[] - */ - public function getGlobalDefinitions(): \Generator; - /** * Returns a Generator providing the Definitions that are in the given FQN * From 188a5df382fcbc63956cd63c5564b0a785ba4077 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Tue, 14 Nov 2017 04:41:10 +0100 Subject: [PATCH 16/35] fix definitions storage collision (member / non member) --- src/Index/Index.php | 98 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 78 insertions(+), 20 deletions(-) diff --git a/src/Index/Index.php b/src/Index/Index.php index af8eae03..28e0c85a 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -16,7 +16,7 @@ class Index implements ReadableIndex, \Serializable /** * An associative array that maps splitted fully qualified symbol names - * to definitions, eg : + * to member definitions, eg : * [ * 'Psr' => [ * '\Log' => [ @@ -29,7 +29,23 @@ class Index implements ReadableIndex, \Serializable * * @var array */ - private $definitions = []; + private $memberDefinitions = []; + + /** + * An associative array that maps splitted fully qualified symbol names + * to non member definitions, eg : + * [ + * 'Psr' => [ + * '\Log' => [ + * '\LoggerInterface' => $definition, + * ], + * ], + * ] + * + * @var array + */ + private $nonMemberDefinitions = []; + /** * An associative array that maps fully qualified symbol names @@ -108,7 +124,8 @@ public function getDefinitions(): \Generator // } // } - yield from $this->generateDefinitionsRecursively($this->definitions); + yield from $this->yieldDefinitionsRecursively($this->memberDefinitions); + yield from $this->yieldDefinitionsRecursively($this->nonMemberDefinitions); } /** @@ -124,12 +141,20 @@ public function getDefinitionsForFqn(string $fqn): \Generator // } $parts = $this->splitFqn($fqn); - $result = $this->getIndexValue($parts, $this->definitions); + $result = $this->getIndexValue($parts, $this->memberDefinitions); if ($result instanceof Definition) { - yield $fqn => $definition; + yield $fqn => $result; } elseif (is_array($result)) { - yield from $this->generateDefinitionsRecursively($result, $fqn); + yield from $this->yieldDefinitionsRecursively($result, $fqn); + } else { + $result = $this->getIndexValue($parts, $this->nonMemberDefinitions); + + if ($result instanceof Definition) { + yield $fqn => $result; + } elseif (is_array($result)) { + yield from $this->yieldDefinitionsRecursively($result, $fqn); + } } } @@ -156,9 +181,18 @@ public function getDefinition(string $fqn, bool $globalFallback = false) // } $parts = $this->splitFqn($fqn); - $result = $this->getIndexValue($parts, $this->definitions); + $result = $this->getIndexValue($parts, $this->memberDefinitions); - return $result instanceof Definition ?? null; + if ($result instanceof Definition) { + return $result; + } + + $result = $this->getIndexValue($parts, $this->nonMemberDefinitions); + + return $result instanceof Definition + ? $result + : null + ; } /** @@ -178,7 +212,12 @@ public function setDefinition(string $fqn, Definition $definition) // $this->fqnDefinitions[$namespacedFqn][$fqn] = $definition; $parts = $this->splitFqn($fqn); - $this->indexDefinition(0, $parts, $this->definitions, $definition); + + if ($definition->isMember) { + $this->indexDefinition(0, $parts, $this->memberDefinitions, $definition); + } else { + $this->indexDefinition(0, $parts, $this->nonMemberDefinitions, $definition); + } $this->emit('definition-added'); } @@ -202,7 +241,10 @@ public function removeDefinition(string $fqn) // } $parts = $this->splitFqn($fqn); - $this->removeIndexedDefinition(0, $parts, $this->definitions); + + if (true !== $this->removeIndexedDefinition(0, $parts, $this->memberDefinitions)) { + $this->removeIndexedDefinition(0, $parts, $this->nonMemberDefinitions); + } unset($this->references[$fqn]); } @@ -287,7 +329,8 @@ public function unserialize($serialized) public function serialize() { return serialize([ - 'definitions' => $this->definitions, + 'memberDefinitions' => $this->memberDefinitions, + 'nonMemberDefinitions' => $this->nonMemberDefinitions, 'references' => $this->references, 'complete' => $this->complete, 'staticComplete' => $this->staticComplete @@ -318,14 +361,13 @@ public function serialize() * @param string $prefix (optional) * @return \Generator */ - private function generateDefinitionsRecursively(array &$storage, string $prefix = ''): \Generator + private function yieldDefinitionsRecursively(array &$storage, string $prefix = ''): \Generator { foreach ($storage as $key => $value) { - $prefix .= $key; if (!is_array($value)) { - yield $prefix => $value; + yield sprintf('%s%s', $prefix, $key) => $value; } else { - yield from generateDefinitionsRecursively($value, $prefix); + yield from $this->yieldDefinitionsRecursively($value, sprintf('%s%s', $prefix, $key)); } } } @@ -346,7 +388,10 @@ private function splitFqn(string $fqn): array // write back the backslach prefix to the first part if it was present if ('' === $parts[0]) { - $parts = array_slice($parts, 1); + if (count($parts) > 1) { + $parts = array_slice($parts, 1); + } + $parts[0] = sprintf('\\%s', $parts[0]); } @@ -394,7 +439,13 @@ private function getIndexValue(array $parts, array &$storage) return $storage[$part]; } - return getIndexValue($parts, $storage[$part]); + if (!is_array($storage[$part]) && count($parts) > 0) { + // we're looking for a member definition in the non member index, + // no matches can be found. + return null; + } + + return $this->getIndexValue($parts, $storage[$part]); } /** @@ -420,6 +471,12 @@ private function indexDefinition(int $level, array $parts, array &$storage, Defi $storage[$part] = []; } + if (!is_array($storage[$part])) { + // it's a non member definition, we can't add it to the member + // definitions index + return; + } + $this->indexDefinition($level + 1, $parts, $storage[$part], $definition); } @@ -434,6 +491,7 @@ private function indexDefinition(int $level, array $parts, array &$storage, Defi * @param array $parts The splitted FQN * @param array &$storage The current array in which to remove data * @param array &$rootStorage The root storage array + * @return boolean|null True when the definition has been found and removed, null otherwise. */ private function removeIndexedDefinition(int $level, array $parts, array &$storage, &$rootStorage) { @@ -446,16 +504,16 @@ private function removeIndexedDefinition(int $level, array $parts, array &$stora if (0 === $level) { // we're at root level, no need to check for parents // w/o children - return; + return true; } array_pop($parts); // parse again the definition tree to see if the parent // can be removed too if it has no more children - removeIndexedDefinition(0, $parts, $rootStorage, $rootStorage); + return $this->removeIndexedDefinition(0, $parts, $rootStorage, $rootStorage); } } else { - removeIndexedDefinition($level + 1, $parts, $storage[$part], $rootStorage); + return $this->removeIndexedDefinition($level + 1, $parts, $storage[$part], $rootStorage); } } } From cacde1ea72e9d8550c9d1dfbbc2c05f330d613d7 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Tue, 14 Nov 2017 13:40:35 +0100 Subject: [PATCH 17/35] use a single array to store definitions --- src/Index/Index.php | 112 +++++++++++++++++++------------------------- src/Indexer.php | 2 +- 2 files changed, 50 insertions(+), 64 deletions(-) diff --git a/src/Index/Index.php b/src/Index/Index.php index 28e0c85a..45663836 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -16,12 +16,13 @@ class Index implements ReadableIndex, \Serializable /** * An associative array that maps splitted fully qualified symbol names - * to member definitions, eg : + * to definitions, eg : * [ * 'Psr' => [ * '\Log' => [ * '\LoggerInterface' => [ - * '->log()' => $definition, + * '' => $def1, // definition for 'Psr\Log\LoggerInterface' which is non-member + * '->log()' => $def2, // definition for 'Psr\Log\LoggerInterface->log()' which is a member definition * ], * ], * ], @@ -29,23 +30,7 @@ class Index implements ReadableIndex, \Serializable * * @var array */ - private $memberDefinitions = []; - - /** - * An associative array that maps splitted fully qualified symbol names - * to non member definitions, eg : - * [ - * 'Psr' => [ - * '\Log' => [ - * '\LoggerInterface' => $definition, - * ], - * ], - * ] - * - * @var array - */ - private $nonMemberDefinitions = []; - + private $definitions = []; /** * An associative array that maps fully qualified symbol names @@ -124,8 +109,7 @@ public function getDefinitions(): \Generator // } // } - yield from $this->yieldDefinitionsRecursively($this->memberDefinitions); - yield from $this->yieldDefinitionsRecursively($this->nonMemberDefinitions); + yield from $this->yieldDefinitionsRecursively($this->definitions); } /** @@ -141,20 +125,18 @@ public function getDefinitionsForFqn(string $fqn): \Generator // } $parts = $this->splitFqn($fqn); - $result = $this->getIndexValue($parts, $this->memberDefinitions); + if ('' === end($parts)) { + // we want to return all the definitions in the given FQN, not only + // the one matching exactly the FQN. + array_pop($parts); + } + + $result = $this->getIndexValue($parts, $this->definitions); if ($result instanceof Definition) { yield $fqn => $result; } elseif (is_array($result)) { yield from $this->yieldDefinitionsRecursively($result, $fqn); - } else { - $result = $this->getIndexValue($parts, $this->nonMemberDefinitions); - - if ($result instanceof Definition) { - yield $fqn => $result; - } elseif (is_array($result)) { - yield from $this->yieldDefinitionsRecursively($result, $fqn); - } } } @@ -181,18 +163,23 @@ public function getDefinition(string $fqn, bool $globalFallback = false) // } $parts = $this->splitFqn($fqn); - $result = $this->getIndexValue($parts, $this->memberDefinitions); + $result = $this->getIndexValue($parts, $this->definitions); if ($result instanceof Definition) { return $result; } - $result = $this->getIndexValue($parts, $this->nonMemberDefinitions); + if ($globalFallback) { + $parts = explode('\\', $fqn); + $fqn = end($parts); - return $result instanceof Definition - ? $result - : null - ; + return $this->getDefinition($fqn); + } + + // return $result instanceof Definition + // ? $result + // : null + // ; } /** @@ -212,12 +199,7 @@ public function setDefinition(string $fqn, Definition $definition) // $this->fqnDefinitions[$namespacedFqn][$fqn] = $definition; $parts = $this->splitFqn($fqn); - - if ($definition->isMember) { - $this->indexDefinition(0, $parts, $this->memberDefinitions, $definition); - } else { - $this->indexDefinition(0, $parts, $this->nonMemberDefinitions, $definition); - } + $this->indexDefinition(0, $parts, $this->definitions, $definition); $this->emit('definition-added'); } @@ -242,9 +224,7 @@ public function removeDefinition(string $fqn) $parts = $this->splitFqn($fqn); - if (true !== $this->removeIndexedDefinition(0, $parts, $this->memberDefinitions)) { - $this->removeIndexedDefinition(0, $parts, $this->nonMemberDefinitions); - } + $this->removeIndexedDefinition(0, $parts, $this->definitions); unset($this->references[$fqn]); } @@ -317,6 +297,14 @@ public function unserialize($serialized) { $data = unserialize($serialized); + if (isset($data['definitions'])) { + foreach ($data['definitions'] as $fqn => $definition) { + $this->setDefinition($fqn, $definition); + } + + unset($data['definitions']); + } + foreach ($data as $prop => $val) { $this->$prop = $val; } @@ -329,8 +317,7 @@ public function unserialize($serialized) public function serialize() { return serialize([ - 'memberDefinitions' => $this->memberDefinitions, - 'nonMemberDefinitions' => $this->nonMemberDefinitions, + 'definitions' => iterator_to_array($this->getDefinitions(), true), 'references' => $this->references, 'complete' => $this->complete, 'staticComplete' => $this->staticComplete @@ -401,10 +388,12 @@ private function splitFqn(string $fqn): array } // split the last part in 2 parts at the operator + $hasOperator = false; $lastPart = end($parts); foreach (['::', '->'] as $operator) { $endParts = explode($operator, $lastPart); if (count($endParts) > 1) { + $hasOperator = true; // replace the last part by its pieces array_pop($parts); $parts[] = $endParts[0]; @@ -413,6 +402,16 @@ private function splitFqn(string $fqn): array } } + if (!$hasOperator) { + // 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 + // ['Psr']['\Log']['\LoggerInterface'][''] to be able to also store + // member definitions, ie 'Psr\Log\LoggerInterface->log()' will be + // stored at ['Psr']['\Log']['\LoggerInterface']['->log()'] + $parts[] = ''; + } + return $parts; } @@ -439,12 +438,6 @@ private function getIndexValue(array $parts, array &$storage) return $storage[$part]; } - if (!is_array($storage[$part]) && count($parts) > 0) { - // we're looking for a member definition in the non member index, - // no matches can be found. - return null; - } - return $this->getIndexValue($parts, $storage[$part]); } @@ -471,12 +464,6 @@ private function indexDefinition(int $level, array $parts, array &$storage, Defi $storage[$part] = []; } - if (!is_array($storage[$part])) { - // it's a non member definition, we can't add it to the member - // definitions index - return; - } - $this->indexDefinition($level + 1, $parts, $storage[$part], $definition); } @@ -491,7 +478,6 @@ private function indexDefinition(int $level, array $parts, array &$storage, Defi * @param array $parts The splitted FQN * @param array &$storage The current array in which to remove data * @param array &$rootStorage The root storage array - * @return boolean|null True when the definition has been found and removed, null otherwise. */ private function removeIndexedDefinition(int $level, array $parts, array &$storage, &$rootStorage) { @@ -504,16 +490,16 @@ private function removeIndexedDefinition(int $level, array $parts, array &$stora if (0 === $level) { // we're at root level, no need to check for parents // w/o children - return true; + return; } array_pop($parts); // parse again the definition tree to see if the parent // can be removed too if it has no more children - return $this->removeIndexedDefinition(0, $parts, $rootStorage, $rootStorage); + $this->removeIndexedDefinition(0, $parts, $rootStorage, $rootStorage); } } else { - return $this->removeIndexedDefinition($level + 1, $parts, $storage[$part], $rootStorage); + $this->removeIndexedDefinition($level + 1, $parts, $storage[$part], $rootStorage); } } } diff --git a/src/Indexer.php b/src/Indexer.php index 2618d43a..f7d7c80e 100644 --- a/src/Indexer.php +++ b/src/Indexer.php @@ -18,7 +18,7 @@ class Indexer /** * @var int The prefix for every cache item */ - const CACHE_VERSION = 2; + const CACHE_VERSION = 1; /** * @var FilesFinder From e162d94e14114601db77d5ae8943b43c1cbf88ac Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Tue, 14 Nov 2017 13:44:23 +0100 Subject: [PATCH 18/35] cleanup --- src/Index/AbstractAggregateIndex.php | 12 ------ src/Index/Index.php | 62 +--------------------------- 2 files changed, 1 insertion(+), 73 deletions(-) diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index 8c88659e..19988894 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -107,10 +107,6 @@ public function isStaticComplete(): bool public function getDefinitions(): \Generator { foreach ($this->getIndexes() as $index) { - // foreach ($index->getDefinitions() as $fqn => $definition) { - // yield $fqn => $definition; - // } - yield from $index->getDefinitions(); } } @@ -124,10 +120,6 @@ public function getDefinitions(): \Generator public function getDefinitionsForFqn(string $fqn): \Generator { foreach ($this->getIndexes() as $index) { - // foreach ($index->getDefinitionsForFqn($fqn) as $symbolFqn => $definition) { - // yield $symbolFqn => $definition; - // } - yield from $index->getDefinitionsForFqn($fqn); } } @@ -157,10 +149,6 @@ public function getDefinition(string $fqn, bool $globalFallback = false) public function getReferenceUris(string $fqn): \Generator { foreach ($this->getIndexes() as $index) { - // foreach ($index->getReferenceUris($fqn) as $uri) { - // yield $uri; - // } - yield from $index->getReferenceUris($fqn); } } diff --git a/src/Index/Index.php b/src/Index/Index.php index 45663836..a2927bde 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -103,12 +103,6 @@ public function isStaticComplete(): bool */ public function getDefinitions(): \Generator { - // foreach ($this->fqnDefinitions as $fqnDefinition) { - // foreach ($fqnDefinition as $fqn => $definition) { - // yield $fqn => $definition; - // } - // } - yield from $this->yieldDefinitionsRecursively($this->definitions); } @@ -120,14 +114,10 @@ public function getDefinitions(): \Generator */ public function getDefinitionsForFqn(string $fqn): \Generator { - // foreach ($this->fqnDefinitions[$fqn] ?? [] as $symbolFqn => $definition) { - // yield $symbolFqn => $definition; - // } - $parts = $this->splitFqn($fqn); if ('' === end($parts)) { // we want to return all the definitions in the given FQN, not only - // the one matching exactly the FQN. + // the one (non member) matching exactly the FQN. array_pop($parts); } @@ -149,19 +139,6 @@ public function getDefinitionsForFqn(string $fqn): \Generator */ public function getDefinition(string $fqn, bool $globalFallback = false) { - // $namespacedFqn = $this->extractNamespacedFqn($fqn); - // $definitions = $this->fqnDefinitions[$namespacedFqn] ?? []; - - // if (isset($definitions[$fqn])) { - // return $definitions[$fqn]; - // } - - // if ($globalFallback) { - // $parts = explode('\\', $fqn); - // $fqn = end($parts); - // return $this->getDefinition($fqn); - // } - $parts = $this->splitFqn($fqn); $result = $this->getIndexValue($parts, $this->definitions); @@ -175,11 +152,6 @@ public function getDefinition(string $fqn, bool $globalFallback = false) return $this->getDefinition($fqn); } - - // return $result instanceof Definition - // ? $result - // : null - // ; } /** @@ -191,13 +163,6 @@ public function getDefinition(string $fqn, bool $globalFallback = false) */ public function setDefinition(string $fqn, Definition $definition) { - // $namespacedFqn = $this->extractNamespacedFqn($fqn); - // if (!isset($this->fqnDefinitions[$namespacedFqn])) { - // $this->fqnDefinitions[$namespacedFqn] = []; - // } - - // $this->fqnDefinitions[$namespacedFqn][$fqn] = $definition; - $parts = $this->splitFqn($fqn); $this->indexDefinition(0, $parts, $this->definitions, $definition); @@ -213,17 +178,7 @@ public function setDefinition(string $fqn, Definition $definition) */ public function removeDefinition(string $fqn) { - // $namespacedFqn = $this->extractNamespacedFqn($fqn); - // if (isset($this->fqnDefinitions[$namespacedFqn])) { - // unset($this->fqnDefinitions[$namespacedFqn][$fqn]); - - // if (empty($this->fqnDefinitions[$namespacedFqn])) { - // unset($this->fqnDefinitions[$namespacedFqn]); - // } - // } - $parts = $this->splitFqn($fqn); - $this->removeIndexedDefinition(0, $parts, $this->definitions); unset($this->references[$fqn]); @@ -324,21 +279,6 @@ public function serialize() ]); } - /** - * @param string $fqn The symbol FQN - * @return string The namespaced FQN extracted from the given symbol FQN - */ - // private function extractNamespacedFqn(string $fqn): string - // { - // foreach (['::', '->'] as $operator) { - // if (false !== ($pos = strpos($fqn, $operator))) { - // return substr($fqn, 0, $pos); - // } - // } - - // return $fqn; - // } - /** * Returns a Genrerator containing all the into the given $storage recursively. * The generator yields key => value pairs, eg From 7437d30d884897050db466c538b81c03a4f4be2e Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Tue, 14 Nov 2017 22:25:54 -0800 Subject: [PATCH 19/35] Fix formatting --- src/Server/TextDocument.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index 46e822ac..1c7d2923 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -220,8 +220,8 @@ public function references( } } $refDocuments = yield Promise\all(iterator_to_array( - $this->getOrLoadReferences($fqn)) - ); + $this->getOrLoadReferences($fqn) + )); foreach ($refDocuments as $document) { $refs = $document->getReferenceNodesByFqn($fqn); if ($refs !== null) { From b118c776991e2f259bb2bb313bf6de14b8cf23fa Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Tue, 14 Nov 2017 22:36:07 -0800 Subject: [PATCH 20/35] Correct some docblocks --- src/Index/Index.php | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Index/Index.php b/src/Index/Index.php index a2927bde..203b8818 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -301,12 +301,12 @@ private function yieldDefinitionsRecursively(array &$storage, string $prefix = ' /** * Splits the given FQN into an array, eg : - * 'Psr\Log\LoggerInterface->log' will be ['Psr', '\Log', '\LoggerInterface', '->log()'] - * '\Exception->getMessage()' will be ['\Exception', '->getMessage()'] - * 'PHP_VERSION' will be ['PHP_VERSION'] + * - `'Psr\Log\LoggerInterface->log'` will be `['Psr', '\Log', '\LoggerInterface', '->log()']` + * - `'\Exception->getMessage()'` will be `['\Exception', '->getMessage()']` + * - `'PHP_VERSION'` will be `['PHP_VERSION']` * * @param string $fqn - * @return array + * @return string[] */ private function splitFqn(string $fqn): array { @@ -360,7 +360,7 @@ private function splitFqn(string $fqn): array * It can be an index node or a Definition if the $parts are precise * enough. Returns null when nothing is found. * - * @param array $parts The splitted FQN + * @param string[] $parts The splitted FQN * @param array &$storage The array in which to store the $definition * @return array|Definition|null */ @@ -386,7 +386,7 @@ private function getIndexValue(array $parts, array &$storage) * array represented as a tree matching the given $parts. * * @param int $level The current level of FQN part - * @param array $parts The splitted FQN + * @param string[] $parts The splitted FQN * @param array &$storage The array in which to store the $definition * @param Definition $definition The Definition to store */ @@ -415,7 +415,7 @@ private function indexDefinition(int $level, array $parts, array &$storage, Defi * in the index. * * @param int $level The current level of FQN part - * @param array $parts The splitted FQN + * @param string[] $parts The splitted FQN * @param array &$storage The current array in which to remove data * @param array &$rootStorage The root storage array */ From b3f30f31f1c7979a6ada016ea536acfc7f7a9939 Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Wed, 15 Nov 2017 09:51:18 +0100 Subject: [PATCH 21/35] cache is backward compatible --- src/Indexer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Indexer.php b/src/Indexer.php index f7d7c80e..2618d43a 100644 --- a/src/Indexer.php +++ b/src/Indexer.php @@ -18,7 +18,7 @@ class Indexer /** * @var int The prefix for every cache item */ - const CACHE_VERSION = 1; + const CACHE_VERSION = 2; /** * @var FilesFinder From fcdf791c2c00afabb47fb25e665c08623885dbef Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Thu, 16 Nov 2017 17:44:04 +0100 Subject: [PATCH 22/35] use string concatenation instead of sprintf --- src/Index/Index.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Index/Index.php b/src/Index/Index.php index 203b8818..7088e03c 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -292,9 +292,9 @@ private function yieldDefinitionsRecursively(array &$storage, string $prefix = ' { foreach ($storage as $key => $value) { if (!is_array($value)) { - yield sprintf('%s%s', $prefix, $key) => $value; + yield $prefix.$key => $value; } else { - yield from $this->yieldDefinitionsRecursively($value, sprintf('%s%s', $prefix, $key)); + yield from $this->yieldDefinitionsRecursively($value, $prefix.$key); } } } @@ -319,12 +319,12 @@ private function splitFqn(string $fqn): array $parts = array_slice($parts, 1); } - $parts[0] = sprintf('\\%s', $parts[0]); + $parts[0] = '\\'.$parts[0]; } // write back the backslashes prefixes for the other parts for ($i = 1; $i < count($parts); $i++) { - $parts[$i] = sprintf('\\%s', $parts[$i]); + $parts[$i] = '\\'.$parts[$i]; } // split the last part in 2 parts at the operator @@ -337,7 +337,7 @@ private function splitFqn(string $fqn): array // replace the last part by its pieces array_pop($parts); $parts[] = $endParts[0]; - $parts[] = sprintf('%s%s', $operator, $endParts[1]); + $parts[] = $operator.$endParts[1]; break; } } From b03950cb53cf492459ec3494d6316d106d324b24 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Sat, 18 Nov 2017 14:53:18 -0800 Subject: [PATCH 23/35] Cleanup --- src/CompletionProvider.php | 4 ++-- src/Index/AbstractAggregateIndex.php | 12 +++++----- src/Index/Index.php | 34 +++++++++++++--------------- src/Index/ReadableIndex.php | 12 +++++----- src/Server/TextDocument.php | 21 ++++------------- 5 files changed, 35 insertions(+), 48 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index fe78ef77..2f223334 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -221,7 +221,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { // Collect fqn definitions - foreach ($this->index->getDefinitionsForFqn($parentFqn) as $fqn => $def) { + foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn) as $fqn => $def) { // Add the object access operator to only get members of all parents $prefix = $parentFqn . '->'; if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember) { @@ -251,7 +251,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { // Collect fqn definitions - foreach ($this->index->getDefinitionsForFqn($parentFqn) as $fqn => $def) { + foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn) as $fqn => $def) { // Append :: operator to only get static members of all parents $prefix = strtolower($parentFqn . '::'); if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && $def->isMember) { diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index 19988894..90490ab2 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -102,7 +102,7 @@ public function isStaticComplete(): bool * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * - * @return \Generator providing Definition[] + * @return \Generator yields Definition */ public function getDefinitions(): \Generator { @@ -112,15 +112,15 @@ public function getDefinitions(): \Generator } /** - * Returns a Generator providing the Definitions that are in the given FQN + * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn - * @return \Generator providing Definitions[] + * @return \Generator yields Definition */ - public function getDefinitionsForFqn(string $fqn): \Generator + public function getDescendantDefinitionsForFqn(string $fqn): \Generator { foreach ($this->getIndexes() as $index) { - yield from $index->getDefinitionsForFqn($fqn); + yield from $index->getDescendantDefinitionsForFqn($fqn); } } @@ -144,7 +144,7 @@ public function getDefinition(string $fqn, bool $globalFallback = false) * Returns a Generator providing all URIs in this index that reference a symbol * * @param string $fqn The fully qualified name of the symbol - * @return \Generator providing string[] + * @return \Generator yields string */ public function getReferenceUris(string $fqn): \Generator { diff --git a/src/Index/Index.php b/src/Index/Index.php index 7088e03c..40002173 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -99,7 +99,7 @@ public function isStaticComplete(): bool * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * - * @return \Generator providing Definition[] + * @return \Generator yields Definition */ public function getDefinitions(): \Generator { @@ -107,12 +107,12 @@ public function getDefinitions(): \Generator } /** - * Returns a Generator providing the Definitions that are in the given FQN + * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn - * @return \Generator providing Definitions[] + * @return \Generator yields Definition */ - public function getDefinitionsForFqn(string $fqn): \Generator + public function getDescendantDefinitionsForFqn(string $fqn): \Generator { $parts = $this->splitFqn($fqn); if ('' === end($parts)) { @@ -188,7 +188,7 @@ public function removeDefinition(string $fqn) * Returns a Generator providing all URIs in this index that reference a symbol * * @param string $fqn The fully qualified name of the symbol - * @return \Generator providing string[] + * @return \Generator yields string */ public function getReferenceUris(string $fqn): \Generator { @@ -280,9 +280,9 @@ public function serialize() } /** - * Returns a Genrerator containing all the into the given $storage recursively. - * The generator yields key => value pairs, eg - * 'Psr\Log\LoggerInterface->log()' => $definition + * Returns a Generator that yields all the Definitions in the given $storage recursively. + * The generator yields key => value pairs, e.g. + * `'Psr\Log\LoggerInterface->log()' => $definition` * * @param array &$storage * @param string $prefix (optional) @@ -319,12 +319,12 @@ private function splitFqn(string $fqn): array $parts = array_slice($parts, 1); } - $parts[0] = '\\'.$parts[0]; + $parts[0] = '\\' . $parts[0]; } // write back the backslashes prefixes for the other parts for ($i = 1; $i < count($parts); $i++) { - $parts[$i] = '\\'.$parts[$i]; + $parts[$i] = '\\' . $parts[$i]; } // split the last part in 2 parts at the operator @@ -337,7 +337,7 @@ private function splitFqn(string $fqn): array // replace the last part by its pieces array_pop($parts); $parts[] = $endParts[0]; - $parts[] = $operator.$endParts[1]; + $parts[] = $operator . $endParts[1]; break; } } @@ -382,8 +382,8 @@ private function getIndexValue(array $parts, array &$storage) } /** - * Recusrive function which store the given definition in the given $storage - * array represented as a tree matching the given $parts. + * Recursive function that stores the given Definition in the given $storage array represented + * as a tree matching the given $parts. * * @param int $level The current level of FQN part * @param string[] $parts The splitted FQN @@ -408,11 +408,9 @@ private function indexDefinition(int $level, array $parts, array &$storage, Defi } /** - * Recusrive function which remove the definition matching the given $parts - * from the given $storage array. - * The function also looks up recursively to remove the parents of the - * definition which no longer has children to avoid to let empty arrays - * in the index. + * Recursive function that removes the definition matching the given $parts from the given + * $storage array. The function also looks up recursively to remove the parents of the + * definition which no longer has children to avoid to let empty arrays in the index. * * @param int $level The current level of FQN part * @param string[] $parts The splitted FQN diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 5c54557d..90ddcc45 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -33,17 +33,17 @@ public function isStaticComplete(): bool; * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * - * @return \Generator providing Definition[] + * @return \Generator yields Definition */ public function getDefinitions(): \Generator; /** - * Returns a Generator providing the Definitions that are in the given FQN + * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn - * @return \Generator providing Definitions[] + * @return \Generator yields Definition */ - public function getDefinitionsForFqn(string $fqn): \Generator; + public function getDescendantDefinitionsForFqn(string $fqn): \Generator; /** * Returns the Definition object by a specific FQN @@ -55,10 +55,10 @@ public function getDefinitionsForFqn(string $fqn): \Generator; public function getDefinition(string $fqn, bool $globalFallback = false); /** - * Returns a Generator providing all URIs in this index that reference a symbol + * Returns a Generator that yields all URIs in this index that reference a symbol * * @param string $fqn The fully qualified name of the symbol - * @return \Generator providing string[] + * @return \Generator yields string */ public function getReferenceUris(string $fqn): \Generator; } diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index 53d804f6..0ad2d815 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -220,9 +220,11 @@ public function references( return []; } } - $refDocuments = yield Promise\all(iterator_to_array( - $this->getOrLoadReferences($fqn) - )); + $refDocumentPromises = []; + foreach ($this->index->getReferenceUris($fqn) as $uri) { + $refDocumentPromises[] = $this->documentLoader->getOrLoad($uri); + } + $refDocuments = yield Promise\all($refDocumentPromises); foreach ($refDocuments as $document) { $refs = $document->getReferenceNodesByFqn($fqn); if ($refs !== null) { @@ -398,17 +400,4 @@ public function xdefinition(TextDocumentIdentifier $textDocument, Position $posi return [new SymbolLocationInformation($descriptor, $def->symbolInformation->location)]; }); } - - /** - * Gets or loads the documents referencing the given FQN. - * - * @param string $fqn - * @return \Generator providing Promise - */ - private function getOrLoadReferences(string $fqn): \Generator - { - foreach ($this->index->getReferenceUris($fqn) as $ref) { - yield $this->documentLoader->getOrLoad($ref); - } - } } From 97ec127312069ca9a4a1b78aed6128022361129b Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Sun, 19 Nov 2017 16:34:51 +0100 Subject: [PATCH 24/35] fix definition removal --- src/Index/Index.php | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/Index/Index.php b/src/Index/Index.php index 40002173..dcb1b8f7 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -179,7 +179,7 @@ public function setDefinition(string $fqn, Definition $definition) public function removeDefinition(string $fqn) { $parts = $this->splitFqn($fqn); - $this->removeIndexedDefinition(0, $parts, $this->definitions); + $this->removeIndexedDefinition(0, $parts, $this->definitions, $this->definitions); unset($this->references[$fqn]); } @@ -417,24 +417,19 @@ private function indexDefinition(int $level, array $parts, array &$storage, Defi * @param array &$storage The current array in which to remove data * @param array &$rootStorage The root storage array */ - private function removeIndexedDefinition(int $level, array $parts, array &$storage, &$rootStorage) + private function removeIndexedDefinition(int $level, array $parts, array &$storage, array &$rootStorage) { $part = $parts[$level]; if ($level + 1 === count($parts)) { - if (isset($storage[$part]) && count($storage[$part]) < 2) { + if (isset($storage[$part])) { unset($storage[$part]); - if (0 === $level) { - // we're at root level, no need to check for parents - // w/o children - return; + if (0 === count($storage)) { + // parse again the definition tree to remove the parent + // when it has no more children + $this->removeIndexedDefinition(0, array_slice($parts, 0, $level), $rootStorage, $rootStorage); } - - array_pop($parts); - // parse again the definition tree to see if the parent - // can be removed too if it has no more children - $this->removeIndexedDefinition(0, $parts, $rootStorage, $rootStorage); } } else { $this->removeIndexedDefinition($level + 1, $parts, $storage[$part], $rootStorage); From 48bbbb5d14a3d1fc01fce7e48383f0ba1690c2bb Mon Sep 17 00:00:00 2001 From: Nicolas MURE Date: Sun, 19 Nov 2017 18:29:23 +0100 Subject: [PATCH 25/35] differenciate member and non member definitions --- src/CompletionProvider.php | 14 ++--- src/Index/AbstractAggregateIndex.php | 10 +-- src/Index/Index.php | 91 ++++++++++++++++++++++----- src/Index/ReadableIndex.php | 6 +- tests/Server/Workspace/SymbolTest.php | 31 +++++---- 5 files changed, 105 insertions(+), 47 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 2f223334..a5f91194 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -221,10 +221,10 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { // Collect fqn definitions - foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn) as $fqn => $def) { + foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn, true) as $fqn => $def) { // Add the object access operator to only get members of all parents $prefix = $parentFqn . '->'; - if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember) { + if (substr($fqn, 0, strlen($prefix)) === $prefix) { $list->items[] = CompletionItem::fromDefinition($def); } } @@ -251,10 +251,10 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { // Collect fqn definitions - foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn) as $fqn => $def) { + foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn, true) as $fqn => $def) { // Append :: operator to only get static members of all parents $prefix = strtolower($parentFqn . '::'); - if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && $def->isMember) { + if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix) { $list->items[] = CompletionItem::fromDefinition($def); } } @@ -321,14 +321,12 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // 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) { + foreach ($this->index->getDefinitions(false) 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 diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index 90490ab2..3458d552 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -102,12 +102,13 @@ public function isStaticComplete(): bool * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * + * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDefinitions(): \Generator + public function getDefinitions(bool $member = null): \Generator { foreach ($this->getIndexes() as $index) { - yield from $index->getDefinitions(); + yield from $index->getDefinitions($member); } } @@ -115,12 +116,13 @@ public function getDefinitions(): \Generator * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn + * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn): \Generator + public function getDescendantDefinitionsForFqn(string $fqn, bool $member = null): \Generator { foreach ($this->getIndexes() as $index) { - yield from $index->getDescendantDefinitionsForFqn($fqn); + yield from $index->getDescendantDefinitionsForFqn($fqn, $member); } } diff --git a/src/Index/Index.php b/src/Index/Index.php index dcb1b8f7..1099fab7 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -16,13 +16,12 @@ class Index implements ReadableIndex, \Serializable /** * An associative array that maps splitted fully qualified symbol names - * to definitions, eg : + * to non-member definitions, eg : * [ * 'Psr' => [ * '\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 + * '' => $definition, * ], * ], * ], @@ -30,7 +29,24 @@ class Index implements ReadableIndex, \Serializable * * @var array */ - private $definitions = []; + private $nonMemberDefinitions = []; + + /** + * An associative array that maps splitted fully qualified symbol names + * to member definitions, eg : + * [ + * 'Psr' => [ + * '\Log' => [ + * '\LoggerInterface' => [ + * '->log()' => $definition, + * ], + * ], + * ], + * ] + * + * @var array + */ + private $memberDefinitions = []; /** * An associative array that maps fully qualified symbol names @@ -99,20 +115,29 @@ public function isStaticComplete(): bool * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * + * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDefinitions(): \Generator + public function getDefinitions(bool $member = null): \Generator { - yield from $this->yieldDefinitionsRecursively($this->definitions); + if (true === $member) { + yield from $this->yieldDefinitionsRecursively($this->memberDefinitions); + } elseif (false === $member) { + yield from $this->yieldDefinitionsRecursively($this->nonMemberDefinitions); + } else { + yield from $this->yieldDefinitionsRecursively($this->memberDefinitions); + yield from $this->yieldDefinitionsRecursively($this->nonMemberDefinitions); + } } /** * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn + * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn): \Generator + public function getDescendantDefinitionsForFqn(string $fqn, bool $member = null): \Generator { $parts = $this->splitFqn($fqn); if ('' === end($parts)) { @@ -121,12 +146,13 @@ public function getDescendantDefinitionsForFqn(string $fqn): \Generator array_pop($parts); } - $result = $this->getIndexValue($parts, $this->definitions); - - if ($result instanceof Definition) { - yield $fqn => $result; - } elseif (is_array($result)) { - yield from $this->yieldDefinitionsRecursively($result, $fqn); + if (true === $member) { + yield from $this->doGetDescendantDefinitionsForFqn($fqn, $parts, $this->memberDefinitions); + } elseif (false === $member) { + yield from $this->doGetDescendantDefinitionsForFqn($fqn, $parts, $this->nonMemberDefinitions); + } else { + yield from $this->doGetDescendantDefinitionsForFqn($fqn, $parts, $this->memberDefinitions); + yield from $this->doGetDescendantDefinitionsForFqn($fqn, $parts, $this->nonMemberDefinitions); } } @@ -140,8 +166,13 @@ public function getDescendantDefinitionsForFqn(string $fqn): \Generator public function getDefinition(string $fqn, bool $globalFallback = false) { $parts = $this->splitFqn($fqn); - $result = $this->getIndexValue($parts, $this->definitions); + $result = $this->getIndexValue($parts, $this->memberDefinitions); + if ($result instanceof Definition) { + return $result; + } + + $result = $this->getIndexValue($parts, $this->nonMemberDefinitions); if ($result instanceof Definition) { return $result; } @@ -164,7 +195,12 @@ public function getDefinition(string $fqn, bool $globalFallback = false) public function setDefinition(string $fqn, Definition $definition) { $parts = $this->splitFqn($fqn); - $this->indexDefinition(0, $parts, $this->definitions, $definition); + + if ($definition->isMember) { + $this->indexDefinition(0, $parts, $this->memberDefinitions, $definition); + } else { + $this->indexDefinition(0, $parts, $this->nonMemberDefinitions, $definition); + } $this->emit('definition-added'); } @@ -179,7 +215,8 @@ public function setDefinition(string $fqn, Definition $definition) public function removeDefinition(string $fqn) { $parts = $this->splitFqn($fqn); - $this->removeIndexedDefinition(0, $parts, $this->definitions, $this->definitions); + $this->removeIndexedDefinition(0, $parts, $this->memberDefinitions, $this->memberDefinitions); + $this->removeIndexedDefinition(0, $parts, $this->nonMemberDefinitions, $this->nonMemberDefinitions); unset($this->references[$fqn]); } @@ -279,6 +316,26 @@ public function serialize() ]); } + /** + * Returns a Generator that yields all the descendant Definitions of a given FQN + * in the given definition index. + * + * @param string $fqn + * @param string[] $parts The splitted FQN + * @param array &$storage The definitions index to look into + * @return \Generator yields Definition + */ + private function doGetDescendantDefinitionsForFqn(string $fqn, array $parts, array &$storage): \Generator + { + $result = $this->getIndexValue($parts, $storage); + + if ($result instanceof Definition) { + yield $fqn => $result; + } elseif (is_array($result)) { + yield from $this->yieldDefinitionsRecursively($result, $fqn); + } + } + /** * Returns a Generator that yields all the Definitions in the given $storage recursively. * The generator yields key => value pairs, e.g. @@ -431,7 +488,7 @@ private function removeIndexedDefinition(int $level, array $parts, array &$stora $this->removeIndexedDefinition(0, array_slice($parts, 0, $level), $rootStorage, $rootStorage); } } - } else { + } elseif (isset($storage[$part])) { $this->removeIndexedDefinition($level + 1, $parts, $storage[$part], $rootStorage); } } diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 90ddcc45..d02f22b0 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -33,17 +33,19 @@ public function isStaticComplete(): bool; * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * + * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDefinitions(): \Generator; + public function getDefinitions(bool $member = null): \Generator; /** * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn + * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn): \Generator; + public function getDescendantDefinitionsForFqn(string $fqn, bool $member = null): \Generator; /** * Returns the Definition object by a specific FQN diff --git a/tests/Server/Workspace/SymbolTest.php b/tests/Server/Workspace/SymbolTest.php index 765841bb..02de5abe 100644 --- a/tests/Server/Workspace/SymbolTest.php +++ b/tests/Server/Workspace/SymbolTest.php @@ -30,41 +30,40 @@ public function testEmptyQueryReturnsAllSymbols() // @codingStandardsIgnoreStart $this->assertEquals([ - new SymbolInformation('TestNamespace', SymbolKind::NAMESPACE, new Location($referencesUri, new Range(new Position(2, 0), new Position(2, 24))), ''), - // Namespaced - new SymbolInformation('TEST_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestNamespace\\TEST_CONST'), 'TestNamespace'), - new SymbolInformation('TestClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\TestClass'), 'TestNamespace'), + // member new SymbolInformation('TEST_CLASS_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestNamespace\\TestClass::TEST_CLASS_CONST'), 'TestNamespace\\TestClass'), new SymbolInformation('staticTestProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestNamespace\\TestClass::staticTestProperty'), 'TestNamespace\\TestClass'), new SymbolInformation('testProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestNamespace\\TestClass::testProperty'), 'TestNamespace\\TestClass'), new SymbolInformation('staticTestMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestNamespace\\TestClass::staticTestMethod()'), 'TestNamespace\\TestClass'), new SymbolInformation('testMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestNamespace\\TestClass::testMethod()'), 'TestNamespace\\TestClass'), + new SymbolInformation('__construct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__construct'), 'TestNamespace\\Example'), + new SymbolInformation('__destruct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__destruct'), 'TestNamespace\\Example'), + new SymbolInformation('TEST_CLASS_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestClass::TEST_CLASS_CONST'), 'TestClass'), + new SymbolInformation('staticTestProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestClass::staticTestProperty'), 'TestClass'), + new SymbolInformation('testProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestClass::testProperty'), 'TestClass'), + new SymbolInformation('staticTestMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestClass::staticTestMethod()'), 'TestClass'), + new SymbolInformation('testMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestClass::testMethod()'), 'TestClass'), + // non member + new SymbolInformation('TEST_DEFINE_CONSTANT', SymbolKind::CONSTANT, $this->getDefinitionLocation('TEST_DEFINE_CONSTANT'), ''), + new SymbolInformation('unusedProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('UnusedClass::unusedProperty'), 'UnusedClass'), + new SymbolInformation('unusedMethod', SymbolKind::METHOD, $this->getDefinitionLocation('UnusedClass::unusedMethod'), 'UnusedClass'), + new SymbolInformation('TestNamespace', SymbolKind::NAMESPACE, new Location($referencesUri, new Range(new Position(2, 0), new Position(2, 24))), ''), + new SymbolInformation('TEST_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestNamespace\\TEST_CONST'), 'TestNamespace'), + new SymbolInformation('TestClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\TestClass'), 'TestNamespace'), new SymbolInformation('TestTrait', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\TestTrait'), 'TestNamespace'), new SymbolInformation('TestInterface', SymbolKind::INTERFACE, $this->getDefinitionLocation('TestNamespace\\TestInterface'), 'TestNamespace'), new SymbolInformation('test_function', SymbolKind::FUNCTION, $this->getDefinitionLocation('TestNamespace\\test_function()'), 'TestNamespace'), new SymbolInformation('ChildClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\ChildClass'), 'TestNamespace'), new SymbolInformation('Example', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\Example'), 'TestNamespace'), - new SymbolInformation('__construct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__construct'), 'TestNamespace\\Example'), - new SymbolInformation('__destruct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__destruct'), 'TestNamespace\\Example'), new SymbolInformation('whatever', SymbolKind::FUNCTION, $this->getDefinitionLocation('TestNamespace\\whatever()'), 'TestNamespace'), - // Global new SymbolInformation('TEST_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TEST_CONST'), ''), new SymbolInformation('TestClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestClass'), ''), - new SymbolInformation('TEST_CLASS_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestClass::TEST_CLASS_CONST'), 'TestClass'), - new SymbolInformation('staticTestProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestClass::staticTestProperty'), 'TestClass'), - new SymbolInformation('testProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestClass::testProperty'), 'TestClass'), - new SymbolInformation('staticTestMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestClass::staticTestMethod()'), 'TestClass'), - new SymbolInformation('testMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestClass::testMethod()'), 'TestClass'), new SymbolInformation('TestTrait', SymbolKind::CLASS_, $this->getDefinitionLocation('TestTrait'), ''), new SymbolInformation('TestInterface', SymbolKind::INTERFACE, $this->getDefinitionLocation('TestInterface'), ''), new SymbolInformation('test_function', SymbolKind::FUNCTION, $this->getDefinitionLocation('test_function()'), ''), new SymbolInformation('ChildClass', SymbolKind::CLASS_, $this->getDefinitionLocation('ChildClass'), ''), - new SymbolInformation('TEST_DEFINE_CONSTANT', SymbolKind::CONSTANT, $this->getDefinitionLocation('TEST_DEFINE_CONSTANT'), ''), new SymbolInformation('UnusedClass', SymbolKind::CLASS_, $this->getDefinitionLocation('UnusedClass'), ''), - new SymbolInformation('unusedProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('UnusedClass::unusedProperty'), 'UnusedClass'), - new SymbolInformation('unusedMethod', SymbolKind::METHOD, $this->getDefinitionLocation('UnusedClass::unusedMethod'), 'UnusedClass'), new SymbolInformation('whatever', SymbolKind::FUNCTION, $this->getDefinitionLocation('whatever()'), ''), - new SymbolInformation('SecondTestNamespace', SymbolKind::NAMESPACE, $this->getDefinitionLocation('SecondTestNamespace'), ''), ], $result); // @codingStandardsIgnoreEnd From 91ca99a8670a7652366bf2ef909dc1989b3c1015 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Thu, 23 Nov 2017 01:20:42 -0800 Subject: [PATCH 26/35] Revert "differenciate member and non member definitions" This reverts commit 48bbbb5d14a3d1fc01fce7e48383f0ba1690c2bb. --- src/CompletionProvider.php | 14 +++-- src/Index/AbstractAggregateIndex.php | 10 ++- src/Index/Index.php | 91 +++++---------------------- src/Index/ReadableIndex.php | 6 +- tests/Server/Workspace/SymbolTest.php | 31 ++++----- 5 files changed, 47 insertions(+), 105 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index a5f91194..2f223334 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -221,10 +221,10 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { // Collect fqn definitions - foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn, true) as $fqn => $def) { + foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn) as $fqn => $def) { // Add the object access operator to only get members of all parents $prefix = $parentFqn . '->'; - if (substr($fqn, 0, strlen($prefix)) === $prefix) { + if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } } @@ -251,10 +251,10 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { // Collect fqn definitions - foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn, true) as $fqn => $def) { + foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn) as $fqn => $def) { // Append :: operator to only get static members of all parents $prefix = strtolower($parentFqn . '::'); - if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix) { + if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && $def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } } @@ -321,12 +321,14 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // 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(false) as $fqn => $def) { + 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 diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index 3458d552..90490ab2 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -102,13 +102,12 @@ public function isStaticComplete(): bool * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * - * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDefinitions(bool $member = null): \Generator + public function getDefinitions(): \Generator { foreach ($this->getIndexes() as $index) { - yield from $index->getDefinitions($member); + yield from $index->getDefinitions(); } } @@ -116,13 +115,12 @@ public function getDefinitions(bool $member = null): \Generator * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn - * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn, bool $member = null): \Generator + public function getDescendantDefinitionsForFqn(string $fqn): \Generator { foreach ($this->getIndexes() as $index) { - yield from $index->getDescendantDefinitionsForFqn($fqn, $member); + yield from $index->getDescendantDefinitionsForFqn($fqn); } } diff --git a/src/Index/Index.php b/src/Index/Index.php index 1099fab7..dcb1b8f7 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -16,12 +16,13 @@ class Index implements ReadableIndex, \Serializable /** * An associative array that maps splitted fully qualified symbol names - * to non-member definitions, eg : + * to definitions, eg : * [ * 'Psr' => [ * '\Log' => [ * '\LoggerInterface' => [ - * '' => $definition, + * '' => $def1, // definition for 'Psr\Log\LoggerInterface' which is non-member + * '->log()' => $def2, // definition for 'Psr\Log\LoggerInterface->log()' which is a member definition * ], * ], * ], @@ -29,24 +30,7 @@ class Index implements ReadableIndex, \Serializable * * @var array */ - private $nonMemberDefinitions = []; - - /** - * An associative array that maps splitted fully qualified symbol names - * to member definitions, eg : - * [ - * 'Psr' => [ - * '\Log' => [ - * '\LoggerInterface' => [ - * '->log()' => $definition, - * ], - * ], - * ], - * ] - * - * @var array - */ - private $memberDefinitions = []; + private $definitions = []; /** * An associative array that maps fully qualified symbol names @@ -115,29 +99,20 @@ public function isStaticComplete(): bool * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * - * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDefinitions(bool $member = null): \Generator + public function getDefinitions(): \Generator { - if (true === $member) { - yield from $this->yieldDefinitionsRecursively($this->memberDefinitions); - } elseif (false === $member) { - yield from $this->yieldDefinitionsRecursively($this->nonMemberDefinitions); - } else { - yield from $this->yieldDefinitionsRecursively($this->memberDefinitions); - yield from $this->yieldDefinitionsRecursively($this->nonMemberDefinitions); - } + yield from $this->yieldDefinitionsRecursively($this->definitions); } /** * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn - * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn, bool $member = null): \Generator + public function getDescendantDefinitionsForFqn(string $fqn): \Generator { $parts = $this->splitFqn($fqn); if ('' === end($parts)) { @@ -146,13 +121,12 @@ public function getDescendantDefinitionsForFqn(string $fqn, bool $member = null) array_pop($parts); } - if (true === $member) { - yield from $this->doGetDescendantDefinitionsForFqn($fqn, $parts, $this->memberDefinitions); - } elseif (false === $member) { - yield from $this->doGetDescendantDefinitionsForFqn($fqn, $parts, $this->nonMemberDefinitions); - } else { - yield from $this->doGetDescendantDefinitionsForFqn($fqn, $parts, $this->memberDefinitions); - yield from $this->doGetDescendantDefinitionsForFqn($fqn, $parts, $this->nonMemberDefinitions); + $result = $this->getIndexValue($parts, $this->definitions); + + if ($result instanceof Definition) { + yield $fqn => $result; + } elseif (is_array($result)) { + yield from $this->yieldDefinitionsRecursively($result, $fqn); } } @@ -166,13 +140,8 @@ public function getDescendantDefinitionsForFqn(string $fqn, bool $member = null) public function getDefinition(string $fqn, bool $globalFallback = false) { $parts = $this->splitFqn($fqn); + $result = $this->getIndexValue($parts, $this->definitions); - $result = $this->getIndexValue($parts, $this->memberDefinitions); - if ($result instanceof Definition) { - return $result; - } - - $result = $this->getIndexValue($parts, $this->nonMemberDefinitions); if ($result instanceof Definition) { return $result; } @@ -195,12 +164,7 @@ public function getDefinition(string $fqn, bool $globalFallback = false) public function setDefinition(string $fqn, Definition $definition) { $parts = $this->splitFqn($fqn); - - if ($definition->isMember) { - $this->indexDefinition(0, $parts, $this->memberDefinitions, $definition); - } else { - $this->indexDefinition(0, $parts, $this->nonMemberDefinitions, $definition); - } + $this->indexDefinition(0, $parts, $this->definitions, $definition); $this->emit('definition-added'); } @@ -215,8 +179,7 @@ public function setDefinition(string $fqn, Definition $definition) public function removeDefinition(string $fqn) { $parts = $this->splitFqn($fqn); - $this->removeIndexedDefinition(0, $parts, $this->memberDefinitions, $this->memberDefinitions); - $this->removeIndexedDefinition(0, $parts, $this->nonMemberDefinitions, $this->nonMemberDefinitions); + $this->removeIndexedDefinition(0, $parts, $this->definitions, $this->definitions); unset($this->references[$fqn]); } @@ -316,26 +279,6 @@ public function serialize() ]); } - /** - * Returns a Generator that yields all the descendant Definitions of a given FQN - * in the given definition index. - * - * @param string $fqn - * @param string[] $parts The splitted FQN - * @param array &$storage The definitions index to look into - * @return \Generator yields Definition - */ - private function doGetDescendantDefinitionsForFqn(string $fqn, array $parts, array &$storage): \Generator - { - $result = $this->getIndexValue($parts, $storage); - - if ($result instanceof Definition) { - yield $fqn => $result; - } elseif (is_array($result)) { - yield from $this->yieldDefinitionsRecursively($result, $fqn); - } - } - /** * Returns a Generator that yields all the Definitions in the given $storage recursively. * The generator yields key => value pairs, e.g. @@ -488,7 +431,7 @@ private function removeIndexedDefinition(int $level, array $parts, array &$stora $this->removeIndexedDefinition(0, array_slice($parts, 0, $level), $rootStorage, $rootStorage); } } - } elseif (isset($storage[$part])) { + } else { $this->removeIndexedDefinition($level + 1, $parts, $storage[$part], $rootStorage); } } diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index d02f22b0..90ddcc45 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -33,19 +33,17 @@ public function isStaticComplete(): bool; * Returns a Generator providing an associative array [string => Definition] * that maps fully qualified symbol names to Definitions (global or not) * - * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDefinitions(bool $member = null): \Generator; + public function getDefinitions(): \Generator; /** * Returns a Generator that yields all the descendant Definitions of a given FQN * * @param string $fqn - * @param boolean|null $member Indicates if we want member or non-member definitions (null for both, default null) * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn, bool $member = null): \Generator; + public function getDescendantDefinitionsForFqn(string $fqn): \Generator; /** * Returns the Definition object by a specific FQN diff --git a/tests/Server/Workspace/SymbolTest.php b/tests/Server/Workspace/SymbolTest.php index 02de5abe..765841bb 100644 --- a/tests/Server/Workspace/SymbolTest.php +++ b/tests/Server/Workspace/SymbolTest.php @@ -30,40 +30,41 @@ public function testEmptyQueryReturnsAllSymbols() // @codingStandardsIgnoreStart $this->assertEquals([ - // member + new SymbolInformation('TestNamespace', SymbolKind::NAMESPACE, new Location($referencesUri, new Range(new Position(2, 0), new Position(2, 24))), ''), + // Namespaced + new SymbolInformation('TEST_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestNamespace\\TEST_CONST'), 'TestNamespace'), + new SymbolInformation('TestClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\TestClass'), 'TestNamespace'), new SymbolInformation('TEST_CLASS_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestNamespace\\TestClass::TEST_CLASS_CONST'), 'TestNamespace\\TestClass'), new SymbolInformation('staticTestProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestNamespace\\TestClass::staticTestProperty'), 'TestNamespace\\TestClass'), new SymbolInformation('testProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestNamespace\\TestClass::testProperty'), 'TestNamespace\\TestClass'), new SymbolInformation('staticTestMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestNamespace\\TestClass::staticTestMethod()'), 'TestNamespace\\TestClass'), new SymbolInformation('testMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestNamespace\\TestClass::testMethod()'), 'TestNamespace\\TestClass'), - new SymbolInformation('__construct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__construct'), 'TestNamespace\\Example'), - new SymbolInformation('__destruct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__destruct'), 'TestNamespace\\Example'), - new SymbolInformation('TEST_CLASS_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestClass::TEST_CLASS_CONST'), 'TestClass'), - new SymbolInformation('staticTestProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestClass::staticTestProperty'), 'TestClass'), - new SymbolInformation('testProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestClass::testProperty'), 'TestClass'), - new SymbolInformation('staticTestMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestClass::staticTestMethod()'), 'TestClass'), - new SymbolInformation('testMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestClass::testMethod()'), 'TestClass'), - // non member - new SymbolInformation('TEST_DEFINE_CONSTANT', SymbolKind::CONSTANT, $this->getDefinitionLocation('TEST_DEFINE_CONSTANT'), ''), - new SymbolInformation('unusedProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('UnusedClass::unusedProperty'), 'UnusedClass'), - new SymbolInformation('unusedMethod', SymbolKind::METHOD, $this->getDefinitionLocation('UnusedClass::unusedMethod'), 'UnusedClass'), - new SymbolInformation('TestNamespace', SymbolKind::NAMESPACE, new Location($referencesUri, new Range(new Position(2, 0), new Position(2, 24))), ''), - new SymbolInformation('TEST_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestNamespace\\TEST_CONST'), 'TestNamespace'), - new SymbolInformation('TestClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\TestClass'), 'TestNamespace'), new SymbolInformation('TestTrait', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\TestTrait'), 'TestNamespace'), new SymbolInformation('TestInterface', SymbolKind::INTERFACE, $this->getDefinitionLocation('TestNamespace\\TestInterface'), 'TestNamespace'), new SymbolInformation('test_function', SymbolKind::FUNCTION, $this->getDefinitionLocation('TestNamespace\\test_function()'), 'TestNamespace'), new SymbolInformation('ChildClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\ChildClass'), 'TestNamespace'), new SymbolInformation('Example', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\Example'), 'TestNamespace'), + new SymbolInformation('__construct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__construct'), 'TestNamespace\\Example'), + new SymbolInformation('__destruct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__destruct'), 'TestNamespace\\Example'), new SymbolInformation('whatever', SymbolKind::FUNCTION, $this->getDefinitionLocation('TestNamespace\\whatever()'), 'TestNamespace'), + // Global new SymbolInformation('TEST_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TEST_CONST'), ''), new SymbolInformation('TestClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestClass'), ''), + new SymbolInformation('TEST_CLASS_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestClass::TEST_CLASS_CONST'), 'TestClass'), + new SymbolInformation('staticTestProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestClass::staticTestProperty'), 'TestClass'), + new SymbolInformation('testProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('TestClass::testProperty'), 'TestClass'), + new SymbolInformation('staticTestMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestClass::staticTestMethod()'), 'TestClass'), + new SymbolInformation('testMethod', SymbolKind::METHOD, $this->getDefinitionLocation('TestClass::testMethod()'), 'TestClass'), new SymbolInformation('TestTrait', SymbolKind::CLASS_, $this->getDefinitionLocation('TestTrait'), ''), new SymbolInformation('TestInterface', SymbolKind::INTERFACE, $this->getDefinitionLocation('TestInterface'), ''), new SymbolInformation('test_function', SymbolKind::FUNCTION, $this->getDefinitionLocation('test_function()'), ''), new SymbolInformation('ChildClass', SymbolKind::CLASS_, $this->getDefinitionLocation('ChildClass'), ''), + new SymbolInformation('TEST_DEFINE_CONSTANT', SymbolKind::CONSTANT, $this->getDefinitionLocation('TEST_DEFINE_CONSTANT'), ''), new SymbolInformation('UnusedClass', SymbolKind::CLASS_, $this->getDefinitionLocation('UnusedClass'), ''), + new SymbolInformation('unusedProperty', SymbolKind::PROPERTY, $this->getDefinitionLocation('UnusedClass::unusedProperty'), 'UnusedClass'), + new SymbolInformation('unusedMethod', SymbolKind::METHOD, $this->getDefinitionLocation('UnusedClass::unusedMethod'), 'UnusedClass'), new SymbolInformation('whatever', SymbolKind::FUNCTION, $this->getDefinitionLocation('whatever()'), ''), + new SymbolInformation('SecondTestNamespace', SymbolKind::NAMESPACE, $this->getDefinitionLocation('SecondTestNamespace'), ''), ], $result); // @codingStandardsIgnoreEnd From fa67f84b73fb8b554d9e45dbc3dd1afe7fba6ecf Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Thu, 23 Nov 2017 02:22:26 -0800 Subject: [PATCH 27/35] perf: get direct children --- src/CompletionProvider.php | 18 ++++++++++-------- src/Index/AbstractAggregateIndex.php | 6 +++--- src/Index/Index.php | 20 ++++++++++++-------- src/Index/ReadableIndex.php | 4 ++-- 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 2f223334..18f00efe 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -220,11 +220,12 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { + // Add the object access operator to only get members of all parents + $prefix = $parentFqn . '->'; + $prefixLen = strlen($prefix); // Collect fqn definitions - foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn) as $fqn => $def) { - // Add the object access operator to only get members of all parents - $prefix = $parentFqn . '->'; - if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember) { + foreach ($this->index->getChildDefinitionsForFqn($parentFqn) as $fqn => $def) { + if (substr($fqn, 0, $prefixLen) === $prefix && $def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } } @@ -250,11 +251,12 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { + // Append :: operator to only get static members of all parents + $prefix = strtolower($parentFqn . '::'); + $prefixLen = strlen($prefix); // Collect fqn definitions - foreach ($this->index->getDescendantDefinitionsForFqn($parentFqn) as $fqn => $def) { - // Append :: operator to only get static members of all parents - $prefix = strtolower($parentFqn . '::'); - if (substr(strtolower($fqn), 0, strlen($prefix)) === $prefix && $def->isMember) { + foreach ($this->index->getChildDefinitionsForFqn($parentFqn) as $fqn => $def) { + if (substr(strtolower($fqn), 0, $prefixLen) === $prefix && $def->isMember) { $list->items[] = CompletionItem::fromDefinition($def); } } diff --git a/src/Index/AbstractAggregateIndex.php b/src/Index/AbstractAggregateIndex.php index 90490ab2..8c8c95a1 100644 --- a/src/Index/AbstractAggregateIndex.php +++ b/src/Index/AbstractAggregateIndex.php @@ -112,15 +112,15 @@ public function getDefinitions(): \Generator } /** - * Returns a Generator that yields all the descendant Definitions of a given FQN + * Returns a Generator that yields all the direct child Definitions of a given FQN * * @param string $fqn * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn): \Generator + public function getChildDefinitionsForFqn(string $fqn): \Generator { foreach ($this->getIndexes() as $index) { - yield from $index->getDescendantDefinitionsForFqn($fqn); + yield from $index->getChildDefinitionsForFqn($fqn); } } diff --git a/src/Index/Index.php b/src/Index/Index.php index dcb1b8f7..6f91e376 100644 --- a/src/Index/Index.php +++ b/src/Index/Index.php @@ -107,12 +107,12 @@ public function getDefinitions(): \Generator } /** - * Returns a Generator that yields all the descendant Definitions of a given FQN + * Returns a Generator that yields all the direct child Definitions of a given FQN * * @param string $fqn * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn): \Generator + public function getChildDefinitionsForFqn(string $fqn): \Generator { $parts = $this->splitFqn($fqn); if ('' === end($parts)) { @@ -122,11 +122,15 @@ public function getDescendantDefinitionsForFqn(string $fqn): \Generator } $result = $this->getIndexValue($parts, $this->definitions); - - if ($result instanceof Definition) { - yield $fqn => $result; - } elseif (is_array($result)) { - yield from $this->yieldDefinitionsRecursively($result, $fqn); + if (!$result) { + return; + } + foreach ($result as $name => $item) { + // Don't yield the parent + if ($name === '') { + continue; + } + yield $fqn.$name => $item; } } @@ -374,7 +378,7 @@ private function getIndexValue(array $parts, array &$storage) $parts = array_slice($parts, 1); // we've reached the last provided part - if (0 === count($parts)) { + if (empty($parts)) { return $storage[$part]; } diff --git a/src/Index/ReadableIndex.php b/src/Index/ReadableIndex.php index 90ddcc45..505bb9a9 100644 --- a/src/Index/ReadableIndex.php +++ b/src/Index/ReadableIndex.php @@ -38,12 +38,12 @@ public function isStaticComplete(): bool; public function getDefinitions(): \Generator; /** - * Returns a Generator that yields all the descendant Definitions of a given FQN + * Returns a Generator that yields all the direct child Definitions of a given FQN * * @param string $fqn * @return \Generator yields Definition */ - public function getDescendantDefinitionsForFqn(string $fqn): \Generator; + public function getChildDefinitionsForFqn(string $fqn): \Generator; /** * Returns the Definition object by a specific FQN From 81c40f2190ec0aebdfde73ee69e8aae47eab98d0 Mon Sep 17 00:00:00 2001 From: Felix Becker Date: Thu, 23 Nov 2017 02:38:23 -0800 Subject: [PATCH 28/35] chore: add completion benchmark --- benchmarks/completion.php | 75 +++++++++++++++++++++++ Performance.php => benchmarks/parsing.php | 2 +- 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 benchmarks/completion.php rename Performance.php => benchmarks/parsing.php (95%) diff --git a/benchmarks/completion.php b/benchmarks/completion.php new file mode 100644 index 00000000..b02d7238 --- /dev/null +++ b/benchmarks/completion.php @@ -0,0 +1,75 @@ +getSize(); + $testProviderArray[] = $file->getRealPath(); + } +} + +if (count($testProviderArray) === 0) { + throw new Exception("ERROR: Validation testsuite frameworks not found - run `git submodule update --init --recursive` to download."); +} + +$index = new Index; +$definitionResolver = new DefinitionResolver($index); +$completionProvider = new CompletionProvider($definitionResolver, $index); +$docBlockFactory = DocBlockFactory::createInstance(); +$completionFile = realpath(__DIR__ . '/../validation/frameworks/symfony/src/Symfony/Component/HttpFoundation/Request.php'); +$parser = new PhpParser\Parser(); +$completionDocument = null; + +echo "Indexing $framework" . PHP_EOL; + +foreach ($testProviderArray as $idx => $testCaseFile) { + if (filesize($testCaseFile) > 100000) { + continue; + } + if ($idx % 100 === 0) { + echo $idx . '/' . count($testProviderArray) . PHP_EOL; + } + + $fileContents = file_get_contents($testCaseFile); + + try { + $d = new PhpDocument($testCaseFile, $fileContents, $index, $parser, $docBlockFactory, $definitionResolver); + if ($testCaseFile === $completionFile) { + $completionDocument = $d; + } + } catch (\Throwable $e) { + echo $e->getMessage() . PHP_EOL; + continue; + } +} + +echo "Getting completion". PHP_EOL; + +$start = microtime(true); +$list = $completionProvider->provideCompletion($completionDocument, new Position(274, 15)); +$end = microtime(true); + +echo count($list->items) . ' completion items' . PHP_EOL; + +echo "Time: " . ($end - $start) . 's' . PHP_EOL; + diff --git a/Performance.php b/benchmarks/parsing.php similarity index 95% rename from Performance.php rename to benchmarks/parsing.php index 5022f847..8eaae6a2 100644 --- a/Performance.php +++ b/benchmarks/parsing.php @@ -17,7 +17,7 @@ $frameworks = ["drupal", "wordpress", "php-language-server", "tolerant-php-parser", "math-php", "symfony", "CodeIgniter", "cakephp"]; foreach($frameworks as $framework) { - $iterator = new RecursiveDirectoryIterator(__DIR__ . "/validation/frameworks/$framework"); + $iterator = new RecursiveDirectoryIterator(__DIR__ . "/../validation/frameworks/$framework"); $testProviderArray = array(); foreach (new RecursiveIteratorIterator($iterator) as $file) { From 439cebe1acf72e191dac1165289557f20817746f Mon Sep 17 00:00:00 2001 From: Declspeck Date: Sat, 3 Feb 2018 23:06:04 +0200 Subject: [PATCH 29/35] fix(tests): fix require in parsing.php benchmark after file move --- benchmarks/parsing.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/benchmarks/parsing.php b/benchmarks/parsing.php index e68bc1bd..bb7c3c48 100644 --- a/benchmarks/parsing.php +++ b/benchmarks/parsing.php @@ -1,7 +1,7 @@ Date: Sat, 3 Feb 2018 23:10:13 +0200 Subject: [PATCH 30/35] refactor(completion): make completion of global symbols use Index more efficiently --- src/CompletionProvider.php | 165 ++++++++++++------- src/Index/Index.php | 41 ++--- tests/Server/TextDocument/CompletionTest.php | 23 +-- 3 files changed, 127 insertions(+), 102 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index fe00c719..9811419e 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -127,8 +127,11 @@ public function __construct(DefinitionResolver $definitionResolver, ReadableInde * @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 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon $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 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon /** The closest NamespaceDefinition Node */ $namespaceNode = $node->getNamespaceDefinition(); - /** @var string The name of the namespace */ - $namespacedPrefix = null; - if ($namespaceNode) { - $namespacedPrefix = (string)PhpParser\ResolvedName::buildName($namespaceNode->name->nameParts, $node->getFileContents()) . '\\' . $prefix; - $namespacedPrefixLen = strlen($namespacedPrefix); + if ($nameNode instanceof Node\QualifiedName) { + /** @var array For Psr\Http\Mess this will be ['Psr', 'Http'] */ + $namePartsWithoutLast = $nameNode->nameParts; + array_pop($namePartsWithoutLast); + /** @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))) { - $list->items[] = CompletionItem::fromDefinition($def); + if (substr($alias, 0, $prefixLen) === $prefix + && ($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); + $item->insertText = $alias; + $list->items[] = $item; } } } - // 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)) { - // Insert the global FQN with leading backslash - $item->insertText = '\\' . $fqn; - } else { - // Insert the FQN without leading backlash - $item->insertText = $fqn; + foreach ($namespacesToSearch as $namespaceToSearch) { + foreach ($this->index->getChildDefinitionsForFqn($namespaceToSearch) as $fqn => $def) { + if (isset($creation) && !$def->canBeInstantiated) { + // Only suggest classes for `new` + continue; } - // 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); + + $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; } - $list->items[] = $item; } } - // If not a class instantiation, also suggest keywords - if (!isset($creation)) { + // 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 @@ private function suggestVariablesAtNode(Node $node, string $namePrefix = ''): ar } } - if ($level instanceof Node\Expression\AnonymousFunctionCreationExpression && $level->anonymousFunctionUseClause !== null && - $level->anonymousFunctionUseClause->useVariableNameList !== null) { + if ($level instanceof Node\Expression\AnonymousFunctionCreationExpression + && $level->anonymousFunctionUseClause !== null + && $level->anonymousFunctionUseClause->useVariableNameList !== null) { foreach ($level->anonymousFunctionUseClause->useVariableNameList->getValues() as $use) { $useName = $use->getName(); if (empty($namePrefix) || strpos($useName, $namePrefix) !== false) { diff --git a/src/Index/Index.php b/src/Index/Index.php index 6f91e376..2459bbbc 100644 --- a/src/Index/Index.php +++ b/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 @@ public function getChildDefinitionsForFqn(string $fqn): \Generator 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 @@ private function splitFqn(string $fqn): array // split fqn at backslashes $parts = explode('\\', $fqn); - // write back the backslach prefix to the first part if it was present - if ('' === $parts[0]) { - if (count($parts) > 1) { - $parts = array_slice($parts, 1); - } - + // write back the backslash prefix to the first part if it was present + if ('' === $parts[0] && count($parts) > 1) { + $parts = array_slice($parts, 1); $parts[0] = '\\' . $parts[0]; } @@ -346,7 +347,8 @@ private function splitFqn(string $fqn): array } } - 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 @@ private function splitFqn(string $fqn): array * 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 array &$storage The array in which to store the $definition + * @param string[] $path The splitted FQN + * @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); - // we've reached the last provided part - if (empty($parts)) { - return $storage[$part]; - } - - return $this->getIndexValue($parts, $storage[$part]); + return $this->getIndexValue($path, $storage[$part]); } /** diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index 8e55263e..a29658d2 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -218,24 +218,6 @@ public function testNewInNamespace() 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 @@ public function testUsedClass() '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); } From 6858bd3513f8dba28eaf6c65e1b3be4d56a73d64 Mon Sep 17 00:00:00 2001 From: Declspeck Date: Sat, 3 Feb 2018 23:25:53 +0200 Subject: [PATCH 31/35] tests(completion): add completion of new| ParameterBag to completion benchmark --- benchmarks/completion.php | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/benchmarks/completion.php b/benchmarks/completion.php index b02d7238..7cc86ddb 100644 --- a/benchmarks/completion.php +++ b/benchmarks/completion.php @@ -65,11 +65,17 @@ echo "Getting completion". PHP_EOL; +// Completion in $this->|request = new ParameterBag($request); $start = microtime(true); $list = $completionProvider->provideCompletion($completionDocument, new Position(274, 15)); $end = microtime(true); - +echo 'Time ($this->|): ' . ($end - $start) . 's' . PHP_EOL; echo count($list->items) . ' completion items' . PHP_EOL; -echo "Time: " . ($end - $start) . 's' . PHP_EOL; - +// Completion in $this->request = new| ParameterBag($request); +// (this only finds ParameterBag though.) +$start = microtime(true); +$list = $completionProvider->provideCompletion($completionDocument, new Position(274, 28)); +$end = microtime(true); +echo 'Time (new|): ' . ($end - $start) . 's' . PHP_EOL; +echo count($list->items) . ' completion items' . PHP_EOL; From d6b4e794918b1897473396dfd1b039c64af2c5ae Mon Sep 17 00:00:00 2001 From: Declspeck Date: Fri, 9 Feb 2018 22:42:22 +0200 Subject: [PATCH 32/35] feat(completion): complete for used namespaces --- fixtures/completion/used_namespace.php | 9 + fixtures/symbols.php | 5 + src/CompletionProvider.php | 234 ++++++++++++------ tests/NodeVisitor/DefinitionCollectorTest.php | 5 +- tests/Server/ServerTestCase.php | 4 +- tests/Server/TextDocument/CompletionTest.php | 27 +- .../TextDocument/DocumentSymbolTest.php | 4 +- tests/Server/Workspace/SymbolTest.php | 4 +- 8 files changed, 203 insertions(+), 89 deletions(-) create mode 100644 fixtures/completion/used_namespace.php diff --git a/fixtures/completion/used_namespace.php b/fixtures/completion/used_namespace.php new file mode 100644 index 00000000..7c822900 --- /dev/null +++ b/fixtures/completion/used_namespace.php @@ -0,0 +1,9 @@ +classTypeDesignator : $node; - /** The typed name */ - $prefix = $nameNode instanceof Node\QualifiedName - ? (string)PhpParser\ResolvedName::buildName($nameNode->nameParts, $nameNode->getFileContents()) - : $nameNode->getText($node->getFileContents()); + $filterNameTokens = static function ($tokens) { + return array_values( + array_filter( + $tokens, + static function ($token): bool { + return $token->kind === PhpParser\TokenKind::Name; + } + ) + ); + }; + + /** @var string[] The written name, exploded by \ */ + $prefix = array_map( + static function ($part) use ($node) : string { + return $part->getText($node->getFileContents()); + }, + $filterNameTokens( + $nameNode instanceof Node\QualifiedName + ? $nameNode->nameParts + : [$nameNode] + ) + ); + + if ($prefix === ['']) { + $prefix = []; + } /** Whether the prefix is qualified (contains at least one backslash) */ $isQualified = $nameNode instanceof Node\QualifiedName && $nameNode->isQualifiedName(); @@ -295,103 +318,100 @@ public function provideCompletion( /** The closest NamespaceDefinition Node */ $namespaceNode = $node->getNamespaceDefinition(); - if ($nameNode instanceof Node\QualifiedName) { - /** @var array For Psr\Http\Mess this will be ['Psr', 'Http'] */ - $namePartsWithoutLast = $nameNode->nameParts; - array_pop($namePartsWithoutLast); - /** @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 = ''; - } + // Get the namespace use statements + // TODO: use function statements, use const statements - /** @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() - ); + /** @var string[] $aliases A map from local alias to fully qualified name */ + list($aliases,,) = $node->getImportTablesForCurrentScope(); - if ($prefixParentNamespace === '') { - $namespacesToSearch[] = $currentNamespace; - } else { - // Partially qualified, concatenate with current namespace. - $namespacesToSearch[] = $currentNamespace . '\\' . $prefixParentNamespace; + /** @var array Array of [fqn=string, requiresRoaming=bool] the prefix may represent. */ + $possibleFqns = []; + + if ($isFullyQualified) { + // Case \Microsoft\PhpParser\Res| + $possibleFqns[] = [$prefix, false]; + } else if ($fqnAfterAlias = $this->tryApplyAlias($aliases, $prefix)) { + // Cases handled here: (i.e. all namespaces involving use clauses) + // + // use Microsoft\PhpParser\Node; //Note that Node is both a class and a namespace. + // Nod| + // Node\Qual| + // + // use Microsoft\PhpParser as TheParser; + // TheParser\Nod| + $possibleFqns[] = [$fqnAfterAlias, false]; + } else if ($namespaceNode) { + // Cases handled here: + // + // namespace Foo; + // Microsoft\PhpParser\Nod| // Can refer only to \Foo\Microsoft, not to \Microsoft. + // + // namespace Foo; + // Test| // Can refer either to functions or constants at the global scope, or to + // // everything below \Foo. (Global fallback / roaming) + /** @var \Microsoft\PhpParser\ResolvedName Declared namespace of the file (or section) */ + $namespacedFqn = array_merge( + array_map( + static function ($token) use ($namespaceNode): string { + return $token->getText($namespaceNode->getFileContents()); + }, + $filterNameTokens($namespaceNode->name->nameParts) + ), + $prefix + ); + $possibleFqns[] = [$namespacedFqn, false]; + if (!$isQualified) { + // Case of global fallback. If nothing is entered, also complete for root-level classnames. + // If something has been entered, complete root-level roamed symbols only. + $possibleFqns[] = [$prefix, !empty($prefix)]; } - /** @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[] = ''; + // Case handled here: (no namespace declaration in file) + // + // Microsoft\PhpParser\N| + $possibleFqns[] = [$prefix, false]; } - /** @var int Length of $namespacedPrefix */ - $namespacedPrefixLen = strlen($namespacedPrefix); + $prefixStr = implode('\\', $prefix); /** @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(); + $prefixLen = strlen($prefixStr); - // If there is a prefix that does not start with a slash, suggest `use`d symbols + // If there is a prefix that does not contain a slash, suggest used names. if (!$isQualified) { foreach ($aliases as $alias => $fqn) { // Suggest symbols that have been `use`d and match the prefix - if (substr($alias, 0, $prefixLen) === $prefix + if (substr($alias, 0, $prefixLen) === $prefixStr && ($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); - $item->insertText = $alias; - $list->items[] = $item; + $list->items[] = CompletionItem::fromDefinition($def); } } } - foreach ($namespacesToSearch as $namespaceToSearch) { + foreach ($possibleFqns as list ($fqnToSearch, $requiresRoaming)) { + $namespaceToSearch = $fqnToSearch; + array_pop($namespaceToSearch); + $namespaceToSearch = implode('\\', $namespaceToSearch); + $fqnToSearch = implode('\\', $fqnToSearch); + $fqnToSearchLen = strlen($fqnToSearch); foreach ($this->index->getChildDefinitionsForFqn($namespaceToSearch) as $fqn => $def) { if (isset($creation) && !$def->canBeInstantiated) { // Only suggest classes for `new` continue; } + if ($requiresRoaming && !$def->roamed) { + 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) - ) { + if (substr($fqn, 0, $fqnToSearchLen) === $fqnToSearch) { $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 + if (($aliasMatch = $this->tryMatchAlias($aliases, $fqn)) !== null) { + $item->insertText = $aliasMatch; + } else if ($namespaceNode && (empty($prefix) || $requiresRoaming)) { + // Insert the global FQN with a leading backslash. + // For empty prefix: Assume that the user wants an FQN. They have not + // started writing anything yet, so we are not second-guessing. + // For roaming: Second-guess that the user doesn't want to depend on + // roaming. $item->insertText = '\\' . $fqn; } else { // Insert the FQN without a leading backslash @@ -410,7 +430,7 @@ public function provideCompletion( // Suggest keywords if (!$isQualified && !isset($creation)) { foreach (self::KEYWORDS as $keyword) { - if (substr($keyword, 0, $prefixLen) === $prefix) { + if (substr($keyword, 0, $prefixLen) === $prefixStr) { $item = new CompletionItem($keyword, CompletionItemKind::KEYWORD); $item->insertText = $keyword; $list->items[] = $item; @@ -422,6 +442,62 @@ public function provideCompletion( return $list; } + private function tryMatchAlias( + array $aliases, + string $fullyQualifiedName + ): ?string { + $fullyQualifiedName = explode('\\', $fullyQualifiedName); + $aliasMatch = null; + $aliasMatchLength = null; + foreach ($aliases as $alias => $aliasFqn) { + $aliasFqn = $aliasFqn->getNameParts(); + $aliasFqnLength = count($aliasFqn); + if ($aliasMatchLength && $aliasFqnLength < $aliasFqnLength) { + // Find the longest possible match. This one won't do. + continue; + } + $fqnStart = array_slice($fullyQualifiedName, 0, $aliasFqnLength); + if ($fqnStart === $aliasFqn) { + $aliasMatch = $alias; + $aliasMatchLength = $aliasFqnLength; + } + } + + if ($aliasMatch === null) { + return null; + } + + $fqnNoAlias = array_slice($fullyQualifiedName, $aliasMatchLength); + return join('\\', array_merge([$aliasMatch], $fqnNoAlias)); + } + + /** + * Tries to convert a partially qualified name to an FQN using aliases. + * + * Example: + * + * use Microsoft\PhpParser as TheParser; + * "TheParser\Node" will convert to "Microsoft\PhpParser\Node" + * + * @param \Microsoft\PhpParser\ResolvedName[] $aliases + * Aliases available in the scope of resolution. Keyed by alias. + * @param string[] $partiallyQualifiedName + **/ + private function tryApplyAlias( + array $aliases, + array $partiallyQualifiedName + ): ?array { + if (empty($partiallyQualifiedName)) { + return null; + } + $head = $partiallyQualifiedName[0]; + $tail = array_slice($partiallyQualifiedName, 1); + if (!isset($aliases[$head])) { + return null; + } + return array_merge($aliases[$head]->getNameParts(), $tail); + } + /** * Yields FQNs from an array along with the FQNs of all parent classes * diff --git a/tests/NodeVisitor/DefinitionCollectorTest.php b/tests/NodeVisitor/DefinitionCollectorTest.php index a092b625..c27822ce 100644 --- a/tests/NodeVisitor/DefinitionCollectorTest.php +++ b/tests/NodeVisitor/DefinitionCollectorTest.php @@ -35,7 +35,9 @@ public function testCollectsSymbols() 'TestNamespace\\ChildClass', 'TestNamespace\\Example', 'TestNamespace\\Example->__construct()', - 'TestNamespace\\Example->__destruct()' + 'TestNamespace\\Example->__destruct()', + 'TestNamespace\\InnerNamespace', + 'TestNamespace\\InnerNamespace\\InnerClass', ], array_keys($defNodes)); $this->assertInstanceOf(Node\ConstElement::class, $defNodes['TestNamespace\\TEST_CONST']); @@ -53,6 +55,7 @@ public function testCollectsSymbols() $this->assertInstanceOf(Node\Statement\ClassDeclaration::class, $defNodes['TestNamespace\\Example']); $this->assertInstanceOf(Node\MethodDeclaration::class, $defNodes['TestNamespace\\Example->__construct()']); $this->assertInstanceOf(Node\MethodDeclaration::class, $defNodes['TestNamespace\\Example->__destruct()']); + $this->assertInstanceOf(Node\Statement\ClassDeclaration::class, $defNodes['TestNamespace\\InnerNamespace\\InnerClass']); } public function testDoesNotCollectReferences() diff --git a/tests/Server/ServerTestCase.php b/tests/Server/ServerTestCase.php index 45d949ff..1403d43e 100644 --- a/tests/Server/ServerTestCase.php +++ b/tests/Server/ServerTestCase.php @@ -107,7 +107,9 @@ public function setUp() 'TestNamespace\\whatever()' => new Location($referencesUri, new Range(new Position(21, 0), new Position(23, 1))), 'TestNamespace\\Example' => new Location($symbolsUri, new Range(new Position(101, 0), new Position(104, 1))), 'TestNamespace\\Example::__construct' => new Location($symbolsUri, new Range(new Position(102, 4), new Position(102, 36))), - 'TestNamespace\\Example::__destruct' => new Location($symbolsUri, new Range(new Position(103, 4), new Position(103, 35))) + 'TestNamespace\\Example::__destruct' => new Location($symbolsUri, new Range(new Position(103, 4), new Position(103, 35))), + 'TestNamespace\\InnerNamespace' => new Location($symbolsUri, new Range(new Position(106, 0), new Position(106, 39))), + 'TestNamespace\\InnerNamespace\\InnerClass' => new Location($symbolsUri, new Range(new Position(108, 0), new Position(109, 1))), ]; $this->referenceLocations = [ diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index a29658d2..f3fa017b 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -213,10 +213,7 @@ public function testNewInNamespace() '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.', - null, - null, - 'TestClass' + 'sint. Officia culpa pariatur laborum nostrud cupidatat consequat mollit.' ), ], true), $items); } @@ -239,10 +236,28 @@ public function testUsedClass() '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.' + ) + ], true), $items); + } + + public function testUsedNamespace() + { + $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/used_namespace.php'); + $this->loader->open($completionUri, file_get_contents($completionUri)); + $items = $this->textDocument->completion( + new TextDocumentIdentifier($completionUri), + new Position(6, 16) + )->wait(); + $this->assertCompletionsListSubset(new CompletionList([ + new CompletionItem( + 'InnerClass', + CompletionItemKind::CLASS_, + 'TestNamespace\\InnerNamespace', null, null, - 'TestClass' + null, + 'AliasNamespace\\InnerClass' ) ], true), $items); } diff --git a/tests/Server/TextDocument/DocumentSymbolTest.php b/tests/Server/TextDocument/DocumentSymbolTest.php index 155e4a29..2f5054d6 100644 --- a/tests/Server/TextDocument/DocumentSymbolTest.php +++ b/tests/Server/TextDocument/DocumentSymbolTest.php @@ -32,7 +32,9 @@ public function test() new SymbolInformation('ChildClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\ChildClass'), 'TestNamespace'), new SymbolInformation('Example', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\Example'), 'TestNamespace'), new SymbolInformation('__construct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__construct'), 'TestNamespace\\Example'), - new SymbolInformation('__destruct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__destruct'), 'TestNamespace\\Example') + new SymbolInformation('__destruct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__destruct'), 'TestNamespace\\Example'), + new SymbolInformation('TestNamespace\\InnerNamespace', SymbolKind::NAMESPACE, $this->getDefinitionLocation('TestNamespace\\InnerNamespace'), 'TestNamespace'), + new SymbolInformation('InnerClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\InnerNamespace\\InnerClass'), 'TestNamespace\\InnerNamespace'), ], $result); // @codingStandardsIgnoreEnd } diff --git a/tests/Server/Workspace/SymbolTest.php b/tests/Server/Workspace/SymbolTest.php index 765841bb..f2eee17d 100644 --- a/tests/Server/Workspace/SymbolTest.php +++ b/tests/Server/Workspace/SymbolTest.php @@ -30,7 +30,7 @@ public function testEmptyQueryReturnsAllSymbols() // @codingStandardsIgnoreStart $this->assertEquals([ - new SymbolInformation('TestNamespace', SymbolKind::NAMESPACE, new Location($referencesUri, new Range(new Position(2, 0), new Position(2, 24))), ''), + new SymbolInformation('TestNamespace', SymbolKind::NAMESPACE, new Location($referencesUri, new Range(new Position(2, 0), new Position(2, 24))), ''), // Namespaced new SymbolInformation('TEST_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TestNamespace\\TEST_CONST'), 'TestNamespace'), new SymbolInformation('TestClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\TestClass'), 'TestNamespace'), @@ -46,6 +46,8 @@ public function testEmptyQueryReturnsAllSymbols() new SymbolInformation('Example', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\Example'), 'TestNamespace'), new SymbolInformation('__construct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__construct'), 'TestNamespace\\Example'), new SymbolInformation('__destruct', SymbolKind::CONSTRUCTOR, $this->getDefinitionLocation('TestNamespace\\Example::__destruct'), 'TestNamespace\\Example'), + new SymbolInformation('TestNamespace\\InnerNamespace', SymbolKind::NAMESPACE, $this->getDefinitionLocation('TestNamespace\\InnerNamespace'), 'TestNamespace'), + new SymbolInformation('InnerClass', SymbolKind::CLASS_, $this->getDefinitionLocation('TestNamespace\\InnerNamespace\\InnerClass'), 'TestNamespace\\InnerNamespace'), new SymbolInformation('whatever', SymbolKind::FUNCTION, $this->getDefinitionLocation('TestNamespace\\whatever()'), 'TestNamespace'), // Global new SymbolInformation('TEST_CONST', SymbolKind::CONSTANT, $this->getDefinitionLocation('TEST_CONST'), ''), From d1933b83322728fba847942b7fe8c5d135f70058 Mon Sep 17 00:00:00 2001 From: Declspeck Date: Thu, 15 Feb 2018 22:47:57 +0200 Subject: [PATCH 33/35] refactor(completion): rewrite global name completion with generators --- fixtures/completion/used_namespace.php | 5 +- src/CompletionProvider.php | 413 +++++++++++-------- src/FqnUtilities.php | 88 ++++ tests/Server/TextDocument/CompletionTest.php | 196 ++++++--- 4 files changed, 468 insertions(+), 234 deletions(-) diff --git a/fixtures/completion/used_namespace.php b/fixtures/completion/used_namespace.php index 7c822900..40a7e50d 100644 --- a/fixtures/completion/used_namespace.php +++ b/fixtures/completion/used_namespace.php @@ -4,6 +4,7 @@ use TestNamespace\InnerNamespace as AliasNamespace; -AliasNamespace\I - class IDontShowUpInCompletion {} + +AliasNamespace\I; +AliasNamespace\; diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 933b8e75..4d982d90 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -18,6 +18,13 @@ use Microsoft\PhpParser\Node; use Microsoft\PhpParser\ResolvedName; use Generator; +use function LanguageServer\FqnUtilities\{ + nameConcat, + nameGetFirstPart, + nameGetParent, + nameStartsWith, + nameWithoutFirstPart +}; class CompletionProvider { @@ -280,224 +287,278 @@ public function provideCompletion( // my_func| // MY_CONS| // MyCla| + // \MyCla| // The name Node under the cursor $nameNode = isset($creation) ? $creation->classTypeDesignator : $node; - $filterNameTokens = static function ($tokens) { - return array_values( - array_filter( - $tokens, - static function ($token): bool { - return $token->kind === PhpParser\TokenKind::Name; - } - ) - ); - }; - - /** @var string[] The written name, exploded by \ */ - $prefix = array_map( - static function ($part) use ($node) : string { - return $part->getText($node->getFileContents()); - }, - $filterNameTokens( - $nameNode instanceof Node\QualifiedName - ? $nameNode->nameParts - : [$nameNode] - ) - ); - - if ($prefix === ['']) { - $prefix = []; + if ($nameNode instanceof Node\QualifiedName) { + /** @var string The typed name. */ + $prefix = (string)PhpParser\ResolvedName::buildName($nameNode->nameParts, $nameNode->getFileContents()); + } else { + $prefix = $nameNode->getText($node->getFileContents()); } - /** Whether the prefix is qualified (contains at least one backslash) */ - $isQualified = $nameNode instanceof Node\QualifiedName && $nameNode->isQualifiedName(); + $namespaceNode = $node->getNamespaceDefinition(); + /** @var string The current namespace without a leading backslash. */ + $currentNamespace = $namespaceNode === null ? '' : $namespaceNode->name->getText(); - /** Whether the prefix is fully qualified (begins with a backslash) */ - $isFullyQualified = $nameNode instanceof Node\QualifiedName && $nameNode->isFullyQualifiedName(); + /** @var bool Whether the prefix is qualified (contains at least one backslash) */ + $isFullyQualified = false; - /** The closest NamespaceDefinition Node */ - $namespaceNode = $node->getNamespaceDefinition(); + /** @var bool Whether the prefix is qualified (contains at least one backslash) */ + $isQualified = false; - // Get the namespace use statements - // TODO: use function statements, use const statements + if ($nameNode instanceof Node\QualifiedName) { + $isFullyQualified = $nameNode->isFullyQualifiedName(); + $isQualified = $nameNode->isQualifiedName(); + } - /** @var string[] $aliases A map from local alias to fully qualified name */ - list($aliases,,) = $node->getImportTablesForCurrentScope(); + /** @var bool Whether we are in a new expression */ + $isCreation = isset($creation); - /** @var array Array of [fqn=string, requiresRoaming=bool] the prefix may represent. */ - $possibleFqns = []; + /** @var array Import (use) tables */ + $importTables = $node->getImportTablesForCurrentScope(); if ($isFullyQualified) { - // Case \Microsoft\PhpParser\Res| - $possibleFqns[] = [$prefix, false]; - } else if ($fqnAfterAlias = $this->tryApplyAlias($aliases, $prefix)) { - // Cases handled here: (i.e. all namespaces involving use clauses) - // - // use Microsoft\PhpParser\Node; //Note that Node is both a class and a namespace. - // Nod| - // Node\Qual| - // - // use Microsoft\PhpParser as TheParser; - // TheParser\Nod| - $possibleFqns[] = [$fqnAfterAlias, false]; - } else if ($namespaceNode) { - // Cases handled here: - // - // namespace Foo; - // Microsoft\PhpParser\Nod| // Can refer only to \Foo\Microsoft, not to \Microsoft. - // - // namespace Foo; - // Test| // Can refer either to functions or constants at the global scope, or to - // // everything below \Foo. (Global fallback / roaming) - /** @var \Microsoft\PhpParser\ResolvedName Declared namespace of the file (or section) */ - $namespacedFqn = array_merge( - array_map( - static function ($token) use ($namespaceNode): string { - return $token->getText($namespaceNode->getFileContents()); - }, - $filterNameTokens($namespaceNode->name->nameParts) - ), - $prefix + // \Prefix\Goes\Here| - Only return completions from the root namespace. + /** @var $items \Generator|CompletionItem[] Generator yielding CompletionItems indexed by their FQN */ + $items = $this->getCompletionsForFqnPrefix($prefix, $isCreation, false); + } else if ($isQualified) { + // Prefix\Goes\Here| + $items = $this->getPartiallyQualifiedCompletions( + $prefix, + $currentNamespace, + $importTables, + $isCreation ); - $possibleFqns[] = [$namespacedFqn, false]; - if (!$isQualified) { - // Case of global fallback. If nothing is entered, also complete for root-level classnames. - // If something has been entered, complete root-level roamed symbols only. - $possibleFqns[] = [$prefix, !empty($prefix)]; - } } else { - // Case handled here: (no namespace declaration in file) - // - // Microsoft\PhpParser\N| - $possibleFqns[] = [$prefix, false]; + // PrefixGoesHere| + $items = $this->getUnqualifiedCompletions($prefix, $currentNamespace, $importTables, $isCreation); } - $prefixStr = implode('\\', $prefix); - /** @var int Length of $prefix */ - $prefixLen = strlen($prefixStr); - - // If there is a prefix that does not contain a slash, suggest used names. - if (!$isQualified) { - foreach ($aliases as $alias => $fqn) { - // Suggest symbols that have been `use`d and match the prefix - if (substr($alias, 0, $prefixLen) === $prefixStr - && ($def = $this->index->getDefinition((string)$fqn))) { - $list->items[] = CompletionItem::fromDefinition($def); - } + $list->items = array_values(iterator_to_array($items)); + foreach ($list->items as $item) { + // Remove () + if (is_string($item->insertText) && substr($item->insertText, strlen($item->insertText) - 2) === '()') { + $item->insertText = substr($item->insertText, 0, strlen($item->insertText) - 2); } } - foreach ($possibleFqns as list ($fqnToSearch, $requiresRoaming)) { - $namespaceToSearch = $fqnToSearch; - array_pop($namespaceToSearch); - $namespaceToSearch = implode('\\', $namespaceToSearch); - $fqnToSearch = implode('\\', $fqnToSearch); - $fqnToSearchLen = strlen($fqnToSearch); - foreach ($this->index->getChildDefinitionsForFqn($namespaceToSearch) as $fqn => $def) { - if (isset($creation) && !$def->canBeInstantiated) { - // Only suggest classes for `new` - continue; - } - if ($requiresRoaming && !$def->roamed) { - continue; - } + } + return $list; + } - if (substr($fqn, 0, $fqnToSearchLen) === $fqnToSearch) { - $item = CompletionItem::fromDefinition($def); - if (($aliasMatch = $this->tryMatchAlias($aliases, $fqn)) !== null) { - $item->insertText = $aliasMatch; - } else if ($namespaceNode && (empty($prefix) || $requiresRoaming)) { - // Insert the global FQN with a leading backslash. - // For empty prefix: Assume that the user wants an FQN. They have not - // started writing anything yet, so we are not second-guessing. - // For roaming: Second-guess that the user doesn't want to depend on - // roaming. - $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; - } - } + private function getPartiallyQualifiedCompletions( + string $prefix, + string $currentNamespace, + array $importTables, + bool $requireCanBeInstantiated + ): \Generator { + // If the first part of the partially qualified name matches a namespace alias, + // only definitions below that alias can be completed. + list($namespaceAliases,,) = $importTables; + $prefixFirstPart = nameGetFirstPart($prefix); + $foundAlias = $foundAliasFqn = null; + foreach ($namespaceAliases as $alias => $aliasFqn) { + if (strcasecmp($prefixFirstPart, $alias) === 0) { + $foundAlias = $alias; + $foundAliasFqn = (string)$aliasFqn; + break; } + } - // Suggest keywords - if (!$isQualified && !isset($creation)) { - foreach (self::KEYWORDS as $keyword) { - if (substr($keyword, 0, $prefixLen) === $prefixStr) { - $item = new CompletionItem($keyword, CompletionItemKind::KEYWORD); - $item->insertText = $keyword; - $list->items[] = $item; - } - } + if ($foundAlias !== null) { + yield from $this->getCompletionsFromAliasedNamespace( + $prefix, + $foundAlias, + $foundAliasFqn, + $requireCanBeInstantiated + ); + } else { + yield from $this->getCompletionsForFqnPrefix( + nameConcat($currentNamespace, $prefix), + $requireCanBeInstantiated, + false + ); + } + } + + /** + * Yields completions for non-qualified global names. + * + * Yields + * - Aliased classes + * - Completions from current namespace + * - Roamed completions from the global namespace (when not creating and not already in root NS) + * - PHP keywords (when not creating) + * + * @return \Generator|CompletionItem[] + * Yields CompletionItems + */ + private function getUnqualifiedCompletions( + string $prefix, + string $currentNamespace, + array $importTables, + bool $requireCanBeInstantiated + ): \Generator { + // Aliases + list($namespaceAliases,,) = $importTables; + // use Foo\Bar + yield from $this->getCompletionsForAliases( + $prefix, + $namespaceAliases, + $requireCanBeInstantiated + ); + + // Completions from the current namespace + yield from $this->getCompletionsForFqnPrefix( + nameConcat($currentNamespace, $prefix), + $requireCanBeInstantiated, + false + ); + + if ($currentNamespace !== '' && $prefix === '') { + // Get additional suggestions from the global namespace. + // When completing e.g. for new |, suggest \DateTime + yield from $this->getCompletionsForFqnPrefix('', $requireCanBeInstantiated, true); + } + + if (!$requireCanBeInstantiated) { + if ($currentNamespace !== '' && $prefix !== '') { + // Roamed definitions (i.e. global constants and functions). The prefix is checked against '', since + // in that case global completions have already been provided (including non-roamed definitions.) + yield from $this->getRoamedCompletions($prefix); } + + // Lastly and least importantly, suggest keywords. + yield from $this->getCompletionsForKeywords($prefix); } + } - return $list; + /** + * Gets completions for prefixes of fully qualified names in their parent namespace. + * + * @param string $prefix Prefix to complete for. Fully qualified. + * @param bool $requireCanBeInstantiated If set, only return classes. + * @param bool $insertFullyQualified If set, return completion with the leading \ inserted. + * @return \Generator|CompletionItem[] + * Yields CompletionItems. + */ + private function getCompletionsForFqnPrefix( + string $prefix, + bool $requireCanBeInstantiated, + bool $insertFullyQualified + ): \Generator { + $namespace = nameGetParent($prefix); + foreach ($this->index->getChildDefinitionsForFqn($namespace) as $fqn => $def) { + if ($requireCanBeInstantiated && !$def->canBeInstantiated) { + continue; + } + if (!nameStartsWith($fqn, $prefix)) { + continue; + } + $completion = CompletionItem::fromDefinition($def); + if ($insertFullyQualified) { + $completion->insertText = '\\' . $fqn; + } + yield $fqn => $completion; + } } - private function tryMatchAlias( + /** + * Gets completions for non-qualified names matching the start of an used class, function, or constant. + * + * @param string $prefix Non-qualified name being completed for + * @param QualifiedName[] $aliases Array of alias FQNs indexed by the alias. + * @return \Generator|CompletionItem[] + * Yields CompletionItems. + */ + private function getCompletionsForAliases( + string $prefix, array $aliases, - string $fullyQualifiedName - ): ?string { - $fullyQualifiedName = explode('\\', $fullyQualifiedName); - $aliasMatch = null; - $aliasMatchLength = null; + bool $requireCanBeInstantiated + ): \Generator { foreach ($aliases as $alias => $aliasFqn) { - $aliasFqn = $aliasFqn->getNameParts(); - $aliasFqnLength = count($aliasFqn); - if ($aliasMatchLength && $aliasFqnLength < $aliasFqnLength) { - // Find the longest possible match. This one won't do. + if (!nameStartsWith($alias, $prefix)) { continue; } - $fqnStart = array_slice($fullyQualifiedName, 0, $aliasFqnLength); - if ($fqnStart === $aliasFqn) { - $aliasMatch = $alias; - $aliasMatchLength = $aliasFqnLength; + $definition = $this->index->getDefinition((string)$aliasFqn); + if ($definition) { + if ($requireCanBeInstantiated && !$definition->canBeInstantiated) { + continue; + } + $completionItem = CompletionItem::fromDefinition($definition); + $completionItem->insertText = $alias; + yield (string)$aliasFqn => $completionItem; } } + } - if ($aliasMatch === null) { - return null; + /** + * Gets completions for partially qualified names, where the first part is matched by an alias. + * + * @return \Generator|CompletionItem[] + * Yields CompletionItems. + */ + private function getCompletionsFromAliasedNamespace( + string $prefix, + string $alias, + string $aliasFqn, + bool $requireCanBeInstantiated + ): \Generator { + $prefixFirstPart = nameGetFirstPart($prefix); + // Matched alias. + $resolvedPrefix = nameConcat($aliasFqn, nameWithoutFirstPart($prefix)); + $completionItems = $this->getCompletionsForFqnPrefix( + $resolvedPrefix, + $requireCanBeInstantiated, + false + ); + // Convert FQNs in the CompletionItems so they are expressed in terms of the alias. + foreach ($completionItems as $fqn => $completionItem) { + /** @var string $fqn with the leading parts determined by the alias removed. Has the leading backslash. */ + $nameWithoutAliasedPart = substr($fqn, strlen($aliasFqn)); + $completionItem->insertText = $alias . $nameWithoutAliasedPart; + yield $fqn => $completionItem; } - - $fqnNoAlias = array_slice($fullyQualifiedName, $aliasMatchLength); - return join('\\', array_merge([$aliasMatch], $fqnNoAlias)); } /** - * Tries to convert a partially qualified name to an FQN using aliases. - * - * Example: - * - * use Microsoft\PhpParser as TheParser; - * "TheParser\Node" will convert to "Microsoft\PhpParser\Node" + * Gets completions for globally defined functions and constants (i.e. symbols which may be used anywhere) * - * @param \Microsoft\PhpParser\ResolvedName[] $aliases - * Aliases available in the scope of resolution. Keyed by alias. - * @param string[] $partiallyQualifiedName - **/ - private function tryApplyAlias( - array $aliases, - array $partiallyQualifiedName - ): ?array { - if (empty($partiallyQualifiedName)) { - return null; + * @return \Generator|CompletionItem[] + * Yields CompletionItems. + */ + private function getRoamedCompletions(string $prefix): \Generator + { + foreach ($this->index->getChildDefinitionsForFqn('') as $fqn => $def) { + if (!$def->roamed || !nameStartsWith($fqn, $prefix)) { + continue; + } + $completionItem = CompletionItem::fromDefinition($def); + // Second-guessing the user here - do not trust roaming to work. If the same symbol is + // inserted in the current namespace, the code will stop working. + $completionItem->insertText = '\\' . $fqn; + yield $fqn => $completionItem; } - $head = $partiallyQualifiedName[0]; - $tail = array_slice($partiallyQualifiedName, 1); - if (!isset($aliases[$head])) { - return null; + } + + /** + * Completes PHP keywords. + * + * @return \Generator|CompletionItem[] + * Yields CompletionItems. + */ + private function getCompletionsForKeywords(string $prefix): \Generator + { + foreach (self::KEYWORDS as $keyword) { + if (nameStartsWith($keyword, $prefix)) { + $item = new CompletionItem($keyword, CompletionItemKind::KEYWORD); + $item->insertText = $keyword; + yield $keyword => $item; + } } - return array_merge($aliases[$head]->getNameParts(), $tail); } /** diff --git a/src/FqnUtilities.php b/src/FqnUtilities.php index b5d01a9d..21f75cb3 100644 --- a/src/FqnUtilities.php +++ b/src/FqnUtilities.php @@ -29,3 +29,91 @@ function getFqnsFromType($type): array } return $fqns; } + +/** + * Returns parent of an FQN. + * + * getFqnParent('') === '' + * getFqnParent('\\') === '' + * getFqnParent('\A') === '' + * getFqnParent('A') === '' + * getFqnParent('\A\') === '\A' // Empty trailing name is considered a name. + * + * @return string + */ +function nameGetParent(string $name): ?string +{ + if ($name === '') { // Special-case handling for the root namespace. + return ''; + } + $parts = explode('\\', $name); + array_pop($parts); + return implode('\\', $parts); +} + +/** + * Concatenates two names. + * + * nameConcat('\Foo\Bar', 'Baz') === '\Foo\Bar\Baz' + * nameConcat('\Foo\Bar\\', '\Baz') === '\Foo\Bar\Baz' + * nameConcat('\\', 'Baz') === '\Baz' + * nameConcat('', 'Baz') === 'Baz' + * + * @return string + */ +function nameConcat(string $a, string $b): string +{ + if ($a === '') { + return $b; + } + $a = rtrim($a, '\\'); + $b = ltrim($b, '\\'); + return "$a\\$b"; +} + +/** + * Returns the first component of $name. + * + * nameGetFirstPart('Foo\Bar') === 'Foo' + * nameGetFirstPart('\Foo\Bar') === 'Foo' + * nameGetFirstPart('') === '' + * nameGetFirstPart('\') === '' + */ +function nameGetFirstPart(string $name): string +{ + $parts = explode('\\', $name, 3); + if ($parts[0] === '' && count($parts) > 1) { + return $parts[1]; + } else { + return $parts[0]; + } +} + +/** + * Removes the first component of $name. + * + * nameWithoutFirstPart('Foo\Bar') === 'Bar' + * nameWithoutFirstPart('\Foo\Bar') === 'Bar' + * nameWithoutFirstPart('') === '' + * nameWithoutFirstPart('\') === '' + */ +function nameWithoutFirstPart(string $name): string +{ + $parts = explode('\\', $name, 3); + if ($parts[0] === '') { + array_shift($parts); + } + array_shift($parts); + return implode('\\', $parts); +} + +/** + * @param string $name Name to match against + * @param string $prefix Prefix $name has to starts with + * @return bool + */ +function nameStartsWith(string $name, string $prefix): bool +{ + return strlen($name) >= strlen($prefix) + && strncmp($name, $prefix, strlen($prefix)) === 0; +} diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index 26a04aa6..de33f9f6 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -47,6 +47,9 @@ public function setUp() $this->textDocument = new Server\TextDocument($this->loader, $definitionResolver, $client, $projectIndex); } + /** + * Tests completion at `$obj->t|` + */ public function testPropertyAndMethodWithPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/property_with_prefix.php'); @@ -71,6 +74,9 @@ public function testPropertyAndMethodWithPrefix() ], true), $items); } + /** + * Tests completion at `public function a() { tes| }` + */ public function testGlobalFunctionInsideNamespaceAndClass() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/inside_namespace_and_method.php'); @@ -92,6 +98,9 @@ public function testGlobalFunctionInsideNamespaceAndClass() ], true), $items); } + /** + * Tests completion at `$obj->|` + */ public function testPropertyAndMethodWithoutPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/property.php'); @@ -116,6 +125,9 @@ public function testPropertyAndMethodWithoutPrefix() ], true), $items); } + /** + * Tests completion at `$|` when variables are defined + */ public function testVariable() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/variable.php'); @@ -148,6 +160,9 @@ public function testVariable() ], true), $items); } + /** + * Tests completion at `$p|` when variables are defined + */ public function testVariableWithPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/variable_with_prefix.php'); @@ -170,6 +185,9 @@ public function testVariableWithPrefix() ], true), $items); } + /** + * Tests completion at `new|` when in a namespace and have used variables. + */ public function testNewInNamespace() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/used_new.php'); @@ -213,11 +231,17 @@ public function testNewInNamespace() '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); } + /** + * Tests completion at `TestC|` with `use TestNamespace\TestClass` + */ public function testUsedClass() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/used_class.php'); @@ -236,32 +260,74 @@ public function testUsedClass() '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); + + $this->assertCompletionsListDoesNotContainLabel('OtherClass', $items); + $this->assertCompletionsListDoesNotContainLabel('TestInterface', $items); } - public function testUsedNamespace() + /** + * Tests completion at `AliasNamespace\I|` with `use TestNamespace\InnerNamespace as AliasNamespace` + */ + public function testUsedNamespaceWithPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/used_namespace.php'); $this->loader->open($completionUri, file_get_contents($completionUri)); $items = $this->textDocument->completion( new TextDocumentIdentifier($completionUri), - new Position(6, 16) + new Position(8, 16) )->wait(); - $this->assertCompletionsListSubset(new CompletionList([ - new CompletionItem( - 'InnerClass', - CompletionItemKind::CLASS_, - 'TestNamespace\\InnerNamespace', - null, - null, - null, - 'AliasNamespace\\InnerClass' - ) - ], true), $items); + $this->assertEquals( + new CompletionList([ + new CompletionItem( + 'InnerClass', + CompletionItemKind::CLASS_, + 'TestNamespace\\InnerNamespace', + null, + null, + null, + 'AliasNamespace\\InnerClass' + ) + ], true), + $items + ); } + /** + * Tests completion at `AliasNamespace\|` with `use TestNamespace\InnerNamespace as AliasNamespace` + */ + public function testUsedNamespaceWithoutPrefix() + { + $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/used_namespace.php'); + $this->loader->open($completionUri, file_get_contents($completionUri)); + $items = $this->textDocument->completion( + new TextDocumentIdentifier($completionUri), + new Position(9, 15) + )->wait(); + $this->assertEquals( + new CompletionList([ + new CompletionItem( + 'InnerClass', + CompletionItemKind::CLASS_, + 'TestNamespace\InnerNamespace', + null, + null, + null, + 'AliasNamespace\InnerClass' + ), + ], true), + $items + ); + } + + /** + * Tests completion at `TestClass::$st|` + */ public function testStaticPropertyWithPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/static_property_with_prefix.php'); @@ -283,6 +349,9 @@ public function testStaticPropertyWithPrefix() ], true), $items); } + /** + * Tests completion at `TestClass::|` + */ public function testStaticWithoutPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/static.php'); @@ -316,6 +385,9 @@ public function testStaticWithoutPrefix() ], true), $items); } + /** + * Tests completion at `TestClass::st|` + */ public function testStaticMethodWithPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/static_method_with_prefix.php'); @@ -325,21 +397,6 @@ public function testStaticMethodWithPrefix() new Position(2, 13) )->wait(); $this->assertCompletionsListSubset(new CompletionList([ - new CompletionItem( - 'TEST_CLASS_CONST', - CompletionItemKind::VARIABLE, - 'int', - 'Anim labore veniam consectetur laboris minim quis aute aute esse nulla ad.' - ), - new CompletionItem( - 'staticTestProperty', - CompletionItemKind::PROPERTY, - '\TestClass[]', - 'Lorem excepteur officia sit anim velit veniam enim.', - null, - null, - '$staticTestProperty' - ), new CompletionItem( 'staticTestMethod', CompletionItemKind::METHOD, @@ -349,6 +406,9 @@ public function testStaticMethodWithPrefix() ], true), $items); } + /** + * Tests completion at `TestClass::TE` at the root level. + */ public function testClassConstWithPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/class_const_with_prefix.php'); @@ -363,25 +423,13 @@ public function testClassConstWithPrefix() CompletionItemKind::VARIABLE, 'int', 'Anim labore veniam consectetur laboris minim quis aute aute esse nulla ad.' - ), - new CompletionItem( - 'staticTestProperty', - CompletionItemKind::PROPERTY, - '\TestClass[]', - 'Lorem excepteur officia sit anim velit veniam enim.', - null, - null, - '$staticTestProperty' - ), - new CompletionItem( - 'staticTestMethod', - CompletionItemKind::METHOD, - 'mixed', - 'Do magna consequat veniam minim proident eiusmod incididunt aute proident.' ) ], true), $items); } + /** + * Test completion at `\TestC|` in a namespace + */ public function testFullyQualifiedClass() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/fully_qualified_class.php'); @@ -400,14 +448,18 @@ public function testFullyQualifiedClass() '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.', - null, - null, - 'TestClass' + 'sint. Officia culpa pariatur laborum nostrud cupidatat consequat mollit.' ) ], true), $items); + // Assert that all results are non-namespaced. + foreach ($items->items as $item) { + $this->assertSame($item->detail, null); + } } + /** + * Tests completion at `cl|` at root level + */ public function testKeywords() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/keywords.php'); @@ -422,6 +474,9 @@ public function testKeywords() ], true), $items); } + /** + * Tests completion in an empty file + */ public function testHtmlWithoutPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/html.php'); @@ -444,6 +499,9 @@ public function testHtmlWithoutPrefix() ], true), $items); } + /** + * Tests completion in `<|` when not within `assertEquals(new CompletionList([], true), $items); } + /** + * Tests completion in `<|` when not within `assertEquals(new CompletionList([], true), $items); } + /** + * Tests completion at `<|` when not within `assertCompletionsListSubset(new CompletionList([ new CompletionItem( 'SomeNamespace', - CompletionItemKind::MODULE, - null, - null, - null, - null, - 'SomeNamespace' + CompletionItemKind::MODULE ) ], true), $items); } - public function testBarePhp() + /** + * Tests completion at `echo $ab|` at the root level. + */ + public function testBarePhpVariable() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/bare_php.php'); $this->loader->open($completionUri, file_get_contents($completionUri)); @@ -776,6 +844,16 @@ private function assertCompletionsListSubset(CompletionList $subsetList, Complet $this->assertEquals($subsetList->isIncomplete, $list->isIncomplete); } + private function assertCompletionsListDoesNotContainLabel(string $label, CompletionList $list) + { + foreach ($list->items as $item) { + $this->assertNotSame($label, $item->label, "Completion list should not contain $label."); + } + } + + /** + * Tests completion for `$this->|` + */ public function testThisWithoutPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/this.php'); @@ -812,6 +890,9 @@ public function testThisWithoutPrefix() ], true), $items); } + /** + * Tests completion at `$this->m|` + */ public function testThisWithPrefix() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/this_with_prefix.php'); @@ -860,6 +941,9 @@ public function testThisWithPrefix() ], true), $items); } + /** + * Tests completion at `$this->foo()->q|` + */ public function testThisReturnValue() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/this_return_value.php'); From 0bc5b81561c355b68aaa05ac0a9a0b6908cdde85 Mon Sep 17 00:00:00 2001 From: Declspeck Date: Thu, 15 Feb 2018 23:13:04 +0200 Subject: [PATCH 34/35] fix(completion): Return type hint --- src/FqnUtilities.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/FqnUtilities.php b/src/FqnUtilities.php index 21f75cb3..5b3d523d 100644 --- a/src/FqnUtilities.php +++ b/src/FqnUtilities.php @@ -41,7 +41,7 @@ function getFqnsFromType($type): array * * @return string */ -function nameGetParent(string $name): ?string +function nameGetParent(string $name): string { if ($name === '') { // Special-case handling for the root namespace. return ''; From 75e07b86a024e458912e380fcdc25930b83db813 Mon Sep 17 00:00:00 2001 From: Declspeck Date: Fri, 16 Feb 2018 00:12:08 +0200 Subject: [PATCH 35/35] feat(completion): completion for relative names --- fixtures/completion/relative.php | 9 ++++ src/CompletionProvider.php | 21 +++++++-- tests/Server/TextDocument/CompletionTest.php | 48 ++++++++++++++++++++ 3 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 fixtures/completion/relative.php diff --git a/fixtures/completion/relative.php b/fixtures/completion/relative.php new file mode 100644 index 00000000..2bac9c60 --- /dev/null +++ b/fixtures/completion/relative.php @@ -0,0 +1,9 @@ +parent) instanceof Node\Expression\ObjectCreationExpression || (($creation = $node) instanceof Node\Expression\ObjectCreationExpression) @@ -288,13 +289,19 @@ public function provideCompletion( // MY_CONS| // MyCla| // \MyCla| + // namespace\| // This is the special case when Node\RelativeSpecifier is matched. + // namespace\MyC| // The name Node under the cursor $nameNode = isset($creation) ? $creation->classTypeDesignator : $node; if ($nameNode instanceof Node\QualifiedName) { - /** @var string The typed name. */ + /** @var string The typed name. If relative, without the namespace\. */ $prefix = (string)PhpParser\ResolvedName::buildName($nameNode->nameParts, $nameNode->getFileContents()); + } else if ($nameNode instanceof Node\RelativeSpecifier) { + // The prefix is supposed to not have the leading namespace\ specifier. The token at point is + // "namespace\", so prefix should be empty. + $prefix = ''; } else { $prefix = $nameNode->getText($node->getFileContents()); } @@ -305,13 +312,16 @@ public function provideCompletion( /** @var bool Whether the prefix is qualified (contains at least one backslash) */ $isFullyQualified = false; - - /** @var bool Whether the prefix is qualified (contains at least one backslash) */ + /** @var bool|null Whether the prefix is qualified (contains at least one backslash) */ $isQualified = false; - + /** @var bool Whether the prefix starts with namespace\ */ + $isRelative = false; if ($nameNode instanceof Node\QualifiedName) { $isFullyQualified = $nameNode->isFullyQualifiedName(); $isQualified = $nameNode->isQualifiedName(); + $isRelative = $nameNode->isRelativeName(); + } else if ($nameNode instanceof Node\RelativeSpecifier) { + $isRelative = true; } /** @var bool Whether we are in a new expression */ @@ -324,6 +334,9 @@ public function provideCompletion( // \Prefix\Goes\Here| - Only return completions from the root namespace. /** @var $items \Generator|CompletionItem[] Generator yielding CompletionItems indexed by their FQN */ $items = $this->getCompletionsForFqnPrefix($prefix, $isCreation, false); + } else if ($isRelative) { + // namespace\Something| - Only return completions from the current namespace. + $items = $this->getCompletionsForFqnPrefix(nameConcat($currentNamespace, $prefix), $isCreation, false); } else if ($isQualified) { // Prefix\Goes\Here| $items = $this->getPartiallyQualifiedCompletions( diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index de33f9f6..5e2663ec 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -325,6 +325,54 @@ public function testUsedNamespaceWithoutPrefix() ); } + /** + * Tests completion at `namespace\|` + */ + public function testRelativeNoPrefix() + { + $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/relative.php'); + $this->loader->open($completionUri, file_get_contents($completionUri)); + $items = $this->textDocument->completion( + new TextDocumentIdentifier($completionUri), + new Position(7, 10) + )->wait(); + $this->assertEquals( + new CompletionList([ + new CompletionItem( + 'InnerClass', + CompletionItemKind::CLASS_, + 'TestNamespace\InnerNamespace', + '' + ), + ], true), + $items + ); + } + + /** + * Tests completion at `namespace\TestCla|` + */ + public function testRelativeWithPrefix() + { + $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/relative.php'); + $this->loader->open($completionUri, file_get_contents($completionUri)); + $items = $this->textDocument->completion( + new TextDocumentIdentifier($completionUri), + new Position(8, 17) + )->wait(); + $this->assertEquals( + new CompletionList([ + new CompletionItem( + 'InnerClass', + CompletionItemKind::CLASS_, + 'TestNamespace\InnerNamespace', + '' + ), + ], true), + $items + ); + } + /** * Tests completion at `TestClass::$st|` */