From 9eea26df711920a7e04d4894fc4b9263ee783e11 Mon Sep 17 00:00:00 2001 From: Phil Nelson Date: Mon, 18 Dec 2017 12:55:12 +1100 Subject: [PATCH] feat: foreach completion (#551) --- fixtures/completion/foreach.php | 37 +++++ src/CompletionProvider.php | 8 + src/DefinitionResolver.php | 40 ++++- src/Server/TextDocument.php | 1 + tests/Server/TextDocument/CompletionTest.php | 140 ++++++++++++++++++ ...rrayValueShouldBeBoolean.php.expected.json | 2 +- .../cases/magicConsts.php.expected.json | 2 +- 7 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 fixtures/completion/foreach.php diff --git a/fixtures/completion/foreach.php b/fixtures/completion/foreach.php new file mode 100644 index 00000000..df8b6df5 --- /dev/null +++ b/fixtures/completion/foreach.php @@ -0,0 +1,37 @@ +test(); +$array1 = [new Bar(), new \stdClass()]; +$array2 = ['foo' => $bar, $bar]; +$array3 = ['foo' => $bar, 'baz' => $bar]; + +foreach ($bars as $value) { + $v + $value-> +} + +foreach ($array1 as $key => $value) { + $ +} + +foreach ($array2 as $key => $value) { + $ +} + +foreach ($array3 as $key => $value) { + $ +} + +foreach ($bar->test() as $value) { + $ +} diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index d774423e..fd895683 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -486,6 +486,14 @@ private function findVariableDefinitionsInNode(Node $node, string $namePrefix = if ($this->isAssignmentToVariableWithPrefix($node, $namePrefix)) { $vars[] = $node->leftOperand; + } elseif ($node instanceof Node\ForeachKey || $node instanceof Node\ForeachValue) { + foreach ($node->getDescendantNodes() as $descendantNode) { + if ($descendantNode instanceof Node\Expression\Variable + && ($namePrefix === '' || strpos($descendantNode->getName(), $namePrefix) !== false) + ) { + $vars[] = $descendantNode; + } + } } else { // Get all descendent variables, then filter to ones that start with $namePrefix. // Avoiding closure usage in tight loop diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php index 2c1f67d6..f36588dd 100644 --- a/src/DefinitionResolver.php +++ b/src/DefinitionResolver.php @@ -568,6 +568,20 @@ public function resolveVariableToNode($var) } break; } + + // If we get to a ForeachStatement, check the keys and values + if ($n instanceof Node\Statement\ForeachStatement) { + if ($n->foreachKey && $n->foreachKey->expression->getName() === $name) { + return $n->foreachKey; + } + if ($n->foreachValue + && $n->foreachValue->expression instanceof Node\Expression\Variable + && $n->foreachValue->expression->getName() === $name + ) { + return $n->foreachValue; + } + } + // Check each previous sibling node for a variable assignment to that variable while (($prevSibling = $n->getPreviousSibling()) !== null && $n = $prevSibling) { if ($n instanceof Node\Statement\ExpressionStatement) { @@ -619,6 +633,9 @@ public function resolveExpressionNodeToType($expr) if ($defNode instanceof Node\Expression\AssignmentExpression || $defNode instanceof Node\UseVariableName) { return $this->resolveExpressionNodeToType($defNode); } + if ($defNode instanceof Node\ForeachKey || $defNode instanceof Node\ForeachValue) { + return $this->getTypeFromNode($defNode); + } if ($defNode instanceof Node\Parameter) { return $this->getTypeFromNode($defNode); } @@ -900,7 +917,7 @@ public function resolveExpressionNodeToType($expr) $keyTypes[] = $item->elementKey ? $this->resolveExpressionNodeToType($item->elementKey) : new Types\Integer; } } - $valueTypes = array_unique($keyTypes); + $valueTypes = array_unique($valueTypes); $keyTypes = array_unique($keyTypes); if (empty($valueTypes)) { $valueType = null; @@ -1080,6 +1097,27 @@ public function getTypeFromNode($node) return new Types\Mixed_; } + // FOREACH KEY/VARIABLE + if ($node instanceof Node\ForeachKey || $node->parent instanceof Node\ForeachKey) { + $foreach = $node->getFirstAncestor(Node\Statement\ForeachStatement::class); + $collectionType = $this->resolveExpressionNodeToType($foreach->forEachCollectionName); + if ($collectionType instanceof Types\Array_) { + return $collectionType->getKeyType(); + } + return new Types\Mixed_(); + } + + // FOREACH VALUE/VARIABLE + if ($node instanceof Node\ForeachValue + || ($node instanceof Node\Expression\Variable && $node->parent instanceof Node\ForeachValue) + ) { + $foreach = $node->getFirstAncestor(Node\Statement\ForeachStatement::class); + $collectionType = $this->resolveExpressionNodeToType($foreach->forEachCollectionName); + if ($collectionType instanceof Types\Array_) { + return $collectionType->getValueType(); + } + } + // PROPERTIES, CONSTS, CLASS CONSTS, ASSIGNMENT EXPRESSIONS // Get the documented type the assignment resolves to. if ( diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index 704ceb8f..371ad361 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -337,6 +337,7 @@ public function hover(TextDocumentIdentifier $textDocument, Position $position): if ($def === null) { return new Hover([], $range); } + $contents = []; if ($def->declarationLine) { $contents[] = new MarkedString('php', "declarationLine); } diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index 424dc3ca..ded59415 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -554,6 +554,146 @@ public function testBarePhp() ], true), $items); } + /** + * @dataProvider foreachProvider + */ + public function testForeach(Position $position, array $expectedItems) + { + $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/foreach.php'); + $this->loader->open($completionUri, file_get_contents($completionUri)); + $items = $this->textDocument->completion( + new TextDocumentIdentifier($completionUri), + $position + )->wait(); + $this->assertCompletionsListSubset(new CompletionList($expectedItems, true), $items); + } + + public function foreachProvider(): array + { + return [ + 'foreach value' => [ + new Position(18, 6), + [ + new CompletionItem( + '$value', + CompletionItemKind::VARIABLE, + '\\Foo\\Bar', + null, + null, + null, + null, + new TextEdit(new Range(new Position(18, 6), new Position(18, 6)), 'alue') + ), + ] + ], + 'foreach value resolved' => [ + new Position(19, 12), + [ + new CompletionItem( + 'foo', + CompletionItemKind::PROPERTY, + 'mixed' + ), + new CompletionItem( + 'test', + CompletionItemKind::METHOD, + '\\Foo\\Bar[]' + ), + ] + ], + 'array creation with multiple objects' => [ + new Position(23, 5), + [ + new CompletionItem( + '$value', + CompletionItemKind::VARIABLE, + '\\Foo\\Bar|\\stdClass', + null, + null, + null, + null, + new TextEdit(new Range(new Position(23, 5), new Position(23, 5)), 'value') + ), + new CompletionItem( + '$key', + CompletionItemKind::VARIABLE, + 'int', + null, + null, + null, + null, + new TextEdit(new Range(new Position(23, 5), new Position(23, 5)), 'key') + ), + ] + ], + 'array creation with string/int keys and object values' => [ + new Position(27, 5), + [ + new CompletionItem( + '$value', + CompletionItemKind::VARIABLE, + '\\Foo\\Bar', + null, + null, + null, + null, + new TextEdit(new Range(new Position(27, 5), new Position(27, 5)), 'value') + ), + new CompletionItem( + '$key', + CompletionItemKind::VARIABLE, + 'string|int', + null, + null, + null, + null, + new TextEdit(new Range(new Position(27, 5), new Position(27, 5)), 'key') + ), + ] + ], + 'array creation with only string keys' => [ + new Position(31, 5), + [ + new CompletionItem( + '$value', + CompletionItemKind::VARIABLE, + '\\Foo\\Bar', + null, + null, + null, + null, + new TextEdit(new Range(new Position(31, 5), new Position(31, 5)), 'value') + ), + new CompletionItem( + '$key', + CompletionItemKind::VARIABLE, + 'string', + null, + null, + null, + null, + new TextEdit(new Range(new Position(31, 5), new Position(31, 5)), 'key') + ), + ] + ], + 'foreach function call' => [ + new Position(35, 5), + [ + new CompletionItem( + '$value', + CompletionItemKind::VARIABLE, + '\\Foo\\Bar', + null, + null, + null, + null, + new TextEdit(new Range(new Position(35, 5), new Position(35, 5)), 'value') + ), + ] + ], + ]; + } + public function testMethodReturnType() { $completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/method_return_type.php'); diff --git a/tests/Validation/cases/arrayValueShouldBeBoolean.php.expected.json b/tests/Validation/cases/arrayValueShouldBeBoolean.php.expected.json index 1f40635f..f0cdb24d 100644 --- a/tests/Validation/cases/arrayValueShouldBeBoolean.php.expected.json +++ b/tests/Validation/cases/arrayValueShouldBeBoolean.php.expected.json @@ -36,7 +36,7 @@ }, "containerName": "A" }, - "type__tostring": "string[]", + "type__tostring": "bool[]", "type": {}, "declarationLine": "protected $foo;", "documentation": null, diff --git a/tests/Validation/cases/magicConsts.php.expected.json b/tests/Validation/cases/magicConsts.php.expected.json index 545f1cbe..27608e55 100644 --- a/tests/Validation/cases/magicConsts.php.expected.json +++ b/tests/Validation/cases/magicConsts.php.expected.json @@ -40,7 +40,7 @@ }, "containerName": "A" }, - "type__tostring": "\\__CLASS__[]", + "type__tostring": "bool[]", "type": {}, "declarationLine": "private static $deprecationsTriggered;", "documentation": null,