Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: LSP closing brace hints #5686

Merged
merged 3 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 146 additions & 9 deletions tooling/lsp/src/requests/inlay_hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@
args.files.get_file_id(&path).map(|file_id| {
let file = args.files.get_file(file_id).unwrap();
let source = file.source();
let (parsed_moduled, _errors) = noirc_frontend::parse_program(source);

Check warning on line 44 in tooling/lsp/src/requests/inlay_hint.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (moduled)

let span = range_to_byte_span(args.files, file_id, &params.range)
.map(|range| Span::from(range.start as u32..range.end as u32));

let mut collector =
InlayHintCollector::new(args.files, file_id, args.interner, span, options);
collector.collect_in_parsed_module(&parsed_moduled);

Check warning on line 51 in tooling/lsp/src/requests/inlay_hint.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (moduled)
collector.inlay_hints
})
});
Expand Down Expand Up @@ -86,25 +86,46 @@
}

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);
}

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, _) 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);
}

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);

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(_) => (),
Expand All @@ -129,18 +150,22 @@
}
}

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);
}
TraitImplItem::Type { .. } => (),
}
}

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);

self.show_closing_brace_hint(span, || format!(" fn {}", noir_function.def.name));
}

fn collect_in_let_statement(&mut self, let_statement: &LetStatement) {
Expand Down Expand Up @@ -479,6 +504,20 @@
fn intersects_span(&self, other_span: Span) -> bool {
self.span.map_or(true, |span| span.intersects(&other_span))
}

fn show_closing_brace_hint<F>(&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<String>) -> InlayHintLabelPart {
Expand Down Expand Up @@ -708,7 +747,7 @@
#[cfg(test)]
mod inlay_hints_tests {
use crate::{
requests::{ParameterHintsOptions, TypeHintsOptions},
requests::{ClosingBraceHintsOptions, ParameterHintsOptions, TypeHintsOptions},
test_utils,
};

Expand Down Expand Up @@ -744,20 +783,31 @@
InlayHintsOptions {
type_hints: TypeHintsOptions { enabled: false },
parameter_hints: ParameterHintsOptions { enabled: false },
closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 25 },
}
}

fn type_hints() -> InlayHintsOptions {
InlayHintsOptions {
type_hints: TypeHintsOptions { enabled: true },
parameter_hints: ParameterHintsOptions { enabled: false },
closing_brace_hints: ClosingBraceHintsOptions { enabled: false, min_lines: 25 },
}
}

fn parameter_hints() -> InlayHintsOptions {
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 },
}
}

Expand Down Expand Up @@ -1025,4 +1075,91 @@
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);
}
}
}
28 changes: 28 additions & 0 deletions tooling/lsp/src/requests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@

#[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)]
Expand All @@ -92,6 +95,15 @@
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
}
Expand All @@ -104,6 +116,7 @@
InlayHintsOptions {
type_hints: default_type_hints(),
parameter_hints: default_parameter_hints(),
closing_brace_hints: default_closing_brace_hints(),
}
}

Expand All @@ -123,6 +136,21 @@
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 {
Expand Down Expand Up @@ -346,7 +374,7 @@
location: noirc_errors::Location,
files: &'a FileMap,
interner: &'a NodeInterner,
interners: &'a HashMap<String, NodeInterner>,

Check warning on line 377 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (interners)
root_crate_name: String,
root_crate_dependencies: &'a Vec<Dependency>,
}
Expand Down Expand Up @@ -403,7 +431,7 @@
location,
files,
interner,
interners: &state.cached_definitions,

Check warning on line 434 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (interners)
root_crate_name: package.name.to_string(),
root_crate_dependencies: &context.crate_graph[context.root_crate_id()].dependencies,
}))
Expand All @@ -411,7 +439,7 @@
pub(crate) fn find_all_references_in_workspace(
location: noirc_errors::Location,
interner: &NodeInterner,
cached_interners: &HashMap<String, NodeInterner>,

Check warning on line 442 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (interners)
files: &FileMap,
include_declaration: bool,
include_self_type_name: bool,
Expand All @@ -423,8 +451,8 @@
// If we found the referenced node, find its location
let referenced_location = interner.reference_location(referenced);

// Now we find all references that point to this location, in all interners

Check warning on line 454 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (interners)
// (there's one interner per package, and all interners in a workspace rely on the

Check warning on line 455 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (interners)
// same FileManager so a Location/FileId in one package is the same as in another package)
let mut locations = find_all_references(
referenced_location,
Expand All @@ -443,7 +471,7 @@
));
}

// The LSP client usually removes duplicate loctions, but we do it here just in case they don't

Check warning on line 474 in tooling/lsp/src/requests/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (loctions)
locations.sort_by_key(|location| {
(
location.uri.to_string(),
Expand Down
16 changes: 16 additions & 0 deletions tooling/lsp/test_programs/inlay_hints/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

}
Loading