diff --git a/fixtures/signature_help/calls.php b/fixtures/signature_help/calls.php index 2dfc7f2..511725b 100644 --- a/fixtures/signature_help/calls.php +++ b/fixtures/signature_help/calls.php @@ -47,7 +47,8 @@ function foo(int $i, bool $b = false) $t = new Test(); $t->foo(); -$t->foo(1, ); +$t->foo(1, +$t->foo(1,); $t->baz(); foo(); diff --git a/src/Server/TextDocument.php b/src/Server/TextDocument.php index 37ee658..94b5082 100644 --- a/src/Server/TextDocument.php +++ b/src/Server/TextDocument.php @@ -58,6 +58,9 @@ class TextDocument */ protected $completionProvider; + /** + * @var SignatureHelpProvider + */ protected $signatureHelpProvider; /** @@ -254,12 +257,13 @@ class TextDocument } /** - * The goto definition request is sent from the client to the server to resolve the definition location of a symbol - * at a given text document position. + * The signature help request is sent from the client to the server to request signature information at a given + * cursor position. * * @param TextDocumentIdentifier $textDocument The text document - * @param Position $position The position inside the text document - * @return Promise + * @param Position $position The position inside the text document + * + * @return Promise */ public function signatureHelp(TextDocumentIdentifier $textDocument, Position $position): Promise { diff --git a/src/SignatureHelpProvider.php b/src/SignatureHelpProvider.php index 1b80b34..3dcae23 100644 --- a/src/SignatureHelpProvider.php +++ b/src/SignatureHelpProvider.php @@ -21,11 +21,15 @@ class SignatureHelpProvider /** @var ReadableIndex */ private $index; + /** @var PhpDocumentLoader */ private $documentLoader; /** + * Constructor + * * @param DefinitionResolver $definitionResolver * @param ReadableIndex $index + * @param PhpDocumentLoader $documentLoader */ public function __construct(DefinitionResolver $definitionResolver, ReadableIndex $index, PhpDocumentLoader $documentLoader) { @@ -34,81 +38,44 @@ class SignatureHelpProvider $this->documentLoader = $documentLoader; } + /** + * Finds signature help for a callable position + * + * @param PhpDocument $doc The document the position belongs to + * @param Position $position The position to detect a call from + * + * @return SignatureHelp + */ public function getSignatureHelp(PhpDocument $doc, Position $position): SignatureHelp { // Find the node under the cursor $node = $doc->getNodeAtPosition($position); - $fqn = null; - - // First find the node that the call belongs to - if ($node instanceof Node\DelimitedList\ArgumentExpressionList) { - $argumentExpressionList = $node; - if ($node->parent instanceof Node\Expression\ObjectCreationExpression) { - $node = $node->parent->classTypeDesignator; - if (!$node instanceof Node\QualifiedName) { - return new SignatureHelp(); - } - $fqn = $this->definitionResolver->resolveReferenceNodeToFqn($node); - $fqn = "{$fqn}->__construct()"; - } else { - $node = $node->parent->getFirstChildNode( - Node\Expression\MemberAccessExpression::class, - Node\Expression\ScopedPropertyAccessExpression::class, - Node\QualifiedName::class - ); - } - } elseif ($node instanceof Node\Expression\CallExpression) { - $argumentExpressionList = $node->getFirstChildNode(Node\DelimitedList\ArgumentExpressionList::class); - $node = $node->getFirstChildNode( - Node\Expression\MemberAccessExpression::class, - Node\Expression\ScopedPropertyAccessExpression::class, - Node\QualifiedName::class - ); - } elseif ($node instanceof Node\Expression\ObjectCreationExpression) { - $argumentExpressionList = $node->getFirstChildNode(Node\DelimitedList\ArgumentExpressionList::class); - //$node = $node->getFirstChildNode(Node\QualifiedName::class); - $node = $node->classTypeDesignator; - if (!$node instanceof Node\QualifiedName) { - return new SignatureHelp(); - } - $fqn = $this->definitionResolver->resolveReferenceNodeToFqn($node); - $fqn = "{$fqn}->__construct()"; - } else { - $node = null; - } - - if (!$node) { - return new SignatureHelp(); - } - - // Now find the definition of the call - $fqn = $fqn ?: DefinitionResolver::getDefinedFqn($node); - if ($fqn) { - $def = $this->index->getDefinition($fqn); - } else { - $def = $this->definitionResolver->resolveReferenceNodeToDefinition($node); - } + // Find the definition of the item being called + list($def, $argumentExpressionList) = $this->getCallingInfo($node); if (!$def) { return new SignatureHelp(); } + // Find the active parameter $activeParam = $argumentExpressionList ? $this->findActiveParameter($argumentExpressionList, $position, $doc) : 0; - $doc = $this->documentLoader->get($def->symbolInformation->location->uri); - if (!$doc) { + // Get information from the item being called to build the signature information + $calledDoc = $this->documentLoader->get($def->symbolInformation->location->uri); + if (!$calledDoc) { return new SignatureHelp(); } - $node = $doc->getNodeAtPosition($def->symbolInformation->location->range->start); - $params = $this->getParameters($node, $doc); - $label = $this->getLabel($node, $params, $doc); + $calledNode = $calledDoc->getNodeAtPosition($def->symbolInformation->location->range->start); + $params = $this->getParameters($calledNode, $calledDoc); + $label = $this->getLabel($calledNode, $params, $calledDoc); + $signatureInformation = new SignatureInformation(); $signatureInformation->label = $label; $signatureInformation->parameters = $params; - $signatureInformation->documentation = $this->definitionResolver->getDocumentationFromNode($node); + $signatureInformation->documentation = $this->definitionResolver->getDocumentationFromNode($calledNode); $signatureHelp = new SignatureHelp(); $signatureHelp->signatures = [$signatureInformation]; $signatureHelp->activeSignature = 0; @@ -117,11 +84,84 @@ class SignatureHelpProvider } /** - * @param Node\MethodDeclaration|Node\Statement\FunctionDeclaration $node + * Given a node that could be a callable, finds the definition of the call and the argument expression list of + * the node + * + * @param Node $node The node to find calling information from + * + * @return array|null */ - private function getLabel($node, array $params, PhpDocument $doc): string + private function getCallingInfo(Node $node) + { + $fqn = null; + $callingNode = null; + if ($node instanceof Node\DelimitedList\ArgumentExpressionList) { + // Cursor is already inside a ( + $argumentExpressionList = $node; + if ($node->parent instanceof Node\Expression\ObjectCreationExpression) { + // Constructing something + $callingNode = $node->parent->classTypeDesignator; + if (!$callingNode instanceof Node\QualifiedName) { + // We only support constructing from a QualifiedName + return null; + } + $fqn = $this->definitionResolver->resolveReferenceNodeToFqn($callingNode); + $fqn = "{$fqn}->__construct()"; + } else { + $callingNode = $node->parent->getFirstChildNode( + Node\Expression\MemberAccessExpression::class, + Node\Expression\ScopedPropertyAccessExpression::class, + Node\QualifiedName::class + ); + } + } elseif ($node instanceof Node\Expression\CallExpression) { + $argumentExpressionList = $node->getFirstChildNode(Node\DelimitedList\ArgumentExpressionList::class); + $callingNode = $node->getFirstChildNode( + Node\Expression\MemberAccessExpression::class, + Node\Expression\ScopedPropertyAccessExpression::class, + Node\QualifiedName::class + ); + } elseif ($node instanceof Node\Expression\ObjectCreationExpression) { + $argumentExpressionList = $node->getFirstChildNode(Node\DelimitedList\ArgumentExpressionList::class); + $callingNode = $node->classTypeDesignator; + if (!$callingNode instanceof Node\QualifiedName) { + // We only support constructing from a QualifiedName + return null; + } + // Manually build the __construct fqn + $fqn = $this->definitionResolver->resolveReferenceNodeToFqn($callingNode); + $fqn = "{$fqn}->__construct()"; + } + + if (!$callingNode) { + return null; + } + + // Now find the definition of the call + $fqn = $fqn ?: DefinitionResolver::getDefinedFqn($callingNode); + if ($fqn) { + $def = $this->index->getDefinition($fqn); + } else { + $def = $this->definitionResolver->resolveReferenceNodeToDefinition($callingNode); + } + + if (!$def) { + return null; + } + return [$def, $argumentExpressionList]; + } + + /** + * Creates a label for SignatureInformation + * + * @param Node\MethodDeclaration|Node\Statement\FunctionDeclaration $node The method/function declaration node + * we are building the label for + * @param ParameterInformation[] $params Parameters that belong to the node + * + * @return string + */ + private function getLabel($node, array $params): string { - //$label = $node->getName() . '('; $label = '('; if ($params) { foreach ($params as $param) { @@ -130,21 +170,16 @@ class SignatureHelpProvider $label = substr($label, 0, -2); } $label .= ')'; - /* - if ($node->returnType) { - $label .= ': '; - if ($node->returnType instanceof QualifiedName) { - $label .= $node->returnType->getResolvedName(); - } else { - $label .= $node->returnType->getText($doc->getContent()); - } - } - */ return $label; } /** - * @param Node\MethodDeclaration|Node\Statement\FunctionDeclaration $node + * Builds ParameterInformation from a node + * + * @param Node\MethodDeclaration|Node\Statement\FunctionDeclaration $node The node to build parameters from + * @param PhpDocument $doc The document the node belongs to + * + * @return ParameterInformation[] */ private function getParameters($node, PhpDocument $doc): array { @@ -165,6 +200,15 @@ class SignatureHelpProvider return $params; } + /** + * Given a position and arguments, finds the "active" argument at the position + * + * @param Node\DelimitedList\ArgumentExpressionList $argumentExpressionList The argument expression list + * @param Position $position The position to detect the active argument from + * @param PhpDocument $doc The document that contains the expression + * + * @return int + */ private function findActiveParameter( Node\DelimitedList\ArgumentExpressionList $argumentExpressionList, Position $position, @@ -177,7 +221,6 @@ class SignatureHelpProvider if ($arg instanceof Node) { $start = $arg->getFullStart(); $end = $arg->getEndPosition(); - ++$i; } else { $start = $arg->fullStart; $end = $start + $arg->length; @@ -187,6 +230,9 @@ class SignatureHelpProvider $found = $i; break; } + if ($arg instanceof Node) { + ++$i; + } } if (is_null($found)) { $found = $i; diff --git a/tests/Server/TextDocument/SignatureHelpTest.php b/tests/Server/TextDocument/SignatureHelpTest.php index 33a36d7..d5482a9 100644 --- a/tests/Server/TextDocument/SignatureHelpTest.php +++ b/tests/Server/TextDocument/SignatureHelpTest.php @@ -80,7 +80,26 @@ class SignatureHelpTest extends TestCase ]), ], 'member call 2nd param active' => [ - new Position(49, 11), + new Position(49, 12), + $this->createSignatureHelp([ + 'label' => '(\\Foo\\SomethingElse $a, int|null $b = null)', + 'documentation' => 'Function doc', + 'parameters' => [ + [ + 'label' => '\\Foo\\SomethingElse $a', + 'documentation' => 'A param with a different doc type', + ], + [ + 'label' => 'int|null $b = null', + 'documentation' => 'Param with default value', + ], + ], + 'activeSignature' => 0, + 'activeParameter' => 1, + ]), + ], + 'member call 2nd param active and closing )' => [ + new Position(50, 11), $this->createSignatureHelp([ 'label' => '(\\Foo\\SomethingElse $a, int|null $b = null)', 'documentation' => 'Function doc', @@ -99,7 +118,7 @@ class SignatureHelpTest extends TestCase ]), ], 'method with no params' => [ - new Position(50, 9), + new Position(51, 9), $this->createSignatureHelp([ 'label' => '()', 'documentation' => 'Method with no params', @@ -133,7 +152,7 @@ class SignatureHelpTest extends TestCase ]), ], 'global function' => [ - new Position(52, 4), + new Position(53, 4), $this->createSignatureHelp([ 'label' => '(int $i, bool $b = false)', 'documentation' => null, @@ -152,7 +171,7 @@ class SignatureHelpTest extends TestCase ]), ], 'static method' => [ - new Position(54, 10), + new Position(55, 10), $this->createSignatureHelp([ 'label' => '(mixed $a)', 'documentation' => null,