Skip to content

Commit

Permalink
fix: suggest trait attributes in LSP (#5972)
Browse files Browse the repository at this point in the history
# Description

## Problem

I noticed LSP didn't suggest trait attributes... because these weren't
visited. I also noticed when an attribute related to a function was
suggested, but the function only had one argument, the parentheses were
included in the suggestion but they aren't needed.

## Summary

Fixes the above problems.


![lsp-trait-attribute](https://github.com/user-attachments/assets/16cdd8ab-1d03-40d1-a1b1-19b26d88e322)

## Additional Context



## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
asterite authored Sep 9, 2024
1 parent cc30d88 commit d6f60d7
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 4 deletions.
5 changes: 5 additions & 0 deletions compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use super::{
pub enum AttributeTarget {
Module,
Struct,
Trait,
Function,
}

Expand Down Expand Up @@ -617,6 +618,10 @@ impl NoirTrait {
}

pub fn accept_children(&self, visitor: &mut impl Visitor) {
for attribute in &self.attributes {
attribute.accept(AttributeTarget::Trait, visitor);
}

for item in &self.items {
item.item.accept(visitor);
}
Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/requests/completion/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<'a> NodeFinder<'a> {

pub(super) fn suggest_builtin_attributes(&mut self, prefix: &str, target: AttributeTarget) {
match target {
AttributeTarget::Module => (),
AttributeTarget::Module | AttributeTarget::Trait => (),
AttributeTarget::Struct => {
self.suggest_one_argument_attributes(prefix, &["abi"]);
}
Expand Down
11 changes: 8 additions & 3 deletions tooling/lsp/src/requests/completion/completion_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ impl<'a> NodeFinder<'a> {
match target {
AttributeTarget::Module => Some(Type::Quoted(QuotedType::Module)),
AttributeTarget::Struct => Some(Type::Quoted(QuotedType::StructDefinition)),
AttributeTarget::Trait => Some(Type::Quoted(QuotedType::TraitDefinition)),
AttributeTarget::Function => Some(Type::Quoted(QuotedType::FunctionDefinition)),
}
} else {
Expand Down Expand Up @@ -226,10 +227,14 @@ impl<'a> NodeFinder<'a> {
function_kind,
skip_first_argument,
);
let label = if insert_text.ends_with("()") {
format!("{}()", name)
let (label, insert_text) = if insert_text.ends_with("()") {
if skip_first_argument {
(name.to_string(), insert_text.strip_suffix("()").unwrap().to_string())
} else {
(format!("{}()", name), insert_text)
}
} else {
format!("{}(…)", name)
(format!("{}(…)", name), insert_text)
};

snippet_completion_item(label, kind, insert_text, Some(description))
Expand Down
36 changes: 36 additions & 0 deletions tooling/lsp/src/requests/completion/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1952,4 +1952,40 @@ mod completion_tests {
)
.await;
}

#[test]
async fn test_suggests_function_attribute_no_arguments() {
let src = r#"
#[some>|<]
fn foo() {}
fn some_attr(f: FunctionDefinition) {}
"#;

assert_completion_excluding_auto_import(
src,
vec![function_completion_item("some_attr", "some_attr", "fn(FunctionDefinition)")],
)
.await;
}

#[test]
async fn test_suggests_trait_attribute() {
let src = r#"
#[some>|<]
trait SomeTrait {}
fn some_attr(t: TraitDefinition, x: Field) {}
"#;

assert_completion_excluding_auto_import(
src,
vec![function_completion_item(
"some_attr(…)",
"some_attr(${1:x})",
"fn(TraitDefinition, Field)",
)],
)
.await;
}
}

0 comments on commit d6f60d7

Please sign in to comment.