From f43a37ad257313f938f4ac6830c7d4aebd090154 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 12 Jan 2025 13:44:38 +0100 Subject: [PATCH 1/2] internal: Compute inlay hint text edits lazily --- crates/ide/src/inlay_hints.rs | 71 +++++++++++++------ crates/ide/src/inlay_hints/adjustment.rs | 12 ++-- crates/ide/src/inlay_hints/bind_pat.rs | 3 +- crates/ide/src/inlay_hints/binding_mode.rs | 29 +++++--- crates/ide/src/inlay_hints/closure_ret.rs | 3 +- crates/ide/src/inlay_hints/discriminant.rs | 7 +- crates/ide/src/inlay_hints/extern_block.rs | 26 ++++--- crates/ide/src/inlay_hints/implicit_static.rs | 4 +- crates/rust-analyzer/src/lsp/to_proto.rs | 19 ++++- 9 files changed, 118 insertions(+), 56 deletions(-) diff --git a/crates/ide/src/inlay_hints.rs b/crates/ide/src/inlay_hints.rs index 040f1f72a6..2fc3629252 100644 --- a/crates/ide/src/inlay_hints.rs +++ b/crates/ide/src/inlay_hints.rs @@ -1,6 +1,6 @@ use std::{ fmt::{self, Write}, - mem::take, + mem::{self, take}, }; use either::Either; @@ -297,6 +297,17 @@ pub struct InlayHintsConfig { pub closing_brace_hints_min_lines: Option, pub fields_to_resolve: InlayFieldsToResolve, } +impl InlayHintsConfig { + fn lazy_text_edit(&self, finish: impl FnOnce() -> TextEdit) -> Lazy { + if self.fields_to_resolve.resolve_text_edits { + Lazy::Lazy + } else { + let edit = finish(); + never!(edit.is_empty(), "inlay hint produced an empty text edit"); + Lazy::Computed(edit) + } + } +} #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub struct InlayFieldsToResolve { @@ -408,12 +419,32 @@ pub struct InlayHint { /// The actual label to show in the inlay hint. pub label: InlayHintLabel, /// Text edit to apply when "accepting" this inlay hint. - pub text_edit: Option, + pub text_edit: Option>, /// Range to recompute inlay hints when trying to resolve for this hint. If this is none, the /// hint does not support resolving. pub resolve_parent: Option, } +/// A type signaling that a value is either computed, or is available for computation. +#[derive(Clone, Debug)] +pub enum Lazy { + Computed(T), + Lazy, +} + +impl Lazy { + pub fn computed(self) -> Option { + match self { + Lazy::Computed(it) => Some(it), + _ => None, + } + } + + pub fn is_lazy(&self) -> bool { + matches!(self, Self::Lazy) + } +} + impl std::hash::Hash for InlayHint { fn hash(&self, state: &mut H) { self.range.hash(state); @@ -422,7 +453,7 @@ impl std::hash::Hash for InlayHint { self.pad_right.hash(state); self.kind.hash(state); self.label.hash(state); - self.text_edit.is_some().hash(state); + mem::discriminant(&self.text_edit).hash(state); } } @@ -439,10 +470,6 @@ impl InlayHint { resolve_parent: None, } } - - pub fn needs_resolve(&self) -> Option { - self.resolve_parent.filter(|_| self.text_edit.is_some() || self.label.needs_resolve()) - } } #[derive(Debug, Hash)] @@ -503,10 +530,6 @@ impl InlayHintLabel { } self.parts.push(part); } - - pub fn needs_resolve(&self) -> bool { - self.parts.iter().any(|part| part.linked_location.is_some() || part.tooltip.is_some()) - } } impl From for InlayHintLabel { @@ -725,19 +748,22 @@ fn hint_iterator( fn ty_to_text_edit( sema: &Semantics<'_, RootDatabase>, + config: &InlayHintsConfig, node_for_hint: &SyntaxNode, ty: &hir::Type, offset_to_insert: TextSize, - prefix: String, -) -> Option { - let scope = sema.scope(node_for_hint)?; + prefix: impl Into, +) -> Option> { // FIXME: Limit the length and bail out on excess somehow? - let rendered = ty.display_source_code(scope.db, scope.module().into(), false).ok()?; - - let mut builder = TextEdit::builder(); - builder.insert(offset_to_insert, prefix); - builder.insert(offset_to_insert, rendered); - Some(builder.finish()) + let rendered = sema + .scope(node_for_hint) + .and_then(|scope| ty.display_source_code(scope.db, scope.module().into(), false).ok())?; + Some(config.lazy_text_edit(|| { + let mut builder = TextEdit::builder(); + builder.insert(offset_to_insert, prefix.into()); + builder.insert(offset_to_insert, rendered); + builder.finish() + })) } fn closure_has_block_body(closure: &ast::ClosureExpr) -> bool { @@ -847,7 +873,7 @@ mod tests { let edits = inlay_hints .into_iter() - .filter_map(|hint| hint.text_edit) + .filter_map(|hint| hint.text_edit?.computed()) .reduce(|mut acc, next| { acc.union(next).expect("merging text edits failed"); acc @@ -867,7 +893,8 @@ mod tests { let (analysis, file_id) = fixture::file(ra_fixture); let inlay_hints = analysis.inlay_hints(&config, file_id, None).unwrap(); - let edits: Vec<_> = inlay_hints.into_iter().filter_map(|hint| hint.text_edit).collect(); + let edits: Vec<_> = + inlay_hints.into_iter().filter_map(|hint| hint.text_edit?.computed()).collect(); assert!(edits.is_empty(), "unexpected edits: {edits:?}"); } diff --git a/crates/ide/src/inlay_hints/adjustment.rs b/crates/ide/src/inlay_hints/adjustment.rs index 4e48baa6f1..013e008bf9 100644 --- a/crates/ide/src/inlay_hints/adjustment.rs +++ b/crates/ide/src/inlay_hints/adjustment.rs @@ -183,7 +183,7 @@ pub(super) fn hints( return None; } if allow_edit { - let edit = { + let edit = Some(config.lazy_text_edit(|| { let mut b = TextEditBuilder::default(); if let Some(pre) = &pre { b.insert( @@ -198,14 +198,14 @@ pub(super) fn hints( ); } b.finish() - }; + })); match (&mut pre, &mut post) { (Some(pre), Some(post)) => { - pre.text_edit = Some(edit.clone()); - post.text_edit = Some(edit); + pre.text_edit = edit.clone(); + post.text_edit = edit; } - (Some(pre), None) => pre.text_edit = Some(edit), - (None, Some(post)) => post.text_edit = Some(edit), + (Some(pre), None) => pre.text_edit = edit, + (None, Some(post)) => post.text_edit = edit, (None, None) => (), } } diff --git a/crates/ide/src/inlay_hints/bind_pat.rs b/crates/ide/src/inlay_hints/bind_pat.rs index 1c2289158b..8a4b9590a1 100644 --- a/crates/ide/src/inlay_hints/bind_pat.rs +++ b/crates/ide/src/inlay_hints/bind_pat.rs @@ -78,13 +78,14 @@ pub(super) fn hints( let text_edit = if let Some(colon_token) = &type_ascriptable { ty_to_text_edit( sema, + config, desc_pat.syntax(), &ty, colon_token .as_ref() .map_or_else(|| pat.syntax().text_range(), |t| t.text_range()) .end(), - if colon_token.is_some() { String::new() } else { String::from(": ") }, + if colon_token.is_some() { "" } else { ": " }, ) } else { None diff --git a/crates/ide/src/inlay_hints/binding_mode.rs b/crates/ide/src/inlay_hints/binding_mode.rs index ed2bd8d2a8..5bbb4fe4e6 100644 --- a/crates/ide/src/inlay_hints/binding_mode.rs +++ b/crates/ide/src/inlay_hints/binding_mode.rs @@ -99,17 +99,24 @@ pub(super) fn hints( } if let hints @ [_, ..] = &mut acc[acc_base..] { - let mut edit = TextEditBuilder::default(); - for h in &mut *hints { - edit.insert( - match h.position { - InlayHintPosition::Before => h.range.start(), - InlayHintPosition::After => h.range.end(), - }, - h.label.parts.iter().map(|p| &*p.text).chain(h.pad_right.then_some(" ")).collect(), - ); - } - let edit = edit.finish(); + let edit = config.lazy_text_edit(|| { + let mut edit = TextEditBuilder::default(); + for h in &mut *hints { + edit.insert( + match h.position { + InlayHintPosition::Before => h.range.start(), + InlayHintPosition::After => h.range.end(), + }, + h.label + .parts + .iter() + .map(|p| &*p.text) + .chain(h.pad_right.then_some(" ")) + .collect(), + ); + } + edit.finish() + }); hints.iter_mut().for_each(|h| h.text_edit = Some(edit.clone())); } diff --git a/crates/ide/src/inlay_hints/closure_ret.rs b/crates/ide/src/inlay_hints/closure_ret.rs index 6827540fa8..7858b1d90a 100644 --- a/crates/ide/src/inlay_hints/closure_ret.rs +++ b/crates/ide/src/inlay_hints/closure_ret.rs @@ -52,13 +52,14 @@ pub(super) fn hints( let text_edit = if has_block_body { ty_to_text_edit( sema, + config, closure.syntax(), &ty, arrow .as_ref() .map_or_else(|| param_list.syntax().text_range(), |t| t.text_range()) .end(), - if arrow.is_none() { String::from(" -> ") } else { String::new() }, + if arrow.is_none() { " -> " } else { "" }, ) } else { None diff --git a/crates/ide/src/inlay_hints/discriminant.rs b/crates/ide/src/inlay_hints/discriminant.rs index 9aef859333..f720c06048 100644 --- a/crates/ide/src/inlay_hints/discriminant.rs +++ b/crates/ide/src/inlay_hints/discriminant.rs @@ -36,13 +36,14 @@ pub(super) fn enum_hints( return None; } for variant in enum_.variant_list()?.variants() { - variant_hints(acc, sema, &enum_, &variant); + variant_hints(acc, config, sema, &enum_, &variant); } Some(()) } fn variant_hints( acc: &mut Vec, + config: &InlayHintsConfig, sema: &Semantics<'_, RootDatabase>, enum_: &ast::Enum, variant: &ast::Variant, @@ -88,7 +89,9 @@ fn variant_hints( }, kind: InlayKind::Discriminant, label, - text_edit: d.ok().map(|val| TextEdit::insert(range.start(), format!("{eq_} {val}"))), + text_edit: d.ok().map(|val| { + config.lazy_text_edit(|| TextEdit::insert(range.start(), format!("{eq_} {val}"))) + }), position: InlayHintPosition::After, pad_left: false, pad_right: false, diff --git a/crates/ide/src/inlay_hints/extern_block.rs b/crates/ide/src/inlay_hints/extern_block.rs index 4cc4925cda..2bc91b68ed 100644 --- a/crates/ide/src/inlay_hints/extern_block.rs +++ b/crates/ide/src/inlay_hints/extern_block.rs @@ -8,7 +8,7 @@ use crate::{InlayHint, InlayHintsConfig}; pub(super) fn extern_block_hints( acc: &mut Vec, FamousDefs(_sema, _): &FamousDefs<'_, '_>, - _config: &InlayHintsConfig, + config: &InlayHintsConfig, _file_id: EditionedFileId, extern_block: ast::ExternBlock, ) -> Option<()> { @@ -23,7 +23,9 @@ pub(super) fn extern_block_hints( pad_right: true, kind: crate::InlayKind::ExternUnsafety, label: crate::InlayHintLabel::from("unsafe"), - text_edit: Some(TextEdit::insert(abi.syntax().text_range().start(), "unsafe ".to_owned())), + text_edit: Some(config.lazy_text_edit(|| { + TextEdit::insert(abi.syntax().text_range().start(), "unsafe ".to_owned()) + })), resolve_parent: Some(extern_block.syntax().text_range()), }); Some(()) @@ -32,7 +34,7 @@ pub(super) fn extern_block_hints( pub(super) fn fn_hints( acc: &mut Vec, FamousDefs(_sema, _): &FamousDefs<'_, '_>, - _config: &InlayHintsConfig, + config: &InlayHintsConfig, _file_id: EditionedFileId, fn_: &ast::Fn, extern_block: &ast::ExternBlock, @@ -42,14 +44,14 @@ pub(super) fn fn_hints( return None; } let fn_ = fn_.fn_token()?; - acc.push(item_hint(extern_block, fn_)); + acc.push(item_hint(config, extern_block, fn_)); Some(()) } pub(super) fn static_hints( acc: &mut Vec, FamousDefs(_sema, _): &FamousDefs<'_, '_>, - _config: &InlayHintsConfig, + config: &InlayHintsConfig, _file_id: EditionedFileId, static_: &ast::Static, extern_block: &ast::ExternBlock, @@ -59,11 +61,15 @@ pub(super) fn static_hints( return None; } let static_ = static_.static_token()?; - acc.push(item_hint(extern_block, static_)); + acc.push(item_hint(config, extern_block, static_)); Some(()) } -fn item_hint(extern_block: &ast::ExternBlock, token: SyntaxToken) -> InlayHint { +fn item_hint( + config: &InlayHintsConfig, + extern_block: &ast::ExternBlock, + token: SyntaxToken, +) -> InlayHint { InlayHint { range: token.text_range(), position: crate::InlayHintPosition::Before, @@ -71,7 +77,7 @@ fn item_hint(extern_block: &ast::ExternBlock, token: SyntaxToken) -> InlayHint { pad_right: true, kind: crate::InlayKind::ExternUnsafety, label: crate::InlayHintLabel::from("unsafe"), - text_edit: { + text_edit: Some(config.lazy_text_edit(|| { let mut builder = TextEdit::builder(); builder.insert(token.text_range().start(), "unsafe ".to_owned()); if extern_block.unsafe_token().is_none() { @@ -79,8 +85,8 @@ fn item_hint(extern_block: &ast::ExternBlock, token: SyntaxToken) -> InlayHint { builder.insert(abi.syntax().text_range().start(), "unsafe ".to_owned()); } } - Some(builder.finish()) - }, + builder.finish() + })), resolve_parent: Some(extern_block.syntax().text_range()), } } diff --git a/crates/ide/src/inlay_hints/implicit_static.rs b/crates/ide/src/inlay_hints/implicit_static.rs index 1560df37d0..ae5b519b43 100644 --- a/crates/ide/src/inlay_hints/implicit_static.rs +++ b/crates/ide/src/inlay_hints/implicit_static.rs @@ -39,7 +39,9 @@ pub(super) fn hints( range: t.text_range(), kind: InlayKind::Lifetime, label: "'static".into(), - text_edit: Some(TextEdit::insert(t.text_range().start(), "'static ".into())), + text_edit: Some(config.lazy_text_edit(|| { + TextEdit::insert(t.text_range().start(), "'static ".into()) + })), position: InlayHintPosition::After, pad_left: false, pad_right: true, diff --git a/crates/rust-analyzer/src/lsp/to_proto.rs b/crates/rust-analyzer/src/lsp/to_proto.rs index 0dda7044f1..a3b54f532c 100644 --- a/crates/rust-analyzer/src/lsp/to_proto.rs +++ b/crates/rust-analyzer/src/lsp/to_proto.rs @@ -547,7 +547,18 @@ pub(crate) fn inlay_hint( file_id: FileId, mut inlay_hint: InlayHint, ) -> Cancellable { - let resolve_range_and_hash = inlay_hint.needs_resolve().map(|range| { + let hint_needs_resolve = |hint: &InlayHint| -> Option { + hint.resolve_parent.filter(|_| { + hint.text_edit.is_some() + || hint + .label + .parts + .iter() + .any(|part| part.linked_location.is_some() || part.tooltip.is_some()) + }) + }; + + let resolve_range_and_hash = hint_needs_resolve(&inlay_hint).map(|range| { ( range, std::hash::BuildHasher::hash_one( @@ -568,7 +579,11 @@ pub(crate) fn inlay_hint( something_to_resolve |= inlay_hint.text_edit.is_some(); None } else { - inlay_hint.text_edit.take().map(|it| text_edit_vec(line_index, it)) + inlay_hint + .text_edit + .take() + .and_then(|it| it.computed()) + .map(|it| text_edit_vec(line_index, it)) }; let (label, tooltip) = inlay_hint_label( snap, From 0f595b07bdb7dcda7822735105d8f05bed0a6dec Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sun, 12 Jan 2025 14:04:35 +0100 Subject: [PATCH 2/2] Fix text edits for discriminant hints --- crates/ide/src/inlay_hints/discriminant.rs | 35 ++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/crates/ide/src/inlay_hints/discriminant.rs b/crates/ide/src/inlay_hints/discriminant.rs index f720c06048..5a147adabc 100644 --- a/crates/ide/src/inlay_hints/discriminant.rs +++ b/crates/ide/src/inlay_hints/discriminant.rs @@ -90,7 +90,7 @@ fn variant_hints( kind: InlayKind::Discriminant, label, text_edit: d.ok().map(|val| { - config.lazy_text_edit(|| TextEdit::insert(range.start(), format!("{eq_} {val}"))) + config.lazy_text_edit(|| TextEdit::insert(range.end(), format!("{eq_} {val}"))) }), position: InlayHintPosition::After, pad_left: false, @@ -102,8 +102,10 @@ fn variant_hints( } #[cfg(test)] mod tests { + use expect_test::expect; + use crate::inlay_hints::{ - tests::{check_with_config, DISABLED_CONFIG}, + tests::{check_edit, check_with_config, DISABLED_CONFIG}, DiscriminantHints, InlayHintsConfig, }; @@ -210,4 +212,33 @@ enum Enum { "#, ); } + + #[test] + fn edit() { + check_edit( + InlayHintsConfig { discriminant_hints: DiscriminantHints::Always, ..DISABLED_CONFIG }, + r#" +#[repr(u8)] +enum Enum { + Variant(), + Variant1, + Variant2 {}, + Variant3, + Variant5, + Variant6, +} +"#, + expect![[r#" + #[repr(u8)] + enum Enum { + Variant() = 0, + Variant1 = 1, + Variant2 {} = 2, + Variant3 = 3, + Variant5 = 4, + Variant6 = 5, + } + "#]], + ); + } }