Auto merge of #17063 - Veykril:inlay-hints-fix, r=Veykril

fix: Fix inlay hint resolution being broken

So, things broke because we now store a hash (u64) in the resolution payload, but javascript and hence JSON only support integers of up to 53 bits (anything beyond gets truncated in various ways) which caused almost all hashes to always differ when resolving them. This masks the hash to 53 bits to work around that.

Fixes https://github.com/rust-lang/rust-analyzer/issues/16962

8 files changed, 89 insertions, 37 deletions

diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs
index dda38ce77e0..15eecd1b54a 100644
--- a/crates/ide/src/inlay_hints.rs
+++ b/crates/ide/src/inlay_hints.rs
@@ -1,6 +1,5 @@
 use std::{
     fmt::{self, Write},
-    hash::{BuildHasher, BuildHasherDefault},
     mem::take,
 };
 
@@ -9,7 +8,7 @@ use hir::{
     known, ClosureStyle, HasVisibility, HirDisplay, HirDisplayError, HirWrite, ModuleDef,
     ModuleDefId, Semantics,
 };
-use ide_db::{base_db::FileRange, famous_defs::FamousDefs, FxHasher, RootDatabase};
+use ide_db::{base_db::FileRange, famous_defs::FamousDefs, RootDatabase};
 use itertools::Itertools;
 use smallvec::{smallvec, SmallVec};
 use stdx::never;
@@ -495,6 +494,7 @@ pub(crate) fn inlay_hints_resolve(
     position: TextSize,
     hash: u64,
     config: &InlayHintsConfig,
+    hasher: impl Fn(&InlayHint) -> u64,
 ) -> Option<InlayHint> {
     let _p = tracing::span!(tracing::Level::INFO, "inlay_hints").entered();
     let sema = Semantics::new(db);
@@ -506,20 +506,16 @@ pub(crate) fn inlay_hints_resolve(
     let mut acc = Vec::new();
 
     let hints = |node| hints(&mut acc, &famous_defs, config, file_id, node);
-    match file.token_at_offset(position).left_biased() {
-        Some(token) => {
-            if let Some(parent_block) = token.parent_ancestors().find_map(ast::BlockExpr::cast) {
-                parent_block.syntax().descendants().for_each(hints)
-            } else if let Some(parent_item) = token.parent_ancestors().find_map(ast::Item::cast) {
-                parent_item.syntax().descendants().for_each(hints)
-            } else {
-                return None;
-            }
-        }
-        None => return None,
+    let token = file.token_at_offset(position).left_biased()?;
+    if let Some(parent_block) = token.parent_ancestors().find_map(ast::BlockExpr::cast) {
+        parent_block.syntax().descendants().for_each(hints)
+    } else if let Some(parent_item) = token.parent_ancestors().find_map(ast::Item::cast) {
+        parent_item.syntax().descendants().for_each(hints)
+    } else {
+        return None;
     }
 
-    acc.into_iter().find(|hint| BuildHasherDefault::<FxHasher>::default().hash_one(hint) == hash)
+    acc.into_iter().find(|hint| hasher(hint) == hash)
 }
 
 fn hints(
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
index e13060e4d79..5f5af20ac5e 100644
--- a/crates/ide/src/lib.rs
+++ b/crates/ide/src/lib.rs
@@ -58,6 +58,8 @@ mod view_item_tree;
 mod view_memory_layout;
 mod view_mir;
 
+use std::panic::UnwindSafe;
+
 use cfg::CfgOptions;
 use fetch_crates::CrateInfo;
 use hir::ChangeWithProcMacros;
@@ -428,8 +430,11 @@ impl Analysis {
         file_id: FileId,
         position: TextSize,
         hash: u64,
+        hasher: impl Fn(&InlayHint) -> u64 + Send + UnwindSafe,
     ) -> Cancellable<Option<InlayHint>> {
-        self.with_db(|db| inlay_hints::inlay_hints_resolve(db, file_id, position, hash, config))
+        self.with_db(|db| {
+            inlay_hints::inlay_hints_resolve(db, file_id, position, hash, config, hasher)
+        })
     }
 
     /// Returns the set of folding ranges.
diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs
index e5c070bf77d..3439ae1d074 100644
--- a/crates/rust-analyzer/src/handlers/request.rs
+++ b/crates/rust-analyzer/src/handlers/request.rs
@@ -1522,11 +1522,18 @@ pub(crate) fn handle_inlay_hints_resolve(
         file_id,
         hint_position,
         resolve_data.hash,
+        |hint| {
+            std::hash::BuildHasher::hash_one(
+                &std::hash::BuildHasherDefault::<ide_db::FxHasher>::default(),
+                hint,
+            )
+            // json only supports numbers up to 2^53 - 1 as integers, so mask the rest
+            & ((1 << 53) - 1)
+        },
     )?;
 
-    let mut resolved_hints = resolve_hints
-        .into_iter()
-        .filter_map(|it| {
+    Ok(resolve_hints
+        .and_then(|it| {
             to_proto::inlay_hint(
                 &snap,
                 &forced_resolve_inlay_hints_config.fields_to_resolve,
@@ -1537,13 +1544,8 @@ pub(crate) fn handle_inlay_hints_resolve(
             .ok()
         })
         .filter(|hint| hint.position == original_hint.position)
-        .filter(|hint| hint.kind == original_hint.kind);
-    if let Some(resolved_hint) = resolved_hints.next() {
-        if resolved_hints.next().is_none() {
-            return Ok(resolved_hint);
-        }
-    }
-    Ok(original_hint)
+        .filter(|hint| hint.kind == original_hint.kind)
+        .unwrap_or(original_hint))
 }
 
 pub(crate) fn handle_call_hierarchy_prepare(
diff --git a/crates/rust-analyzer/src/lsp/ext.rs b/crates/rust-analyzer/src/lsp/ext.rs
index eac982f1b27..1ef49b5c111 100644
--- a/crates/rust-analyzer/src/lsp/ext.rs
+++ b/crates/rust-analyzer/src/lsp/ext.rs
@@ -463,13 +463,6 @@ pub struct TestInfo {
     pub runnable: Runnable,
 }
 
-#[derive(Serialize, Deserialize, Debug)]
-#[serde(rename_all = "camelCase")]
-pub struct InlayHintsParams {
-    pub text_document: TextDocumentIdentifier,
-    pub range: Option<lsp_types::Range>,
-}
-
 pub enum Ssr {}
 
 impl Request for Ssr {
diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs
index d8bb12528b9..04a5486e941 100644
--- a/crates/rust-analyzer/src/lsp/to_proto.rs
+++ b/crates/rust-analyzer/src/lsp/to_proto.rs
@@ -453,6 +453,8 @@ pub(crate) fn inlay_hint(
             &std::hash::BuildHasherDefault::<FxHasher>::default(),
             &inlay_hint,
         )
+        // json only supports numbers up to 2^53 - 1 as integers, so mask the rest
+         & ((1 << 53) - 1)
     });
 
     let mut something_to_resolve = false;
diff --git a/crates/rust-analyzer/tests/slow-tests/main.rs b/crates/rust-analyzer/tests/slow-tests/main.rs
index 439b006977d..8c982b46879 100644
--- a/crates/rust-analyzer/tests/slow-tests/main.rs
+++ b/crates/rust-analyzer/tests/slow-tests/main.rs
@@ -23,12 +23,12 @@ use lsp_types::{
     notification::DidOpenTextDocument,
     request::{
         CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest,
-        WillRenameFiles, WorkspaceSymbolRequest,
+        InlayHintRequest, InlayHintResolveRequest, WillRenameFiles, WorkspaceSymbolRequest,
     },
     CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams,
     DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams,
-    PartialResultParams, Position, Range, RenameFilesParams, TextDocumentItem,
-    TextDocumentPositionParams, WorkDoneProgressParams,
+    InlayHint, InlayHintLabel, InlayHintParams, PartialResultParams, Position, Range,
+    RenameFilesParams, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams,
 };
 use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams, UnindexedProject};
 use serde_json::json;
@@ -76,6 +76,48 @@ use std::collections::Spam;
 }
 
 #[test]
+fn resolves_inlay_hints() {
+    if skip_slow_tests() {
+        return;
+    }
+
+    let server = Project::with_fixture(
+        r#"
+//- /Cargo.toml
+[package]
+name = "foo"
+version = "0.0.0"
+
+//- /src/lib.rs
+struct Foo;
+fn f() {
+    let x = Foo;
+}
+"#,
+    )
+    .server()
+    .wait_until_workspace_is_loaded();
+
+    let res = server.send_request::<InlayHintRequest>(InlayHintParams {
+        range: Range::new(Position::new(0, 0), Position::new(3, 1)),
+        text_document: server.doc_id("src/lib.rs"),
+        work_done_progress_params: WorkDoneProgressParams::default(),
+    });
+    let mut hints = serde_json::from_value::<Option<Vec<InlayHint>>>(res).unwrap().unwrap();
+    let hint = hints.pop().unwrap();
+    assert!(hint.data.is_some());
+    assert!(
+        matches!(&hint.label, InlayHintLabel::LabelParts(parts) if parts[1].location.is_none())
+    );
+    let res = server.send_request::<InlayHintResolveRequest>(hint);
+    let hint = serde_json::from_value::<InlayHint>(res).unwrap();
+    assert!(hint.data.is_none());
+    assert!(
+        matches!(&hint.label, InlayHintLabel::LabelParts(parts) if parts[1].location.is_some())
+    );
+}
+
+#[test]
 fn test_runnables_project() {
     if skip_slow_tests() {
         return;
diff --git a/crates/rust-analyzer/tests/slow-tests/support.rs b/crates/rust-analyzer/tests/slow-tests/support.rs
index 8bbe6ff3724..c6778bf6564 100644
--- a/crates/rust-analyzer/tests/slow-tests/support.rs
+++ b/crates/rust-analyzer/tests/slow-tests/support.rs
@@ -159,6 +159,18 @@ impl Project<'_> {
                         content_format: Some(vec![lsp_types::MarkupKind::Markdown]),
                         ..Default::default()
                     }),
+                    inlay_hint: Some(lsp_types::InlayHintClientCapabilities {
+                        resolve_support: Some(lsp_types::InlayHintResolveClientCapabilities {
+                            properties: vec![
+                                "textEdits".to_owned(),
+                                "tooltip".to_owned(),
+                                "label.tooltip".to_owned(),
+                                "label.location".to_owned(),
+                                "label.command".to_owned(),
+                            ],
+                        }),
+                        ..Default::default()
+                    }),
                     ..Default::default()
                 }),
                 window: Some(lsp_types::WindowClientCapabilities {
diff --git a/docs/dev/lsp-extensions.md b/docs/dev/lsp-extensions.md
index 939b1819c7e..1b7534a549f 100644
--- a/docs/dev/lsp-extensions.md
+++ b/docs/dev/lsp-extensions.md
@@ -1,5 +1,5 @@
 <!---
-lsp/ext.rs hash: 223f48a89a5126a0
+lsp/ext.rs hash: 4aacf4cca1c9ff5e
 
 If you need to change the above hash to make the test pass, please check if you
 need to adjust this doc as well and ping this issue:
@@ -444,7 +444,7 @@ interface DiscoverTestResults {
     // For each file which its uri is in this list, the response
     // contains all tests that are located in this file, and
     // client should remove old tests not included in the response.
-    scopeFile: lc.TextDocumentIdentifier[] | undefined;    
+    scopeFile: lc.TextDocumentIdentifier[] | undefined;
 }
 ```