From 0e989cdad326d6f08470a5ffbb709b85005d19dd Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 24 Jun 2019 11:39:31 +0200 Subject: [PATCH] Implement signature help - closes #1841 --- src/Psalm/Codebase.php | 54 +++++++++ .../Expression/Call/MethodCallAnalyzer.php | 110 ++++++++++++++++++ src/Psalm/Internal/Codebase/Analyzer.php | 93 +++++++++++++-- .../LanguageServer/LanguageServer.php | 5 +- .../LanguageServer/Server/TextDocument.php | 46 ++++++++ tests/LanguageServer/SymbolLookupTest.php | 32 +++++ 6 files changed, 329 insertions(+), 11 deletions(-) diff --git a/src/Psalm/Codebase.php b/src/Psalm/Codebase.php index 7f9f3b93610..3c997d90080 100644 --- a/src/Psalm/Codebase.php +++ b/src/Psalm/Codebase.php @@ -1123,6 +1123,60 @@ public function getReferenceAtPosition(string $file_path, Position $position) return [$reference, $range]; } + /** + * @return array{0: string, 1: int, 2: Range}|null + */ + public function getFunctionArgumentAtPosition(string $file_path, Position $position) + { + $is_open = $this->file_provider->isOpen($file_path); + + if (!$is_open) { + throw new \Psalm\Exception\UnanalyzedFileException($file_path . ' is not open'); + } + + $file_contents = $this->getFileContents($file_path); + + $offset = $position->toOffset($file_contents); + + list(,,, $argument_map) = $this->analyzer->getMapsForFile($file_path); + + $reference = null; + $argument_number = null; + + if (!$argument_map) { + return null; + } + + $start_pos = null; + $end_pos = null; + + ksort($argument_map); + + foreach ($argument_map as $start_pos => list($end_pos, $possible_reference, $possible_argument_number)) { + if ($offset < $start_pos) { + break; + } + + if ($offset > $end_pos) { + continue; + } + + $reference = $possible_reference; + $argument_number = $possible_argument_number; + } + + if ($reference === null || $argument_number === null) { + return null; + } + + $range = new Range( + self::getPositionFromOffset($start_pos, $file_contents), + self::getPositionFromOffset($end_pos, $file_contents) + ); + + return [$reference, $argument_number, $range]; + } + /** * @return array{0: string, 1: '->'|'::'|'symbol', 2: array}|null */ diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php index 226c7d8efa1..b8269ffd8b4 100644 --- a/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php +++ b/src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php @@ -954,6 +954,13 @@ function (PhpParser\Node\Arg $arg) { } } + self::recordArgumentPositions( + $statements_analyzer, + $stmt, + $codebase, + $method_id + ); + if (self::checkMethodArgs( $method_id, $args, @@ -1643,4 +1650,107 @@ private static function checkMagicGetterOrSetterProperty( return true; } + + private static function recordArgumentPositions( + StatementsAnalyzer $statements_analyzer, + PhpParser\Node\Expr\MethodCall $stmt, + Codebase $codebase, + string $method_id + ): void { + $file_content = $codebase->file_provider->getContents($statements_analyzer->getFilePath()); + + // Find opening paren + $first_argument = $stmt->args[0] ?? null; + $first_argument_character = $first_argument !== null + ? $first_argument->getStartFilePos() + : ($stmt->getEndFilePos() - 1); + $method_name_and_first_parent_source_code = substr( + $file_content, + $stmt->getStartFilePos(), + $first_argument_character - $stmt->getStartFilePos() + ); + $method_name_and_first_parent_tokens = token_get_all('args[0] ?? null; + $first_argument_starting_position = $first_argument !== null + ? $first_argument->getStartFilePos() + : $stmt->getEndFilePos() - 1; + $first_range_starting_position = $opening_paren_position + 1; + if ($first_range_starting_position !== $first_argument_starting_position) { + $ranges[] = [$first_range_starting_position, $first_argument_starting_position]; + } + + // Add range between arguments + foreach ($stmt->args as $i => $argument) { + $range_start = $argument->getEndFilePos() + 1; + $next_argument = $stmt->args[$i + 1] ?? null; + $range_end = $next_argument !== null + ? $next_argument->getStartFilePos() + : $stmt->getEndFilePos(); + + if ($range_start !== $range_end) { + $ranges[] = [$range_start, $range_end]; + } + } + + $commas = []; + foreach ($ranges as $range) { + $position = $range[0]; + $length = $range[1] - $position; + $range_source_code = substr($file_content, $position, $length); + $range_tokens = token_get_all('analyzer->addNodeArgument( + $statements_analyzer->getFilePath(), + $argument_start_position, + $comma, + $method_id, + $argument_number + ); + + ++$argument_number; + $argument_start_position = $comma + 1; + } + + $codebase->analyzer->addNodeArgument( + $statements_analyzer->getFilePath(), + $argument_start_position, + $stmt->getEndFilePos(), + $method_id, + $argument_number + ); + } } diff --git a/src/Psalm/Internal/Codebase/Analyzer.php b/src/Psalm/Internal/Codebase/Analyzer.php index 2bdb4c3c2f5..d10bddd60bc 100644 --- a/src/Psalm/Internal/Codebase/Analyzer.php +++ b/src/Psalm/Internal/Codebase/Analyzer.php @@ -145,6 +145,11 @@ class Analyzer */ private $alias_map = []; + /** + * @var array> + */ + private $argument_map = []; + /** * @var array> */ @@ -425,10 +430,12 @@ function () { FileManipulationBuffer::add($file_path, $manipulations); } - foreach ($pool_data['file_maps'] as $file_path => list($reference_map, $type_map, $alias_map)) { + foreach ($pool_data['file_maps'] as $file_path => list($reference_map, $type_map, $alias_map, $argument_map) + ) { $this->reference_map[$file_path] = $reference_map; $this->type_map[$file_path] = $type_map; $this->alias_map[$file_path] = $alias_map; + $this->argument_map[$file_path] = $argument_map; } } @@ -512,10 +519,11 @@ public function loadCachedResults(ProjectAnalyzer $project_analyzer) $this->existing_issues = $codebase->file_reference_provider->getExistingIssues(); $file_maps = $codebase->file_reference_provider->getFileMaps(); - foreach ($file_maps as $file_path => list($reference_map, $type_map, $alias_map)) { + foreach ($file_maps as $file_path => list($reference_map, $type_map, $alias_map, $argument_map)) { $this->reference_map[$file_path] = $reference_map; $this->type_map[$file_path] = $type_map; $this->alias_map[$file_path] = $alias_map; + $this->argument_map[$file_path] = $argument_map; } } @@ -905,6 +913,40 @@ public function shiftFileOffsets(array $diff_map) } } } + + foreach ($this->argument_map as $file_path => &$argument_map) { + if (!isset($this->analyzed_methods[$file_path])) { + unset($this->argument_map[$file_path]); + continue; + } + + $file_diff_map = $diff_map[$file_path] ?? []; + + if (!$file_diff_map) { + continue; + } + + $first_diff_offset = $file_diff_map[0][0]; + $last_diff_offset = $file_diff_map[count($file_diff_map) - 1][1]; + + foreach ($argument_map as $argument_from => list($argument_to, $method_id, $argument_number)) { + if ($argument_to < $first_diff_offset || $argument_from > $last_diff_offset) { + continue; + } + + + foreach ($file_diff_map as list($from, $to, $file_offset)) { + if ($argument_from >= $from && $argument_from <= $to) { + unset($argument_map[$argument_from]); + $argument_map[$argument_from += $file_offset] = [ + $argument_to += $file_offset, + $method_id, + $argument_number, + ]; + } + } + } + } } /** @@ -1055,6 +1097,20 @@ public function addNodeReference(string $file_path, PhpParser\Node $node, string ]; } + public function addNodeArgument( + string $file_path, + int $start_position, + int $end_position, + string $reference, + int $argument_number + ): void { + $this->argument_map[$file_path][$start_position] = [ + $end_position, + $reference, + $argument_number + ]; + } + /** * @return void */ @@ -1297,6 +1353,14 @@ public function removeExistingDataForFile($file_path, $start, $end) } } } + + if (isset($this->argument_map[$file_path])) { + foreach ($this->argument_map[$file_path] as $map_start => $_) { + if ($map_start >= $start && $map_start <= $end) { + unset($this->argument_map[$file_path][$map_start]); + } + } + } } /** @@ -1313,7 +1377,8 @@ public function getAnalyzedMethods() * array{ * 0: TaggedCodeType, * 1: TaggedCodeType, - * 2: array}> + * 2: array}>, + * 3: array * } * > */ @@ -1322,14 +1387,14 @@ public function getFileMaps() $file_maps = []; foreach ($this->reference_map as $file_path => $reference_map) { - $file_maps[$file_path] = [$reference_map, [], []]; + $file_maps[$file_path] = [$reference_map, [], [], []]; } foreach ($this->type_map as $file_path => $type_map) { if (isset($file_maps[$file_path])) { $file_maps[$file_path][1] = $type_map; } else { - $file_maps[$file_path] = [[], $type_map, []]; + $file_maps[$file_path] = [[], $type_map, [], []]; } } @@ -1337,7 +1402,15 @@ public function getFileMaps() if (isset($file_maps[$file_path])) { $file_maps[$file_path][2] = $alias_map; } else { - $file_maps[$file_path] = [[], [], $alias_map]; + $file_maps[$file_path] = [[], [], $alias_map, []]; + } + } + + foreach ($this->argument_map as $file_path => $argument_map) { + if (isset($file_maps[$file_path])) { + $file_maps[$file_path][3] = $argument_map; + } else { + $file_maps[$file_path] = [[], [], [], $argument_map]; } } @@ -1345,7 +1418,12 @@ public function getFileMaps() } /** - * @return array{0: TaggedCodeType, 1: TaggedCodeType, 2: array}>} + * @return array{ + * 0: TaggedCodeType, + * 1: TaggedCodeType, + * 2: array}> + * 3: array + * } */ public function getMapsForFile(string $file_path) { @@ -1353,6 +1431,7 @@ public function getMapsForFile(string $file_path) $this->reference_map[$file_path] ?? [], $this->type_map[$file_path] ?? [], $this->alias_map[$file_path] ?? [], + $this->argument_map[$file_path] ?? [], ]; } diff --git a/src/Psalm/Internal/LanguageServer/LanguageServer.php b/src/Psalm/Internal/LanguageServer/LanguageServer.php index 81d96337156..c5242e5575e 100644 --- a/src/Psalm/Internal/LanguageServer/LanguageServer.php +++ b/src/Psalm/Internal/LanguageServer/LanguageServer.php @@ -224,10 +224,7 @@ function () use ($capabilities, $rootPath, $processId) { $serverCapabilities->completionProvider->triggerCharacters = ['$', '>', ':']; } - /* - $serverCapabilities->signatureHelpProvider = new SignatureHelpOptions(); - $serverCapabilities->signatureHelpProvider->triggerCharacters = ['(', ',']; - */ + $serverCapabilities->signatureHelpProvider = new SignatureHelpOptions(['(', ',']); // Support global references $serverCapabilities->xworkspaceReferencesProvider = false; diff --git a/src/Psalm/Internal/LanguageServer/Server/TextDocument.php b/src/Psalm/Internal/LanguageServer/Server/TextDocument.php index e8afdced3e4..3b6604c0e62 100644 --- a/src/Psalm/Internal/LanguageServer/Server/TextDocument.php +++ b/src/Psalm/Internal/LanguageServer/Server/TextDocument.php @@ -270,4 +270,50 @@ public function completion(TextDocumentIdentifier $textDocument, Position $posit return new Success(new CompletionList($completion_items, false)); } + + public function signatureHelp(TextDocumentIdentifier $textDocument, Position $position): Promise + { + $file_path = LanguageServer::uriToPath($textDocument->uri); + + $argument_location = $this->codebase->getFunctionArgumentAtPosition($file_path, $position); + if ($argument_location === null) { + error_log('No argument location'); + return new Success(new \LanguageServerProtocol\SignatureHelp()); + } + + list($method_symbol, $argument_number) = $argument_location; + + $declaring_method_id = $this->codebase->methods->getDeclaringMethodId($method_symbol); + if ($declaring_method_id === null) { + error_log('No declaring method id'); + return new Success(new \LanguageServerProtocol\SignatureHelp()); + } + + $method_storage = $this->codebase->methods->getStorage($declaring_method_id); + + $signature_label = '('; + $parameters = []; + foreach ($method_storage->params as $i => $param) { + $parameter_label = ($param->type ?: 'mixed') . ' $' . $param->name; + $parameters[] = new \LanguageServerProtocol\ParameterInformation([ + strlen($signature_label), + strlen($signature_label) + strlen($parameter_label), + ]) ; + $signature_label .= $parameter_label; + + if ($i < (count($method_storage->params) - 1)) { + $signature_label .= ', '; + } + } + $signature_label .= ')'; + + error_log('Argument ' . $argument_number . ' of ' . $method_storage->cased_name); + + return new Success(new \LanguageServerProtocol\SignatureHelp([ + new \LanguageServerProtocol\SignatureInformation( + $signature_label, + $parameters + ), + ], null, $argument_number)); + } } diff --git a/tests/LanguageServer/SymbolLookupTest.php b/tests/LanguageServer/SymbolLookupTest.php index 3c853bd4774..7815e66067a 100644 --- a/tests/LanguageServer/SymbolLookupTest.php +++ b/tests/LanguageServer/SymbolLookupTest.php @@ -295,4 +295,36 @@ class A { $this->assertSame('Exception', $symbol_at_position[0]); } + + /** + * @return void + */ + public function testGetSignatureHelp() + { + $codebase = $this->project_analyzer->getCodebase(); + $config = $codebase->config; + $config->throw_exception = false; + + $this->addFile( + 'somefile.php', + 'foo("test", "Hello world!", "foo",); + } + }' + ); + + $codebase->file_provider->openFile('somefile.php'); + $codebase->scanFiles(); + $this->analyzeFile('somefile.php', new Context()); + + $reference_location = $codebase->getFunctionArgumentAtPosition('somefile.php', new Position(5, 35)); + + list($symbol, $argument_number) = $reference_location; + $this->assertSame('B\A::foo', $symbol); + $this->assertSame(0, $argument_number); + } }