From e92b519bdfbd2a149a46745ad2ecffdd0e91f3f1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 29 Oct 2024 11:27:00 -0300 Subject: [PATCH] feat: better LSP hover for functions (#6376) --- tooling/lsp/src/requests/hover.rs | 179 ++++++++++++++++-- .../test_programs/workspace/two/src/lib.nr | 30 ++- 2 files changed, 193 insertions(+), 16 deletions(-) diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index 5087955ea77..64974a42133 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/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 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()"#, + pub fn function_one()"#, ) .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 Foo + fn new() -> Foo" + )); + } + + #[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 Bar for Foo + pub fn bar_stuff(self)" + )); + } } diff --git a/tooling/lsp/test_programs/workspace/two/src/lib.nr b/tooling/lsp/test_programs/workspace/two/src/lib.nr index c6b516d88ab..2dec902f327 100644 --- a/tooling/lsp/test_programs/workspace/two/src/lib.nr +++ b/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() {} } -} \ No newline at end of file +} + +struct Foo {} + +impl Foo { + fn new() -> Self { + Foo {} + } +} + +fn new_foo() -> Foo { + Foo::new() +} + +trait Bar { + fn bar_stuff(self); +} + +impl Bar for Foo { + fn bar_stuff(self) {} +} + +fn bar_stuff() { + let foo = Foo::new(); + foo.bar_stuff(); +} +