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

fix: let LSP autocompletion work in more contexts #5719

Merged
merged 7 commits into from
Aug 14, 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
3 changes: 2 additions & 1 deletion compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,8 @@ impl<'context> Elaborator<'context> {
function_args.push((typ, arg, span));
}

let location = Location::new(span, self.file);
let call_span = Span::from(object_span.start()..method_name_span.end());
let location = Location::new(call_span, self.file);
let method = method_call.method_name;
let turbofish_generics = generics.clone();
let is_macro_call = method_call.is_macro_call;
Expand Down
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ pub enum ParserErrorReason {
ExpectedIdentifierAfterDot,
#[error("expected an identifier after ::")]
ExpectedIdentifierAfterColons,
#[error("expected {{ after if condition")]
ExpectedLeftBraceAfterIfCondition,
#[error("Expected a ; separating these two statements")]
MissingSeparatingSemi,
#[error("constrain keyword is deprecated")]
Expand Down
48 changes: 44 additions & 4 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@

use chumsky::prelude::*;
use iter_extended::vecmap;
use lalrpop_util::lalrpop_mod;

Check warning on line 54 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lalrpop)

Check warning on line 54 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lalrpop)
use noirc_errors::{Span, Spanned};

mod assertion;
Expand All @@ -65,8 +65,8 @@
pub(super) mod traits;
mod types;

// synthesized by LALRPOP

Check warning on line 68 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (LALRPOP)
lalrpop_mod!(pub noir_parser);

Check warning on line 69 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lalrpop)

#[cfg(test)]
mod test_helpers;
Expand All @@ -91,12 +91,12 @@

if cfg!(feature = "experimental_parser") {
for parsed_item in &parsed_module.items {
if lalrpop_parser_supports_kind(&parsed_item.kind) {

Check warning on line 94 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lalrpop)
match &parsed_item.kind {
ItemKind::Import(parsed_use_tree) => {
prototype_parse_use_tree(Some(parsed_use_tree), source_program);
}
// other kinds prevented by lalrpop_parser_supports_kind

Check warning on line 99 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (lalrpop)
_ => unreachable!(),
}
}
Expand All @@ -113,7 +113,7 @@
}

let mut lexer = Lexer::new(input);
lexer = lexer.skip_whitespaces(false);

Check warning on line 116 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (whitespaces)
let mut errors = Vec::new();

// NOTE: this is a hack to get the references working
Expand Down Expand Up @@ -944,10 +944,32 @@

keyword(Keyword::If)
.ignore_then(expr_no_constructors)
.then(if_block)
.then(keyword(Keyword::Else).ignore_then(else_block).or_not())
.map(|((condition, consequence), alternative)| {
ExpressionKind::If(Box::new(IfExpression { condition, consequence, alternative }))
.then(if_block.then(keyword(Keyword::Else).ignore_then(else_block).or_not()).or_not())
.validate(|(condition, consequence_and_alternative), span, emit_error| {
if let Some((consequence, alternative)) = consequence_and_alternative {
ExpressionKind::If(Box::new(IfExpression {
condition,
consequence,
alternative,
}))
} else {
// We allow `if cond` without a block mainly for LSP, so that it parses right
// and autocompletion works there.
emit_error(ParserError::with_reason(
ParserErrorReason::ExpectedLeftBraceAfterIfCondition,
span,
));

let span_end = condition.span.end();
ExpressionKind::If(Box::new(IfExpression {
condition,
consequence: Expression::new(
ExpressionKind::Error,
Span::from(span_end..span_end),
),
alternative: None,
}))
}
})
})
}
Expand Down Expand Up @@ -1420,6 +1442,24 @@
);
}

#[test]
fn parse_if_without_block() {
let src = "if foo";
let parser = if_expr(expression_no_constructors(expression()), fresh_statement());
let (expression_kind, errors) = parse_recover(parser, src);

let expression_kind = expression_kind.unwrap();
let ExpressionKind::If(if_expression) = expression_kind else {
panic!("Expected an if expression, got {:?}", expression_kind);
};

assert_eq!(if_expression.consequence.kind, ExpressionKind::Error);
assert_eq!(if_expression.alternative, None);

assert_eq!(errors.len(), 1);
assert_eq!(errors[0].message, "expected { after if condition");
}

#[test]
fn parse_module_declaration() {
parse_with(module_declaration(), "mod foo").unwrap();
Expand Down
125 changes: 111 additions & 14 deletions tooling/lsp/src/requests/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
};

use async_lsp::ResponseError;
use builtins::{builtin_integer_types, keyword_builtin_function, keyword_builtin_type};

Check warning on line 7 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (builtins)
use fm::{FileId, PathString};
use lsp_types::{
CompletionItem, CompletionItemKind, CompletionItemLabelDetails, CompletionParams,
Expand Down Expand Up @@ -39,7 +39,7 @@

use super::process_request;

mod builtins;

Check warning on line 42 in tooling/lsp/src/requests/completion.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (builtins)

/// When finding items in a module, whether to show only direct children or all visible items.
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
Expand Down Expand Up @@ -450,7 +450,18 @@

fn find_in_lvalue(&mut self, lvalue: &LValue) {
match lvalue {
LValue::Ident(_) => (),
LValue::Ident(ident) => {
if self.byte == Some(b'.') && ident.span().end() as usize == self.byte_index - 1 {
let location = Location::new(ident.span(), self.file);
if let Some(ReferenceId::Local(definition_id)) =
self.interner.find_referenced(location)
{
let typ = self.interner.definition_type(definition_id);
let prefix = "";
self.complete_type_fields_and_methods(&typ, prefix);
}
}
}
LValue::MemberAccess { object, field_name: _, span: _ } => self.find_in_lvalue(object),
LValue::Index { array, index, span: _ } => {
self.find_in_lvalue(array);
Expand All @@ -477,19 +488,6 @@
}

fn find_in_expression(&mut self, expression: &Expression) {
// "foo." (no identifier afterwards) is parsed as the expression on the left hand-side of the dot.
// Here we check if there's a dot at the completion position, and if the expression
// ends right before the dot. If so, it means we want to complete the expression's type fields and methods.
if self.byte == Some(b'.') && expression.span.end() as usize == self.byte_index - 1 {
let location = Location::new(expression.span, self.file);
if let Some(typ) = self.interner.type_at_location(location) {
let typ = typ.follow_bindings();
let prefix = "";
self.complete_type_fields_and_methods(&typ, prefix);
return;
}
}

match &expression.kind {
noirc_frontend::ast::ExpressionKind::Literal(literal) => self.find_in_literal(literal),
noirc_frontend::ast::ExpressionKind::Block(block_expression) => {
Expand Down Expand Up @@ -551,6 +549,23 @@
| noirc_frontend::ast::ExpressionKind::Resolved(_)
| noirc_frontend::ast::ExpressionKind::Error => (),
}

// "foo." (no identifier afterwards) is parsed as the expression on the left hand-side of the dot.
// Here we check if there's a dot at the completion position, and if the expression
// ends right before the dot. If so, it means we want to complete the expression's type fields and methods.
// We only do this after visiting nested expressions, because in an expression like `foo & bar.` we want
// to complete for `bar`, not for `foo & bar`.
if self.completion_items.is_empty()
&& self.byte == Some(b'.')
&& expression.span.end() as usize == self.byte_index - 1
{
let location = Location::new(expression.span, self.file);
if let Some(typ) = self.interner.type_at_location(location) {
let typ = typ.follow_bindings();
let prefix = "";
self.complete_type_fields_and_methods(&typ, prefix);
}
}
}

fn find_in_literal(&mut self, literal: &Literal) {
Expand Down Expand Up @@ -2877,4 +2892,86 @@
)
.await;
}

#[test]
async fn test_completes_in_broken_if_after_dot() {
let src = r#"
struct Some {
foo: i32,
}

fn foo(s: Some) {
if s.>|<
}
"#;
assert_completion(
src,
vec![simple_completion_item("foo", CompletionItemKind::FIELD, Some("i32".to_string()))],
)
.await;
}

#[test]
async fn test_completes_in_nested_expression() {
let src = r#"
struct Foo { bar: Bar }
struct Bar { baz: i32 }

fn foo(f: Foo) {
f.bar & f.>|<
}
"#;
assert_completion(
src,
vec![simple_completion_item("bar", CompletionItemKind::FIELD, Some("Bar".to_string()))],
)
.await;
}

#[test]
async fn test_completes_in_call_chain() {
let src = r#"
struct Foo {}

impl Foo {
fn foo(self) -> Foo { self }
}

fn foo(f: Foo) {
f.foo().>|<
}
"#;
assert_completion(
src,
vec![snippet_completion_item(
"foo()",
CompletionItemKind::FUNCTION,
"foo()",
Some("fn(self) -> Foo".to_string()),
)],
)
.await;
}

#[test]
async fn test_completes_when_assignment_follows() {
let src = r#"
struct Foo {
bar: i32,
}

fn foo(f: Foo) {
let mut x = 1;

f.>|<

x = 2;
}
"#;
assert_completion(
src,
vec![simple_completion_item("bar", CompletionItemKind::FIELD, Some("i32".to_string()))],
)
.await;
}
}
Loading