Skip to content

Commit

Permalink
feat: LSP autocompletion for attributes (#5963)
Browse files Browse the repository at this point in the history
# Description

## Problem

While working a lot with attributes lately I thought it would be nice if
we had autocompletion there too.

## Summary

Built-in attributes are suggested (I don't know if I got them all right,
I don't know which attributes are allowed on structs) and also "function
attributes" (we need to give this a name) are suggested if their first
argument type matches that of the item the annotation is on (and with
auto-import, we effectively get all annotations available to be placed
on a function/struct/module).


![lsp-complete-attribute](https://github.com/user-attachments/assets/ab918a88-203d-42e1-88a8-412e5c67029c)

## Additional Context

If we add new attributes we'll have to remember to update the LSP code.
But... I think this might be rare, so it might be fine to have to
remember this. Also eventually if would be nice if all attributes had an
associated function (even if it's a built-in one) and then we wouldn't
need special code for that.

## 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 6, 2024
1 parent af50a7b commit b7b9e3f
Show file tree
Hide file tree
Showing 7 changed files with 238 additions and 16 deletions.
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ mod traits;
mod type_alias;
mod visitor;

pub use visitor::AttributeTarget;
pub use visitor::Visitor;

pub use expression::*;
Expand Down
55 changes: 50 additions & 5 deletions compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
QuotedTypeId,
},
parser::{Item, ItemKind, ParsedSubModule},
token::{SecondaryAttribute, Tokens},
token::{CustomAtrribute, SecondaryAttribute, Tokens},
ParsedModule, QuotedType,
};

Expand All @@ -26,6 +26,13 @@ use super::{
UnresolvedTypeExpression,
};

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
pub enum AttributeTarget {
Module,
Struct,
Function,
}

/// Implements the [Visitor pattern](https://en.wikipedia.org/wiki/Visitor_pattern) for Noir's AST.
///
/// In this implementation, methods must return a bool:
Expand Down Expand Up @@ -433,7 +440,15 @@ pub trait Visitor {
true
}

fn visit_secondary_attribute(&mut self, _: &SecondaryAttribute, _: Span) {}
fn visit_secondary_attribute(
&mut self,
_: &SecondaryAttribute,
_target: AttributeTarget,
) -> bool {
true
}

fn visit_custom_attribute(&mut self, _: &CustomAtrribute, _target: AttributeTarget) {}
}

impl ParsedModule {
Expand Down Expand Up @@ -484,14 +499,18 @@ impl Item {
module_declaration.accept(self.span, visitor);
}
ItemKind::InnerAttribute(attribute) => {
attribute.accept(self.span, visitor);
attribute.accept(AttributeTarget::Module, visitor);
}
}
}
}

impl ParsedSubModule {
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
for attribute in &self.outer_attributes {
attribute.accept(AttributeTarget::Module, visitor);
}

if visitor.visit_parsed_submodule(self, span) {
self.accept_children(visitor);
}
Expand All @@ -510,6 +529,10 @@ impl NoirFunction {
}

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

for param in &self.def.parameters {
param.typ.accept(visitor);
}
Expand Down Expand Up @@ -674,6 +697,10 @@ impl NoirStruct {
}

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

for (_name, unresolved_type) in &self.fields {
unresolved_type.accept(visitor);
}
Expand All @@ -694,6 +721,10 @@ impl NoirTypeAlias {

impl ModuleDeclaration {
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
for attribute in &self.outer_attributes {
attribute.accept(AttributeTarget::Module, visitor);
}

visitor.visit_module_declaration(self, span);
}
}
Expand Down Expand Up @@ -1295,8 +1326,22 @@ impl Pattern {
}

impl SecondaryAttribute {
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
visitor.visit_secondary_attribute(self, span);
pub fn accept(&self, target: AttributeTarget, visitor: &mut impl Visitor) {
if visitor.visit_secondary_attribute(self, target) {
self.accept_children(target, visitor);
}
}

pub fn accept_children(&self, target: AttributeTarget, visitor: &mut impl Visitor) {
if let SecondaryAttribute::Custom(custom) = self {
custom.accept(target, visitor);
}
}
}

impl CustomAtrribute {
pub fn accept(&self, target: AttributeTarget, visitor: &mut impl Visitor) {
visitor.visit_custom_attribute(self, target);
}
}

Expand Down
75 changes: 69 additions & 6 deletions tooling/lsp/src/requests/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{

use async_lsp::ResponseError;
use completion_items::{
crate_completion_item, field_completion_item, simple_completion_item,
crate_completion_item, field_completion_item, simple_completion_item, snippet_completion_item,
struct_field_completion_item,
};
use convert_case::{Case, Casing};
Expand All @@ -15,18 +15,19 @@ use lsp_types::{CompletionItem, CompletionItemKind, CompletionParams, Completion
use noirc_errors::{Location, Span};
use noirc_frontend::{
ast::{
AsTraitPath, BlockExpression, CallExpression, ConstructorExpression, Expression,
ExpressionKind, ForLoopStatement, GenericTypeArgs, Ident, IfExpression, ItemVisibility,
Lambda, LetStatement, MemberAccessExpression, MethodCallExpression, NoirFunction,
NoirStruct, NoirTraitImpl, Path, PathKind, Pattern, Statement, TypeImpl, UnresolvedGeneric,
UnresolvedGenerics, UnresolvedType, UseTree, UseTreeKind, Visitor,
AsTraitPath, AttributeTarget, BlockExpression, CallExpression, ConstructorExpression,
Expression, ExpressionKind, ForLoopStatement, GenericTypeArgs, Ident, IfExpression,
ItemVisibility, Lambda, LetStatement, MemberAccessExpression, MethodCallExpression,
NoirFunction, NoirStruct, NoirTraitImpl, Path, PathKind, Pattern, Statement, TypeImpl,
UnresolvedGeneric, UnresolvedGenerics, UnresolvedType, UseTree, UseTreeKind, Visitor,
},
graph::{CrateId, Dependency},
hir::def_map::{CrateDefMap, LocalModuleId, ModuleId},
hir_def::traits::Trait,
macros_api::{ModuleDefId, NodeInterner},
node_interner::ReferenceId,
parser::{Item, ItemKind, ParsedSubModule},
token::CustomAtrribute,
ParsedModule, StructType, Type,
};
use sort_text::underscore_sort_text;
Expand Down Expand Up @@ -359,6 +360,7 @@ impl<'a> NodeFinder<'a> {
self.builtin_types_completion(&prefix);
self.type_parameters_completion(&prefix);
}
RequestedItems::OnlyAttributeFunctions(..) => (),
}
self.complete_auto_imports(&prefix, requested_items, function_completion_kind);
}
Expand Down Expand Up @@ -606,6 +608,7 @@ impl<'a> NodeFinder<'a> {
func_id,
function_completion_kind,
function_kind,
None, // attribute first type
self_prefix,
) {
self.completion_items.push(completion_item);
Expand All @@ -632,6 +635,7 @@ impl<'a> NodeFinder<'a> {
*func_id,
function_completion_kind,
function_kind,
None, // attribute first type
self_prefix,
) {
self.completion_items.push(completion_item);
Expand Down Expand Up @@ -786,6 +790,49 @@ impl<'a> NodeFinder<'a> {
None
}

fn suggest_attributes(&mut self, prefix: &str, target: AttributeTarget) {
self.suggest_builtin_attributes(prefix, target);

let function_completion_kind = FunctionCompletionKind::NameAndParameters;
let requested_items = RequestedItems::OnlyAttributeFunctions(target);

self.complete_in_module(
self.module_id,
prefix,
PathKind::Plain,
true,
function_completion_kind,
requested_items,
);

self.complete_auto_imports(prefix, requested_items, function_completion_kind);
}

fn suggest_no_arguments_attributes(&mut self, prefix: &str, attributes: &[&str]) {
for name in attributes {
if name_matches(name, prefix) {
self.completion_items.push(simple_completion_item(
*name,
CompletionItemKind::METHOD,
None,
));
}
}
}

fn suggest_one_argument_attributes(&mut self, prefix: &str, attributes: &[&str]) {
for name in attributes {
if name_matches(name, prefix) {
self.completion_items.push(snippet_completion_item(
format!("{}(…)", name),
CompletionItemKind::METHOD,
format!("{}(${{1:name}})", name),
None,
));
}
}
}

fn try_set_self_type(&mut self, pattern: &Pattern) {
match pattern {
Pattern::Identifier(ident) => {
Expand Down Expand Up @@ -855,6 +902,10 @@ impl<'a> Visitor for NodeFinder<'a> {
}

fn visit_noir_function(&mut self, noir_function: &NoirFunction, span: Span) -> bool {
for attribute in noir_function.secondary_attributes() {
attribute.accept(AttributeTarget::Function, self);
}

let old_type_parameters = self.type_parameters.clone();
self.collect_type_parameters_in_generics(&noir_function.def.generics);

Expand Down Expand Up @@ -915,6 +966,10 @@ impl<'a> Visitor for NodeFinder<'a> {
}

fn visit_noir_struct(&mut self, noir_struct: &NoirStruct, _: Span) -> bool {
for attribute in &noir_struct.attributes {
attribute.accept(AttributeTarget::Struct, self);
}

self.type_parameters.clear();
self.collect_type_parameters_in_generics(&noir_struct.generics);

Expand Down Expand Up @@ -1263,6 +1318,14 @@ impl<'a> Visitor for NodeFinder<'a> {
unresolved_types.accept(self);
false
}

fn visit_custom_attribute(&mut self, attribute: &CustomAtrribute, target: AttributeTarget) {
if self.byte_index != attribute.contents_span.end() as usize {
return;
}

self.suggest_attributes(&attribute.contents, target);
}
}

/// Returns true if name matches a prefix written in code.
Expand Down
36 changes: 35 additions & 1 deletion tooling/lsp/src/requests/completion/builtins.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use lsp_types::CompletionItemKind;
use noirc_frontend::token::Keyword;
use noirc_frontend::{ast::AttributeTarget, token::Keyword};
use strum::IntoEnumIterator;

use super::{
Expand Down Expand Up @@ -84,6 +84,40 @@ impl<'a> NodeFinder<'a> {
}
}
}

pub(super) fn suggest_builtin_attributes(&mut self, prefix: &str, target: AttributeTarget) {
match target {
AttributeTarget::Module => (),
AttributeTarget::Struct => {
self.suggest_one_argument_attributes(prefix, &["abi"]);
}
AttributeTarget::Function => {
let no_arguments_attributes = &[
"contract_library_method",
"deprecated",
"export",
"fold",
"no_predicates",
"recursive",
"test",
"varargs",
];
self.suggest_no_arguments_attributes(prefix, no_arguments_attributes);

let one_argument_attributes = &["abi", "field", "foreign", "oracle"];
self.suggest_one_argument_attributes(prefix, one_argument_attributes);

if name_matches("test", prefix) || name_matches("should_fail_with", prefix) {
self.completion_items.push(snippet_completion_item(
"test(should_fail_with=\"...\")",
CompletionItemKind::METHOD,
"test(should_fail_with=\"${1:message}\")",
None,
));
}
}
}
}
}

pub(super) fn builtin_integer_types() -> [&'static str; 8] {
Expand Down
Loading

0 comments on commit b7b9e3f

Please sign in to comment.