Cache hover text responses in language server mode

For phan#3252

When requesting a hover for the exact same position with the
exact same file open, return the cached hover response data.

NEWS.md

@@ -26,6 +26,7 @@ Language Server/Daemon mode:
 + Catch exception seen when printing debug info about not being able to parse a file.
 + Warn when Phan's language server dependencies were installed for php 7.2+
   but the language server gets run in php 7.1. (phpdocumentor/reflection-docblock 5.0 requires php 7.2)
++ Immediately return cached hover text when the client repeats an identical hover request. (#3252)
 
 Miscellaneous:
 + PHP 8.0-dev compatibility fixes, analysis for some new functions of PHP 8.0-dev.

src/Phan/LanguageServer/CachedHoverResponse.php

@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Phan\LanguageServer;
+
+use Phan\LanguageServer\Protocol\Hover;
+use Phan\LanguageServer\Protocol\Position;
+
+/**
+ * Caches the data for a hover response and information to check if the request was equivalent.
+ *
+ * @phan-read-only
+ */
+class CachedHoverResponse
+{
+    /** @var string the URI of the file where the hover cursor was. */
+    private $uri;
+    /** @var Position the position in the file where the hover cursor was. */
+    private $position;
+    /** @var string a hash of the FileMapping for open files in the editor. */
+    private $file_mapping_hash;
+    /** @var ?Hover the cached hover response */
+    private $hover;
+
+    public function __construct(string $uri, Position $position, FileMapping $file_mapping, ?Hover $hover)
+    {
+        $this->uri = $uri;
+        $this->position = $position;
+        $this->file_mapping_hash = $file_mapping->getHash();
+        $this->hover = $hover;
+    }
+
+    /**
+     * Checks if this is effectively the same request as the previous request
+     */
+    public function isSameRequest(string $uri, Position $position, FileMapping $file_mapping): bool
+    {
+        return $this->uri === $uri && $this->position->compare($position) === 0 && $this->file_mapping_hash === $file_mapping->getHash();
+    }
+
+    /**
+     * Get the hover response data or null.
+     * Callers should check that isSameRequest() is true first.
+     */
+    public function getHoverResponse(): ?Hover
+    {
+        return $this->hover;
+    }
+}

src/Phan/LanguageServer/FileMapping.php

@@ -39,6 +39,12 @@ public function getOverrides(): array
         return $this->overrides;
     }
 
+    public function getHash(): string
+    {
+        \uksort($this->overrides, 'strcmp');
+        return \sha1(\var_export($this->overrides, true));
+    }
+
     public function addOverrideURI(string $uri, ?string $new_contents): void
     {
         $path = Utils::uriToPath($uri);

src/Phan/LanguageServer/GoToDefinitionRequest.php

@@ -362,12 +362,13 @@ public function getHoverResponse(): ?Hover
      *
      * @param ?Hover|?array $hover
      */
-    public function setHoverResponse($hover): void
+    public function setHoverResponse($hover): ?Hover
     {
         if (is_array($hover)) {
             $hover = Hover::fromArray($hover);
         }
         $this->hover_response = $hover;
+        return $hover;
     }
 
     /**

src/Phan/LanguageServer/LanguageServer.php

@@ -161,12 +161,18 @@ class LanguageServer extends AdvancedJsonRpc\Dispatcher
     /**
      * @var ?NodeInfoRequest
      *
-     * Contains the promise for the most recent "Go to definition" request
+     * Contains the promise for the most recent "Go to definition" request.
      * If more than one such request exists, the earlier requests will be discarded.
      *
      * TODO: Will need to Resolve(null) for the older requests.
      */
     protected $most_recent_node_info_request = null;
+    /**
+     * @var ?CachedHoverResponse
+     *
+     * Contains the result of the previous hover response and information used to validate the cached information
+     */
+    protected $cached_hover_response = null;
 
     /**
      * Constructs the only instance of the language server
@@ -445,6 +451,8 @@ public function analyzeURIAsync(string $uri): void
         $path_to_analyze = Utils::uriToPath($uri);
         Logger::logInfo("Called analyzeURIAsync, uri=$uri, path=$path_to_analyze");
         $this->analyze_request_set[$path_to_analyze] = $uri;
+        // Clear the cached hover response - a file was probably opened, modified, or saved/closed.
+        $this->cached_hover_response = null;
         // Don't call file_path_lister immediately -
         // That has to walk the directories in .phan/config.php to see if the requested path is included and not excluded.
     }
@@ -478,19 +486,24 @@ public function awaitDefinition(
     /**
      * Asynchronously generates the hover text for a given URL and position.
      *
-     * @return Promise <Location|Location[]|null>
+     * @return Promise <Hover|null>
      */
     public function awaitHover(
         string $uri,
         Position $position
     ): Promise {
+        MarkupDescription::eagerlyLoadAllDescriptionMaps();
         // TODO: Add a way to "go to definition" without emitting analysis results as a side effect
         $path_to_analyze = Utils::uriToPath($uri);
         Logger::logInfo("Called LanguageServer->awaitHover, uri=$uri, position=" . StringUtil::jsonEncode($position));
-        $this->discardPreviousNodeInfoRequest();
         $request = new GoToDefinitionRequest($uri, $position, GoToDefinitionRequest::REQUEST_HOVER);
+        $this->discardPreviousNodeInfoRequest();
         $this->most_recent_node_info_request = $request;
-        MarkupDescription::eagerlyLoadAllDescriptionMaps();
+        $early_response = $this->generateQuickHoverResponse($request);
+        if ($early_response) {
+            return $early_response;
+        }
+        $this->cached_hover_response = null;
 
         // We analyze this url so that Phan is aware enough of the types and namespace maps to trigger "Go to definition"
         // E.g. going to the definition of `Bar` in `use Foo as Bar; Bar::method();` requires parsing other statements in this file, not just the name in question.
@@ -500,6 +513,29 @@ public function awaitHover(
         return $request->getPromise();
     }
 
+    /**
+     * Immediately generates the hover text for a given URL and position.
+     *
+     * @return ?Promise <?Hover>
+     */
+    private function generateQuickHoverResponse(GoToDefinitionRequest $request): ?Promise
+    {
+        $description = "uri={$request->getUrl()}, position=" . StringUtil::jsonEncode($request->getPosition());
+        Logger::logInfo("Looking for quick hover response for $description");
+        if (!$this->cached_hover_response) {
+            return null;
+        }
+        if (!$this->cached_hover_response->isSameRequest($request->getUrl(), $request->getPosition(), $this->file_mapping)) {
+            return null;
+        }
+        $promise = new Promise();
+        $cached_response = $this->cached_hover_response->getHoverResponse();
+        Logger::logInfo("Returning cached hover response for $description");
+
+        $promise->fulfill($cached_response);
+        return $promise;
+    }
+
     /**
      * Asynchronously generates the definition for a given URL
      * @return Promise <Location|Location[]|null>
@@ -767,7 +803,16 @@ private function handleJSONResponseFromWorker(array $uris_to_analyze, array $res
             if ($most_recent_node_info_request instanceof GoToDefinitionRequest) {
                 // @phan-suppress-next-line PhanPartialTypeMismatchArgument, PhanTypeMismatchArgumentNullable
                 $most_recent_node_info_request->recordDefinitionLocationList($response_data['definitions'] ?? null);
-                $most_recent_node_info_request->setHoverResponse($response_data['hover_response'] ?? null);
+                if ($most_recent_node_info_request->isHoverRequest()) {
+                    $normalized_hover = $most_recent_node_info_request->setHoverResponse($response_data['hover_response'] ?? null);
+                    // \fwrite(\STDERR, "Creating cached_hover_response\n");
+                    $this->cached_hover_response = new CachedHoverResponse(
+                        $most_recent_node_info_request->getUrl(),
+                        $most_recent_node_info_request->getPosition(),
+                        $this->file_mapping,
+                        $normalized_hover
+                    );
+                }
             } elseif ($most_recent_node_info_request instanceof CompletionRequest) {
                 $most_recent_node_info_request->recordCompletionList($response_data['completions'] ?? null);
             }

src/Phan/LanguageServer/Protocol/Position.php

@@ -46,10 +46,6 @@ public function __construct(int $line = null, int $character = null)
      */
     public function compare(Position $position): int
     {
-        if ($this->line === $position->line && $this->character === $position->character) {
-            return 0;
-        }
-
         if ($this->line !== $position->line) {
             return $this->line - $position->line;
         }

tests/Phan/LanguageServer/LanguageServerIntegrationTest.php

@@ -1349,8 +1349,8 @@ public function runTestHoverInOtherFileWithPcntlSetting(
 
             // Request the definition of the class "MyExample" with the cursor in the middle of that word
             // NOTE: Line numbers are 0-based for Position
-            $perform_hover_request = /** @return array<string,mixed> */ function () use ($proc_in, $proc_out, $position, $requested_uri): array {
-                return $this->writeHoverRequestAndAwaitResponse($proc_in, $proc_out, $position, $requested_uri);
+            $perform_hover_request = /** @return array<string,mixed> */ function (bool $is_repeated = false) use ($proc_in, $proc_out, $position, $requested_uri): array {
+                return $this->writeHoverRequestAndAwaitResponse($proc_in, $proc_out, $position, $requested_uri, $is_repeated);
             };
             $hover_response = $perform_hover_request();
 
@@ -1380,15 +1380,16 @@ public function runTestHoverInOtherFileWithPcntlSetting(
                 (string)\json_encode($cur_line),
                 (string)\substr($cur_line, $position->character, 10)
             );
-            $this->assertSame($expected_hover_response, $hover_response, $message);  // slightly better diff view than assertSame
+            $this->assertEquals($expected_hover_response, $hover_response, $message);  // slightly better diff view than assertSame
             $this->assertSame($expected_hover_response, $hover_response, $message);
 
             // This operation should be idempotent.
             // If it's repeated, it should give the same response
             // (and it shouldn't crash the server)
+            // (and it should avoid repeating the analysis step)
             $expected_hover_response['id'] = 3;
 
-            $hover_response = $perform_hover_request();
+            $hover_response = $perform_hover_request(true);
             $this->assertSame($expected_hover_response, $hover_response, $message);  // slightly better diff view than assertSame
             $this->assertSame($expected_hover_response, $hover_response, $message);
 
@@ -1827,7 +1828,7 @@ private function writeTypeDefinitionRequestAndAwaitResponse($proc_in, $proc_out,
      * @return array<string,mixed> the response of the server to the hover request
      * @throws InvalidArgumentException
      */
-    private function writeHoverRequestAndAwaitResponse($proc_in, $proc_out, Position $position, string $requested_uri = null): array
+    private function writeHoverRequestAndAwaitResponse($proc_in, $proc_out, Position $position, string $requested_uri = null, bool $is_repeated = false): array
     {
         $requested_uri = $requested_uri ?? self::getDefaultFileURI();
         // Implementation detail: We simultaneously emit a notification with new diagnostics
@@ -1839,7 +1840,7 @@ private function writeHoverRequestAndAwaitResponse($proc_in, $proc_out, Position
             'position' => $position,
         ];
         $this->writeMessage($proc_in, 'textDocument/hover', $params);
-        if (self::shouldExpectDiagnosticNotificationForURI($requested_uri)) {
+        if (!$is_repeated && self::shouldExpectDiagnosticNotificationForURI($requested_uri)) {
             $this->assertHasEmptyPublishDiagnosticsNotification($proc_out, $requested_uri);
         }