diff --git a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index 49b8eee1755..6be22bdea24 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -18,7 +18,7 @@ use crate::ast::{ }; use crate::hir::resolution::errors::ResolverError; use crate::node_interner::{ModuleAttributes, NodeInterner, ReferenceId, TypeId}; -use crate::token::SecondaryAttribute; +use crate::token::{SecondaryAttribute, TestScope}; use crate::usage_tracker::{UnusedItem, UsageTracker}; use crate::{Generics, Kind, ResolvedGeneric, Type, TypeVariable}; use crate::{ @@ -1009,8 +1009,10 @@ pub fn collect_function( let module_data = &mut def_map[module.local_id]; - let is_test = function.def.attributes.is_test_function(); - let is_fuzzing_harness = function.def.attributes.is_fuzzing_harness(); + let test_attribute = function.def.attributes.as_test_function(); + let is_test = test_attribute.is_some(); + let fuzz_attribute = function.def.attributes.as_fuzzing_harness(); + let is_fuzzing_harness = fuzz_attribute.is_some(); let is_entry_point_function = if module_data.is_contract { function.attributes().is_contract_entry_point() } else { @@ -1040,6 +1042,22 @@ pub fn collect_function( interner.set_doc_comments(ReferenceId::Function(func_id), doc_comments); + if let Some((test_scope, location)) = test_attribute { + if function.def.parameters.is_empty() + && matches!(test_scope, TestScope::OnlyFailWith { .. }) + { + let error = DefCollectorErrorKind::TestOnlyFailWithWithoutParameters { location }; + errors.push(error.into()); + } + } + + if let Some((_, location)) = fuzz_attribute { + if function.def.parameters.is_empty() { + let error = DefCollectorErrorKind::FuzzingHarnessWithoutParameters { location }; + errors.push(error.into()); + } + } + // Add function to scope/ns of the module let result = module_data.declare_function(name, visibility, func_id); if let Err((first_def, second_def)) = result { diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index 9679c37fa05..216a323572f 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -73,6 +73,12 @@ pub enum DefCollectorErrorKind { TestOnAssociatedFunction { location: Location }, #[error("The `#[export]` attribute may only be used on a non-associated function")] ExportOnAssociatedFunction { location: Location }, + #[error( + "The `#[test(only_fail_with = \"..\")]` attribute may only be used on functions with parameters" + )] + TestOnlyFailWithWithoutParameters { location: Location }, + #[error("The `#[fuzz]` attribute may only be used on functions with parameters")] + FuzzingHarnessWithoutParameters { location: Location }, } impl DefCollectorErrorKind { @@ -104,7 +110,9 @@ impl DefCollectorErrorKind { | DefCollectorErrorKind::ModuleOriginallyDefined { location, .. } | DefCollectorErrorKind::TraitImplOrphaned { location } | DefCollectorErrorKind::TraitMissingMethod { trait_impl_location: location, .. } - | DefCollectorErrorKind::ForeignImpl { location, .. } => *location, + | DefCollectorErrorKind::ForeignImpl { location, .. } + | DefCollectorErrorKind::TestOnlyFailWithWithoutParameters { location } + | DefCollectorErrorKind::FuzzingHarnessWithoutParameters { location } => *location, DefCollectorErrorKind::NotATrait { not_a_trait_name: path } | DefCollectorErrorKind::TraitNotFound { trait_path: path } => path.location, DefCollectorErrorKind::UnsupportedNumericGenericType( @@ -279,6 +287,16 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic { String::new(), *location, ), + DefCollectorErrorKind::TestOnlyFailWithWithoutParameters { location } => Diagnostic::simple_error( + "The `#[test(only_fail_with = \"..\")]` attribute may only be used on functions with parameters".into(), + String::new(), + *location, + ), + DefCollectorErrorKind::FuzzingHarnessWithoutParameters { location } => Diagnostic::simple_error( + "The `#[fuzz]` attribute may only be used on functions with parameters".into(), + String::new(), + *location, + ), } } } diff --git a/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 6a869d47e67..bf50c5688c0 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -387,7 +387,7 @@ impl TestFunction { pub fn should_fail(&self) -> bool { match self.scope { TestScope::ShouldFailWith { .. } => true, - TestScope::None => false, + TestScope::OnlyFailWith { .. } | TestScope::None => false, } } @@ -397,6 +397,7 @@ impl TestFunction { match &self.scope { TestScope::None => None, TestScope::ShouldFailWith { reason } => reason.as_deref(), + TestScope::OnlyFailWith { reason } => Some(reason.as_str()), } } } diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index b9886e8638f..2644b5903d5 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -732,6 +732,9 @@ pub enum TestScope { /// if it fails with the specified reason. If the reason is None, then /// the test must unconditionally fail ShouldFailWith { reason: Option }, + /// If a test has a scope of OnlyFailWith, then it can only fail + /// if it fails with the specified reason. + OnlyFailWith { reason: String }, /// No scope is applied and so the test must pass None, } @@ -744,6 +747,9 @@ impl fmt::Display for TestScope { Some(failure_reason) => write!(f, "(should_fail_with = {failure_reason:?})"), None => write!(f, "(should_fail)"), }, + TestScope::OnlyFailWith { reason } => { + write!(f, "(only_fail_with = {reason:?})") + } } } } @@ -812,14 +818,31 @@ impl Attributes { } pub fn is_test_function(&self) -> bool { - matches!(self.function().map(|attr| &attr.kind), Some(FunctionAttributeKind::Test(_))) + self.as_test_function().is_some() + } + + pub fn as_test_function(&self) -> Option<(&TestScope, Location)> { + self.function().and_then(|attr| { + if let FunctionAttributeKind::Test(scope) = &attr.kind { + Some((scope, attr.location)) + } else { + None + } + }) } pub fn is_fuzzing_harness(&self) -> bool { - matches!( - self.function().map(|attr| &attr.kind), - Some(FunctionAttributeKind::FuzzingHarness(_)) - ) + self.as_fuzzing_harness().is_some() + } + + pub fn as_fuzzing_harness(&self) -> Option<(&FuzzingScope, Location)> { + self.function().and_then(|attr| { + if let FunctionAttributeKind::FuzzingHarness(scope) = &attr.kind { + Some((scope, attr.location)) + } else { + None + } + }) } /// True if these attributes mean the given function is an entry point function if it was diff --git a/compiler/noirc_frontend/src/parser/labels.rs b/compiler/noirc_frontend/src/parser/labels.rs index 604c87801f8..2f56bc65448 100644 --- a/compiler/noirc_frontend/src/parser/labels.rs +++ b/compiler/noirc_frontend/src/parser/labels.rs @@ -23,6 +23,7 @@ pub enum ParsingRuleLabel { Path, Pattern, Statement, + String, Term, TraitBound, TraitImplItem, @@ -54,6 +55,7 @@ impl fmt::Display for ParsingRuleLabel { ParsingRuleLabel::Path => write!(f, "path"), ParsingRuleLabel::Pattern => write!(f, "pattern"), ParsingRuleLabel::Statement => write!(f, "statement"), + ParsingRuleLabel::String => write!(f, "string"), ParsingRuleLabel::Term => write!(f, "term"), ParsingRuleLabel::TraitBound => write!(f, "trait bound"), ParsingRuleLabel::TraitImplItem => write!(f, "trait impl item"), diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 0862e408ea3..88b1f3c3f7a 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -544,6 +544,10 @@ impl<'a> Parser<'a> { self.expected_label(ParsingRuleLabel::Identifier); } + fn expected_string(&mut self) { + self.expected_label(ParsingRuleLabel::String); + } + fn expected_token(&mut self, token: Token) { self.errors.push(ParserError::expected_token( token, diff --git a/compiler/noirc_frontend/src/parser/parser/attributes.rs b/compiler/noirc_frontend/src/parser/parser/attributes.rs index 1caf9a41a13..068dc486256 100644 --- a/compiler/noirc_frontend/src/parser/parser/attributes.rs +++ b/compiler/noirc_frontend/src/parser/parser/attributes.rs @@ -333,6 +333,15 @@ impl Parser<'_> { Some(TestScope::ShouldFailWith { reason: None }) } } + "only_fail_with" => { + self.eat_or_error(Token::Assign); + if let Some(reason) = self.eat_str() { + Some(TestScope::OnlyFailWith { reason }) + } else { + self.expected_string(); + None + } + } _ => None, } } else { @@ -687,6 +696,14 @@ mod tests { parse_function_attribute_no_errors(src, expected); } + #[test] + fn parses_attribute_test_only_fail_with() { + let src = "#[test(only_fail_with = \"reason\")]"; + let reason = "reason".to_string(); + let expected = FunctionAttributeKind::Test(TestScope::OnlyFailWith { reason }); + parse_function_attribute_no_errors(src, expected); + } + #[test] fn parses_meta_attribute_single_identifier_no_arguments() { let src = "#[foo]"; diff --git a/docs/docs/tooling/testing.md b/docs/docs/tooling/testing.md index a87903543bc..3518bf05cf4 100644 --- a/docs/docs/tooling/testing.md +++ b/docs/docs/tooling/testing.md @@ -89,7 +89,7 @@ fn test_basic(a: Field, b: Field) { } ``` The test above is not expected to fail. By default, the fuzzer will run for 1 second and use 100000 executions (whichever comes first). All available threads will be used for each fuzz test. -The fuzz tests also work with `#[test(should_fail)]` and `#[test(should_fail_with = "")]`. For example: +The fuzz tests also work with `#[test(should_fail)]`, `#[test(should_fail_with = "")]` and `#[test(only_fail_with = "")]`. For example: ```rust #[test(should_fail)] @@ -105,7 +105,7 @@ or ```rust #[test(should_fail_with = "This is the message that will be checked for")] -fn fuzz_should_fail_with(a: [bool; 32]) { +fn test_should_fail_with(a: [bool; 32]) { let mut or_sum= false; for i in 0..32 { or_sum=or_sum|(a[i]==((i&1)as bool)); @@ -115,6 +115,13 @@ fn fuzz_should_fail_with(a: [bool; 32]) { } ``` +```rust +#[test(only_fail_with = "This is the message that will be checked for")] +fn test_add(a: u64, b: u64) { + assert((a+b-15)!=(a-b+30), "This is the message that will be checked for"); +} +``` + The underlying fuzzing mechanism is described in the [Fuzzing](../tooling/fuzzing) documentation. There are some fuzzing-specific options that can be used with `nargo test`: diff --git a/test_programs/compile_failure/test_and_fuzz_attribute_errors/Nargo.toml b/test_programs/compile_failure/test_and_fuzz_attribute_errors/Nargo.toml new file mode 100644 index 00000000000..142c31bd3cb --- /dev/null +++ b/test_programs/compile_failure/test_and_fuzz_attribute_errors/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "test_and_fuzz_attribute_errors" +type = "bin" +authors = [""] +compiler_version = ">=0.26.0" + +[dependencies] diff --git a/test_programs/compile_failure/test_and_fuzz_attribute_errors/src/main.nr b/test_programs/compile_failure/test_and_fuzz_attribute_errors/src/main.nr new file mode 100644 index 00000000000..475a1240449 --- /dev/null +++ b/test_programs/compile_failure/test_and_fuzz_attribute_errors/src/main.nr @@ -0,0 +1,9 @@ +// This is an error as we can't fuzz if there are no arguments +#[fuzz] +fn fuzz_no_arguments() {} + +// This is an error as `only_fail_with` implies fuzzing, but there are no arguments +#[test(only_fail_with = "error")] +fn test() {} + +fn main() {} diff --git a/test_programs/compile_failure/test_and_fuzz_attribute_errors/stderr.txt b/test_programs/compile_failure/test_and_fuzz_attribute_errors/stderr.txt new file mode 100644 index 00000000000..793a2789019 --- /dev/null +++ b/test_programs/compile_failure/test_and_fuzz_attribute_errors/stderr.txt @@ -0,0 +1,15 @@ +error: The `#[fuzz]` attribute may only be used on functions with parameters + ┌─ src/main.nr:2:1 + │ +2 │ #[fuzz] + │ ------- + │ + +error: The `#[test(only_fail_with = "..")]` attribute may only be used on functions with parameters + ┌─ src/main.nr:6:1 + │ +6 │ #[test(only_fail_with = "error")] + │ --------------------------------- + │ + +Aborting due to 2 previous errors diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 31597d29d5f..548bb8bbced 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -233,6 +233,7 @@ where TestScope::ShouldFailWith { reason } => { FuzzingScope::ShouldFailWith { reason: reason.clone() } } + TestScope::OnlyFailWith { reason } => FuzzingScope::OnlyFailWith { reason: reason.clone() }, TestScope::None => FuzzingScope::None, }; let location = test_function.location; diff --git a/tooling/nargo_fmt/src/formatter/attribute.rs b/tooling/nargo_fmt/src/formatter/attribute.rs index 02c29211de9..9ac178566e9 100644 --- a/tooling/nargo_fmt/src/formatter/attribute.rs +++ b/tooling/nargo_fmt/src/formatter/attribute.rs @@ -131,10 +131,10 @@ impl Formatter<'_> { self.write_current_token_and_bump(); // should_fail self.write_right_paren(); // ) } - TestScope::ShouldFailWith { reason: Some(..) } => { + TestScope::ShouldFailWith { reason: Some(..) } | TestScope::OnlyFailWith { .. } => { self.write_left_paren(); // ( self.skip_comments_and_whitespace(); - self.write_current_token_and_bump(); // should_fail_with + self.write_current_token_and_bump(); // should_fail_with | only_fail_with self.write_space(); self.write_token(Token::Assign); self.write_space(); @@ -389,6 +389,13 @@ mod tests { assert_format_attribute(src, expected); } + #[test] + fn format_test_only_fail_with_reason_attribute() { + let src = " #[ test ( only_fail_with=\"reason\" )] "; + let expected = "#[test(only_fail_with = \"reason\")]"; + assert_format_attribute(src, expected); + } + #[test] fn format_fuzz_attribute() { let src = " #[ fuzz ] "; @@ -397,7 +404,7 @@ mod tests { } #[test] - fn format_test_only_fail_with_reason_attribute() { + fn format_fuzz_only_fail_with_reason_attribute() { let src = " #[ fuzz ( only_fail_with=\"reason\" )] "; let expected = "#[fuzz(only_fail_with = \"reason\")]"; assert_format_attribute(src, expected);