Implement signature help - closes vimeo#1841

composer.json

@@ -15,7 +15,7 @@
         "openlss/lib-array2xml": "^1.0",
         "ocramius/package-versions": "^1.2",
         "composer/xdebug-handler": "^1.1",
-        "felixfbecker/language-server-protocol": "^1.3",
+        "felixfbecker/language-server-protocol": "^1.4",
         "felixfbecker/advanced-json-rpc": "^3.0.3",
         "netresearch/jsonmapper": "^1.0",
         "webmozart/glob": "^4.1",

src/Psalm/Codebase.php

@@ -1142,6 +1142,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 || $start_pos === null || $end_pos === 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: int}|null
      */

src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php

@@ -46,6 +46,11 @@
 use function array_search;
 use function array_keys;
 use function in_array;
+use function substr;
+use function token_get_all;
+use function array_reverse;
+use function strlen;
+use function reset;
 
 /**
  * @internal
@@ -967,6 +972,13 @@ function (PhpParser\Node\Arg $arg) {
             }
         }
 
+        self::recordArgumentPositions(
+            $statements_analyzer,
+            $stmt,
+            $codebase,
+            $method_id
+        );
+
         if (self::checkMethodArgs(
             $method_id,
             $args,
@@ -1656,4 +1668,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();
+        $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('<?php ' . $method_name_and_first_parent_source_code);
+        $opening_paren_position = $first_argument_character;
+        foreach (array_reverse($method_name_and_first_parent_tokens) as $token) {
+            $token = is_string($token) ? $token : $token[1];
+            $opening_paren_position -= strlen($token);
+
+            if ($token === '(') {
+                break;
+            }
+        }
+
+        // Record ranges of the source code that need to be tokenized to find commas
+        /** @var array{0: int, 1: int}[] $ranges */
+        $ranges = [];
+
+        // Add range between opening paren and first argument
+        $first_argument = $stmt->args[0] ?? null;
+        $first_argument_starting_position = $first_argument !== null
+            ? $first_argument->getStartFilePos()
+            : $stmt->getEndFilePos();
+        $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('<?php ' . $range_source_code);
+            array_shift($range_tokens);
+
+            $current_position = $position;
+            foreach ($range_tokens as $token) {
+                $token = is_string($token) ? $token : $token[1];
+
+                if ($token === ',') {
+                    $commas[] = $current_position;
+                }
+
+                $current_position += strlen($token);
+            }
+        }
+
+        $argument_start_position = $opening_paren_position + 1;
+        $argument_number = 0;
+        while (!empty($commas)) {
+            $comma = reset($commas);
+            array_shift($commas);
+
+            $codebase->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
+        );
+    }
 }

src/Psalm/Internal/Codebase/Analyzer.php

@@ -61,7 +61,8 @@
  *          string,
  *          array{
  *              0: TaggedCodeType,
- *              1: TaggedCodeType
+ *              1: TaggedCodeType,
+ *              2: array<int, array{0: int, 1: string, 2: int}>
  *         }
  *      >,
  *      class_locations: array<string, array<int, \Psalm\CodeLocation>>,
@@ -151,6 +152,11 @@ class Analyzer
      */
     private $type_map = [];
 
+    /**
+     * @var array<string, array<int, array{0: int, 1: string, 2: int}>>
+     */
+    private $argument_map = [];
+
     /**
      * @var array<string, array<int, \Psalm\Type\Union>>
      */
@@ -434,9 +440,11 @@ function () {
                     FileManipulationBuffer::add($file_path, $manipulations);
                 }
 
-                foreach ($pool_data['file_maps'] as $file_path => list($reference_map, $type_map)) {
+                foreach ($pool_data['file_maps'] as $file_path => $file_maps) {
+                    list($reference_map, $type_map, $argument_map) = $file_maps;
                     $this->reference_map[$file_path] = $reference_map;
                     $this->type_map[$file_path] = $type_map;
+                    $this->argument_map[$file_path] = $argument_map;
                 }
             }
 
@@ -520,9 +528,10 @@ 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)) {
+            foreach ($file_maps as $file_path => list($reference_map, $type_map, $argument_map)) {
                 $this->reference_map[$file_path] = $reference_map;
                 $this->type_map[$file_path] = $type_map;
+                $this->argument_map[$file_path] = $argument_map;
             }
         }
 
@@ -879,6 +888,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,
+                        ];
+                    }
+                }
+            }
+        }
     }
 
     /**
@@ -1002,6 +1045,20 @@ public function addNodeType(
         ];
     }
 
+    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
      */
@@ -1255,6 +1312,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]);
+                }
+            }
+        }
     }
 
     /**
@@ -1270,7 +1335,8 @@ public function getAnalyzedMethods()
      *      string,
      *      array{
      *          0: TaggedCodeType,
-     *          1: TaggedCodeType
+     *          1: TaggedCodeType,
+     *          2: array<int, array{0: int, 1: string, 2: int}>
      *      }
      * >
      */
@@ -1279,28 +1345,41 @@ 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, []];
+            }
+        }
+
+        foreach ($this->argument_map as $file_path => $argument_map) {
+            if (isset($file_maps[$file_path])) {
+                $file_maps[$file_path][2] = $argument_map;
+            } else {
+                $file_maps[$file_path] = [[], [], $argument_map];
             }
         }
 
         return $file_maps;
     }
 
     /**
-     * @return array{0: TaggedCodeType, 1: TaggedCodeType}
+     * @return array{
+     *     0: TaggedCodeType,
+     *     1: TaggedCodeType,
+     *     2: array<int, array{0: int, 1: string, 2: int}>
+     * }
      */
     public function getMapsForFile(string $file_path)
     {
         return [
             $this->reference_map[$file_path] ?? [],
-            $this->type_map[$file_path] ?? []
+            $this->type_map[$file_path] ?? [],
+            $this->argument_map[$file_path] ?? [],
         ];
     }
 

src/Psalm/Internal/LanguageServer/LanguageServer.php

@@ -241,10 +241,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;