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: suggest removing ! from macro call that doesn't return Quoted #6384

Merged
merged 5 commits into from
Oct 29, 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
14 changes: 9 additions & 5 deletions compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
TypeKindMismatch { expected_kind: String, expr_kind: String, expr_span: Span },
// TODO(https://github.com/noir-lang/noir/issues/6238): implement handling for larger types
#[error("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)")]
EvaluatedGlobalIsntU32 { expected_kind: String, expr_kind: String, expr_span: Span },

Check warning on line 53 in compiler/noirc_frontend/src/hir/type_check/errors.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
#[error("Expected {expected:?} found {found:?}")]
ArityMisMatch { expected: usize, found: usize, span: Span },
#[error("Return type in a function cannot be public")]
Expand Down Expand Up @@ -234,7 +234,7 @@
}
// TODO(https://github.com/noir-lang/noir/issues/6238): implement
// handling for larger types
TypeCheckError::EvaluatedGlobalIsntU32 { expected_kind, expr_kind, expr_span } => {

Check warning on line 237 in compiler/noirc_frontend/src/hir/type_check/errors.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
Diagnostic::simple_warning(
format!("Expected type {expected_kind} when evaluating globals, but found {expr_kind} (this warning may become an error in the future)"),
String::new(),
Expand Down Expand Up @@ -434,11 +434,15 @@
let msg = format!("Expected {expected_count} generic{expected_plural} from this function, but {actual_count} {actual_plural} provided");
Diagnostic::simple_error(msg, "".into(), *span)
},
TypeCheckError::MacroReturningNonExpr { typ, span } => Diagnostic::simple_error(
format!("Expected macro call to return a `Quoted` but found a(n) `{typ}`"),
"Macro calls must return quoted values, otherwise there is no code to insert".into(),
*span,
),
TypeCheckError::MacroReturningNonExpr { typ, span } => {
let mut error = Diagnostic::simple_error(
format!("Expected macro call to return a `Quoted` but found a(n) `{typ}`"),
"Macro calls must return quoted values, otherwise there is no code to insert.".into(),
*span,
);
error.add_secondary("Hint: remove the `!` from the end of the function name.".to_string(), *span);
error
},
TypeCheckError::UnsupportedTurbofishUsage { span } => {
let msg = "turbofish (`::<_>`) usage at this position isn't supported yet";
Diagnostic::simple_error(msg.to_string(), "".to_string(), *span)
Expand Down
34 changes: 33 additions & 1 deletion tooling/lsp/src/requests/code_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
};
use noirc_errors::Span;
use noirc_frontend::{
ast::{ConstructorExpression, ItemVisibility, NoirTraitImpl, Path, UseTree, Visitor},
ast::{
CallExpression, ConstructorExpression, ItemVisibility, MethodCallExpression, NoirTraitImpl,
Path, UseTree, Visitor,
},
graph::CrateId,
hir::def_map::{CrateDefMap, LocalModuleId, ModuleId},
node_interner::NodeInterner,
Expand All @@ -29,6 +32,7 @@
mod fill_struct_fields;
mod implement_missing_members;
mod import_or_qualify;
mod remove_bang_from_call;
mod remove_unused_import;
mod tests;

Expand Down Expand Up @@ -168,7 +172,7 @@

CodeAction {
title,
kind: Some(CodeActionKind::QUICKFIX),

Check warning on line 175 in tooling/lsp/src/requests/code_action.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (QUICKFIX)
diagnostics: None,
edit: Some(workspace_edit),
command: None,
Expand Down Expand Up @@ -250,4 +254,32 @@

true
}

fn visit_call_expression(&mut self, call: &CallExpression, span: Span) -> bool {
if !self.includes_span(span) {
return false;
}

if call.is_macro_call {
self.remove_bang_from_call(call.func.span);
}

true
}

fn visit_method_call_expression(
&mut self,
method_call: &MethodCallExpression,
span: Span,
) -> bool {
if !self.includes_span(span) {
return false;
}

if method_call.is_macro_call {
self.remove_bang_from_call(method_call.method_name.span());
}

true
}
}
97 changes: 97 additions & 0 deletions tooling/lsp/src/requests/code_action/remove_bang_from_call.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
use lsp_types::TextEdit;
use noirc_errors::{Location, Span};
use noirc_frontend::{node_interner::ReferenceId, QuotedType, Type};

use crate::byte_span_to_range;

use super::CodeActionFinder;

impl<'a> CodeActionFinder<'a> {
pub(super) fn remove_bang_from_call(&mut self, span: Span) {
// If we can't find the referenced function, there's nothing we can do
let Some(ReferenceId::Function(func_id)) =
self.interner.find_referenced(Location::new(span, self.file))
else {
return;
};

// If the return type is Quoted, all is good
let func_meta = self.interner.function_meta(&func_id);
if let Type::Quoted(QuotedType::Quoted) = func_meta.return_type() {
return;
}

// The `!` comes right after the name
let byte_span = span.end() as usize..span.end() as usize + 1;
let Some(range) = byte_span_to_range(self.files, self.file, byte_span) else {
return;
};

let title = "Remove `!` from call".to_string();
let text_edit = TextEdit { range, new_text: "".to_string() };

let code_action = self.new_quick_fix(title, text_edit);
self.code_actions.push(code_action);
}
}

#[cfg(test)]
mod tests {
use tokio::test;

use crate::requests::code_action::tests::assert_code_action;

#[test]
async fn test_removes_bang_from_call() {
let title = "Remove `!` from call";

let src = r#"
fn foo() {}

fn main() {
fo>|<o!();
}
"#;

let expected = r#"
fn foo() {}

fn main() {
foo();
}
"#;

assert_code_action(title, src, expected).await;
}

#[test]
async fn test_removes_bang_from_method_call() {
let title = "Remove `!` from call";

let src = r#"
struct Foo {}

impl Foo {
fn foo(self) {}
}

fn bar(foo: Foo) {
foo.fo>|<o!();
}
"#;

let expected = r#"
struct Foo {}

impl Foo {
fn foo(self) {}
}

fn bar(foo: Foo) {
foo.foo();
}
"#;

assert_code_action(title, src, expected).await;
}
}
Loading