Repository: rust-lang/rust-analyzer (rust)

Issue:
Incorrect parameter names preview when tuple destructuring is involed.
**rust-analyzer version**: `rust-analyzer 1.80.0 (05147895 2024-07-21)`

**rustc version**: `rustc 1.80.0 (051478957 2024-07-21)`

**editor or extension**: Vim ([coc-rust-analyzer](https://github.com/fannheyward/coc-rust-analyzer))

**code snippet to reproduce**: cited from #4960:
```rust
fn main() {
    let range = (3, 5);
    test_1(range);
}

fn test_1((start_range, end_range): (u32, u32)) {
    println!("{} - {}", start_range, end_range);
}
```

## Actual

The popup documentation window shows the parameter name as `_`. This information is *completely useless* and I'm always forced to jump to the definition just to see the correct parameter name.

![2024-08-12_17_21_26](https://github.com/user-attachments/assets/a89e93ba-a4c5-49f3-89bb-961c0afb5740)

The situation becomes far more worse when two or more tuple destructurings are involed:

```rust
pub fn unchecked_move_forward(
    &self,
    (i, j): (usize, usize),
    (h_exclusive, w_exclusive): (usize, usize),
) -> (usize, usize) {
```

In this case, I get confused which is which.

![2024-08-12_17_37_41](https://github.com/user-attachments/assets/4aac09c2-7633-4251-ae73-c96845e32295)

## Expected

The popup documentation window shows the parameter name as `(start_range, end_range)` or anything meaningful.

<!-- TRIAGEBOT_START -->

<!-- TRIAGEBOT_ASSIGN_START -->

<!-- TRIAGEBOT_ASSIGN_DATA_START$${"user":"roife"}$$TRIAGEBOT_ASSIGN_DATA_END -->

<!-- TRIAGEBOT_ASSIGN_END -->
<!-- TRIAGEBOT_END -->

Pull request:
feat: render patterns in params for hovering
Fix #17858 

This PR introduces an option to [hir-def/src/body/pretty.rs](https://github.com/rust-lang/rust-analyzer/blob/08c7bbc2dbe4dcc8968484f1a0e1e6fe7a1d4f6d/crates/hir-def/src/body/pretty.rs) to render the result as a single line, which is then reused for rendering patterns in parameters for hovering.

Patch:
diff --git a/crates/hir-def/src/body.rs b/crates/hir-def/src/body.rs
index f18083d387..6493826f07 100644
--- a/crates/hir-def/src/body.rs
+++ b/crates/hir-def/src/body.rs
@@ -227,6 +227,17 @@ impl Body {
         pretty::print_expr_hir(db, self, owner, expr, edition)
     }
 
+    pub fn pretty_print_pat(
+        &self,
+        db: &dyn DefDatabase,
+        owner: DefWithBodyId,
+        pat: PatId,
+        oneline: bool,
+        edition: Edition,
+    ) -> String {
+        pretty::print_pat_hir(db, self, owner, pat, oneline, edition)
+    }
+
     fn new(
         db: &dyn DefDatabase,
         owner: DefWithBodyId,
diff --git a/crates/hir-def/src/body/pretty.rs b/crates/hir-def/src/body/pretty.rs
index 55740a68ac..37167fcb81 100644
--- a/crates/hir-def/src/body/pretty.rs
+++ b/crates/hir-def/src/body/pretty.rs
@@ -16,6 +16,13 @@ use crate::{
 
 use super::*;
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(super) enum LineFormat {
+    Oneline,
+    Newline,
+    Indentation,
+}
+
 pub(super) fn print_body_hir(
     db: &dyn DefDatabase,
     body: &Body,
@@ -52,7 +59,14 @@ pub(super) fn print_body_hir(
         }
     };
 
-    let mut p = Printer { db, body, buf: header, indent_level: 0, needs_indent: false, edition };
+    let mut p = Printer {
+        db,
+        body,
+        buf: header,
+        indent_level: 0,
+        line_format: LineFormat::Newline,
+        edition,
+    };
     if let DefWithBodyId::FunctionId(it) = owner {
         p.buf.push('(');
         let function_data = &db.function_data(it);
@@ -95,12 +109,38 @@ pub(super) fn print_expr_hir(
     expr: ExprId,
     edition: Edition,
 ) -> String {
-    let mut p =
-        Printer { db, body, buf: String::new(), indent_level: 0, needs_indent: false, edition };
+    let mut p = Printer {
+        db,
+        body,
+        buf: String::new(),
+        indent_level: 0,
+        line_format: LineFormat::Newline,
+        edition,
+    };
     p.print_expr(expr);
     p.buf
 }
 
+pub(super) fn print_pat_hir(
+    db: &dyn DefDatabase,
+    body: &Body,
+    _owner: DefWithBodyId,
+    pat: PatId,
+    oneline: bool,
+    edition: Edition,
+) -> String {
+    let mut p = Printer {
+        db,
+        body,
+        buf: String::new(),
+        indent_level: 0,
+        line_format: if oneline { LineFormat::Oneline } else { LineFormat::Newline },
+        edition,
+    };
+    p.print_pat(pat);
+    p.buf
+}
+
 macro_rules! w {
     ($dst:expr, $($arg:tt)*) => {
         { let _ = write!($dst, $($arg)*); }
@@ -109,10 +149,10 @@ macro_rules! w {
 
 macro_rules! wln {
     ($dst:expr) => {
-        { let _ = writeln!($dst); }
+        { $dst.newline(); }
     };
     ($dst:expr, $($arg:tt)*) => {
-        { let _ = writeln!($dst, $($arg)*); }
+        { let _ = w!($dst, $($arg)*); $dst.newline(); }
     };
 }
 
@@ -121,24 +161,30 @@ struct Printer<'a> {
     body: &'a Body,
     buf: String,
     indent_level: usize,
-    needs_indent: bool,
+    line_format: LineFormat,
     edition: Edition,
 }
 
 impl Write for Printer<'_> {
     fn write_str(&mut self, s: &str) -> fmt::Result {
         for line in s.split_inclusive('\n') {
-            if self.needs_indent {
+            if matches!(self.line_format, LineFormat::Indentation) {
                 match self.buf.chars().rev().find(|ch| *ch != ' ') {
                     Some('\n') | None => {}
                     _ => self.buf.push('\n'),
                 }
                 self.buf.push_str(&"    ".repeat(self.indent_level));
-                self.needs_indent = false;
             }
 
             self.buf.push_str(line);
-            self.needs_indent = line.ends_with('\n');
+
+            if matches!(self.line_format, LineFormat::Newline | LineFormat::Indentation) {
+                self.line_format = if line.ends_with('\n') {
+                    LineFormat::Indentation
+                } else {
+                    LineFormat::Newline
+                };
+            }
         }
 
         Ok(())
@@ -161,14 +207,28 @@ impl Printer<'_> {
         }
     }
 
+    // Add a newline if the current line is not empty.
+    // If the current line is empty, add a space instead.
+    //
+    // Do not use [`writeln!()`] or [`wln!()`] here, which will result in
+    // infinite recursive calls to this function.
     fn newline(&mut self) {
-        match self.buf.chars().rev().find_position(|ch| *ch != ' ') {
-            Some((_, '\n')) | None => {}
-            Some((idx, _)) => {
-                if idx != 0 {
-                    self.buf.drain(self.buf.len() - idx..);
+        if matches!(self.line_format, LineFormat::Oneline) {
+            match self.buf.chars().last() {
+                Some(' ') | None => {}
+                Some(_) => {
+                    w!(self, " ");
+                }
+            }
+        } else {
+            match self.buf.chars().rev().find_position(|ch| *ch != ' ') {
+                Some((_, '\n')) | None => {}
+                Some((idx, _)) => {
+                    if idx != 0 {
+                        self.buf.drain(self.buf.len() - idx..);
+                    }
+                    w!(self, "\n");
                 }
-                writeln!(self).unwrap()
             }
         }
     }
@@ -539,12 +599,14 @@ impl Printer<'_> {
                 w!(self, ")");
             }
             Pat::Or(pats) => {
+                w!(self, "(");
                 for (i, pat) in pats.iter().enumerate() {
                     if i != 0 {
                         w!(self, " | ");
                     }
                     self.print_pat(*pat);
                 }
+                w!(self, ")");
             }
             Pat::Record { path, args, ellipsis } => {
                 match path {
@@ -554,12 +616,37 @@ impl Printer<'_> {
 
                 w!(self, " {{");
                 let edition = self.edition;
+                let oneline = matches!(self.line_format, LineFormat::Oneline);
                 self.indented(|p| {
-                    for arg in args.iter() {
-                        w!(p, "{}: ", arg.name.display(self.db.upcast(), edition));
-                        p.print_pat(arg.pat);
-                        wln!(p, ",");
+                    for (idx, arg) in args.iter().enumerate() {
+                        let field_name = arg.name.display(self.db.upcast(), edition).to_string();
+
+                        let mut same_name = false;
+                        if let Pat::Bind { id, subpat: None } = &self.body[arg.pat] {
+                            if let Binding { name, mode: BindingAnnotation::Unannotated, .. } =
+                                &self.body.bindings[*id]
+                            {
+                                if name.as_str() == field_name {
+                                    same_name = true;
+                                }
+                            }
+                        }
+
+                        w!(p, "{}", field_name);
+
+                        if !same_name {
+                            w!(p, ": ");
+                            p.print_pat(arg.pat);
+                        }
+
+                        // Do not print the extra comma if the line format is oneline
+                        if oneline && idx == args.len() - 1 {
+                            w!(p, " ");
+                        } else {
+                            wln!(p, ",");
+                        }
                     }
+
                     if *ellipsis {
                         wln!(p, "..");
                     }
diff --git a/crates/hir/src/display.rs b/crates/hir/src/display.rs
index 923dca6466..c2b2fbef75 100644
--- a/crates/hir/src/display.rs
+++ b/crates/hir/src/display.rs
@@ -99,17 +99,20 @@ impl HirDisplay for Function {
         }
 
         // FIXME: Use resolved `param.ty` once we no longer discard lifetimes
+        let body = db.body(self.id.into());
         for (type_ref, param) in data.params.iter().zip(self.assoc_fn_params(db)).skip(skip_self) {
-            let local = param.as_local(db).map(|it| it.name(db));
             if !first {
                 f.write_str(", ")?;
             } else {
                 first = false;
             }
-            match local {
-                Some(name) => write!(f, "{}: ", name.display(f.db.upcast(), f.edition()))?,
-                None => f.write_str("_: ")?,
-            }
+
+            let pat_id = body.params[param.idx - body.self_param.is_some() as usize];
+            let pat_str =
+                body.pretty_print_pat(db.upcast(), self.id.into(), pat_id, true, f.edition());
+            f.write_str(&pat_str)?;
+
+            f.write_str(": ")?;
             type_ref.hir_fmt(f)?;
         }
 
diff --git a/crates/ide-db/src/syntax_helpers/suggest_name.rs b/crates/ide-db/src/syntax_helpers/suggest_name.rs
index e60deb3bf5..6ee526a67e 100644
--- a/crates/ide-db/src/syntax_helpers/suggest_name.rs
+++ b/crates/ide-db/src/syntax_helpers/suggest_name.rs
@@ -18,9 +18,7 @@ const USELESS_TRAITS: &[&str] = &["Send", "Sync", "Copy", "Clone", "Eq", "Partia
 ///
 /// **NOTE**: they all must be snake lower case
 const USELESS_NAMES: &[&str] =
-    &["new", "default", "option", "some", "none", "ok", "err", "str", "string", "from", "into"];
-
-const USELESS_NAME_PREFIXES: &[&str] = &["from_", "with_", "into_"];
+    &["new", "default", "option", "some", "none", "ok", "err", "str", "string"];
 
 /// Generic types replaced by their first argument
 ///
@@ -191,10 +189,6 @@ fn normalize(name: &str) -> Option<String> {
         return None;
     }
 
-    if USELESS_NAME_PREFIXES.iter().any(|prefix| name.starts_with(prefix)) {
-        return None;
-    }
-
     if !is_valid_name(&name) {
         return None;
     }
@@ -837,92 +831,4 @@ fn foo<T>(some_struct: S<T>) { $0some_struct.some_field$0 }
             "some_field",
         );
     }
-
-    #[test]
-    fn from_and_to_func() {
-        check(
-            r#"
-//- minicore: from
-struct Foo;
-struct Bar;
-
-impl From<Foo> for Bar {
-    fn from(_: Foo) -> Self {
-        Bar;
-    }
-}
-
-fn f(_: Bar) {}
-
-fn main() {
-    let foo = Foo {};
-    f($0Bar::from(foo)$0);
-}
-"#,
-            "bar",
-        );
-
-        check(
-            r#"
-//- minicore: from
-struct Foo;
-struct Bar;
-
-impl From<Foo> for Bar {
-    fn from(_: Foo) -> Self {
-        Bar;
-    }
-}
-
-fn f(_: Bar) {}
-
-fn main() {
-    let foo = Foo {};
-    f($0Into::<Bar>::into(foo)$0);
-}
-"#,
-            "bar",
-        );
-    }
-
-    #[test]
-    fn useless_name_prefix() {
-        check(
-            r#"
-struct Foo;
-struct Bar;
-
-impl Bar {
-    fn from_foo(_: Foo) -> Self {
-        Foo {}
-    }
-}
-
-fn main() {
-    let foo = Foo {};
-    let _ = $0Bar::from_foo(foo)$0;
-}
-"#,
-            "bar",
-        );
-
-        check(
-            r#"
-struct Foo;
-struct Bar;
-
-impl Bar {
-    fn with_foo(_: Foo) -> Self {
-        Bar {}
-    }
-}
-
-fn main() {
-    let foo = Foo {};
-    let _ = $0Bar::with_foo(foo)$0;
-}
-"#,
-            "bar",
-        );
-    }
 }


Test patch:
diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs
index f4528dd754..f2e5d24fcc 100644
--- a/crates/ide/src/hover/tests.rs
+++ b/crates/ide/src/hover/tests.rs
@@ -8742,3 +8742,181 @@ fn foo() {
         "#]],
     );
 }
+
+#[test]
+fn test_hover_function_with_pat_param() {
+    check(
+        r#"fn test_1$0((start_range, end_range): (u32, u32), a: i32) {}"#,
+        expect![[r#"
+            *test_1*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_1((start_range, end_range): (u32, u32), a: i32)
+            ```
+        "#]],
+    );
+
+    // Test case with tuple pattern and mutable parameters
+    check(
+        r#"fn test_2$0((mut x, y): (i32, i32)) {}"#,
+        expect![[r#"
+            *test_2*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_2((mut x, y): (i32, i32))
+            ```
+        "#]],
+    );
+
+    // Test case with a pattern in a reference type
+    check(
+        r#"fn test_3$0(&(a, b): &(i32, i32)) {}"#,
+        expect![[r#"
+            *test_3*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_3(&(a, b): &(i32, i32))
+            ```
+        "#]],
+    );
+
+    // Test case with complex pattern (struct destructuring)
+    check(
+        r#"struct Point { x: i32, y: i32 } fn test_4$0(Point { x, y }: Point) {}"#,
+        expect![[r#"
+            *test_4*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_4(Point { x, y }: Point)
+            ```
+        "#]],
+    );
+
+    // Test case with a nested pattern
+    check(
+        r#"fn test_5$0(((a, b), c): ((i32, i32), i32)) {}"#,
+        expect![[r#"
+            *test_5*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_5(((a, b), c): ((i32, i32), i32))
+            ```
+        "#]],
+    );
+
+    // Test case with an unused variable in the pattern
+    check(
+        r#"fn test_6$0((_, y): (i32, i64)) {}"#,
+        expect![[r#"
+            *test_6*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_6((_, y): (i32, i64))
+            ```
+        "#]],
+    );
+
+    // Test case with a complex pattern involving both tuple and struct
+    check(
+        r#"struct Foo { a: i32, b: i32 } fn test_7$0((x, Foo { a, b }): (i32, Foo)) {}"#,
+        expect![[r#"
+            *test_7*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_7((x, Foo { a, b }): (i32, Foo))
+            ```
+        "#]],
+    );
+
+    // Test case with Enum and Or pattern
+    check(
+        r#"enum MyEnum { A(i32), B(i32) } fn test_8$0((MyEnum::A(x) | MyEnum::B(x)): MyEnum) {}"#,
+        expect![[r#"
+            *test_8*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_8((MyEnum::A(x) | MyEnum::B(x)): MyEnum)
+            ```
+        "#]],
+    );
+
+    // Test case with a pattern as a function parameter
+    check(
+        r#"struct Foo { a: i32, b: i32 } fn test_9$0(Foo { a, b }: Foo) {}"#,
+        expect![[r#"
+            *test_9*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_9(Foo { a, b }: Foo)
+            ```
+        "#]],
+    );
+
+    // Test case with a pattern as a function parameter with a different name
+    check(
+        r#"struct Foo { a: i32, b: i32 } fn test_10$0(Foo { a, b: b1 }: Foo) {}"#,
+        expect![[r#"
+            *test_10*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_10(Foo { a, b: b1 }: Foo)
+            ```
+        "#]],
+    );
+
+    // Test case with a pattern as a function parameter with annotations
+    check(
+        r#"struct Foo { a: i32, b: i32 } fn test_10$0(Foo { a, b: mut b }: Foo) {}"#,
+        expect![[r#"
+            *test_10*
+
+            ```rust
+            test
+            ```
+
+            ```rust
+            fn test_10(Foo { a, b: mut b }: Foo)
+            ```
+        "#]],
+    );
+}