From 81d20eb853045530636f2a9be7d17e32bdee1926 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 6 Aug 2024 11:49:08 -0300 Subject: [PATCH 1/3] feat: LSP closing brace hints --- tooling/lsp/src/requests/inlay_hint.rs | 178 +++++++++++++++++- tooling/lsp/src/requests/mod.rs | 28 +++ .../lsp/test_programs/inlay_hints/src/main.nr | 16 ++ 3 files changed, 213 insertions(+), 9 deletions(-) diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 2ed441c623e..71d495069ab 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -86,25 +86,73 @@ impl<'a> InlayHintCollector<'a> { } match &item.kind { - ItemKind::Function(noir_function) => self.collect_in_noir_function(noir_function), + ItemKind::Function(noir_function) => { + self.collect_in_noir_function(noir_function, item.span) + } ItemKind::Trait(noir_trait) => { for item in &noir_trait.items { self.collect_in_trait_item(item); } } ItemKind::TraitImpl(noir_trait_impl) => { - for item in &noir_trait_impl.items { - self.collect_in_trait_impl_item(item); + for trait_impl_item in &noir_trait_impl.items { + self.collect_in_trait_impl_item(trait_impl_item, item.span); + } + + if self.options.closing_brace_hints.enabled { + if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, item.span) + { + let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; + if lines >= self.options.closing_brace_hints.min_lines { + self.push_text_hint( + lsp_location.range.end, + format!( + " impl {} for {}", + noir_trait_impl.trait_name, noir_trait_impl.object_type + ), + ); + } + } } } ItemKind::Impl(type_impl) => { - for (noir_function, _) in &type_impl.methods { - self.collect_in_noir_function(noir_function); + for (noir_function, span) in &type_impl.methods { + self.collect_in_noir_function(noir_function, *span); + } + + if self.options.closing_brace_hints.enabled { + if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, item.span) + { + let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; + if lines >= self.options.closing_brace_hints.min_lines { + self.push_text_hint( + lsp_location.range.end, + format!(" impl {}", type_impl.object_type), + ); + } + } } } ItemKind::Global(let_statement) => self.collect_in_let_statement(let_statement), ItemKind::Submodules(parsed_submodule) => { self.collect_in_parsed_module(&parsed_submodule.contents); + + if self.options.closing_brace_hints.enabled { + if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, item.span) + { + let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; + if lines >= self.options.closing_brace_hints.min_lines { + self.push_text_hint( + lsp_location.range.end, + if parsed_submodule.is_contract { + format!(" contract {}", parsed_submodule.name) + } else { + format!(" mod {}", parsed_submodule.name) + }, + ); + } + } + } } ItemKind::ModuleDecl(_) => (), ItemKind::Import(_) => (), @@ -129,9 +177,11 @@ impl<'a> InlayHintCollector<'a> { } } - fn collect_in_trait_impl_item(&mut self, item: &TraitImplItem) { + fn collect_in_trait_impl_item(&mut self, item: &TraitImplItem, span: Span) { match item { - TraitImplItem::Function(noir_function) => self.collect_in_noir_function(noir_function), + TraitImplItem::Function(noir_function) => { + self.collect_in_noir_function(noir_function, span) + } TraitImplItem::Constant(_name, _typ, default_value) => { self.collect_in_expression(default_value); } @@ -139,8 +189,20 @@ impl<'a> InlayHintCollector<'a> { } } - fn collect_in_noir_function(&mut self, noir_function: &NoirFunction) { + fn collect_in_noir_function(&mut self, noir_function: &NoirFunction, span: Span) { self.collect_in_block_expression(&noir_function.def.body); + + if self.options.closing_brace_hints.enabled { + if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, span) { + let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; + if lines >= self.options.closing_brace_hints.min_lines { + self.push_text_hint( + lsp_location.range.end, + format!(" fn {}", noir_function.def.name), + ); + } + } + } } fn collect_in_let_statement(&mut self, let_statement: &LetStatement) { @@ -708,7 +770,7 @@ fn character_to_line_offset(line: &str, character: u32) -> Option { #[cfg(test)] mod inlay_hints_tests { use crate::{ - requests::{ParameterHintsOptions, TypeHintsOptions}, + requests::{ClosingBraceHintsOptions, ParameterHintsOptions, TypeHintsOptions}, test_utils, }; @@ -744,6 +806,7 @@ mod inlay_hints_tests { InlayHintsOptions { type_hints: TypeHintsOptions { enabled: false }, parameter_hints: ParameterHintsOptions { enabled: false }, + closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 25 }, } } @@ -751,6 +814,7 @@ mod inlay_hints_tests { InlayHintsOptions { type_hints: TypeHintsOptions { enabled: true }, parameter_hints: ParameterHintsOptions { enabled: false }, + closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 25 }, } } @@ -758,6 +822,15 @@ mod inlay_hints_tests { InlayHintsOptions { type_hints: TypeHintsOptions { enabled: false }, parameter_hints: ParameterHintsOptions { enabled: true }, + closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 25 }, + } + } + + fn closing_braces_hints(min_lines: u32) -> InlayHintsOptions { + InlayHintsOptions { + type_hints: TypeHintsOptions { enabled: false }, + parameter_hints: ParameterHintsOptions { enabled: false }, + closing_brace_hints: ClosingBraceHintsOptions { enabled: true, min_lines: min_lines }, } } @@ -1025,4 +1098,91 @@ mod inlay_hints_tests { let inlay_hints = get_inlay_hints(89, 92, parameter_hints()).await; assert!(inlay_hints.is_empty()); } + + #[test] + async fn test_does_not_show_closing_brace_inlay_hints_if_disabled() { + let inlay_hints = get_inlay_hints(41, 46, no_hints()).await; + assert!(inlay_hints.is_empty()); + } + + #[test] + async fn test_does_not_show_closing_brace_inlay_hints_if_enabled_but_not_lines() { + let inlay_hints = get_inlay_hints(41, 46, closing_braces_hints(6)).await; + assert!(inlay_hints.is_empty()); + } + + #[test] + async fn test_shows_closing_brace_inlay_hints_for_a_function() { + let inlay_hints = get_inlay_hints(41, 46, closing_braces_hints(5)).await; + assert_eq!(inlay_hints.len(), 1); + + let inlay_hint = &inlay_hints[0]; + assert_eq!(inlay_hint.position, Position { line: 45, character: 1 }); + assert_eq!(inlay_hint.text_edits, None); + if let InlayHintLabel::String(label) = &inlay_hint.label { + assert_eq!(label, " fn call_where_name_matches"); + } else { + panic!("Expected InlayHintLabel::String, got {:?}", inlay_hint.label); + } + } + + #[test] + async fn test_shows_closing_brace_inlay_hints_for_impl() { + let inlay_hints = get_inlay_hints(32, 34, closing_braces_hints(2)).await; + assert_eq!(inlay_hints.len(), 1); + + let inlay_hint = &inlay_hints[0]; + assert_eq!(inlay_hint.position, Position { line: 34, character: 1 }); + assert_eq!(inlay_hint.text_edits, None); + if let InlayHintLabel::String(label) = &inlay_hint.label { + assert_eq!(label, " impl SomeStruct"); + } else { + panic!("Expected InlayHintLabel::String, got {:?}", inlay_hint.label); + } + } + + #[test] + async fn test_shows_closing_brace_inlay_hints_for_trait_impl() { + let inlay_hints = get_inlay_hints(111, 113, closing_braces_hints(2)).await; + assert_eq!(inlay_hints.len(), 1); + + let inlay_hint = &inlay_hints[0]; + assert_eq!(inlay_hint.position, Position { line: 113, character: 1 }); + assert_eq!(inlay_hint.text_edits, None); + if let InlayHintLabel::String(label) = &inlay_hint.label { + assert_eq!(label, " impl SomeTrait for SomeStruct"); + } else { + panic!("Expected InlayHintLabel::String, got {:?}", inlay_hint.label); + } + } + + #[test] + async fn test_shows_closing_brace_inlay_hints_for_module() { + let inlay_hints = get_inlay_hints(115, 117, closing_braces_hints(2)).await; + assert_eq!(inlay_hints.len(), 1); + + let inlay_hint = &inlay_hints[0]; + assert_eq!(inlay_hint.position, Position { line: 117, character: 1 }); + assert_eq!(inlay_hint.text_edits, None); + if let InlayHintLabel::String(label) = &inlay_hint.label { + assert_eq!(label, " mod some_module"); + } else { + panic!("Expected InlayHintLabel::String, got {:?}", inlay_hint.label); + } + } + + #[test] + async fn test_shows_closing_brace_inlay_hints_for_contract() { + let inlay_hints = get_inlay_hints(119, 121, closing_braces_hints(2)).await; + assert_eq!(inlay_hints.len(), 1); + + let inlay_hint = &inlay_hints[0]; + assert_eq!(inlay_hint.position, Position { line: 121, character: 1 }); + assert_eq!(inlay_hint.text_edits, None); + if let InlayHintLabel::String(label) = &inlay_hint.label { + assert_eq!(label, " contract some_contract"); + } else { + panic!("Expected InlayHintLabel::String, got {:?}", inlay_hint.label); + } + } } diff --git a/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 09794574709..aef5d782c46 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -78,6 +78,9 @@ pub(crate) struct InlayHintsOptions { #[serde(rename = "parameterHints", default = "default_parameter_hints")] pub(crate) parameter_hints: ParameterHintsOptions, + + #[serde(rename = "closingBraceHints", default = "default_closing_brace_hints")] + pub(crate) closing_brace_hints: ClosingBraceHintsOptions, } #[derive(Debug, Deserialize, Serialize, Copy, Clone)] @@ -92,6 +95,15 @@ pub(crate) struct ParameterHintsOptions { pub(crate) enabled: bool, } +#[derive(Debug, Deserialize, Serialize, Copy, Clone)] +pub(crate) struct ClosingBraceHintsOptions { + #[serde(rename = "enabled", default = "default_closing_brace_hints_enabled")] + pub(crate) enabled: bool, + + #[serde(rename = "minLines", default = "default_closing_brace_min_lines")] + pub(crate) min_lines: u32, +} + fn default_enable_code_lens() -> bool { true } @@ -104,6 +116,7 @@ fn default_inlay_hints() -> InlayHintsOptions { InlayHintsOptions { type_hints: default_type_hints(), parameter_hints: default_parameter_hints(), + closing_brace_hints: default_closing_brace_hints(), } } @@ -123,6 +136,21 @@ fn default_parameter_hints_enabled() -> bool { true } +fn default_closing_brace_hints() -> ClosingBraceHintsOptions { + ClosingBraceHintsOptions { + enabled: default_closing_brace_hints_enabled(), + min_lines: default_closing_brace_min_lines(), + } +} + +fn default_closing_brace_hints_enabled() -> bool { + true +} + +fn default_closing_brace_min_lines() -> u32 { + 25 +} + impl Default for LspInitializationOptions { fn default() -> Self { Self { diff --git a/tooling/lsp/test_programs/inlay_hints/src/main.nr b/tooling/lsp/test_programs/inlay_hints/src/main.nr index b2bbed2b1e5..46a6d3bc558 100644 --- a/tooling/lsp/test_programs/inlay_hints/src/main.nr +++ b/tooling/lsp/test_programs/inlay_hints/src/main.nr @@ -104,3 +104,19 @@ fn hint_on_lambda_parameter() { let value: i32 = 1; let _: i32 = some_map(value, |x| x + 1); } + +trait SomeTrait { + +} + +impl SomeTrait for SomeStruct { + +} + +mod some_module { + +} + +contract some_contract { + +} From ccfb2249db08fe2c3ffc570f514aae9d9df8f8f5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 6 Aug 2024 12:01:49 -0300 Subject: [PATCH 2/3] Refactor --- tooling/lsp/src/requests/inlay_hint.rs | 83 ++++++++++---------------- 1 file changed, 30 insertions(+), 53 deletions(-) diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 71d495069ab..3eb6034b9a0 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -99,60 +99,33 @@ impl<'a> InlayHintCollector<'a> { self.collect_in_trait_impl_item(trait_impl_item, item.span); } - if self.options.closing_brace_hints.enabled { - if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, item.span) - { - let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; - if lines >= self.options.closing_brace_hints.min_lines { - self.push_text_hint( - lsp_location.range.end, - format!( - " impl {} for {}", - noir_trait_impl.trait_name, noir_trait_impl.object_type - ), - ); - } - } - } + self.show_closing_brace_hint(item.span, || { + format!( + " impl {} for {}", + noir_trait_impl.trait_name, noir_trait_impl.object_type + ) + }); } ItemKind::Impl(type_impl) => { for (noir_function, span) in &type_impl.methods { self.collect_in_noir_function(noir_function, *span); } - if self.options.closing_brace_hints.enabled { - if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, item.span) - { - let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; - if lines >= self.options.closing_brace_hints.min_lines { - self.push_text_hint( - lsp_location.range.end, - format!(" impl {}", type_impl.object_type), - ); - } - } - } + self.show_closing_brace_hint(item.span, || { + format!(" impl {}", type_impl.object_type) + }); } ItemKind::Global(let_statement) => self.collect_in_let_statement(let_statement), ItemKind::Submodules(parsed_submodule) => { self.collect_in_parsed_module(&parsed_submodule.contents); - if self.options.closing_brace_hints.enabled { - if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, item.span) - { - let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; - if lines >= self.options.closing_brace_hints.min_lines { - self.push_text_hint( - lsp_location.range.end, - if parsed_submodule.is_contract { - format!(" contract {}", parsed_submodule.name) - } else { - format!(" mod {}", parsed_submodule.name) - }, - ); - } + self.show_closing_brace_hint(item.span, || { + if parsed_submodule.is_contract { + format!(" contract {}", parsed_submodule.name) + } else { + format!(" mod {}", parsed_submodule.name) } - } + }); } ItemKind::ModuleDecl(_) => (), ItemKind::Import(_) => (), @@ -192,17 +165,7 @@ impl<'a> InlayHintCollector<'a> { fn collect_in_noir_function(&mut self, noir_function: &NoirFunction, span: Span) { self.collect_in_block_expression(&noir_function.def.body); - if self.options.closing_brace_hints.enabled { - if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, span) { - let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; - if lines >= self.options.closing_brace_hints.min_lines { - self.push_text_hint( - lsp_location.range.end, - format!(" fn {}", noir_function.def.name), - ); - } - } - } + self.show_closing_brace_hint(span, || format!(" fn {}", noir_function.def.name)); } fn collect_in_let_statement(&mut self, let_statement: &LetStatement) { @@ -541,6 +504,20 @@ impl<'a> InlayHintCollector<'a> { fn intersects_span(&self, other_span: Span) -> bool { self.span.map_or(true, |span| span.intersects(&other_span)) } + + fn show_closing_brace_hint(&mut self, span: Span, f: F) + where + F: FnOnce() -> String, + { + if self.options.closing_brace_hints.enabled { + if let Some(lsp_location) = to_lsp_location(self.files, self.file_id, span) { + let lines = lsp_location.range.end.line - lsp_location.range.start.line + 1; + if lines >= self.options.closing_brace_hints.min_lines { + self.push_text_hint(lsp_location.range.end, f()); + } + } + } + } } fn string_part(str: impl Into) -> InlayHintLabelPart { From ea16af39963eb24dada04974758ff79be34f9979 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Tue, 6 Aug 2024 12:08:04 -0300 Subject: [PATCH 3/3] clippy --- tooling/lsp/src/requests/inlay_hint.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 3eb6034b9a0..c222d5bca01 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -87,7 +87,7 @@ impl<'a> InlayHintCollector<'a> { match &item.kind { ItemKind::Function(noir_function) => { - self.collect_in_noir_function(noir_function, item.span) + self.collect_in_noir_function(noir_function, item.span); } ItemKind::Trait(noir_trait) => { for item in &noir_trait.items { @@ -153,7 +153,7 @@ impl<'a> InlayHintCollector<'a> { fn collect_in_trait_impl_item(&mut self, item: &TraitImplItem, span: Span) { match item { TraitImplItem::Function(noir_function) => { - self.collect_in_noir_function(noir_function, span) + self.collect_in_noir_function(noir_function, span); } TraitImplItem::Constant(_name, _typ, default_value) => { self.collect_in_expression(default_value); @@ -807,7 +807,7 @@ mod inlay_hints_tests { InlayHintsOptions { type_hints: TypeHintsOptions { enabled: false }, parameter_hints: ParameterHintsOptions { enabled: false }, - closing_brace_hints: ClosingBraceHintsOptions { enabled: true, min_lines: min_lines }, + closing_brace_hints: ClosingBraceHintsOptions { enabled: true, min_lines }, } }