Skip to content

Commit

Permalink
Add documentation to LSP (#5267)
Browse files Browse the repository at this point in the history 
* Add documention to LSP

Add descriptions for all Classes, Functions, Methods, Class Constants for LSP methods for Hover, SignatureInformation and Completions

* Descriptions for class name completions

* PHPCS

* Fix docblock being overriden

* Remove trailing comma in args

* Add description to function param before early `continue`

* Update php-language-server-protocol to 1.5

* Break up long array docblocks

* Break up docblock onto newline

Co-authored-by: Matthew Brown <github@muglug.com>
  • Loading branch information
@joehoyle @muglug
joehoyle and muglug authored Feb 24, 2021
1 parent f8cbb22 commit e59670e
Show file tree
Hide file tree
Showing 21 changed files with 452 additions and 51 deletions.
2 changes: 1 addition & 1 deletion composer.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"composer/xdebug-handler": "^1.1",
"dnoegel/php-xdg-base-dir": "^0.1.1",
"felixfbecker/advanced-json-rpc": "^3.0.3",
"felixfbecker/language-server-protocol": "^1.4",
"felixfbecker/language-server-protocol": "^1.5",
"netresearch/jsonmapper": "^1.0 || ^2.0 || ^3.0 || ^4.0",
"nikic/php-parser": "^4.10.1",
"openlss/lib-array2xml": "^1.0",
Expand Down
107 changes: 79 additions & 28 deletions src/Psalm/Codebase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
use function array_pop;
use function implode;
use function array_reverse;
use function dirname;

class Codebase
{
Expand Down Expand Up @@ -941,10 +942,15 @@ public function getFunctionStorageForSymbol(string $file_path, string $symbol):
return $function;
}

public function getSymbolInformation(string $file_path, string $symbol): ?string
/**
* @param string $file_path
* @param string $symbol
* @return array{ type: string, description?: string|null}|null
*/
public function getSymbolInformation(string $file_path, string $symbol): ?array
{
if (\is_numeric($symbol[0])) {
return \preg_replace('/^[^:]*:/', '', $symbol);
return ['type' => \preg_replace('/^[^:]*:/', '', $symbol)];
}

try {
Expand All @@ -963,15 +969,21 @@ public function getSymbolInformation(string $file_path, string $symbol): ?string

$storage = $this->methods->getStorage($declaring_method_id);

return '<?php ' . $storage->getSignature(true);
return [
'type' => '<?php ' . $storage->getSignature(true),
'description' => $storage->description,
];
}

[, $symbol_name] = explode('::', $symbol);

if (strpos($symbol, '$') !== false) {
$storage = $this->properties->getStorage($symbol);

return '<?php ' . $storage->getInfo() . ' ' . $symbol_name;
return [
'type' => '<?php ' . $storage->getInfo() . ' ' . $symbol_name,
'description' => $storage->description,
];
}

[$fq_classlike_name, $const_name] = explode('::', $symbol);
Expand All @@ -985,7 +997,10 @@ public function getSymbolInformation(string $file_path, string $symbol): ?string
return null;
}

return '<?php ' . $const_name;
return [
'type' => '<?php ' . $const_name,
'description' => $class_constants[$const_name]->description,
];
}

if (strpos($symbol, '()')) {
Expand All @@ -995,27 +1010,36 @@ public function getSymbolInformation(string $file_path, string $symbol): ?string
if (isset($file_storage->functions[$function_id])) {
$function_storage = $file_storage->functions[$function_id];

return '<?php ' . $function_storage->getSignature(true);
return [
'type' => '<?php ' . $function_storage->getSignature(true),
'description' => $function_storage->description,
];
}

if (!$function_id) {
return null;
}

$function = $this->functions->getStorage(null, $function_id);
return '<?php ' . $function->getSignature(true);
return [
'type' => '<?php ' . $function->getSignature(true),
'description' => $function->description,
];
}

if (strpos($symbol, '$') === 0) {
$type = VariableFetchAnalyzer::getGlobalType($symbol);
if (!$type->isMixed()) {
return '<?php ' . $type;
return ['type' => '<?php ' . $type];
}
}

try {
$storage = $this->classlike_storage_provider->get($symbol);
return '<?php ' . ($storage->abstract ? 'abstract ' : '') . 'class ' . $storage->name;
return [
'type' => '<?php ' . ($storage->abstract ? 'abstract ' : '') . 'class ' . $storage->name,
'description' => $storage->description,
];
} catch (\InvalidArgumentException $e) {
}

Expand All @@ -1030,17 +1054,17 @@ public function getSymbolInformation(string $file_path, string $symbol): ?string
);
if (isset($namespace_constants[$const_name])) {
$type = $namespace_constants[$const_name];
return '<?php const ' . $symbol . ' ' . $type;
return ['type' => '<?php const ' . $symbol . ' ' . $type];
}
} else {
$file_storage = $this->file_storage_provider->get($file_path);
if (isset($file_storage->constants[$symbol])) {
return '<?php const ' . $symbol . ' ' . $file_storage->constants[$symbol];
return ['type' => '<?php const ' . $symbol . ' ' . $file_storage->constants[$symbol]];
}
$constant = ConstFetchAnalyzer::getGlobalConstType($this, $symbol, $symbol);

if ($constant) {
return '<?php const ' . $symbol . ' ' . $constant;
return ['type' => '<?php const ' . $symbol . ' ' . $constant];
}
}
return null;
Expand Down Expand Up @@ -1246,8 +1270,12 @@ public function getFunctionArgumentAtPosition(string $file_path, Position $posit
/**
* @param non-empty-string $function_symbol
*/
public function getSignatureInformation(string $function_symbol) : ?\LanguageServerProtocol\SignatureInformation
{
public function getSignatureInformation(
string $function_symbol,
string $file_path = null
): ?\LanguageServerProtocol\SignatureInformation {
$signature_label = '';
$signature_documentation = null;
if (strpos($function_symbol, '::') !== false) {
/** @psalm-suppress ArgumentTypeCoercion */
$method_id = new \Psalm\Internal\MethodIdentifier(...explode('::', $function_symbol));
Expand All @@ -1260,11 +1288,23 @@ public function getSignatureInformation(string $function_symbol) : ?\LanguageSer

$method_storage = $this->methods->getStorage($declaring_method_id);
$params = $method_storage->params;
$signature_label = $method_storage->cased_name;
$signature_documentation = $method_storage->description;
} else {
try {
$function_storage = $this->functions->getStorage(null, strtolower($function_symbol));

if ($file_path) {
$function_storage = $this->functions->getStorage(
null,
strtolower($function_symbol),
dirname($file_path),
$file_path
);
} else {
$function_storage = $this->functions->getStorage(null, strtolower($function_symbol));
}
$params = $function_storage->params;
$signature_label = $function_storage->cased_name;
$signature_documentation = $function_storage->description;
} catch (\Exception $exception) {
if (InternalCallMapHandler::inCallMap($function_symbol)) {
$callables = InternalCallMapHandler::getCallablesFromCallMap($function_symbol);
Expand All @@ -1280,15 +1320,18 @@ public function getSignatureInformation(string $function_symbol) : ?\LanguageSer
}
}

$signature_label = '(';
$signature_label .= '(';
$parameters = [];

foreach ($params as $i => $param) {
$parameter_label = ($param->type ?: 'mixed') . ' $' . $param->name;
$parameters[] = new \LanguageServerProtocol\ParameterInformation([
strlen($signature_label),
strlen($signature_label) + strlen($parameter_label),
]);
$parameters[] = new \LanguageServerProtocol\ParameterInformation(
[
strlen($signature_label),
strlen($signature_label) + strlen($parameter_label),
],
$param->description ?? null
);

$signature_label .= $parameter_label;

Expand All @@ -1301,7 +1344,8 @@ public function getSignatureInformation(string $function_symbol) : ?\LanguageSer

return new \LanguageServerProtocol\SignatureInformation(
$signature_label,
$parameters
$parameters,
$signature_documentation
);
}

Expand Down Expand Up @@ -1440,7 +1484,7 @@ public function getCompletionItemsForClassishThing(string $type_string, string $
$method_storage->cased_name,
\LanguageServerProtocol\CompletionItemKind::METHOD,
(string)$method_storage,
null,
$method_storage->description,
(string)$method_storage->visibility,
$method_storage->cased_name,
$method_storage->cased_name . (count($method_storage->params) !== 0 ? '($0)' : '()'),
Expand All @@ -1466,7 +1510,7 @@ public function getCompletionItemsForClassishThing(string $type_string, string $
'$' . $property_name,
\LanguageServerProtocol\CompletionItemKind::PROPERTY,
$property_storage->getInfo(),
null,
$property_storage->description,
(string)$property_storage->visibility,
$property_name,
($gap === '::' ? '$' : '') . $property_name
Expand All @@ -1479,12 +1523,12 @@ public function getCompletionItemsForClassishThing(string $type_string, string $
}
}

foreach ($class_storage->constants as $const_name => $_) {
foreach ($class_storage->constants as $const_name => $const) {
$static_completion_items[] = new \LanguageServerProtocol\CompletionItem(
$const_name,
\LanguageServerProtocol\CompletionItemKind::VARIABLE,
'const ' . $const_name,
null,
$const->description,
null,
$const_name,
$const_name
Expand Down Expand Up @@ -1606,11 +1650,18 @@ public function getCompletionItemsForPartialSymbol(
$insertion_text = $class_name;
}

try {
$class_storage = $this->classlike_storage_provider->get($fq_class_name);
$description = $class_storage->description;
} catch (\Exception $e) {
$description = null;
}

$completion_items[] = new \LanguageServerProtocol\CompletionItem(
$fq_class_name,
\LanguageServerProtocol\CompletionItemKind::CLASS_,
null,
null,
$description,
null,
$fq_class_name,
$insertion_text,
Expand Down Expand Up @@ -1656,7 +1707,7 @@ public function getCompletionItemsForPartialSymbol(
$function_name,
\LanguageServerProtocol\CompletionItemKind::FUNCTION,
$function->getSignature(false),
null,
$function->description,
null,
$function_name,
$function_name . (count($function->params) !== 0 ? '($0)' : '()'),
Expand Down
19 changes: 16 additions & 3 deletions src/Psalm/Internal/Analyzer/CommentAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ public static function arrayToDocblocks(
$line_parts = self::splitDocLine($var_line);

$line_number = $comment->getStartLine() + substr_count($comment_text, "\n", 0, $offset);
$description = $parsed_docblock->description;

if ($line_parts && $line_parts[0]) {
$type_start = $offset + $comment->getStartFilePos();
Expand Down Expand Up @@ -131,8 +132,14 @@ public static function arrayToDocblocks(

$var_line_number = $line_number;

if (count($line_parts) > 1 && $line_parts[1][0] === '$') {
$var_id = $line_parts[1];
if (count($line_parts) > 1) {
if ($line_parts[1][0] === '$') {
$var_id = $line_parts[1];
$description = trim(substr($var_line, strlen($line_parts[0]) + strlen($line_parts[1]) + 2));
} else {
$description = trim(substr($var_line, strlen($line_parts[0]) + 1));
}
$description = preg_replace('/\\n \\*\\s+/um', ' ', $description);
}
}

Expand Down Expand Up @@ -168,6 +175,7 @@ public static function arrayToDocblocks(
$var_comment->line_number = $var_line_number;
$var_comment->type_start = $type_start;
$var_comment->type_end = $type_end;
$var_comment->description = $description;

self::decorateVarDocblockComment($var_comment, $parsed_docblock);

Expand All @@ -182,7 +190,8 @@ public static function arrayToDocblocks(
|| isset($parsed_docblock->tags['psalm-readonly'])
|| isset($parsed_docblock->tags['psalm-readonly-allow-private-mutation'])
|| isset($parsed_docblock->tags['psalm-taint-escape'])
|| isset($parsed_docblock->tags['psalm-internal']))
|| isset($parsed_docblock->tags['psalm-internal'])
|| $parsed_docblock->description)
) {
$var_comment = new VarDocblockComment();

Expand All @@ -208,6 +217,10 @@ private static function decorateVarDocblockComment(
= isset($parsed_docblock->tags['psalm-allow-private-mutation'])
|| isset($parsed_docblock->tags['psalm-readonly-allow-private-mutation']);

if (!$var_comment->description) {
$var_comment->description = $parsed_docblock->description;
}

if (isset($parsed_docblock->tags['psalm-taint-escape'])) {
foreach ($parsed_docblock->tags['psalm-taint-escape'] as $param) {
$param = trim($param);
Expand Down
15 changes: 11 additions & 4 deletions src/Psalm/Internal/LanguageServer/Server/TextDocument.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
use LanguageServerProtocol\CompletionList;
use LanguageServerProtocol\Hover;
use LanguageServerProtocol\Location;
use LanguageServerProtocol\MarkedString;
use LanguageServerProtocol\MarkupContent;
use LanguageServerProtocol\MarkupKind;
use LanguageServerProtocol\Position;
use LanguageServerProtocol\Range;
use LanguageServerProtocol\TextDocumentIdentifier;
Expand Down Expand Up @@ -211,8 +212,14 @@ public function hover(TextDocumentIdentifier $textDocument, Position $position):
return new Success(null);
}

$contents = [];
$contents[] = new MarkedString('php', $symbol_information);
$content = "```php\n" . $symbol_information['type'] . "\n```";
if (isset($symbol_information['description'])) {
$content .= "\n---\n" . $symbol_information['description'];
}
$contents = new MarkupContent(
MarkupKind::MARKDOWN,
$content
);

return new Success(new Hover($contents, $range));
}
Expand Down Expand Up @@ -295,7 +302,7 @@ public function signatureHelp(TextDocumentIdentifier $textDocument, Position $po
return new Success(new \LanguageServerProtocol\SignatureHelp());
}

$signature_information = $this->codebase->getSignatureInformation($argument_location[0]);
$signature_information = $this->codebase->getSignatureInformation($argument_location[0], $file_path);

if (!$signature_information) {
return new Success(new \LanguageServerProtocol\SignatureHelp());
Expand Down
4 changes: 4 additions & 0 deletions src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeDocblockParser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,10 @@ public static function parse(
$info->stub_override = true;
}

if ($parsed_docblock->description) {
$info->description = $parsed_docblock->description;
}

self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'property');
self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'psalm-property');
self::addMagicPropertyToInfo($comment, $info, $parsed_docblock->tags, 'property-read');
Expand Down
Loading

0 comments on commit e59670e

Please sign in to comment.