Merge #11481

11481: Display parameter names when hovering over a function pointer r=Veykril a=Vannevelj

Implements #11474

The idea is pretty straightforward: previously we constructed the hover based on just the parameter types, now we pass in the parameter names as well. I went for a quick-hit approach here but I expect someone will be able to point me to a better way of resolving the identifier.

I haven't figured out yet how to actually run my rust-analyzer locally so I can see it in action but the unit test indicates it should work.

Co-authored-by: Jeroen Vannevel <jer_vannevel@outlook.com>

5 files changed, 105 insertions, 24 deletions

diff --git a/crates/hir_def/src/item_tree/pretty.rs b/crates/hir_def/src/item_tree/pretty.rs
index eaaff5a21f7..0df6e97dd46 100644
--- a/crates/hir_def/src/item_tree/pretty.rs
+++ b/crates/hir_def/src/item_tree/pretty.rs
@@ -493,14 +493,14 @@ impl<'a> Printer<'a> {
                 w!(self, "]");
             }
             TypeRef::Fn(args_and_ret, varargs) => {
-                let (ret, args) =
+                let ((_, return_type), args) =
                     args_and_ret.split_last().expect("TypeRef::Fn is missing return type");
                 w!(self, "fn(");
-                for (i, arg) in args.iter().enumerate() {
+                for (i, (_, typeref)) in args.iter().enumerate() {
                     if i != 0 {
                         w!(self, ", ");
                     }
-                    self.print_type_ref(arg);
+                    self.print_type_ref(&typeref);
                 }
                 if *varargs {
                     if !args.is_empty() {
@@ -509,7 +509,7 @@ impl<'a> Printer<'a> {
                     w!(self, "...");
                 }
                 w!(self, ") -> ");
-                self.print_type_ref(ret);
+                self.print_type_ref(&return_type);
             }
             TypeRef::Macro(_ast_id) => {
                 w!(self, "<macro>");
diff --git a/crates/hir_def/src/type_ref.rs b/crates/hir_def/src/type_ref.rs
index cfc69feccc6..ee8ef6caa30 100644
--- a/crates/hir_def/src/type_ref.rs
+++ b/crates/hir_def/src/type_ref.rs
@@ -1,9 +1,12 @@
 //! HIR for references to types. Paths in these are not yet resolved. They can
 //! be directly created from an ast::TypeRef, without further queries.
 
-use hir_expand::{name::Name, AstId, InFile};
+use hir_expand::{
+    name::{AsName, Name},
+    AstId, InFile,
+};
 use std::convert::TryInto;
-use syntax::ast;
+use syntax::ast::{self, HasName};
 
 use crate::{body::LowerCtx, intern::Interned, path::Path};
 
@@ -89,7 +92,7 @@ pub enum TypeRef {
     Array(Box<TypeRef>, ConstScalar),
     Slice(Box<TypeRef>),
     /// A fn pointer. Last element of the vector is the return type.
-    Fn(Vec<TypeRef>, bool /*varargs*/),
+    Fn(Vec<(Option<Name>, TypeRef)>, bool /*varargs*/),
     // For
     ImplTrait(Vec<Interned<TypeBound>>),
     DynTrait(Vec<Interned<TypeBound>>),
@@ -188,11 +191,22 @@ impl TypeRef {
                         is_varargs = param.dotdotdot_token().is_some();
                     }
 
-                    pl.params().map(|p| p.ty()).map(|it| TypeRef::from_ast_opt(ctx, it)).collect()
+                    pl.params()
+                        .map(|it| {
+                            let type_ref = TypeRef::from_ast_opt(ctx, it.ty());
+                            let name = match it.pat() {
+                                Some(ast::Pat::IdentPat(it)) => Some(
+                                    it.name().map(|nr| nr.as_name()).unwrap_or_else(Name::missing),
+                                ),
+                                _ => None,
+                            };
+                            (name, type_ref)
+                        })
+                        .collect()
                 } else {
                     Vec::new()
                 };
-                params.push(ret_ty);
+                params.push((None, ret_ty));
                 TypeRef::Fn(params, is_varargs)
             }
             // for types are close enough for our purposes to the inner type for now...
@@ -230,9 +244,10 @@ impl TypeRef {
         fn go(type_ref: &TypeRef, f: &mut impl FnMut(&TypeRef)) {
             f(type_ref);
             match type_ref {
-                TypeRef::Fn(types, _) | TypeRef::Tuple(types) => {
-                    types.iter().for_each(|t| go(t, f))
+                TypeRef::Fn(params, _) => {
+                    params.iter().for_each(|(_, param_type)| go(&param_type, f))
                 }
+                TypeRef::Tuple(types) => types.iter().for_each(|t| go(t, f)),
                 TypeRef::RawPtr(type_ref, _)
                 | TypeRef::Reference(type_ref, ..)
                 | TypeRef::Array(type_ref, _)
diff --git a/crates/hir_ty/src/display.rs b/crates/hir_ty/src/display.rs
index f02f4ac0247..0e75ddeabcd 100644
--- a/crates/hir_ty/src/display.rs
+++ b/crates/hir_ty/src/display.rs
@@ -1094,20 +1094,32 @@ impl HirDisplay for TypeRef {
                 inner.hir_fmt(f)?;
                 write!(f, "]")?;
             }
-            TypeRef::Fn(tys, is_varargs) => {
+            TypeRef::Fn(parameters, is_varargs) => {
                 // FIXME: Function pointer qualifiers.
                 write!(f, "fn(")?;
-                f.write_joined(&tys[..tys.len() - 1], ", ")?;
-                if *is_varargs {
-                    write!(f, "{}...", if tys.len() == 1 { "" } else { ", " })?;
-                }
-                write!(f, ")")?;
-                let ret_ty = tys.last().unwrap();
-                match ret_ty {
-                    TypeRef::Tuple(tup) if tup.is_empty() => {}
-                    _ => {
-                        write!(f, " -> ")?;
-                        ret_ty.hir_fmt(f)?;
+                if let Some(((_, return_type), function_parameters)) = parameters.split_last() {
+                    for index in 0..function_parameters.len() {
+                        let (param_name, param_type) = &function_parameters[index];
+                        if let Some(name) = param_name {
+                            write!(f, "{}: ", name)?;
+                        }
+
+                        param_type.hir_fmt(f)?;
+
+                        if index != function_parameters.len() - 1 {
+                            write!(f, ", ")?;
+                        }
+                    }
+                    if *is_varargs {
+                        write!(f, "{}...", if parameters.len() == 1 { "" } else { ", " })?;
+                    }
+                    write!(f, ")")?;
+                    match &return_type {
+                        TypeRef::Tuple(tup) if tup.is_empty() => {}
+                        _ => {
+                            write!(f, " -> ")?;
+                            return_type.hir_fmt(f)?;
+                        }
                     }
                 }
             }
diff --git a/crates/hir_ty/src/lower.rs b/crates/hir_ty/src/lower.rs
index 55b1a67ea7b..a140dd4057c 100644
--- a/crates/hir_ty/src/lower.rs
+++ b/crates/hir_ty/src/lower.rs
@@ -201,7 +201,7 @@ impl<'a> TyLoweringContext<'a> {
             TypeRef::Placeholder => TyKind::Error.intern(Interner),
             TypeRef::Fn(params, is_varargs) => {
                 let substs = self.with_shifted_in(DebruijnIndex::ONE, |ctx| {
-                    Substitution::from_iter(Interner, params.iter().map(|tr| ctx.lower_ty(tr)))
+                    Substitution::from_iter(Interner, params.iter().map(|(_, tr)| ctx.lower_ty(tr)))
                 });
                 TyKind::Function(FnPointer {
                     num_binders: 0, // FIXME lower `for<'a> fn()` correctly
diff --git a/crates/ide/src/hover/tests.rs b/crates/ide/src/hover/tests.rs
index ed76c84ab47..c5c531c30b1 100644
--- a/crates/ide/src/hover/tests.rs
+++ b/crates/ide/src/hover/tests.rs
@@ -1311,6 +1311,60 @@ fn test_hover_function_show_qualifiers() {
 }
 
 #[test]
+fn test_hover_function_show_types() {
+    check(
+        r#"fn foo$0(a: i32, b:i32) -> i32 { 0 }"#,
+        expect![[r#"
+                *foo*
+
+                ```rust
+                test
+                ```
+
+                ```rust
+                fn foo(a: i32, b: i32) -> i32
+                ```
+            "#]],
+    );
+}
+
+#[test]
+fn test_hover_function_pointer_show_identifiers() {
+    check(
+        r#"type foo$0 = fn(a: i32, b: i32) -> i32;"#,
+        expect![[r#"
+                *foo*
+
+                ```rust
+                test
+                ```
+
+                ```rust
+                type foo = fn(a: i32, b: i32) -> i32
+                ```
+            "#]],
+    );
+}
+
+#[test]
+fn test_hover_function_pointer_no_identifier() {
+    check(
+        r#"type foo$0 = fn(i32, _: i32) -> i32;"#,
+        expect![[r#"
+                *foo*
+
+                ```rust
+                test
+                ```
+
+                ```rust
+                type foo = fn(i32, i32) -> i32
+                ```
+            "#]],
+    );
+}
+
+#[test]
 fn test_hover_trait_show_qualifiers() {
     check_actions(
         r"unsafe trait foo$0() {}",