Skip to content
Open
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
21 changes: 21 additions & 0 deletions crates/bulloak/tests/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -736,3 +736,24 @@ fn checks_no_submodule_multi_root_error() {

assert!(stderr.contains(r#"an error occurred while parsing the tree: separator missing at tree root #1 "no_submodule_multi_root". Expected to find `::` between the contract name and the function name when multiple roots exist"#));
}

/// Regression test for https://github.com/defi-wonderland/bulloak/pull/9#issuecomment-3710452952
/// When multiple roots share the same condition, check should report the missing hoisted setup hook.
#[test]
fn checks_missing_hoisted_setup_hook_for_shared_condition() {
let cwd = env::current_dir().unwrap();
let binary_path = get_binary_path();
let tree_path = cwd
.join("tests")
.join("check")
.join("hoisted_hook_regression.tree");

let output = cmd(&binary_path, "check", &tree_path, &["-l", "noir"]);
let stderr = String::from_utf8(output.stderr).unwrap();

assert!(
stderr.contains(r#"Missing setup hook "when_passing_valid_parameters""#),
"Expected check to report missing setup hook 'when_passing_valid_parameters', but it did not.\nStderr:\n{}",
stderr
);
}
9 changes: 9 additions & 0 deletions crates/bulloak/tests/check/hoisted_hook_regression.tree
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
hoisted_hook_regression::constructor_with_minter
└── when passing valid parameters
├── it sets name
└── it sets symbol

hoisted_hook_regression::constructor_with_initial_supply
└── when passing valid parameters
├── it sets name
└── it sets symbol
22 changes: 22 additions & 0 deletions crates/bulloak/tests/check/hoisted_hook_regression_test.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Generated by bulloak


mod constructor_with_minter {
#[test]
unconstrained fn test_when_passing_valid_parameters() {
when_passing_valid_parameters();
// it sets name
// it sets symbol
}

}

mod constructor_with_initial_supply {
#[test]
unconstrained fn test_when_passing_valid_parameters() {
when_passing_valid_parameters();
// it sets name
// it sets symbol
}

}
2 changes: 2 additions & 0 deletions crates/bulloak/tests/check/hoisted_setup_hooks_test.nr
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ unconstrained fn when_b() {


mod foo {
use super::when_b;
#[test]
unconstrained fn test_when_b() {
when_b();
Expand All @@ -21,6 +22,7 @@ mod foo {
}

mod bar {
// import is not present in this module. `check` ignores imports.
#[test]
unconstrained fn test_when_b() {
when_b();
Expand Down
40 changes: 40 additions & 0 deletions crates/bulloak/tests/scaffold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,3 +350,43 @@ fn scaffold_dissambiguates_function_name_collisions() {
assert_eq!(expected.trim(), actual.trim());
}
}

/// Regression test for https://github.com/defi-wonderland/bulloak/pull/9#issuecomment-3710452952
/// When multiple roots share the same condition, the hoisted setup hook should be generated.
#[test]
fn scaffold_noir_generates_hoisted_setup_hook_for_shared_condition() {
let cwd = env::current_dir().unwrap();
let binary_path = get_binary_path();
let tests_path = cwd.join("tests").join("scaffold");
let tree_path = tests_path.join("hoisted_hook_regression.tree");

let output = cmd(&binary_path, "scaffold", &tree_path, &["-l", "noir"]);
let actual = String::from_utf8(output.stdout).unwrap();

assert!(
actual.contains("unconstrained fn when_passing_valid_parameters()"),
"Expected hoisted setup hook 'when_passing_valid_parameters' to be generated, but it was not.\nActual output:\n{}",
actual
);

// Also verify the full expected output matches
let noir_filename = tests_path.join("hoisted_hook_regression_test.nr");
let expected = fs::read_to_string(noir_filename).unwrap();
assert_eq!(expected.trim(), actual.trim());
}

#[test]
fn scaffold_noir_generates_skips_modifiers_on_imported_hoks() {
let cwd = env::current_dir().unwrap();
let binary_path = get_binary_path();
let tests_path = cwd.join("tests").join("scaffold");
let tree_path = tests_path.join("hoisted_hook_skip_modifiers.tree");

let output = cmd(&binary_path, "scaffold", &tree_path, &["-l", "noir", "-m"]);
let actual = String::from_utf8(output.stdout).unwrap();

// Verify the full expected output matches
let noir_filename = tests_path.join("hoisted_hook_skip_modifiers_test.nr");
let expected = fs::read_to_string(noir_filename).unwrap();
assert_eq!(expected.trim(), actual.trim());
}
9 changes: 9 additions & 0 deletions crates/bulloak/tests/scaffold/hoisted_hook_regression.tree
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
hoisted_hook_regression::constructor_with_minter
└── when passing valid parameters
├── it sets name
└── it sets symbol

hoisted_hook_regression::constructor_with_initial_supply
└── when passing valid parameters
├── it sets name
└── it sets symbol
28 changes: 28 additions & 0 deletions crates/bulloak/tests/scaffold/hoisted_hook_regression_test.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Generated by bulloak

/// Setup hook for condition
unconstrained fn when_passing_valid_parameters() {
}


mod constructor_with_minter {
use super::when_passing_valid_parameters;
#[test]
unconstrained fn test_when_passing_valid_parameters() {
when_passing_valid_parameters();
// it sets name
// it sets symbol
}

}

mod constructor_with_initial_supply {
use super::when_passing_valid_parameters;
#[test]
unconstrained fn test_when_passing_valid_parameters() {
when_passing_valid_parameters();
// it sets name
// it sets symbol
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
hoisted_hook_skip_modifiers::constructor_with_minter
└── when passing valid parameters
├── it sets name
└── it sets symbol

hoisted_hook_skip_modifiers::constructor_with_initial_supply
└── when passing valid parameters
├── it sets name
└── it sets symbol
20 changes: 20 additions & 0 deletions crates/bulloak/tests/scaffold/hoisted_hook_skip_modifiers_test.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Generated by bulloak


mod constructor_with_minter {
#[test]
unconstrained fn test_when_passing_valid_parameters() {
// it sets name
// it sets symbol
}

}

mod constructor_with_initial_supply {
#[test]
unconstrained fn test_when_passing_valid_parameters() {
// it sets name
// it sets symbol
}

}
21 changes: 19 additions & 2 deletions crates/noir/src/check/rules/structural_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,7 @@ mod compare_trees_test {
functions: vec![],
modules: vec![Module {
name: "module1".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::SetupHook(SetupHook {
name: "when_something".to_string(),
})],
Expand Down Expand Up @@ -938,6 +939,7 @@ mod compare_trees_test {
functions: vec![],
modules: vec![Module {
name: "wrong_name".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::TestFunction(TestFunction {
name: "test_something".to_string(),
expect_fail: false,
Expand All @@ -950,6 +952,7 @@ mod compare_trees_test {
functions: vec![],
modules: vec![Module {
name: "correct_name".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::TestFunction(TestFunction {
name: "test_something".to_string(),
expect_fail: false,
Expand All @@ -974,6 +977,7 @@ mod compare_trees_test {
modules: vec![
Module {
name: "module_b".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::TestFunction(TestFunction {
name: "test_b".to_string(),
expect_fail: false,
Expand All @@ -983,6 +987,7 @@ mod compare_trees_test {
},
Module {
name: "module_a".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::TestFunction(TestFunction {
name: "test_a".to_string(),
expect_fail: false,
Expand All @@ -997,6 +1002,7 @@ mod compare_trees_test {
modules: vec![
Module {
name: "module_a".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::TestFunction(TestFunction {
name: "test_a".to_string(),
expect_fail: false,
Expand All @@ -1006,6 +1012,7 @@ mod compare_trees_test {
},
Module {
name: "module_b".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::TestFunction(TestFunction {
name: "test_b".to_string(),
expect_fail: false,
Expand Down Expand Up @@ -1040,9 +1047,14 @@ mod compare_trees_test {
let actual = Root {
functions: vec![],
modules: vec![
Module { name: "module_a".to_string(), functions: vec![] },
Module {
name: "module_a".to_string(),
imported_hooks: Vec::new(),
functions: vec![],
},
Module {
name: "module_b".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::TestFunction(TestFunction {
name: "test_foo".to_string(),
expect_fail: false,
Expand All @@ -1057,14 +1069,19 @@ mod compare_trees_test {
modules: vec![
Module {
name: "module_a".to_string(),
imported_hooks: Vec::new(),
functions: vec![Function::TestFunction(TestFunction {
name: "test_foo".to_string(),
expect_fail: false,
setup_hooks: vec![],
actions: vec![],
})],
},
Module { name: "module_b".to_string(), functions: vec![] },
Module {
name: "module_b".to_string(),
imported_hooks: Vec::new(),
functions: vec![],
},
],
};
let violations =
Expand Down
1 change: 1 addition & 0 deletions crates/noir/src/noir/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl ParsedNoirFile {
self.find_functions_recursive(node, &mut functions);
modules.push(Module {
name: self.extract_module_name(node),
imported_hooks: Vec::new(),
functions,
});
}
Expand Down
5 changes: 5 additions & 0 deletions crates/noir/src/scaffold/scaffoldable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ impl Scaffoldable for Module {
fn scaffold(&self, generate_setup_hooks: bool) -> String {
let mut output = format!("\nmod {} {{\n", self.name);

if generate_setup_hooks {
for import in &self.imported_hooks {
output.push_str(format!(" use super::{};\n", import.name).as_str());
}
}
Comment on lines +72 to +76
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice to have: pack super imports in the mods, this

use super::{
    when_shares_are_held,
    when_called_on_behalf_of_other_with_authwit
};

instead of the current behaviour

use super::when_shares_are_held;
use super::when_called_on_behalf_of_other_with_authwit;

That was generated with this change

Suggested change
if generate_setup_hooks {
for import in &self.imported_hooks {
output.push_str(format!(" use super::{};\n", import.name).as_str());
}
}
if generate_setup_hooks && !self.imported_hooks.is_empty() {
if self.imported_hooks.len() == 1 {
output.push_str(format!(" use super::{};\n", self.imported_hooks[0].name).as_str());
} else {
output.push_str(" use super::{\n");
for (i, import) in self.imported_hooks.iter().enumerate() {
output.push_str(&format!(" {}", import.name));
if i < self.imported_hooks.len() - 1 {
output.push_str(",\n");
} else {
output.push('\n');
}
}
output.push_str(" };\n");
}
output.push('\n');
}

// indent the definitions inside the module
for func in &self.functions {
for line in func.scaffold(generate_setup_hooks).lines() {
Expand Down
Loading