Decline a stale inlay hint request instead of answering it with nothing

docs/CHANGELOG.md

@@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- **Parameter name hints stay on the argument they name while you type.** The symbol map a file's hints are read from is rebuilt on a background task, so a request arriving between a keystroke and that rebuild was answered with offsets describing the previous text. Hints drifted into the middle of the arguments they label, and because the visible range was measured against the newer text while the hints were not, the following call's hints were pulled onto the line being edited and drawn a second time beside its own. A request landing in that window is now declined rather than answered, so the editor keeps the hints it is already showing where they are instead of blanking the line, and the refresh sent once the rebuild finishes re-pulls them. Editors that do not understand a decline are no worse off than before. Contributed by @SuperFes.
 - **Hover, completion, go-to-definition, signature help, and inlay hints parse the document once per request.** The type engine reads the syntax tree from several places while resolving an expression, and only diagnostics and code actions were sharing one parse between them; every other request re-parsed the whole file once per resolution step, which on a large file made a hover noticeably slower than the diagnostics for the same line.
 - **A deleted service provider no longer keeps its macros, gates, commands, morph aliases, and storage drivers registered.** What a file contributes to those Laravel indexes was replaced only when the file was parsed again, which a file removed from disk never is, so a `Str::macro()` or `Gate::define()` that lived only in a since-deleted provider stayed available for the rest of the session. They now go with the file, along with the container bindings and resource paths the provider registered.
 - **Variables shared through the fully qualified `View` facade reach templates.** A provider writing `\Illuminate\Support\Facades\View::share('appName', …)` rather than importing the facade was skipped when shared view variables were collected, so `$appName` was unknown in every template. The qualified spelling now counts the same as the short one.

src/inlay_hints.rs

@@ -25,6 +25,10 @@ use crate::symbol_map::{CallSite, UntypedClosureSite};
 use crate::text_position::{offset_to_position, position_to_offset};
 use crate::types::{ClassLikeKind, FileContext};
 
+/// LSP's `ContentModified` error code, which `tower-lsp`'s `ErrorCode` has
+/// no variant for.
+const CONTENT_MODIFIED: i64 = -32801;
+
 impl Backend {
     /// Entry point for the `textDocument/inlayHint` request.
     ///
@@ -40,15 +44,34 @@ impl Backend {
         // and the editor re-requests them on each scroll and each refresh a
         // keystroke triggers, so the work runs off the request task.
         let backend = self.clone_for_blocking();
-        let result = crate::server::run_blocking_cancel_safe("inlay_hint", move || {
+        let outcome = crate::server::run_blocking_cancel_safe("inlay_hint", move || {
             backend.with_file_content("textDocument/inlayHint", &uri, None, |content, _| {
                 backend.handle_inlay_hints(&uri, content, range)
             })
         })
         .await
         .flatten();
 
-        Ok(result.flatten())
+        match outcome {
+            Some(Some(hints)) => Ok(Some(hints)),
+            // Declining is not the same answer as "no hints here", and a
+            // null result is the only way the client can read it: it would
+            // replace the labels it is already showing with an empty set and
+            // leave the line bare until something re-pulls. `ContentModified`
+            // is the spec's own way to say this request could not be answered
+            // about this document state -- a conforming client keeps what it
+            // has and re-pulls on the `inlayHint/refresh` `did_change` sends
+            // once the new map commits.
+            Some(None) => Err(jsonrpc::Error {
+                code: jsonrpc::ErrorCode::ServerError(CONTENT_MODIFIED),
+                message: "inlay hints are not current for this document version".into(),
+                data: None,
+            }),
+            // No content to work from, or the blocking task died -- neither
+            // is a document-version problem, and re-requesting would not
+            // change either one.
+            None => Ok(None),
+        }
     }
 
     /// Handle a `textDocument/inlayHint` request.
@@ -62,6 +85,20 @@ impl Backend {
         range: Range,
     ) -> Option<Vec<InlayHint>> {
         let symbol_map = self.symbol_maps.read().get(uri).cloned()?;
+
+        // A map rebuilt on the background parse task lags the buffer by a
+        // keystroke, and its offsets only index the text it was built from.
+        // Resolved against `content` they land inside the very tokens they
+        // label, and the viewport window -- converted from `content` -- no
+        // longer selects the same call sites, so neighbouring lines' hints
+        // pile onto the edited one. `did_change` sends `inlayHint/refresh`
+        // once the new map commits, which re-pulls what this declines -- see
+        // `inlay_hint_request` for why declining answers `ContentModified`
+        // rather than an empty result.
+        if !symbol_map.matches_source(content) {
+            return None;
+        }
+
         let ctx = self.file_context(uri);
 
         // A template's request range arrives in Blade coordinates; the
@@ -887,6 +924,65 @@ mod tests {
         assert!(!eq_ignore_case_snake("foo", "bar"));
     }
 
+    /// Declining must not look like "no hints here" to the client: an empty
+    /// result replaces the labels it is already showing, where
+    /// `ContentModified` leaves them alone and re-pulls on the refresh that
+    /// follows.
+    #[tokio::test]
+    async fn a_decline_answers_content_modified_rather_than_an_empty_result() {
+        let backend = Backend::new_test();
+        let uri = "file:///test/declined_inlay.php";
+        let text =
+            "<?php\nfunction makeThing(string $needle, int $count): void {}\nmakeThing('aa', 1);\n";
+
+        backend
+            .open_files
+            .write()
+            .insert(uri.to_string(), std::sync::Arc::new(text.to_string()));
+        backend.update_ast(uri, text);
+        backend.workspace_indexed.store(true, Ordering::Release);
+
+        let params = InlayHintParams {
+            text_document: TextDocumentIdentifier {
+                uri: Url::parse(uri).unwrap(),
+            },
+            range: Range {
+                start: Position {
+                    line: 0,
+                    character: 0,
+                },
+                end: Position {
+                    line: 100,
+                    character: 0,
+                },
+            },
+            work_done_progress_params: Default::default(),
+        };
+
+        let answered = backend.inlay_hint_request(params.clone()).await;
+        assert!(
+            matches!(answered, Ok(Some(ref hints)) if !hints.is_empty()),
+            "a request the map describes must still answer with hints: {answered:?}"
+        );
+
+        // The buffer one keystroke burst ahead of the map describing it,
+        // which is the state a background parse leaves behind.
+        let edited = text.replace("'aa'", "'aaYYYYYYYYYY'");
+        backend
+            .open_files
+            .write()
+            .insert(uri.to_string(), std::sync::Arc::new(edited));
+
+        match backend.inlay_hint_request(params).await {
+            Err(error) => assert_eq!(
+                error.code,
+                jsonrpc::ErrorCode::ServerError(CONTENT_MODIFIED),
+                "a decline must be ContentModified, not any other error: {error:?}"
+            ),
+            Ok(hints) => panic!("declined request answered {hints:?} instead of ContentModified"),
+        }
+    }
+
     #[test]
     fn test_is_obvious_single_param() {
         assert!(is_obvious_single_param("strlen", "string"));

tests/integration/inlay_hints.rs

@@ -1500,3 +1500,70 @@ makeClosure('1', '2')('test');
         a_count, line6_labels
     );
 }
+
+// ─── Staleness: map offsets vs. buffer content ──────────────────────────────
+
+#[tokio::test]
+async fn stale_symbol_map_declines_rather_than_misplacing_hints() {
+    // The symbol map is rebuilt on a background task, so a request that
+    // arrives mid-keystroke is handed content the map does not describe.
+    // Resolving its offsets against that content used to put labels inside
+    // the arguments they annotate and, because the viewport window is
+    // converted from the newer text, pull the *next* call's hints onto the
+    // edited line as duplicates.
+    let backend = create_test_backend();
+    let uri = Url::parse("file:///test/stale_inlay.php").unwrap();
+    let text = r#"<?php
+function makeThing(string $needle, int $count): void {}
+makeThing('aa', 1);
+makeThing('bb', 2);
+"#;
+
+    backend
+        .did_open(DidOpenTextDocumentParams {
+            text_document: TextDocumentItem {
+                uri: uri.clone(),
+                language_id: "php".to_string(),
+                version: 1,
+                text: text.to_string(),
+            },
+        })
+        .await;
+
+    let whole_file = Range {
+        start: Position {
+            line: 0,
+            character: 0,
+        },
+        end: Position {
+            line: 100,
+            character: 0,
+        },
+    };
+
+    let fresh = backend
+        .handle_inlay_hints(uri.as_ref(), text, whole_file)
+        .unwrap_or_default();
+    assert_eq!(
+        hints_at_line(&fresh, 2).len(),
+        2,
+        "the matching-content case must still hint: {:?}",
+        labels(&hints_at_line(&fresh, 2))
+    );
+
+    // The buffer as it stands one keystroke burst later, before the
+    // background parse has published a map for it.
+    let edited = text.replace("'aa'", &format!("'aa{}'", "Y".repeat(30)));
+
+    let stale = backend
+        .handle_inlay_hints(uri.as_ref(), &edited, whole_file)
+        .unwrap_or_default();
+    assert!(
+        stale.is_empty(),
+        "hints resolved against content the map does not describe: {:?}",
+        stale
+            .iter()
+            .map(|h| (h.position.line, h.position.character, hint_label(h)))
+            .collect::<Vec<_>>()
+    );
+}