Skip to content

Fix: Resolve HIR display length issues and improve adjustment tooltips #20031

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
109 changes: 76 additions & 33 deletions crates/ide/src/inlay_hints/adjustment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,50 +109,90 @@ pub(super) fn hints(
}
has_adjustments = true;

// FIXME: Add some nicer tooltips to each of these
let (text, coercion) = match kind {
let (text, coercion, detailed_tooltip) = match kind {
Adjust::NeverToAny if config.adjustment_hints == AdjustmentHints::Always => {
allow_edit = false;
("<never-to-any>", "never to any")
}
Adjust::Deref(None) => ("*", "dereference"),
Adjust::Deref(Some(OverloadedDeref(Mutability::Shared))) => {
("*", "`Deref` dereference")
}
Adjust::Deref(Some(OverloadedDeref(Mutability::Mut))) => {
("*", "`DerefMut` dereference")
}
Adjust::Borrow(AutoBorrow::Ref(Mutability::Shared)) => ("&", "borrow"),
Adjust::Borrow(AutoBorrow::Ref(Mutability::Mut)) => ("&mut ", "unique borrow"),
Adjust::Borrow(AutoBorrow::RawPtr(Mutability::Shared)) => {
("&raw const ", "const pointer borrow")
}
Adjust::Borrow(AutoBorrow::RawPtr(Mutability::Mut)) => {
("&raw mut ", "mut pointer borrow")
(
"<never-to-any>",
"never to any",
"Coerces the never type `!` into any other type. This happens in code paths that never return, like after `panic!()` or `return`.",
)
}
Adjust::Deref(None) => (
"*",
"dereference",
"Built-in dereference of a reference to access the underlying value. The compiler inserts `*` to get the value from `&T`.",
),
Adjust::Deref(Some(OverloadedDeref(Mutability::Shared))) => (
"*",
"`Deref` dereference",
"Dereference via the `Deref` trait. Used for types like `Box<T>` or `Rc<T>` so they act like plain `T`.",
),
Adjust::Deref(Some(OverloadedDeref(Mutability::Mut))) => (
"*",
"`DerefMut` dereference",
"Mutable dereference using the `DerefMut` trait. Enables smart pointers to give mutable access to their inner values.",
),
Adjust::Borrow(AutoBorrow::Ref(Mutability::Shared)) => (
"&",
"shared borrow",
"Inserts `&` to create a shared reference. Lets you use a value without moving or cloning it.",
),
Adjust::Borrow(AutoBorrow::Ref(Mutability::Mut)) => (
"&mut ",
"mutable borrow",
"Inserts `&mut` to create a unique, mutable reference. Lets you modify a value without taking ownership.",
),
Adjust::Borrow(AutoBorrow::RawPtr(Mutability::Shared)) => (
"&raw const ",
"const raw pointer",
"Converts a reference to a raw const pointer `*const T`. Often used when working with FFI or unsafe code.",
),
Adjust::Borrow(AutoBorrow::RawPtr(Mutability::Mut)) => (
"&raw mut ",
"mut raw pointer",
"Converts a mutable reference to a raw mutable pointer `*mut T`. Allows mutation in unsafe contexts.",
),
// some of these could be represented via `as` casts, but that's not too nice and
// handling everything as a prefix expr makes the `(` and `)` insertion easier
Adjust::Pointer(cast) if config.adjustment_hints == AdjustmentHints::Always => {
allow_edit = false;
match cast {
PointerCast::ReifyFnPointer => {
("<fn-item-to-fn-pointer>", "fn item to fn pointer")
}
PointerCast::ReifyFnPointer => (
"<fn-item-to-fn-pointer>",
"fn item to fn pointer",
"Converts a named function to a function pointer `fn()`. Useful when passing functions as values.",
),
PointerCast::UnsafeFnPointer => (
"<safe-fn-pointer-to-unsafe-fn-pointer>",
"safe fn pointer to unsafe fn pointer",
"Coerces a safe function pointer to an unsafe one. Allows calling it in an unsafe context.",
),
PointerCast::ClosureFnPointer(Safety::Unsafe) => (
"<closure-to-unsafe-fn-pointer>",
"closure to unsafe fn pointer",
"Converts a non-capturing closure to an unsafe function pointer. Required for use in `extern` or unsafe APIs.",
),
PointerCast::ClosureFnPointer(Safety::Safe) => (
"<closure-to-fn-pointer>",
"closure to fn pointer",
"Converts a non-capturing closure to a function pointer. Lets closures behave like plain functions.",
),
PointerCast::MutToConstPointer => (
"<mut-ptr-to-const-ptr>",
"mut ptr to const ptr",
"Coerces `*mut T` to `*const T`. Safe because const pointers restrict what you can do.",
),
PointerCast::ArrayToPointer => (
"<array-ptr-to-element-ptr>",
"array to pointer",
"Converts an array to a pointer to its first element. Similar to how arrays decay to pointers in C.",
),
PointerCast::Unsize => (
"<unsize>",
"unsize coercion",
"Converts a sized type to an unsized one. Used for things like turning arrays into slices or concrete types into trait objects.",
),
PointerCast::ClosureFnPointer(Safety::Unsafe) => {
("<closure-to-unsafe-fn-pointer>", "closure to unsafe fn pointer")
}
PointerCast::ClosureFnPointer(Safety::Safe) => {
("<closure-to-fn-pointer>", "closure to fn pointer")
}
PointerCast::MutToConstPointer => {
("<mut-ptr-to-const-ptr>", "mut ptr to const ptr")
}
PointerCast::ArrayToPointer => ("<array-ptr-to-element-ptr>", ""),
PointerCast::Unsize => ("<unsize>", "unsize"),
}
}
_ => continue,
Expand All @@ -162,9 +202,12 @@ pub(super) fn hints(
linked_location: None,
tooltip: Some(config.lazy_tooltip(|| {
InlayTooltip::Markdown(format!(
"`{}` → `{}` ({coercion} coercion)",
"`{}` → `{}`\n\n**{}**\n\n{}",
source.display(sema.db, display_target),
target.display(sema.db, display_target),
coercion.chars().next().unwrap().to_uppercase().collect::<String>()
+ &coercion[1..],
detailed_tooltip
Comment on lines +208 to +210
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's just manually uppercase the string literal instead of doing this

))
})),
};
Expand Down
102 changes: 92 additions & 10 deletions crates/ide/src/inlay_hints/closing_brace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ pub(super) fn hints(
match_ast! {
match parent {
ast::Fn(it) => {
// FIXME: this could include parameters, but `HirDisplay` prints too much info
// and doesn't respect the max length either, so the hints end up way too long
(format!("fn {}", it.name()?), it.name().map(name))
let hint_text = format_function_hint(&it, config.max_length?)
.unwrap_or_else(|| format!("fn {}", it.name().map(|n| n.to_string()).unwrap_or_default()));
(hint_text, it.name().map(name))
},
ast::Static(it) => (format!("static {}", it.name()?), it.name().map(name)),
ast::Const(it) => {
Expand Down Expand Up @@ -156,6 +156,47 @@ pub(super) fn hints(
None
}

fn format_function_hint(func: &ast::Fn, max_length: usize) -> Option<String> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imo we should do this as described in my comment in #19207 (comment) instead

let name = func.name()?;
let name_str = name.to_string();

let params = if let Some(param_list) = func.param_list() {
let mut param_parts = Vec::new();
let mut total_len = 0;
let max_param_len = max_length.saturating_sub(name_str.len() + 4);

for param in param_list.params() {
let param_text = if let Some(pat) = param.pat() {
if let Some(ty) = param.ty() { format!("{}: {}", pat, ty) } else { pat.to_string() }
} else if let Some(ty) = param.ty() {
format!("_: {}", ty)
} else {
let param_source = param.syntax().text().to_string();
if param_source.trim() == "..." { "...".to_owned() } else { "_".to_owned() }
};

let param_len = param_text.len() + if param_parts.is_empty() { 0 } else { 2 };
if total_len + param_len > max_param_len {
param_parts.push("...".to_owned());
break;
}

total_len += param_len;
param_parts.push(param_text);
}

if param_parts.is_empty() {
"()".to_owned()
} else {
format!("({})", param_parts.join(", "))
}
} else {
"()".to_owned()
};

Some(format!("fn {}{}", name_str, params))
}

#[cfg(test)]
mod tests {
use crate::{
Expand All @@ -166,7 +207,11 @@ mod tests {
#[test]
fn hints_closing_brace() {
check_with_config(
InlayHintsConfig { closing_brace_hints_min_lines: Some(2), ..DISABLED_CONFIG },
InlayHintsConfig {
closing_brace_hints_min_lines: Some(2),
max_length: Some(30),
..DISABLED_CONFIG
},
r#"
fn a() {}

Expand All @@ -175,17 +220,17 @@ fn f() {

fn g() {
}
//^ fn g
//^ fn g()

fn h<T>(with: T, arguments: u8, ...) {
}
//^ fn h
//^ fn h(with: T, arguments: u8, ...)

trait Tr {
fn f();
fn g() {
}
//^ fn g
//^ fn g()
}
//^ trait Tr
impl Tr for () {
Expand Down Expand Up @@ -222,15 +267,19 @@ fn f() {
let v = vec![
];
}
//^ fn f
//^ fn f()
"#,
);
}

#[test]
fn hints_closing_brace_for_block_expr() {
check_with_config(
InlayHintsConfig { closing_brace_hints_min_lines: Some(2), ..DISABLED_CONFIG },
InlayHintsConfig {
closing_brace_hints_min_lines: Some(2),
max_length: Some(10),
..DISABLED_CONFIG
},
r#"
fn test() {
'end: {
Expand Down Expand Up @@ -258,7 +307,40 @@ fn test() {
//^ 'a

}
//^ fn test
//^ fn test()
"#,
);
}

#[test]
fn hints_closing_brace_function_parameters() {
check_with_config(
InlayHintsConfig {
closing_brace_hints_min_lines: Some(1),
max_length: Some(50),
..DISABLED_CONFIG
},
r#"
fn simple() {
let v = vec![
];
}
//^ fn simple()

fn with_params(x: i32, y: String) {

}
//^ fn with_params(x: i32, y: String)

fn long_params(very_long_parameter_name: ComplexType, another: AnotherType) {

}
//^ fn long_params(...)

fn many_params(a: i32, b: i32, c: i32, d: i32, e: i32) {

}
//^ fn many_params(a: i32, b: i32, c: i32, d: i32, ...)
"#,
);
}
Expand Down