feat: better LSP hover for functions

tooling/lsp/src/requests/hover.rs

@@ -4,7 +4,7 @@ use async_lsp::ResponseError;
 use fm::{FileMap, PathString};
 use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind};
 use noirc_frontend::{
-    ast::Visibility,
+    ast::{ItemVisibility, Visibility},
     elaborator::types::try_eval_array_length_id,
     hir::def_map::ModuleId,
     hir_def::{
@@ -13,9 +13,9 @@ use noirc_frontend::{
         traits::Trait,
     },
     node_interner::{
-        DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId,
+        DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, NodeInterner, ReferenceId,
+        StructId, TraitId, TypeAliasId,
     },
-    node_interner::{NodeInterner, StructId},
     Generics, Shared, StructType, Type, TypeAlias, TypeBinding, TypeVariable,
 };
 
@@ -300,26 +300,113 @@ fn get_exprs_global_value(interner: &NodeInterner, exprs: &[ExprId]) -> Option<S
 
 fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String {
     let func_meta = args.interner.function_meta(&id);
+    let func_modifiers = args.interner.function_modifiers(&id);
+
     let func_name_definition_id = args.interner.definition(func_meta.name.id);
 
     let mut string = String::new();
     let formatted_parent_module =
         format_parent_module(ReferenceId::Function(id), args, &mut string);
-    let formatted_parent_struct = if let Some(struct_id) = func_meta.struct_id {
+
+    let formatted_parent_type = if let Some(trait_impl_id) = func_meta.trait_impl {
+        let trait_impl = args.interner.get_trait_implementation(trait_impl_id);
+        let trait_impl = trait_impl.borrow();
+        let trait_ = args.interner.get_trait(trait_impl.trait_id);
+
+        let generics: Vec<_> =
+            trait_impl
+                .trait_generics
+                .iter()
+                .filter_map(|generic| {
+                    if let Type::NamedGeneric(_, name) = generic {
+                        Some(name)
+                    } else {
+                        None
+                    }
+                })
+                .collect();
+
+        string.push('\n');
+        string.push_str("    impl");
+        if !generics.is_empty() {
+            string.push('<');
+            for (index, generic) in generics.into_iter().enumerate() {
+                if index > 0 {
+                    string.push_str(", ");
+                }
+                string.push_str(generic);
+            }
+            string.push('>');
+        }
+
+        string.push(' ');
+        string.push_str(&trait_.name.0.contents);
+        if !trait_impl.trait_generics.is_empty() {
+            string.push('<');
+            for (index, generic) in trait_impl.trait_generics.iter().enumerate() {
+                if index > 0 {
+                    string.push_str(", ");
+                }
+                string.push_str(&generic.to_string());
+            }
+            string.push('>');
+        }
+
+        string.push_str(" for ");
+        string.push_str(&trait_impl.typ.to_string());
+
+        true
+    } else if let Some(trait_id) = func_meta.trait_id {
+        let trait_ = args.interner.get_trait(trait_id);
+        string.push('\n');
+        string.push_str("    trait ");
+        string.push_str(&trait_.name.0.contents);
+        format_generics(&trait_.generics, &mut string);
+
+        true
+    } else if let Some(struct_id) = func_meta.struct_id {
         let struct_type = args.interner.get_struct(struct_id);
         let struct_type = struct_type.borrow();
         if formatted_parent_module {
             string.push_str("::");
         }
         string.push_str(&struct_type.name.0.contents);
+        string.push('\n');
+        string.push_str("    ");
+        string.push_str("impl");
+
+        let impl_generics: Vec<_> = func_meta
+            .all_generics
+            .iter()
+            .take(func_meta.all_generics.len() - func_meta.direct_generics.len())
+            .cloned()
+            .collect();
+        format_generics(&impl_generics, &mut string);
+
+        string.push(' ');
+        string.push_str(&struct_type.name.0.contents);
+        format_generic_names(&impl_generics, &mut string);
+
         true
     } else {
         false
     };
-    if formatted_parent_module || formatted_parent_struct {
+    if formatted_parent_module || formatted_parent_type {
         string.push('\n');
     }
     string.push_str("    ");
+
+    if func_modifiers.visibility != ItemVisibility::Private {
+        string.push_str(&func_modifiers.visibility.to_string());
+        string.push(' ');
+    }
+    if func_modifiers.is_unconstrained {
+        string.push_str("unconstrained ");
+    }
+    if func_modifiers.is_comptime {
+        string.push_str("comptime ");
+    }
+
     string.push_str("fn ");
     string.push_str(&func_name_definition_id.name);
     format_generics(&func_meta.direct_generics, &mut string);
@@ -411,21 +498,55 @@ fn format_local(id: DefinitionId, args: &ProcessRequestCallbackArgs) -> String {
     string
 }
 
-/// Some doc comments
 fn format_generics(generics: &Generics, string: &mut String) {
+    format_generics_impl(
+        generics, false, // only show names
+        string,
+    );
+}
+
+fn format_generic_names(generics: &Generics, string: &mut String) {
+    format_generics_impl(
+        generics, true, // only show names
+        string,
+    );
+}
+
+fn format_generics_impl(generics: &Generics, only_show_names: bool, string: &mut String) {
     if generics.is_empty() {
         return;
     }
 
     string.push('<');
     for (index, generic) in generics.iter().enumerate() {
-        string.push_str(&generic.name);
-        if index != generics.len() - 1 {
+        if index > 0 {
             string.push_str(", ");
         }
+
+        if only_show_names {
+            string.push_str(&generic.name);
+        } else {
+            match generic.kind() {
+                noirc_frontend::Kind::Any | noirc_frontend::Kind::Normal => {
+                    string.push_str(&generic.name);
+                }
+                noirc_frontend::Kind::IntegerOrField | noirc_frontend::Kind::Integer => {
+                    string.push_str("let ");
+                    string.push_str(&generic.name);
+                    string.push_str(": u32");
+                }
+                noirc_frontend::Kind::Numeric(typ) => {
+                    string.push_str("let ");
+                    string.push_str(&generic.name);
+                    string.push_str(": ");
+                    string.push_str(&typ.to_string());
+                }
+            }
+        }
     }
     string.push('>');
 }
+
 fn format_pattern(pattern: &HirPattern, interner: &NodeInterner, string: &mut String) {
     match pattern {
         HirPattern::Identifier(ident) => {
@@ -647,6 +768,11 @@ mod hover_tests {
     use tokio::test;
 
     async fn assert_hover(directory: &str, file: &str, position: Position, expected_text: &str) {
+        let hover_text = get_hover_text(directory, file, position).await;
+        assert_eq!(hover_text, expected_text);
+    }
+
+    async fn get_hover_text(directory: &str, file: &str, position: Position) -> String {
         let (mut state, noir_text_document) = test_utils::init_lsp_server(directory).await;
 
         // noir_text_document is always `src/main.nr` in the workspace directory, so let's go to the workspace dir
@@ -673,7 +799,7 @@ mod hover_tests {
             panic!("Expected hover contents to be Markup");
         };
 
-        assert_eq!(markup.value, expected_text);
+        markup.value
     }
 
     #[test]
@@ -759,7 +885,7 @@ mod hover_tests {
             "two/src/lib.nr",
             Position { line: 3, character: 4 },
             r#"    one
-    fn function_one<A, B>()"#,
+    pub fn function_one<A, B>()"#,
         )
         .await;
     }
@@ -771,7 +897,7 @@ mod hover_tests {
             "two/src/lib.nr",
             Position { line: 2, character: 7 },
             r#"    two
-    fn function_two()"#,
+    pub fn function_two()"#,
         )
         .await;
     }
@@ -783,6 +909,7 @@ mod hover_tests {
             "two/src/lib.nr",
             Position { line: 20, character: 6 },
             r#"    one::subone::SubOneStruct
+    impl SubOneStruct
     fn foo(self, x: i32, y: i32) -> Field"#,
         )
         .await;
@@ -892,7 +1019,7 @@ mod hover_tests {
         assert_hover(
             "workspace",
             "two/src/lib.nr",
-            Position { line: 43, character: 4 },
+            Position { line: 42, character: 4 },
             r#"    two
     mod other"#,
         )
@@ -904,7 +1031,7 @@ mod hover_tests {
         assert_hover(
             "workspace",
             "two/src/lib.nr",
-            Position { line: 44, character: 11 },
+            Position { line: 43, character: 11 },
             r#"    two
     mod other"#,
         )
@@ -955,8 +1082,32 @@ mod hover_tests {
             "workspace",
             "two/src/lib.nr",
             Position { line: 54, character: 2 },
-            "    two\n    fn attr(_: FunctionDefinition) -> Quoted",
+            "    two\n    comptime fn attr(_: FunctionDefinition) -> Quoted",
         )
         .await;
     }
+
+    #[test]
+    async fn hover_on_generic_struct_function() {
+        let hover_text =
+            get_hover_text("workspace", "two/src/lib.nr", Position { line: 70, character: 11 })
+                .await;
+        assert!(hover_text.starts_with(
+            "    two::Foo
+    impl<U> Foo<U>
+    fn new() -> Foo<U>"
+        ));
+    }
+
+    #[test]
+    async fn hover_on_trait_impl_function_call() {
+        let hover_text =
+            get_hover_text("workspace", "two/src/lib.nr", Position { line: 83, character: 16 })
+                .await;
+        assert!(hover_text.starts_with(
+            "    two
+    impl<A> Bar<A, i32> for Foo<A>
+    pub fn bar_stuff(self)"
+        ));
+    }
 }

tooling/lsp/test_programs/workspace/two/src/lib.nr

@@ -41,8 +41,8 @@ fn use_impl_method() {
 }
 
 mod other;
-use other::another_function;
 use crate::other::other_function;
+use other::another_function;
 
 use one::subone::GenericStruct;
 
@@ -57,4 +57,30 @@ pub fn foo() {}
 
 comptime fn attr(_: FunctionDefinition) -> Quoted {
     quote { pub fn hello() {} }
-}
+}
+
+struct Foo<T> {}
+
+impl<U> Foo<U> {
+    fn new() -> Self {
+        Foo {}
+    }
+}
+
+fn new_foo() -> Foo<i32> {
+    Foo::new()
+}
+
+trait Bar<T, U> {
+    fn bar_stuff(self);
+}
+
+impl<A> Bar<A, i32> for Foo<A> {
+    fn bar_stuff(self) {}
+}
+
+fn bar_stuff() {
+    let foo = Foo::new();
+    foo.bar_stuff();
+}
+