diff --git a/compiler/noirc_frontend/src/ast/statement.rs b/compiler/noirc_frontend/src/ast/statement.rs index 299c42b85c6..38cf2cb1d80 100644 --- a/compiler/noirc_frontend/src/ast/statement.rs +++ b/compiler/noirc_frontend/src/ast/statement.rs @@ -141,13 +141,14 @@ impl StatementKind { pattern: Pattern, r#type: UnresolvedType, expression: Expression, + attributes: Vec, ) -> StatementKind { StatementKind::Let(LetStatement { pattern, r#type, expression, comptime: false, - attributes: vec![], + attributes, }) } @@ -814,6 +815,7 @@ impl ForRange { Pattern::Identifier(array_ident.clone()), UnresolvedTypeData::Unspecified.with_span(Default::default()), array, + vec![], ), span: array_span, }; @@ -858,6 +860,7 @@ impl ForRange { Pattern::Identifier(identifier), UnresolvedTypeData::Unspecified.with_span(Default::default()), Expression::new(loop_element, array_span), + vec![], ), span: array_span, }; diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 80442d29398..6ce8a70a0c5 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -32,6 +32,7 @@ pub enum AttributeTarget { Struct, Trait, Function, + Let, } /// Implements the [Visitor pattern](https://en.wikipedia.org/wiki/Visitor_pattern) for Noir's AST. @@ -1097,6 +1098,10 @@ impl Statement { impl LetStatement { pub fn accept(&self, visitor: &mut impl Visitor) { + for attribute in &self.attributes { + attribute.accept(AttributeTarget::Let, visitor); + } + if visitor.visit_let_statement(self) { self.accept_children(visitor); } diff --git a/compiler/noirc_frontend/src/debug/mod.rs b/compiler/noirc_frontend/src/debug/mod.rs index ed9265536f9..66de265f869 100644 --- a/compiler/noirc_frontend/src/debug/mod.rs +++ b/compiler/noirc_frontend/src/debug/mod.rs @@ -146,6 +146,7 @@ impl DebugInstrumenter { ast::Pattern::Identifier(ident("__debug_expr", ret_expr.span)), ast::UnresolvedTypeData::Unspecified.with_span(Default::default()), ret_expr.clone(), + vec![], ), span: ret_expr.span, }; @@ -249,6 +250,7 @@ impl DebugInstrumenter { }), span: let_stmt.expression.span, }, + vec![], ), span: *span, } @@ -274,6 +276,7 @@ impl DebugInstrumenter { ast::Pattern::Identifier(ident("__debug_expr", assign_stmt.expression.span)), ast::UnresolvedTypeData::Unspecified.with_span(Default::default()), assign_stmt.expression.clone(), + vec![], ); let expression_span = assign_stmt.expression.span; let new_assign_stmt = match &assign_stmt.lvalue { diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index a3b71f3e211..9a74e77215a 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -790,7 +790,7 @@ impl<'context> Elaborator<'context> { let parameter = DefinitionKind::Local(None); let typ = self.resolve_inferred_type(typ); arg_types.push(typ.clone()); - (self.elaborate_pattern(pattern, typ.clone(), parameter), typ) + (self.elaborate_pattern(pattern, typ.clone(), parameter, true), typ) }); let return_type = self.resolve_inferred_type(lambda.return_type); diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index f9016b1ca65..c36231bcf3b 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -362,8 +362,12 @@ impl<'context> Elaborator<'context> { if let Kind::Numeric(typ) = &generic.kind { let definition = DefinitionKind::GenericType(generic.type_var.clone()); let ident = Ident::new(generic.name.to_string(), generic.span); - let hir_ident = - self.add_variable_decl_inner(ident, false, false, false, definition); + let hir_ident = self.add_variable_decl( + ident, false, // mutable + false, // allow_shadowing + false, // warn_if_unused + definition, + ); self.interner.push_definition_type(hir_ident.id, *typ.clone()); } } @@ -760,6 +764,7 @@ impl<'context> Elaborator<'context> { typ.clone(), DefinitionKind::Local(None), &mut parameter_idents, + true, // warn_if_unused None, ); diff --git a/compiler/noirc_frontend/src/elaborator/patterns.rs b/compiler/noirc_frontend/src/elaborator/patterns.rs index 7afa3215566..6ed59a61e4e 100644 --- a/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -26,6 +26,7 @@ impl<'context> Elaborator<'context> { pattern: Pattern, expected_type: Type, definition_kind: DefinitionKind, + warn_if_unused: bool, ) -> HirPattern { self.elaborate_pattern_mut( pattern, @@ -33,6 +34,7 @@ impl<'context> Elaborator<'context> { definition_kind, None, &mut Vec::new(), + warn_if_unused, None, ) } @@ -45,6 +47,7 @@ impl<'context> Elaborator<'context> { expected_type: Type, definition_kind: DefinitionKind, created_ids: &mut Vec, + warn_if_unused: bool, global_id: Option, ) -> HirPattern { self.elaborate_pattern_mut( @@ -53,10 +56,12 @@ impl<'context> Elaborator<'context> { definition_kind, None, created_ids, + warn_if_unused, global_id, ) } + #[allow(clippy::too_many_arguments)] fn elaborate_pattern_mut( &mut self, pattern: Pattern, @@ -64,6 +69,7 @@ impl<'context> Elaborator<'context> { definition: DefinitionKind, mutable: Option, new_definitions: &mut Vec, + warn_if_unused: bool, global_id: Option, ) -> HirPattern { match pattern { @@ -80,7 +86,13 @@ impl<'context> Elaborator<'context> { let location = Location::new(name.span(), self.file); HirIdent::non_trait_method(id, location) } else { - self.add_variable_decl(name, mutable.is_some(), true, definition) + self.add_variable_decl( + name, + mutable.is_some(), + true, // allow_shadowing + warn_if_unused, + definition, + ) }; self.interner.push_definition_type(ident.id, expected_type); new_definitions.push(ident.clone()); @@ -97,6 +109,7 @@ impl<'context> Elaborator<'context> { definition, Some(span), new_definitions, + warn_if_unused, global_id, ); let location = Location::new(span, self.file); @@ -128,6 +141,7 @@ impl<'context> Elaborator<'context> { definition.clone(), mutable, new_definitions, + warn_if_unused, global_id, ) }); @@ -151,6 +165,7 @@ impl<'context> Elaborator<'context> { definition, mutable, new_definitions, + warn_if_unused, global_id, ) } @@ -180,7 +195,7 @@ impl<'context> Elaborator<'context> { // shadowing here lets us avoid further errors if we define ERROR_IDENT // multiple times. let name = ERROR_IDENT.into(); - let identifier = this.add_variable_decl(name, false, true, definition.clone()); + let identifier = this.add_variable_decl(name, false, true, true, definition.clone()); HirPattern::Identifier(identifier) }; @@ -263,6 +278,7 @@ impl<'context> Elaborator<'context> { definition.clone(), mutable, new_definitions, + true, // warn_if_unused None, ); @@ -295,16 +311,6 @@ impl<'context> Elaborator<'context> { } pub(super) fn add_variable_decl( - &mut self, - name: Ident, - mutable: bool, - allow_shadowing: bool, - definition: DefinitionKind, - ) -> HirIdent { - self.add_variable_decl_inner(name, mutable, allow_shadowing, true, definition) - } - - pub fn add_variable_decl_inner( &mut self, name: Ident, mutable: bool, diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index 2d46c4c6341..55b641ca3d4 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -98,12 +98,16 @@ impl<'context> Elaborator<'context> { } } + let warn_if_unused = + !let_stmt.attributes.iter().any(|attr| attr.is_allow_unused_variables()); + let r#type = annotated_type; let pattern = self.elaborate_pattern_and_store_ids( let_stmt.pattern, r#type.clone(), definition, &mut Vec::new(), + warn_if_unused, global_id, ); @@ -215,7 +219,12 @@ impl<'context> Elaborator<'context> { // TODO: For loop variables are currently mutable by default since we haven't // yet implemented syntax for them to be optionally mutable. let kind = DefinitionKind::Local(None); - let identifier = self.add_variable_decl(identifier, false, true, kind); + let identifier = self.add_variable_decl( + identifier, false, // mutable + true, // allow_shadowing + true, // warn_if_unused + kind, + ); // Check that start range and end range have the same types let range_span = start_span.merge(end_span); diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 972826f5b7c..4344d19829a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -29,7 +29,7 @@ impl HirStatement { let pattern = let_stmt.pattern.to_display_ast(interner); let r#type = interner.id_type(let_stmt.expression).to_display_ast(); let expression = let_stmt.expression.to_display_ast(interner); - StatementKind::new_let(pattern, r#type, expression) + StatementKind::new_let(pattern, r#type, expression, let_stmt.attributes.clone()) } HirStatement::Constrain(constrain) => { let expr = constrain.0.to_display_ast(interner); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 4678d29a452..2e118eb4f0e 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -2338,6 +2338,7 @@ fn function_def_set_parameters( parameter_type.clone(), DefinitionKind::Local(None), &mut parameter_idents, + true, // warn_if_unused None, ) }); diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index f7e0a85c79c..97faea4b445 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -804,6 +804,7 @@ impl Attribute { } ["varargs"] => Attribute::Secondary(SecondaryAttribute::Varargs), ["use_callers_scope"] => Attribute::Secondary(SecondaryAttribute::UseCallersScope), + ["allow", tag] => Attribute::Secondary(SecondaryAttribute::Allow(tag.to_string())), tokens => { tokens.iter().try_for_each(|token| validate(token))?; Attribute::Secondary(SecondaryAttribute::Custom(CustomAttribute { @@ -925,6 +926,9 @@ pub enum SecondaryAttribute { /// within the scope of the calling function/module rather than this one. /// This affects functions such as `Expression::resolve` or `Quoted::as_type`. UseCallersScope, + + /// Allow chosen warnings to happen so they are silenced. + Allow(String), } impl SecondaryAttribute { @@ -948,6 +952,14 @@ impl SecondaryAttribute { SecondaryAttribute::Abi(_) => Some("abi".to_string()), SecondaryAttribute::Varargs => Some("varargs".to_string()), SecondaryAttribute::UseCallersScope => Some("use_callers_scope".to_string()), + SecondaryAttribute::Allow(_) => Some("allow".to_string()), + } + } + + pub(crate) fn is_allow_unused_variables(&self) -> bool { + match self { + SecondaryAttribute::Allow(string) => string == "unused_variables", + _ => false, } } } @@ -966,6 +978,7 @@ impl fmt::Display for SecondaryAttribute { SecondaryAttribute::Abi(ref k) => write!(f, "#[abi({k})]"), SecondaryAttribute::Varargs => write!(f, "#[varargs]"), SecondaryAttribute::UseCallersScope => write!(f, "#[use_callers_scope]"), + SecondaryAttribute::Allow(ref k) => write!(f, "#[allow(#{k})]"), } } } @@ -1011,7 +1024,9 @@ impl AsRef for SecondaryAttribute { SecondaryAttribute::Deprecated(Some(string)) => string, SecondaryAttribute::Deprecated(None) => "", SecondaryAttribute::Custom(attribute) => &attribute.contents, - SecondaryAttribute::Field(string) | SecondaryAttribute::Abi(string) => string, + SecondaryAttribute::Field(string) + | SecondaryAttribute::Abi(string) + | SecondaryAttribute::Allow(string) => string, SecondaryAttribute::ContractLibraryMethod => "", SecondaryAttribute::Export => "", SecondaryAttribute::Varargs => "", diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index b007653062b..0eb49ae975a 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -550,8 +550,12 @@ fn declaration<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - let_statement(expr_parser) - .map(|((pattern, typ), expr)| StatementKind::new_let(pattern, typ, expr)) + attributes().then(let_statement(expr_parser)).validate( + |(attributes, ((pattern, typ), expr)), span, emit| { + let attributes = attributes::validate_secondary_attributes(attributes, span, emit); + StatementKind::new_let(pattern, typ, expr, attributes) + }, + ) } pub fn pattern() -> impl NoirParser { diff --git a/compiler/noirc_frontend/src/tests/unused_items.rs b/compiler/noirc_frontend/src/tests/unused_items.rs index b49414d8b03..da0a40d93cf 100644 --- a/compiler/noirc_frontend/src/tests/unused_items.rs +++ b/compiler/noirc_frontend/src/tests/unused_items.rs @@ -1,4 +1,7 @@ -use crate::hir::{def_collector::dc_crate::CompilationError, resolution::errors::ResolverError}; +use crate::{ + hir::{def_collector::dc_crate::CompilationError, resolution::errors::ResolverError}, + tests::assert_no_errors, +}; use super::get_program_errors; @@ -157,3 +160,14 @@ fn errors_on_unused_trait() { assert_eq!(ident.to_string(), "Foo"); assert_eq!(*item_type, "trait"); } + +#[test] +fn silences_unused_variable_warning() { + let src = r#" + fn main() { + #[allow(unused_variables)] + let x = 1; + } + "#; + assert_no_errors(src); +} diff --git a/tooling/lsp/src/requests/completion/builtins.rs b/tooling/lsp/src/requests/completion/builtins.rs index 6812ebc135b..cf2af4036f7 100644 --- a/tooling/lsp/src/requests/completion/builtins.rs +++ b/tooling/lsp/src/requests/completion/builtins.rs @@ -116,6 +116,15 @@ impl<'a> NodeFinder<'a> { )); } } + AttributeTarget::Let => { + if name_matches("allow", prefix) || name_matches("unused_variables", prefix) { + self.completion_items.push(simple_completion_item( + "allow(unused_variables)", + CompletionItemKind::METHOD, + None, + )); + } + } } } } diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index c0155096dc8..809988c34a5 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -51,6 +51,10 @@ impl<'a> NodeFinder<'a> { AttributeTarget::Struct => Some(Type::Quoted(QuotedType::StructDefinition)), AttributeTarget::Trait => Some(Type::Quoted(QuotedType::TraitDefinition)), AttributeTarget::Function => Some(Type::Quoted(QuotedType::FunctionDefinition)), + AttributeTarget::Let => { + // No item can be suggested for a let statement attribute + return Vec::new(); + } } } else { None diff --git a/tooling/lsp/src/requests/completion/tests.rs b/tooling/lsp/src/requests/completion/tests.rs index 45eb79bd1c2..68cb3870f63 100644 --- a/tooling/lsp/src/requests/completion/tests.rs +++ b/tooling/lsp/src/requests/completion/tests.rs @@ -1942,6 +1942,26 @@ mod completion_tests { .await; } + #[test] + async fn test_suggests_built_in_let_attribute() { + let src = r#" + fn foo() { + #[allo>|<] + let x = 1; + } + "#; + + assert_completion_excluding_auto_import( + src, + vec![simple_completion_item( + "allow(unused_variables)", + CompletionItemKind::METHOD, + None, + )], + ) + .await; + } + #[test] async fn test_suggests_function_attribute() { let src = r#"