-
Notifications
You must be signed in to change notification settings - Fork 185
Signature help #547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Signature help #547
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
f2fdfa5
very rough first go at signatureHelp
phil-nelson 07cecaa
add some tests
phil-nelson ba44967
tidy up
phil-nelson 9ea0a3f
use getOrLoad()->wait(). TODO - work out how to make this better
phil-nelson 9627854
Merge remote-tracking branch 'felixfbecker/master' into signatureHelp
phil-nelson 5db2db0
fix test
phil-nelson a1347db
replace is_null with === null
phil-nelson 5ba7724
add/utilise constructors
phil-nelson fa561f8
use a coroutine instead of using ->wait()
phil-nelson afd7215
remove createSignatureHelp and use constructors in data provider
phil-nelson c7dc5af
add signatureHelp to index
phil-nelson 4c50cc0
formatting
phil-nelson 86acf44
add negative tests
phil-nelson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
<?php | ||
|
||
namespace Foo; | ||
|
||
class Test | ||
{ | ||
/** | ||
* Constructor comment goes here | ||
* | ||
* @param string $first First param | ||
* @param int $second Second param | ||
* @param Test $third Third param with a longer description | ||
*/ | ||
public function __construct(string $first, int $second, Test $third) | ||
{ | ||
} | ||
|
||
/** | ||
* Function doc | ||
* | ||
* @param SomethingElse $a A param with a different doc type | ||
* @param int|null $b Param with default value | ||
*/ | ||
public function foo(\DateTime $a, int $b = null) | ||
{ | ||
} | ||
|
||
public static function bar($a) | ||
{ | ||
} | ||
|
||
/** | ||
* Method with no params | ||
*/ | ||
public function baz() | ||
{ | ||
} | ||
} | ||
|
||
/** | ||
* @param int $i Global function param one | ||
* @param bool $b Default false param | ||
*/ | ||
function foo(int $i, bool $b = false) | ||
{ | ||
} | ||
|
||
$t = new Test(); | ||
$t->foo(); | ||
$t->foo(1, | ||
$t->foo(1,); | ||
$t->baz(); | ||
|
||
foo(); | ||
|
||
Test::bar(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,242 @@ | ||
<?php | ||
declare(strict_types = 1); | ||
|
||
namespace LanguageServer; | ||
|
||
use LanguageServer\Index\ReadableIndex; | ||
use LanguageServer\Protocol\{ | ||
Position, | ||
SignatureHelp, | ||
SignatureInformation, | ||
ParameterInformation | ||
}; | ||
use Microsoft\PhpParser; | ||
use Microsoft\PhpParser\Node; | ||
|
||
class SignatureHelpProvider | ||
{ | ||
/** @var DefinitionResolver */ | ||
private $definitionResolver; | ||
|
||
/** @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) | ||
{ | ||
$this->definitionResolver = $definitionResolver; | ||
$this->index = $index; | ||
$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); | ||
|
||
// 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; | ||
|
||
// Get information from the item being called to build the signature information | ||
$calledDoc = $this->documentLoader->getOrLoad($def->symbolInformation->location->uri)->wait(); | ||
if (!$calledDoc) { | ||
return new SignatureHelp(); | ||
} | ||
$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($calledNode); | ||
$signatureHelp = new SignatureHelp(); | ||
$signatureHelp->signatures = [$signatureInformation]; | ||
$signatureHelp->activeSignature = 0; | ||
$signatureHelp->activeParameter = $activeParam; | ||
return $signatureHelp; | ||
} | ||
|
||
/** | ||
* 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 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 = '('; | ||
if ($params) { | ||
foreach ($params as $param) { | ||
$label .= $param->label . ', '; | ||
} | ||
$label = substr($label, 0, -2); | ||
} | ||
$label .= ')'; | ||
return $label; | ||
} | ||
|
||
/** | ||
* 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 | ||
{ | ||
$params = []; | ||
if ($node->parameters) { | ||
foreach ($node->parameters->getElements() as $element) { | ||
$param = (string) $this->definitionResolver->getTypeFromNode($element); | ||
$param .= ' ' . $element->variableName->getText($doc->getContent()); | ||
if ($element->default) { | ||
$param .= ' = ' . $element->default->getText($doc->getContent()); | ||
} | ||
$info = new ParameterInformation(); | ||
$info->label = $param; | ||
$info->documentation = $this->definitionResolver->getDocumentationFromNode($element); | ||
$params[] = $info; | ||
} | ||
} | ||
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, | ||
PhpDocument $doc | ||
): int { | ||
$args = $argumentExpressionList->children; | ||
$i = 0; | ||
$found = null; | ||
foreach ($args as $arg) { | ||
if ($arg instanceof Node) { | ||
$start = $arg->getFullStart(); | ||
$end = $arg->getEndPosition(); | ||
} else { | ||
$start = $arg->fullStart; | ||
$end = $start + $arg->length; | ||
} | ||
$offset = $position->toOffset($doc->getContent()); | ||
if ($offset >= $start && $offset <= $end) { | ||
$found = $i; | ||
break; | ||
} | ||
if ($arg instanceof Node) { | ||
++$i; | ||
} | ||
} | ||
if (is_null($found)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use === null There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
$found = $i; | ||
} | ||
return $found; | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never use
wait
except in tests, it blocks the event loop. Make it a coroutine andyield
the PromiseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'm still getting my head around this event loop business. I've updated this to wrap this function in a coroutine.