From 618f060648e85389ef27c0e9e3cbf7769bb8b3dc Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Thu, 19 Jan 2017 14:13:32 +0200 Subject: [PATCH 01/10] added $this autocompletion --- fixtures/completion/this.php | 15 ++++ fixtures/completion/this_with_prefix.php | 15 ++++ src/CompletionProvider.php | 14 +++- tests/LanguageServerTest.php | 4 +- tests/Server/TextDocument/CompletionTest.php | 84 ++++++++++++++++++++ 5 files changed, 127 insertions(+), 5 deletions(-) create mode 100644 fixtures/completion/this.php create mode 100644 fixtures/completion/this_with_prefix.php diff --git a/fixtures/completion/this.php b/fixtures/completion/this.php new file mode 100644 index 0000000..b3d684c --- /dev/null +++ b/fixtures/completion/this.php @@ -0,0 +1,15 @@ + + } +} diff --git a/fixtures/completion/this_with_prefix.php b/fixtures/completion/this_with_prefix.php new file mode 100644 index 0000000..6325b99 --- /dev/null +++ b/fixtures/completion/this_with_prefix.php @@ -0,0 +1,15 @@ +m + } +} diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 99a5d57..d24162c 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -141,9 +141,17 @@ class CompletionProvider // If the name is an Error node, just filter by the class if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\PropertyFetch) { // For instances, resolve the variable type - $prefixes = DefinitionResolver::getFqnsFromType( - $this->definitionResolver->resolveExpressionNodeToType($node->var) - ); + $classNode = null; + if ($node->var instanceof Node\Expr\Variable && $node->var->name === 'this') { + $classNode = getClosestNode($node, Node\Stmt\Class_::class); + } + if ($classNode !== null && !$classNode->isAnonymous()) { + $prefixes = [(string)$classNode->namespacedName]; + } else { + $prefixes = DefinitionResolver::getFqnsFromType( + $this->definitionResolver->resolveExpressionNodeToType($node->var) + ); + } } else { // Static member reference $prefixes = [$node->class instanceof Node\Name ? (string)$node->class : '']; diff --git a/tests/LanguageServerTest.php b/tests/LanguageServerTest.php index 6bfa3c9..b824427 100644 --- a/tests/LanguageServerTest.php +++ b/tests/LanguageServerTest.php @@ -57,7 +57,7 @@ class LanguageServerTest extends TestCase if ($msg->body->method === 'window/logMessage' && $promise->state === Promise::PENDING) { if ($msg->body->params->type === MessageType::ERROR) { $promise->reject(new Exception($msg->body->params->message)); - } else if (strpos($msg->body->params->message, 'All 25 PHP files parsed') !== false) { + } else if (strpos($msg->body->params->message, 'All 27 PHP files parsed') !== false) { $promise->fulfill(); } } @@ -103,7 +103,7 @@ class LanguageServerTest extends TestCase if ($promise->state === Promise::PENDING) { $promise->reject(new Exception($msg->body->params->message)); } - } else if (strpos($msg->body->params->message, 'All 25 PHP files parsed') !== false) { + } else if (strpos($msg->body->params->message, 'All 27 PHP files parsed') !== false) { if ($run === 1) { $run++; } else { diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index 15349ed..3711837 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -433,4 +433,88 @@ class CompletionTest extends TestCase ) ], true), $items); } + + public function testThisWithoutPrefix() + { + $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/this.php'); + $this->loader->open($completionUri, file_get_contents($completionUri)); + $items = $this->textDocument->completion( + new TextDocumentIdentifier($completionUri), + new Position(12, 15) + )->wait(); + $this->assertEquals(new CompletionList([ + new CompletionItem( + 'foo', + CompletionItemKind::PROPERTY, + 'mixed', // Type of the property + null + ), + new CompletionItem( + 'bar', + CompletionItemKind::PROPERTY, + 'mixed', // Type of the property + null + ), + new CompletionItem( + 'method', + CompletionItemKind::METHOD, + 'mixed', // Return type of the method + null + ), + new CompletionItem( + 'test', + CompletionItemKind::METHOD, + 'mixed', // Return type of the method + null + ) + ], true), $items); + } + + public function testThisWithPrefix() + { + $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/this_with_prefix.php'); + $this->loader->open($completionUri, file_get_contents($completionUri)); + $items = $this->textDocument->completion( + new TextDocumentIdentifier($completionUri), + 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, + 'mixed', // Type of the property + null + ), + new CompletionItem( + 'bar', + CompletionItemKind::PROPERTY, + 'mixed', // Type of the property + null + ), + new CompletionItem( + 'method', + CompletionItemKind::METHOD, + 'mixed', // Return type of the method + null + ), + new CompletionItem( + 'test', + CompletionItemKind::METHOD, + 'mixed', // Return type of the method + null + ) + ], true), $items); + } } From 975485535e1f7e688d693259beff28ec52455e04 Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Thu, 19 Jan 2017 14:59:12 +0200 Subject: [PATCH 02/10] moved this autocomplete fix to DefinitionResolver --- src/CompletionProvider.php | 15 +++------------ src/DefinitionResolver.php | 4 ++++ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index d24162c..b2beb41 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -140,18 +140,9 @@ class CompletionProvider ) { // If the name is an Error node, just filter by the class if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\PropertyFetch) { - // For instances, resolve the variable type - $classNode = null; - if ($node->var instanceof Node\Expr\Variable && $node->var->name === 'this') { - $classNode = getClosestNode($node, Node\Stmt\Class_::class); - } - if ($classNode !== null && !$classNode->isAnonymous()) { - $prefixes = [(string)$classNode->namespacedName]; - } else { - $prefixes = DefinitionResolver::getFqnsFromType( - $this->definitionResolver->resolveExpressionNodeToType($node->var) - ); - } + $prefixes = DefinitionResolver::getFqnsFromType( + $this->definitionResolver->resolveExpressionNodeToType($node->var) + ); } else { // Static member reference $prefixes = [$node->class instanceof Node\Name ? (string)$node->class : '']; diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index d1f756e..87ab68a 100644 --- a/src/DefinitionResolver.php +++ b/src/DefinitionResolver.php @@ -417,6 +417,10 @@ class DefinitionResolver { if ($expr instanceof Node\Expr\Variable || $expr instanceof Node\Expr\ClosureUse) { if ($expr instanceof Node\Expr\Variable && $expr->name === 'this') { + $classNode = getClosestNode($expr, Node\Stmt\Class_::class); + if ($classNode) { + return self::resolveClassNameToType($classNode->namespacedName); + } return new Types\This; } // Find variable definition From c86ea128a831582a29f051be4d804544e607f866 Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Thu, 19 Jan 2017 15:00:43 +0200 Subject: [PATCH 03/10] did not revert properly --- src/CompletionProvider.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index b2beb41..99a5d57 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -140,6 +140,7 @@ class CompletionProvider ) { // If the name is an Error node, just filter by the class if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\PropertyFetch) { + // For instances, resolve the variable type $prefixes = DefinitionResolver::getFqnsFromType( $this->definitionResolver->resolveExpressionNodeToType($node->var) ); From 2f50f4a430cdc2370ca4347866dd601ed5cfa8cf Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Fri, 20 Jan 2017 21:25:23 +0200 Subject: [PATCH 04/10] Completion for inherited this --- src/CompletionProvider.php | 6 +++++- src/DefinitionResolver.php | 42 ++++++++++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 99a5d57..1e3f7e8 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -130,6 +130,10 @@ class CompletionProvider $list = new CompletionList; $list->isIncomplete = true; + //echo get_class($node->var); + //var_dump((string)$node->var->name); + //die(); + // A non-free node means we do NOT suggest global symbols if ( $node instanceof Node\Expr\MethodCall @@ -141,7 +145,7 @@ class CompletionProvider // If the name is an Error node, just filter by the class if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\PropertyFetch) { // For instances, resolve the variable type - $prefixes = DefinitionResolver::getFqnsFromType( + $prefixes = DefinitionResolver::getFqnsFromType( $this->definitionResolver->resolveExpressionNodeToType($node->var) ); } else { diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index 754473b..7cd0bf9 100644 --- a/src/DefinitionResolver.php +++ b/src/DefinitionResolver.php @@ -423,7 +423,7 @@ class DefinitionResolver if ($expr instanceof Node\Expr\Variable && $expr->name === 'this') { $classNode = getClosestNode($expr, Node\Stmt\Class_::class); if ($classNode) { - return self::resolveClassNameToType($classNode->namespacedName); + return self::resolveClassNameToType($classNode->namespacedName); } return new Types\This; } @@ -479,13 +479,19 @@ class DefinitionResolver } else { $classFqn = substr((string)$t->getFqsen(), 1); } - $fqn = $classFqn . '->' . $expr->name; - if ($expr instanceof Node\Expr\MethodCall) { - $fqn .= '()'; - } - $def = $this->index->getDefinition($fqn); - if ($def !== null) { - return $def->type; + $extended = $this->expandParentFqns([$classFqn]); + foreach ($extended as $f) { + $fqn = $f . '->' . $expr->name; + if ($expr instanceof Node\Expr\MethodCall) { + $fqn .= '()'; + } + $def = $this->index->getDefinition($fqn); + if ($def !== null) { + if ($def->type instanceof Types\This || $def->type instanceof Types\Self_) { + return $this->resolveExpressionNodeToType($expr->var); + } + return $def->type; + } } } } @@ -651,6 +657,26 @@ class DefinitionResolver return new Types\Mixed; } + /** + * Adds the FQNs of all parent classes to an array of FQNs of classes + * + * @param string[] $fqns + * @return string[] + */ + private function expandParentFqns(array $fqns): array + { + $expanded = $fqns; + foreach ($fqns as $fqn) { + $def = $this->index->getDefinition($fqn); + if ($def) { + foreach ($this->expandParentFqns($def->extends) as $parent) { + $expanded[] = $parent; + } + } + } + return $expanded; + } + /** * Takes any class name node (from a static method call, or new node) and returns a Type object * Resolves keywords like self, static and parent From 0d261db7b4b4fa00bd4a2fb7e2a54b2eee16fb8c Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Fri, 20 Jan 2017 21:33:46 +0200 Subject: [PATCH 05/10] fixed coding style --- src/DefinitionResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index 7cd0bf9..d73da7c 100644 --- a/src/DefinitionResolver.php +++ b/src/DefinitionResolver.php @@ -423,7 +423,7 @@ class DefinitionResolver if ($expr instanceof Node\Expr\Variable && $expr->name === 'this') { $classNode = getClosestNode($expr, Node\Stmt\Class_::class); if ($classNode) { - return self::resolveClassNameToType($classNode->namespacedName); + return self::resolveClassNameToType($classNode->namespacedName); } return new Types\This; } From 60153923cb1a0bce04e674fe08b012dc6d759421 Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Fri, 20 Jan 2017 21:40:15 +0200 Subject: [PATCH 06/10] Fixed error on null --- src/DefinitionResolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index d73da7c..5c8bd95 100644 --- a/src/DefinitionResolver.php +++ b/src/DefinitionResolver.php @@ -668,7 +668,7 @@ class DefinitionResolver $expanded = $fqns; foreach ($fqns as $fqn) { $def = $this->index->getDefinition($fqn); - if ($def) { + if ($def && $def->extends) { foreach ($this->expandParentFqns($def->extends) as $parent) { $expanded[] = $parent; } From 7b4d0d1c6094c3cd760477a111dbf66c4786c297 Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Sat, 21 Jan 2017 17:48:15 +0200 Subject: [PATCH 07/10] Switched to a generator when traversing parents --- src/DefinitionResolver.php | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index 5c8bd95..665394a 100644 --- a/src/DefinitionResolver.php +++ b/src/DefinitionResolver.php @@ -479,14 +479,12 @@ class DefinitionResolver } else { $classFqn = substr((string)$t->getFqsen(), 1); } - $extended = $this->expandParentFqns([$classFqn]); - foreach ($extended as $f) { - $fqn = $f . '->' . $expr->name; + foreach ($this->getParentsDefinition($classFqn) as $parent) { + $fqn = $parent->fqn . '->' . $expr->name; if ($expr instanceof Node\Expr\MethodCall) { $fqn .= '()'; } - $def = $this->index->getDefinition($fqn); - if ($def !== null) { + if ($def = $this->index->getDefinition($fqn)) { if ($def->type instanceof Types\This || $def->type instanceof Types\Self_) { return $this->resolveExpressionNodeToType($expr->var); } @@ -660,21 +658,24 @@ class DefinitionResolver /** * Adds the FQNs of all parent classes to an array of FQNs of classes * - * @param string[] $fqns - * @return string[] + * @param string $fqns + * @return Definition[] */ - private function expandParentFqns(array $fqns): array + private function getParentsDefinition(string $fqn) { - $expanded = $fqns; - foreach ($fqns as $fqn) { - $def = $this->index->getDefinition($fqn); - if ($def && $def->extends) { - foreach ($this->expandParentFqns($def->extends) as $parent) { - $expanded[] = $parent; + $def = $this->index->getDefinition($fqn); + while ($def) { + yield $def; + if ($def->extends) { + foreach ($def->extends as $parent) { + if (($tmp = $this->index->getDefinition($parent)) && $tmp->canBeInstantiated) { + $def = $tmp; + continue; + } } } + break; } - return $expanded; } /** From af53973dd5d478f7ede74d970a11987bf84fe1a2 Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Sat, 21 Jan 2017 17:59:19 +0200 Subject: [PATCH 08/10] Removed fluent interface resolution --- src/DefinitionResolver.php | 41 ++++++++------------------------------ 1 file changed, 8 insertions(+), 33 deletions(-) diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index 665394a..a7ab566 100644 --- a/src/DefinitionResolver.php +++ b/src/DefinitionResolver.php @@ -479,17 +479,15 @@ class DefinitionResolver } else { $classFqn = substr((string)$t->getFqsen(), 1); } - foreach ($this->getParentsDefinition($classFqn) as $parent) { - $fqn = $parent->fqn . '->' . $expr->name; - if ($expr instanceof Node\Expr\MethodCall) { - $fqn .= '()'; - } - if ($def = $this->index->getDefinition($fqn)) { - if ($def->type instanceof Types\This || $def->type instanceof Types\Self_) { - return $this->resolveExpressionNodeToType($expr->var); - } - return $def->type; + $fqn = $classFqn . '->' . $expr->name; + if ($expr instanceof Node\Expr\MethodCall) { + $fqn .= '()'; + } + if ($def = $this->index->getDefinition($fqn)) { + if ($def->type instanceof Types\This || $def->type instanceof Types\Self_) { + return $this->resolveExpressionNodeToType($expr->var); } + return $def->type; } } } @@ -655,29 +653,6 @@ class DefinitionResolver return new Types\Mixed; } - /** - * Adds the FQNs of all parent classes to an array of FQNs of classes - * - * @param string $fqns - * @return Definition[] - */ - private function getParentsDefinition(string $fqn) - { - $def = $this->index->getDefinition($fqn); - while ($def) { - yield $def; - if ($def->extends) { - foreach ($def->extends as $parent) { - if (($tmp = $this->index->getDefinition($parent)) && $tmp->canBeInstantiated) { - $def = $tmp; - continue; - } - } - } - break; - } - } - /** * Takes any class name node (from a static method call, or new node) and returns a Type object * Resolves keywords like self, static and parent From 66634a40458cfa67706d366883c4db07a7acab44 Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Sat, 21 Jan 2017 19:08:46 +0200 Subject: [PATCH 09/10] Reverted unnecessary changes --- src/CompletionProvider.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 1e3f7e8..99a5d57 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -130,10 +130,6 @@ class CompletionProvider $list = new CompletionList; $list->isIncomplete = true; - //echo get_class($node->var); - //var_dump((string)$node->var->name); - //die(); - // A non-free node means we do NOT suggest global symbols if ( $node instanceof Node\Expr\MethodCall @@ -145,7 +141,7 @@ class CompletionProvider // If the name is an Error node, just filter by the class if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\PropertyFetch) { // For instances, resolve the variable type - $prefixes = DefinitionResolver::getFqnsFromType( + $prefixes = DefinitionResolver::getFqnsFromType( $this->definitionResolver->resolveExpressionNodeToType($node->var) ); } else { From faf7b12d47a49675c75c65710ba3e1f914bc51b8 Mon Sep 17 00:00:00 2001 From: Ivan Bozhanov Date: Thu, 26 Jan 2017 19:03:08 +0200 Subject: [PATCH 10/10] Fixed setting a variable inside an IF --- src/DefinitionResolver.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index a7ab566..55160f1 100644 --- a/src/DefinitionResolver.php +++ b/src/DefinitionResolver.php @@ -483,7 +483,8 @@ class DefinitionResolver if ($expr instanceof Node\Expr\MethodCall) { $fqn .= '()'; } - if ($def = $this->index->getDefinition($fqn)) { + $def = $this->index->getDefinition($fqn); + if ($def !== null) { if ($def->type instanceof Types\This || $def->type instanceof Types\Self_) { return $this->resolveExpressionNodeToType($expr->var); }