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 #16962

Author: bors

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(

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.

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(

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 {

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;

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;
@@ -75,6 +75,48 @@ use std::collections::Spam;
     assert!(res.to_string().contains("HashMap"));
 }
 
+#[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() {

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 {

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;
 }
 ```