Unnamed repository; edit this file 'description' to name the repository.
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
| -rw-r--r-- | crates/ide/src/inlay_hints.rs | 24 | ||||
| -rw-r--r-- | crates/ide/src/lib.rs | 7 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/handlers/request.rs | 22 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/lsp/ext.rs | 7 | ||||
| -rw-r--r-- | crates/rust-analyzer/src/lsp/to_proto.rs | 2 | ||||
| -rw-r--r-- | crates/rust-analyzer/tests/slow-tests/main.rs | 48 | ||||
| -rw-r--r-- | crates/rust-analyzer/tests/slow-tests/support.rs | 12 | ||||
| -rw-r--r-- | docs/dev/lsp-extensions.md | 4 |
8 files changed, 89 insertions, 37 deletions
diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index dda38ce77e..15eecd1b54 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 e13060e4d7..5f5af20ac5 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 e5c070bf77..3439ae1d07 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 eac982f1b2..1ef49b5c11 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 d8bb12528b..04a5486e94 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 439b006977..8c982b4687 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 8bbe6ff372..c6778bf656 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 939b1819c7..1b7534a549 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; } ``` |