Skip to content

fix: Fix inlay hint resolution being broken #17063

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions crates/ide/src/inlay_hints.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{
fmt::{self, Write},
hash::{BuildHasher, BuildHasherDefault},
mem::take,
};

Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand Down
7 changes: 6 additions & 1 deletion crates/ide/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
22 changes: 12 additions & 10 deletions crates/rust-analyzer/src/handlers/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1508,11 +1508,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,
Expand All @@ -1523,13 +1530,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(
Expand Down
7 changes: 0 additions & 7 deletions crates/rust-analyzer/src/lsp/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 2 additions & 0 deletions crates/rust-analyzer/src/lsp/to_proto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
48 changes: 45 additions & 3 deletions crates/rust-analyzer/tests/slow-tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
12 changes: 12 additions & 0 deletions crates/rust-analyzer/tests/slow-tests/support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions docs/dev/lsp-extensions.md
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -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;
}
```

Expand Down