Skip to content

Commit

Permalink
feat: better LSP hover for functions (#6376)
Browse files Browse the repository at this point in the history
  • Loading branch information
@asterite
asterite authored Oct 29, 2024
1 parent c9ff9a3 commit e92b519
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 16 deletions.
179 changes: 165 additions & 14 deletions tooling/lsp/src/requests/hover.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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,
};

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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
Expand All @@ -673,7 +799,7 @@ mod hover_tests {
panic!("Expected hover contents to be Markup");
};

assert_eq!(markup.value, expected_text);
markup.value
}

#[test]
Expand Down Expand Up @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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"#,
)
Expand All @@ -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"#,
)
Expand Down Expand Up @@ -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)"
));
}
}
30 changes: 28 additions & 2 deletions tooling/lsp/test_programs/workspace/two/src/lib.nr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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();
}

0 comments on commit e92b519

Please sign in to comment.