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
This commit is contained in:
bors 2024-04-14 06:28:40 +00:00
commit 7dad0a231e
8 changed files with 89 additions and 37 deletions

View File

@ -1,6 +1,5 @@
use std::{ use std::{
fmt::{self, Write}, fmt::{self, Write},
hash::{BuildHasher, BuildHasherDefault},
mem::take, mem::take,
}; };
@ -9,7 +8,7 @@
known, ClosureStyle, HasVisibility, HirDisplay, HirDisplayError, HirWrite, ModuleDef, known, ClosureStyle, HasVisibility, HirDisplay, HirDisplayError, HirWrite, ModuleDef,
ModuleDefId, Semantics, 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 itertools::Itertools;
use smallvec::{smallvec, SmallVec}; use smallvec::{smallvec, SmallVec};
use stdx::never; use stdx::never;
@ -495,6 +494,7 @@ pub(crate) fn inlay_hints_resolve(
position: TextSize, position: TextSize,
hash: u64, hash: u64,
config: &InlayHintsConfig, config: &InlayHintsConfig,
hasher: impl Fn(&InlayHint) -> u64,
) -> Option<InlayHint> { ) -> Option<InlayHint> {
let _p = tracing::span!(tracing::Level::INFO, "inlay_hints").entered(); let _p = tracing::span!(tracing::Level::INFO, "inlay_hints").entered();
let sema = Semantics::new(db); let sema = Semantics::new(db);
@ -506,8 +506,7 @@ pub(crate) fn inlay_hints_resolve(
let mut acc = Vec::new(); let mut acc = Vec::new();
let hints = |node| hints(&mut acc, &famous_defs, config, file_id, node); let hints = |node| hints(&mut acc, &famous_defs, config, file_id, node);
match file.token_at_offset(position).left_biased() { let token = file.token_at_offset(position).left_biased()?;
Some(token) => {
if let Some(parent_block) = token.parent_ancestors().find_map(ast::BlockExpr::cast) { if let Some(parent_block) = token.parent_ancestors().find_map(ast::BlockExpr::cast) {
parent_block.syntax().descendants().for_each(hints) parent_block.syntax().descendants().for_each(hints)
} else if let Some(parent_item) = token.parent_ancestors().find_map(ast::Item::cast) { } else if let Some(parent_item) = token.parent_ancestors().find_map(ast::Item::cast) {
@ -515,11 +514,8 @@ pub(crate) fn inlay_hints_resolve(
} else { } else {
return None; return None;
} }
}
None => return None,
}
acc.into_iter().find(|hint| BuildHasherDefault::<FxHasher>::default().hash_one(hint) == hash) acc.into_iter().find(|hint| hasher(hint) == hash)
} }
fn hints( fn hints(

View File

@ -58,6 +58,8 @@
mod view_memory_layout; mod view_memory_layout;
mod view_mir; mod view_mir;
use std::panic::UnwindSafe;
use cfg::CfgOptions; use cfg::CfgOptions;
use fetch_crates::CrateInfo; use fetch_crates::CrateInfo;
use hir::ChangeWithProcMacros; use hir::ChangeWithProcMacros;
@ -428,8 +430,11 @@ pub fn inlay_hints_resolve(
file_id: FileId, file_id: FileId,
position: TextSize, position: TextSize,
hash: u64, hash: u64,
hasher: impl Fn(&InlayHint) -> u64 + Send + UnwindSafe,
) -> Cancellable<Option<InlayHint>> { ) -> 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. /// Returns the set of folding ranges.

View File

@ -1522,11 +1522,18 @@ pub(crate) fn handle_inlay_hints_resolve(
file_id, file_id,
hint_position, hint_position,
resolve_data.hash, 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 Ok(resolve_hints
.into_iter() .and_then(|it| {
.filter_map(|it| {
to_proto::inlay_hint( to_proto::inlay_hint(
&snap, &snap,
&forced_resolve_inlay_hints_config.fields_to_resolve, &forced_resolve_inlay_hints_config.fields_to_resolve,
@ -1537,13 +1544,8 @@ pub(crate) fn handle_inlay_hints_resolve(
.ok() .ok()
}) })
.filter(|hint| hint.position == original_hint.position) .filter(|hint| hint.position == original_hint.position)
.filter(|hint| hint.kind == original_hint.kind); .filter(|hint| hint.kind == original_hint.kind)
if let Some(resolved_hint) = resolved_hints.next() { .unwrap_or(original_hint))
if resolved_hints.next().is_none() {
return Ok(resolved_hint);
}
}
Ok(original_hint)
} }
pub(crate) fn handle_call_hierarchy_prepare( pub(crate) fn handle_call_hierarchy_prepare(

View File

@ -463,13 +463,6 @@ pub struct TestInfo {
pub runnable: Runnable, 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 {} pub enum Ssr {}
impl Request for Ssr { impl Request for Ssr {

View File

@ -453,6 +453,8 @@ pub(crate) fn inlay_hint(
&std::hash::BuildHasherDefault::<FxHasher>::default(), &std::hash::BuildHasherDefault::<FxHasher>::default(),
&inlay_hint, &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; let mut something_to_resolve = false;

View File

@ -23,12 +23,12 @@
notification::DidOpenTextDocument, notification::DidOpenTextDocument,
request::{ request::{
CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest, CodeActionRequest, Completion, Formatting, GotoTypeDefinition, HoverRequest,
WillRenameFiles, WorkspaceSymbolRequest, InlayHintRequest, InlayHintResolveRequest, WillRenameFiles, WorkspaceSymbolRequest,
}, },
CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams, CodeActionContext, CodeActionParams, CompletionParams, DidOpenTextDocumentParams,
DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams, DocumentFormattingParams, FileRename, FormattingOptions, GotoDefinitionParams, HoverParams,
PartialResultParams, Position, Range, RenameFilesParams, TextDocumentItem, InlayHint, InlayHintLabel, InlayHintParams, PartialResultParams, Position, Range,
TextDocumentPositionParams, WorkDoneProgressParams, RenameFilesParams, TextDocumentItem, TextDocumentPositionParams, WorkDoneProgressParams,
}; };
use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams, UnindexedProject}; use rust_analyzer::lsp::ext::{OnEnter, Runnables, RunnablesParams, UnindexedProject};
use serde_json::json; use serde_json::json;
@ -75,6 +75,48 @@ fn completes_items_from_standard_library() {
assert!(res.to_string().contains("HashMap")); 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] #[test]
fn test_runnables_project() { fn test_runnables_project() {
if skip_slow_tests() { if skip_slow_tests() {

View File

@ -159,6 +159,18 @@ pub(crate) fn server(self) -> Server {
content_format: Some(vec![lsp_types::MarkupKind::Markdown]), content_format: Some(vec![lsp_types::MarkupKind::Markdown]),
..Default::default() ..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() ..Default::default()
}), }),
window: Some(lsp_types::WindowClientCapabilities { window: Some(lsp_types::WindowClientCapabilities {

View File

@ -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 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: need to adjust this doc as well and ping this issue: