feat: add --skip-modifiers flag (#68)

README.md

@@ -114,9 +114,12 @@ $ bulloak scaffold -wf ./**/*.tree
 
 Note all tests are showing as passing when their body is empty. To prevent this,
 you can use the `-S` (or `--vm-skip`) option to add a `vm.skip(true);` at the
-beginning of each test function. This option will also add an import for 
+beginning of each test function. This option will also add an import for
 forge-std's `Test.sol` and all test contracts will inherit from it.
 
+You can skip emitting the modifiers by passing the `-m` (or `--skip--modifiers`)
+option. This way, the generated files will only include the test functions.
+
 ### Check That Your Code And Spec Match
 
 You can use `bulloak check` to make sure that your Solidity files match your
@@ -194,6 +197,10 @@ automatically fixed, and bulloak's output will reflect that.
 warn: 13 checks failed (run `bulloak check --fix <.tree files>` to apply 11 fixes)
 ```
 
+You can skip checking that the modifiers are present by passing the `-m`
+(or `--skip--modifiers`) option. This way, `bulloak` will not warn when a
+modifier is missing from the generated file.
+
 #### Rules
 
 The following rules are currently implemented:

src/check/context.rs

@@ -8,8 +8,11 @@ use std::{
     path::{Path, PathBuf},
 };
 
-use crate::hir::{self, Hir};
 use crate::{check::Violation, scaffold::emitter::Emitter};
+use crate::{
+    config::Config,
+    hir::{self, Hir},
+};
 
 use super::{location::Location, violation::ViolationKind};
 
@@ -35,14 +38,16 @@ pub(crate) struct Context {
     pub(crate) pt: SourceUnit,
     /// The comments present in the Solidity file.
     pub(crate) comments: Comments,
+    /// The config passed to `bulloak check`.
+    pub(crate) cfg: Config,
 }
 
 impl Context {
     /// Creates a new `Context`.
     ///
     /// This structure contains everything necessary to perform checks between
     /// trees and Solidity files.
-    pub(crate) fn new(tree: PathBuf) -> Result<Self, Violation> {
+    pub(crate) fn new(tree: PathBuf, cfg: Config) -> Result<Self, Violation> {
         let tree_path_cow = tree.to_string_lossy();
         let tree_contents = try_read_to_string(&tree)?;
         let hir = crate::hir::translate(&tree_contents, &Default::default()).map_err(|e| {
@@ -71,6 +76,7 @@ impl Context {
             src,
             pt,
             comments,
+            cfg: cfg.clone(),
         })
     }
 

src/check/mod.rs

@@ -32,14 +32,17 @@ pub struct Check {
     /// The set of tree files to use as spec.
     ///
     /// Solidity file names are inferred from the specs.
-    files: Vec<PathBuf>,
+    pub files: Vec<PathBuf>,
     /// Whether to fix any issues found.
     #[arg(long, group = "fix-violations", default_value_t = false)]
-    fix: bool,
+    pub fix: bool,
     /// When `--fix` is passed, use `--stdout` to direct output
     /// to standard output instead of writing to files.
     #[arg(long, requires = "fix-violations", default_value_t = false)]
-    stdout: bool,
+    pub stdout: bool,
+    /// Whether to emit modifiers.
+    #[arg(short = 'm', long, default_value_t = false)]
+    pub skip_modifiers: bool,
 }
 
 impl Default for Check {
@@ -52,13 +55,13 @@ impl Check {
     /// Entrypoint for `bulloak check`.
     ///
     /// Note that we don't deal with `solang_parser` errors at all.
-    pub(crate) fn run(&self, _cfg: &Config) -> anyhow::Result<()> {
+    pub(crate) fn run(&self, cfg: &Config) -> anyhow::Result<()> {
         let mut violations = Vec::new();
         let ctxs: Vec<Context> = self
             .files
             .iter()
             .filter_map(|tree_path| {
-                Context::new(tree_path.clone())
+                Context::new(tree_path.clone(), cfg.clone())
                     .map_err(|violation| violations.push(violation))
                     .ok()
             })

src/check/rules/structural_match.rs

@@ -129,13 +129,17 @@ fn check_fns_structure(
                 Some((sol_idx, _)) => present_fn_indices.push((hir_idx, sol_idx)),
                 // We didn't find a matching function, so this is a
                 // violation.
-                None => violations.push(Violation::new(
-                    ViolationKind::MatchingFunctionMissing(fn_hir.clone(), hir_idx),
-                    Location::Code(
-                        ctx.tree.to_string_lossy().into_owned(),
-                        fn_hir.span.start.line,
-                    ),
-                )),
+                None => {
+                    if !ctx.cfg.check().skip_modifiers {
+                        violations.push(Violation::new(
+                            ViolationKind::MatchingFunctionMissing(fn_hir.clone(), hir_idx),
+                            Location::Code(
+                                ctx.tree.to_string_lossy().into_owned(),
+                                fn_hir.span.start.line,
+                            ),
+                        ))
+                    }
+                }
             }
         };
     }

src/config.rs

@@ -20,15 +20,24 @@ impl Config {
         }
     }
 
+    pub(crate) fn check(&self) -> crate::check::Check {
+        match self.command.clone() {
+            Commands::Check(c) => c,
+            Commands::Scaffold(_) => Default::default(),
+        }
+    }
+
     /// Return a cloned `Config` instance with `with_vm_skip` set to the passed
     /// value.
-    #[must_use] pub fn with_vm_skip(&self, with_vm_skip: bool) -> Config {
+    #[must_use]
+    pub fn with_vm_skip(&self, with_vm_skip: bool) -> Config {
         if let Commands::Scaffold(ref s) = self.command {
             return Config {
                 command: Commands::Scaffold(crate::scaffold::Scaffold {
                     with_vm_skip,
                     ..s.clone()
                 }),
+                ..Config::default()
             };
         }
 

src/hir/combiner.rs

@@ -406,7 +406,6 @@ bulloak error: contract name missing at tree root #2";
         // Append a comment HIR to the hirs.
         hirs.push(root(vec![comment("this is a random comment".to_owned())]));
 
-        println!("{:#?}", hirs);
         let text = trees.join("\n\n");
         let children = match combine(&text, hirs).unwrap() {
             Hir::Root(root) => root.children,

src/hir/mod.rs

@@ -9,7 +9,9 @@ pub mod visitor;
 
 pub use hir::*;
 
-use crate::{config::Config, scaffold::modifiers::ModifierDiscoverer, utils::split_trees};
+use crate::{
+    config::Config, error, scaffold::modifiers::ModifierDiscoverer, syntax, utils::split_trees,
+};
 
 /// High-level function that returns a HIR given the contents of a `.tree` file.
 ///
@@ -19,25 +21,25 @@ pub fn translate(text: &str, cfg: &Config) -> anyhow::Result<Hir> {
     Ok(translate_and_combine_trees(text, cfg)?)
 }
 
-/// Generates the HIR for a single tree.
-///
-/// This function leverages [`crate::syntax::parse`] and [`crate::hir::translator::Translator::translate`]
-/// to hide away most of the complexity of `bulloak`'s internal compiler.
-pub fn translate_tree_to_hir(tree: &str, cfg: &Config) -> crate::error::Result<Hir> {
-    let ast = crate::syntax::parse(tree)?;
-    let mut discoverer = ModifierDiscoverer::new();
-    let modifiers = discoverer.discover(&ast);
-    Ok(translator::Translator::new().translate(&ast, modifiers, cfg))
-}
-
 /// High-level function that returns a HIR given the contents of a `.tree` file.
 ///
 /// This function leverages [`translate_tree_to_hir`] to generate the HIR for each tree,
 /// and [`crate::hir::combiner::Combiner::combine`] to combine the HIRs into a single HIR.
-pub(crate) fn translate_and_combine_trees(text: &str, cfg: &Config) -> crate::error::Result<Hir> {
+pub(crate) fn translate_and_combine_trees(text: &str, cfg: &Config) -> error::Result<Hir> {
     let trees = split_trees(text);
     let hirs = trees
         .map(|tree| translate_tree_to_hir(tree, cfg))
-        .collect::<crate::error::Result<Vec<Hir>>>()?;
+        .collect::<error::Result<Vec<Hir>>>()?;
     Ok(combiner::Combiner::new().combine(text, hirs)?)
 }
+
+/// Generates the HIR for a single tree.
+///
+/// This function leverages [`crate::syntax::parse`] and [`crate::hir::translator::Translator::translate`]
+/// to hide away most of the complexity of `bulloak`'s internal compiler.
+pub fn translate_tree_to_hir(tree: &str, cfg: &Config) -> error::Result<Hir> {
+    let ast = syntax::parse(tree)?;
+    let mut discoverer = ModifierDiscoverer::new();
+    let modifiers = discoverer.discover(&ast);
+    Ok(translator::Translator::new().translate(&ast, modifiers, cfg))
+}

src/scaffold/mod.rs

@@ -45,6 +45,9 @@ pub struct Scaffold {
     /// Whether to add vm.skip(true) at the begining of each test.
     #[arg(short = 'S', long = "vm-skip", default_value_t = false)]
     pub with_vm_skip: bool,
+    /// Whether to emit modifiers.
+    #[arg(short = 'm', long, default_value_t = false)]
+    pub skip_modifiers: bool,
 }
 
 impl Default for Scaffold {

src/sol/translator.rs

@@ -38,17 +38,21 @@ pub(crate) struct Translator {
     sol_version: String,
     /// A flag indicating if there is a forge-std dependency.
     with_forge_std: bool,
+    /// Whether to emit modifiers.
+    skip_modifiers: bool,
 }
 
 impl Translator {
     /// Create a new translator.
     #[must_use]
     pub(crate) fn new(cfg: &Config) -> Self {
-        let cfg = cfg.scaffold();
-        let with_forge_std = [cfg.with_vm_skip].into_iter().any(|f| f);
+        let scaffold_cfg = cfg.scaffold();
+        let with_forge_std = [scaffold_cfg.with_vm_skip].into_iter().any(|f| f);
+
         Self {
-            sol_version: cfg.solidity_version,
+            sol_version: scaffold_cfg.solidity_version,
             with_forge_std,
+            skip_modifiers: scaffold_cfg.skip_modifiers,
         }
     }
 
@@ -441,6 +445,9 @@ impl Visitor for TranslatorI {
         let mut parts = Vec::with_capacity(contract.children.len());
         for child in &contract.children {
             if let Hir::FunctionDefinition(function) = child {
+                if function.is_modifier() && self.translator.skip_modifiers {
+                    continue;
+                }
                 parts.push(self.visit_function(function)?);
             }
         }

tests/check.rs

@@ -51,6 +51,20 @@ fn checks_valid_structural_match() {
     assert!(stdout.contains("All checks completed successfully! No issues found."));
 }
 
+#[test]
+fn checks_modifiers_skipped() {
+    let cwd = env::current_dir().unwrap();
+    let binary_path = get_binary_path();
+    let tree_path = cwd.join("tests").join("check").join("skip_modifiers.tree");
+
+    let output = cmd(&binary_path, "check", &tree_path, &["-m"]);
+    let stderr = String::from_utf8(output.stderr).unwrap();
+    let stdout = String::from_utf8(output.stdout).unwrap();
+
+    assert_eq!("", stderr);
+    assert!(stdout.contains("All checks completed successfully! No issues found."));
+}
+
 #[test]
 fn checks_missing_sol_file() {
     let cwd = env::current_dir().unwrap();
@@ -241,7 +255,6 @@ fn fixes_extra_fn_plus_wrong_order() {
 
     let output = cmd(&binary_path, "check", &tree_path, &["--fix", "--stdout"]);
     let actual = String::from_utf8(output.stdout).unwrap();
-    println!("what {}", actual);
 
     let expected = r"// SPDX-License-Identifier: UNLICENSED
 pragma solidity 0.8.0;

tests/check/skip_modifiers.t.sol

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.8.0;
+
+contract HashPairTestSanitize {
+    function test_ShouldNeverRevert() external {
+        // It should never revert.
+    }
+
+    function test_WhenFirstArgIsSmallerThanSecondArg() external whenFirstArgIsSmallerThanSecondArg {
+        // It should match the result of `keccak256(abi.encodePacked(a,b))`.
+    }
+
+    function test_WhenFirstArgIsZero() external whenFirstArgIsSmallerThanSecondArg {
+        // It should do something.
+    }
+
+    function test_WhenFirstArgIsBiggerThanSecondArg() external {
+        // It should match the result of `keccak256(abi.encodePacked(b,a))`.
+    }
+}

tests/check/skip_modifiers.tree

@@ -0,0 +1,8 @@
+HashPairTest.Sanitize
+├── It should never revert.
+├── When first arg is smaller than second arg
+│   ├── When first arg is zero
+│   │   └── It should do something.
+│   └── It should match the result of `keccak256(abi.encodePacked(a,b))`.
+└── When first arg is bigger than second arg
+    └── It should match the result of `keccak256(abi.encodePacked(b,a))`.

tests/scaffold.rs

@@ -69,6 +69,27 @@ fn scaffolds_trees_with_vm_skip() {
     }
 }
 
+#[test]
+fn scaffolds_trees_with_skip_modifiers() {
+    let cwd = env::current_dir().unwrap();
+    let binary_path = get_binary_path();
+    let tests_path = cwd.join("tests").join("scaffold");
+    let trees = ["skip_modifiers.tree"];
+
+    for tree_name in trees {
+        let tree_path = tests_path.join(tree_name);
+        let output = cmd(&binary_path, "scaffold", &tree_path, &["-m"]);
+        let actual = String::from_utf8(output.stdout).unwrap();
+
+        let mut output_file = tree_path.clone();
+        output_file.set_extension("t.sol");
+        let expected = fs::read_to_string(output_file).unwrap();
+
+        // We trim here because we don't care about ending newlines.
+        assert_eq!(expected.trim(), actual.trim());
+    }
+}
+
 #[test]
 fn skips_trees_when_file_exists() {
     let cwd = env::current_dir().unwrap();

tests/scaffold/skip_modifiers.t.sol

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.8.0;
+
+contract HashPairTestSanitize {
+    function test_ShouldNeverRevert() external {
+        // It should never revert.
+    }
+
+    function test_WhenFirstArgIsSmallerThanSecondArg() external whenFirstArgIsSmallerThanSecondArg {
+        // It should match the result of `keccak256(abi.encodePacked(a,b))`.
+    }
+
+    function test_WhenFirstArgIsZero() external whenFirstArgIsSmallerThanSecondArg {
+        // It should do something.
+    }
+
+    function test_WhenFirstArgIsBiggerThanSecondArg() external {
+        // It should match the result of `keccak256(abi.encodePacked(b,a))`.
+    }
+}

tests/scaffold/skip_modifiers.tree

@@ -0,0 +1,8 @@
+HashPairTest.Sanitize
+├── It should never revert.
+├── When first arg is smaller than second arg
+│   ├── When first arg is zero
+│   │   └── It should do something.
+│   └── It should match the result of `keccak256(abi.encodePacked(a,b))`.
+└── When first arg is bigger than second arg
+    └── It should match the result of `keccak256(abi.encodePacked(b,a))`.