From 8fd577a244fb426cbc155068a8e8ecccd1dc1c25 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Fri, 14 Apr 2023 09:00:22 +0200 Subject: [PATCH 01/57] Parse FuzzConfig from string (brief impl) Unit tests --- config/src/fuzz.rs | 110 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 02937b64f4a00..ec5431289bbc3 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -1,8 +1,12 @@ //! Configuration for fuzz testing +use std::error::Error; + use ethers_core::types::U256; use serde::{Deserialize, Serialize}; +use crate::Config; + /// Contains for fuzz testing #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub struct FuzzConfig { @@ -35,6 +39,67 @@ impl Default for FuzzConfig { } } +impl FuzzConfig { + /// Parses a [`FuzzConfig`] from Solidity function comments.
+ /// This is intended to override general fuzzer configs for the current + /// execution profile (see [`Config`]). + /// + /// An example of compatible Solidity comments + /// + /// ```solidity + /// contract MyTest is Test { + /// // forge-config: default.fuzz.runs = 100 + /// // forge-config: ci.fuzz.runs = 500 + /// function test_SimpleFuzzTest(uint256 x) public {...} + /// + /// // forge-config: default.fuzz.runs = 500 + /// // forge-config: ci.fuzz.runs = 10000 + /// function test_ImportantFuzzTest(uint256 x) public {...} + /// } + /// ``` + pub fn parse>(text: S) -> Result> { + let profile = Config::selected_profile().to_string(); + let prefix = format!("forge-config:{profile}.fuzz."); + + let mut conf = Self::default(); + + // Get all lines containing a `forge-config:` prefix + let lines = text + .as_ref() + .split('\n') + .map(Self::remove_whitespaces) + .filter(|l| l.starts_with(&prefix)); + + // i.e. line = "forge-config:default.fuzz.runs=500" + for line in lines { + // i.e. pair = ["forge-config:default.fuzz.", "runs=500"] + let pair = line.split(&prefix).collect::>(); + + // i.e. assignment = "runs=500" + if let Some(assignment) = pair.last() { + // i.e. key_value = "['runs', '500']" + let key_value = assignment.split('=').collect::>(); + if let Some(key) = key_value.first() { + if let Some(value) = key_value.last() { + match key.to_owned() { + "runs" => conf.runs = value.parse()?, + "max-test-rejects" => conf.max_test_rejects = value.parse()?, + "seed" => conf.seed = Some(U256::zero()), + _ => {} + } + } + } + } + } + + Ok(conf) + } + + fn remove_whitespaces(s: &str) -> String { + s.chars().filter(|c| !c.is_whitespace()).collect() + } +} + /// Contains for fuzz testing #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub struct FuzzDictionaryConfig { @@ -70,3 +135,48 @@ impl Default for FuzzDictionaryConfig { } } } + +#[cfg(test)] +mod tests { + use crate::FuzzConfig; + + #[test] + fn parse_config_default_profile() { + let conf = "forge-config: default.fuzz.runs = 600 \n forge-config: ci.fuzz.runs = 500 "; + let parsed = FuzzConfig::parse(conf).expect("Valid config"); + assert_eq!(parsed.runs, 600); + } + + #[test] + fn parse_config_white_spaces() { + let conf = "forge-config: default.fuzz.runs = 600 "; + let parsed = FuzzConfig::parse(conf).expect("Valid config"); + assert_eq!(parsed.runs, 600); + } + + #[test] + fn parse_config_noisy_text() { + let conf = "Free text comment forge-config: default.fuzz.runs = 600 "; + let parsed = FuzzConfig::parse(conf).expect("Valid config"); + let conf = FuzzConfig::default(); + assert_eq!(parsed, conf); + } + + #[test] + fn parse_config_error() { + let conf = "forge-config:default.fuzz.runs = foo \n "; + let parsed = FuzzConfig::parse(conf); + assert!(parsed.is_err()); + } + + #[test] + fn parse_config_ci_profile() { + figment::Jail::expect_with(|jail| { + jail.set_env("FOUNDRY_PROFILE", "ci"); + let conf = "forge-config: default.fuzz.runs = 500 \n forge-config: ci.fuzz.runs = 500 "; + let parsed = FuzzConfig::parse(conf).expect("Valid config"); + assert_eq!(parsed.runs, 500); + Ok(()) + }); + } +} From d8f91b8fc31212f0830b0e04a7fdb1ab5544fadd Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Fri, 14 Apr 2023 17:53:37 +0200 Subject: [PATCH 02/57] ConfParser trait is able to extract configurations out of a structured text Unit tests --- config/src/conf_parser.rs | 121 ++++++++++++++++++++++++++++++++++++++ config/src/lib.rs | 2 + 2 files changed, 123 insertions(+) create mode 100644 config/src/conf_parser.rs diff --git a/config/src/conf_parser.rs b/config/src/conf_parser.rs new file mode 100644 index 0000000000000..aa933fe304404 --- /dev/null +++ b/config/src/conf_parser.rs @@ -0,0 +1,121 @@ +use std::num::ParseIntError; + +/// Errors returned by the [`ConfParser`] trait. +#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] +pub enum ConfParserError { + #[error("'{0}' is not a valid config property")] + InvalidConfigProperty(String), + #[error(transparent)] + ParseIntError(#[from] ParseIntError), +} + +/// This trait is intended to parse configurations from +/// structured text. Foundry users can annotate Solidity test functions, +/// providing special configs just for the execution of a specific test. +/// +/// An example: +/// +/// ```solidity +/// contract MyTest is Test { +/// // forge-config: default.fuzz.runs = 100 +/// // forge-config: ci.fuzz.runs = 500 +/// function test_SimpleFuzzTest(uint256 x) public {...} +/// +/// // forge-config: default.fuzz.runs = 500 +/// // forge-config: ci.fuzz.runs = 10000 +/// function test_ImportantFuzzTest(uint256 x) public {...} +/// } +/// ``` +pub trait ConfParser { + /// Returns a prefix that is common to all valid configuration lines. + /// That helps the parser to extract correct values out of a text. + fn config_prefix() -> String; + + /// Returns + /// * `Some(Self)`in case `text` contains a valid configuration for `Self`. + /// * `None` in case `text` does NOT contain any configuration matching `config_prefix`. + /// * `Err(ConfParserError)` in case of wrong configuration. + fn parse>(text: S) -> Result, ConfParserError> + where + Self: Sized + 'static; + + /// Given a configuration `text` returns all available pairs (key, value) + /// matching the `config_prefix` + fn config_variables>(text: S) -> Vec<(String, String)> { + let mut result: Vec<(String, String)> = vec![]; + + let prefix = Self::config_prefix(); + + text.as_ref() + .split('\n') + .map(remove_whitespaces) + .filter(|l| l.starts_with(&prefix)) + .for_each(|line| { + // i.e. ["forge-config:default.fuzz.", "runs=500"] + let pair = line.split(&prefix).collect::>(); + // i.e. "runs=500" + if let Some(assignment) = pair.last() { + // i.e. "['runs', '500']" + let key_value = assignment.split('=').collect::>(); + + if let Some(key) = key_value.first() { + if let Some(value) = key_value.last() { + result.push((key.to_string(), value.to_string())); + } + } + } + }); + + result + } +} + +fn remove_whitespaces(s: &str) -> String { + s.chars().filter(|c| !c.is_whitespace()).collect() +} + +#[cfg(test)] +mod tests { + use super::{ConfParser, ConfParserError}; + + #[test] + fn config_variables() { + let text = r#" + forge-config: default.fuzz.runs = 600 + forge-config: default.fuzz.foo = 700 + forge-config: default.fuzz.bar = 800 + invalid-prefix + "#; + + let vars = TestParser::config_variables(text); + assert_eq!( + vec![ + ("runs".to_string(), "600".to_string()), + ("foo".to_string(), "700".to_string()), + ("bar".to_string(), "800".to_string()) + ], + vars + ); + } + + #[test] + fn white_spaces_are_ignored() { + let text = "forge-config: default. fuzz.runs = 600"; + let vars = TestParser::config_variables(text); + assert_eq!(vec![("runs".to_string(), "600".to_string())], vars); + } + + struct TestParser; + impl ConfParser for TestParser { + fn config_prefix() -> String { + "forge-config:default.fuzz.".to_string() + } + + fn parse>(_text: S) -> Result, ConfParserError> + where + Self: Sized + 'static, + { + Ok(None) + } + } +} diff --git a/config/src/lib.rs b/config/src/lib.rs index b6dac70e17cf1..208dcd7598180 100644 --- a/config/src/lib.rs +++ b/config/src/lib.rs @@ -92,6 +92,8 @@ use crate::fs_permissions::PathPermission; pub use invariant::InvariantConfig; use providers::remappings::RemappingsProvider; +mod conf_parser; + /// Foundry configuration /// /// # Defaults From 94a18461e4b4579ad9d984fd25f21f6a60b7e2f0 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Fri, 14 Apr 2023 17:54:06 +0200 Subject: [PATCH 03/57] cargo +nightly fmt --- config/src/conf_parser.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/config/src/conf_parser.rs b/config/src/conf_parser.rs index aa933fe304404..2f5c2bb40edaf 100644 --- a/config/src/conf_parser.rs +++ b/config/src/conf_parser.rs @@ -12,7 +12,7 @@ pub enum ConfParserError { /// This trait is intended to parse configurations from /// structured text. Foundry users can annotate Solidity test functions, /// providing special configs just for the execution of a specific test. -/// +/// /// An example: /// /// ```solidity @@ -32,8 +32,8 @@ pub trait ConfParser { fn config_prefix() -> String; /// Returns - /// * `Some(Self)`in case `text` contains a valid configuration for `Self`. - /// * `None` in case `text` does NOT contain any configuration matching `config_prefix`. + /// * `Some(Self)`in case `text` contains a valid configuration for `Self`. + /// * `None` in case `text` does NOT contain any configuration matching `config_prefix`. /// * `Err(ConfParserError)` in case of wrong configuration. fn parse>(text: S) -> Result, ConfParserError> where From efbaacb46d6bf70984b90c375252389444ea7f96 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Fri, 14 Apr 2023 17:55:18 +0200 Subject: [PATCH 04/57] FuzzConfig implements ConfParser trait Unit tests --- config/src/fuzz.rs | 133 ++++++++++++++++----------------------------- 1 file changed, 47 insertions(+), 86 deletions(-) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index ec5431289bbc3..020a3f71356d8 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -1,11 +1,12 @@ //! Configuration for fuzz testing -use std::error::Error; - use ethers_core::types::U256; use serde::{Deserialize, Serialize}; -use crate::Config; +use crate::{ + conf_parser::{ConfParser, ConfParserError}, + Config, +}; /// Contains for fuzz testing #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -39,64 +40,32 @@ impl Default for FuzzConfig { } } -impl FuzzConfig { - /// Parses a [`FuzzConfig`] from Solidity function comments.
- /// This is intended to override general fuzzer configs for the current - /// execution profile (see [`Config`]). - /// - /// An example of compatible Solidity comments - /// - /// ```solidity - /// contract MyTest is Test { - /// // forge-config: default.fuzz.runs = 100 - /// // forge-config: ci.fuzz.runs = 500 - /// function test_SimpleFuzzTest(uint256 x) public {...} - /// - /// // forge-config: default.fuzz.runs = 500 - /// // forge-config: ci.fuzz.runs = 10000 - /// function test_ImportantFuzzTest(uint256 x) public {...} - /// } - /// ``` - pub fn parse>(text: S) -> Result> { +impl ConfParser for FuzzConfig { + fn config_prefix() -> String { let profile = Config::selected_profile().to_string(); - let prefix = format!("forge-config:{profile}.fuzz."); - - let mut conf = Self::default(); - - // Get all lines containing a `forge-config:` prefix - let lines = text - .as_ref() - .split('\n') - .map(Self::remove_whitespaces) - .filter(|l| l.starts_with(&prefix)); + format!("forge-config:{profile}.fuzz.") + } - // i.e. line = "forge-config:default.fuzz.runs=500" - for line in lines { - // i.e. pair = ["forge-config:default.fuzz.", "runs=500"] - let pair = line.split(&prefix).collect::>(); + fn parse>(text: S) -> Result, ConfParserError> + where + Self: Sized + 'static, + { + let vars: Vec<(String, String)> = Self::config_variables::(text); + if vars.is_empty() { + return Ok(None) + } - // i.e. assignment = "runs=500" - if let Some(assignment) = pair.last() { - // i.e. key_value = "['runs', '500']" - let key_value = assignment.split('=').collect::>(); - if let Some(key) = key_value.first() { - if let Some(value) = key_value.last() { - match key.to_owned() { - "runs" => conf.runs = value.parse()?, - "max-test-rejects" => conf.max_test_rejects = value.parse()?, - "seed" => conf.seed = Some(U256::zero()), - _ => {} - } - } - } + let mut conf = Self::default(); + for pair in vars { + let key = pair.0; + let value = pair.1; + match key.as_str() { + "runs" => conf.runs = value.parse()?, + "max-test-rejects" => conf.max_test_rejects = value.parse()?, + _ => Err(ConfParserError::InvalidConfigProperty(key.to_string()))?, } } - - Ok(conf) - } - - fn remove_whitespaces(s: &str) -> String { - s.chars().filter(|c| !c.is_whitespace()).collect() + Ok(Some(conf)) } } @@ -138,45 +107,37 @@ impl Default for FuzzDictionaryConfig { #[cfg(test)] mod tests { - use crate::FuzzConfig; + use crate::{conf_parser::ConfParser, FuzzConfig}; #[test] - fn parse_config_default_profile() { - let conf = "forge-config: default.fuzz.runs = 600 \n forge-config: ci.fuzz.runs = 500 "; - let parsed = FuzzConfig::parse(conf).expect("Valid config"); - assert_eq!(parsed.runs, 600); - } - - #[test] - fn parse_config_white_spaces() { - let conf = "forge-config: default.fuzz.runs = 600 "; - let parsed = FuzzConfig::parse(conf).expect("Valid config"); - assert_eq!(parsed.runs, 600); - } - - #[test] - fn parse_config_noisy_text() { - let conf = "Free text comment forge-config: default.fuzz.runs = 600 "; - let parsed = FuzzConfig::parse(conf).expect("Valid config"); - let conf = FuzzConfig::default(); - assert_eq!(parsed, conf); - } - - #[test] - fn parse_config_error() { - let conf = "forge-config:default.fuzz.runs = foo \n "; - let parsed = FuzzConfig::parse(conf); - assert!(parsed.is_err()); + fn parse_config_default_profile() -> eyre::Result<()> { + let conf = "forge-config: default.fuzz.runs = 1024"; + let parsed = FuzzConfig::parse(conf)?.expect("Parsed config exists"); + assert_eq!(parsed.runs, 1024); + Ok(()) } #[test] fn parse_config_ci_profile() { figment::Jail::expect_with(|jail| { jail.set_env("FOUNDRY_PROFILE", "ci"); - let conf = "forge-config: default.fuzz.runs = 500 \n forge-config: ci.fuzz.runs = 500 "; - let parsed = FuzzConfig::parse(conf).expect("Valid config"); - assert_eq!(parsed.runs, 500); + let conf = r#" + forge-config: default.fuzz.runs = 1024 + forge-config: ci.fuzz.runs = 2048"#; + + let parsed = FuzzConfig::parse(conf).unwrap().expect("Parsed config exists"); + assert_eq!(parsed.runs, 2048); Ok(()) }); } + + #[test] + fn unrecognized_property() { + let conf = "forge-config: default.fuzz.unknownprop = 200"; + if let Err(e) = FuzzConfig::parse(conf) { + assert_eq!(e.to_string(), "'unknownprop' is not a valid config property"); + } else { + assert!(false) + } + } } From 6e770e0d1c40405f97831bf34b0d35ba8a61c26f Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Fri, 14 Apr 2023 18:11:35 +0200 Subject: [PATCH 05/57] InvariantConfig implements ConfParser trait Unit tests --- config/src/conf_parser.rs | 4 +- config/src/invariant.rs | 90 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/config/src/conf_parser.rs b/config/src/conf_parser.rs index 2f5c2bb40edaf..22af5cfdcbbaf 100644 --- a/config/src/conf_parser.rs +++ b/config/src/conf_parser.rs @@ -1,4 +1,4 @@ -use std::num::ParseIntError; +use std::{num::ParseIntError, str::ParseBoolError}; /// Errors returned by the [`ConfParser`] trait. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] @@ -7,6 +7,8 @@ pub enum ConfParserError { InvalidConfigProperty(String), #[error(transparent)] ParseIntError(#[from] ParseIntError), + #[error(transparent)] + ParseBoolError(#[from] ParseBoolError), } /// This trait is intended to parse configurations from diff --git a/config/src/invariant.rs b/config/src/invariant.rs index b71b88d525e62..e423c09ce9190 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -1,6 +1,10 @@ //! Configuration for invariant testing -use crate::fuzz::FuzzDictionaryConfig; +use crate::{ + conf_parser::{ConfParser, ConfParserError}, + fuzz::FuzzDictionaryConfig, + Config, +}; use serde::{Deserialize, Serialize}; /// Contains for invariant testing @@ -31,3 +35,87 @@ impl Default for InvariantConfig { } } } + +impl ConfParser for InvariantConfig { + fn config_prefix() -> String { + let profile = Config::selected_profile().to_string(); + format!("forge-config:{profile}.invariant.") + } + + fn parse>(text: S) -> Result, ConfParserError> + where + Self: Sized + 'static, + { + let vars: Vec<(String, String)> = Self::config_variables::(text); + if vars.is_empty() { + return Ok(None) + } + + let mut conf = Self::default(); + for pair in vars { + let key = pair.0; + let value = pair.1; + match key.as_str() { + "runs" => conf.runs = value.parse()?, + "depth" => conf.depth = value.parse()?, + "fail-on-revert" => conf.fail_on_revert = value.parse()?, + "call-override" => conf.call_override = value.parse()?, + _ => Err(ConfParserError::InvalidConfigProperty(key.to_string()))?, + } + } + Ok(Some(conf)) + } +} + +#[cfg(test)] +mod tests { + use crate::{conf_parser::ConfParser, InvariantConfig}; + + #[test] + fn parse_config_default_profile() -> eyre::Result<()> { + let conf = r#" + forge-config: default.invariant.runs = 1024 + forge-config: default.invariant.depth = 30 + forge-config: default.invariant.fail-on-revert = true + forge-config: default.invariant.call-override = false + "#; + + let parsed = InvariantConfig::parse(conf)?.expect("Parsed config exists"); + assert_eq!(parsed.runs, 1024); + assert_eq!(parsed.depth, 30); + assert_eq!(parsed.fail_on_revert, true); + assert_eq!(parsed.call_override, false); + + Ok(()) + } + + #[test] + fn parse_config_ci_profile() { + figment::Jail::expect_with(|jail| { + jail.set_env("FOUNDRY_PROFILE", "ci"); + let conf = r#" + forge-config: ci.invariant.runs = 1024 + forge-config: ci.invariant.depth = 30 + forge-config: ci.invariant.fail-on-revert = true + forge-config: ci.invariant.call-override = false + "#; + + let parsed = InvariantConfig::parse(conf).unwrap().expect("Parsed config exists"); + assert_eq!(parsed.runs, 1024); + assert_eq!(parsed.depth, 30); + assert_eq!(parsed.fail_on_revert, true); + assert_eq!(parsed.call_override, false); + Ok(()) + }); + } + + #[test] + fn unrecognized_property() { + let conf = "forge-config: default.invariant.unknownprop = 200"; + if let Err(e) = InvariantConfig::parse(conf) { + assert_eq!(e.to_string(), "'unknownprop' is not a valid config property"); + } else { + assert!(false) + } + } +} From b1db51f8572889aef772de397312e8bbe566aff1 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 15 Apr 2023 17:28:31 +0200 Subject: [PATCH 06/57] Parsing logic optimized Meaningful e2e test --- Cargo.lock | 1 + config/Cargo.toml | 1 + config/src/conf_parser.rs | 25 ++++++++++++------------- config/src/fuzz.rs | 31 +++++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6e87b6467883f..dc8c532a2e4d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2595,6 +2595,7 @@ dependencies = [ "semver", "serde", "serde_regex", + "solang-parser", "tempfile", "thiserror", "toml 0.7.3", diff --git a/config/Cargo.toml b/config/Cargo.toml index f71f98b6f698a..84c85dc370f19 100644 --- a/config/Cargo.toml +++ b/config/Cargo.toml @@ -46,3 +46,4 @@ path-slash = "0.2.1" pretty_assertions = "1.3.0" figment = { version = "0.10", features = ["test"] } tempfile = "3.5" +solang-parser = "0.2.3" diff --git a/config/src/conf_parser.rs b/config/src/conf_parser.rs index 22af5cfdcbbaf..b58bb4a13bdde 100644 --- a/config/src/conf_parser.rs +++ b/config/src/conf_parser.rs @@ -1,5 +1,7 @@ use std::{num::ParseIntError, str::ParseBoolError}; +use regex::Regex; + /// Errors returned by the [`ConfParser`] trait. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum ConfParserError { @@ -45,25 +47,22 @@ pub trait ConfParser { /// matching the `config_prefix` fn config_variables>(text: S) -> Vec<(String, String)> { let mut result: Vec<(String, String)> = vec![]; - let prefix = Self::config_prefix(); text.as_ref() .split('\n') .map(remove_whitespaces) - .filter(|l| l.starts_with(&prefix)) + .filter(|l| l.contains(&prefix)) + .map(|l| { + let pattern = format!("^.*{prefix}"); + let re = Regex::new(&pattern).unwrap(); + re.replace(&l, "").to_string() + }) .for_each(|line| { - // i.e. ["forge-config:default.fuzz.", "runs=500"] - let pair = line.split(&prefix).collect::>(); - // i.e. "runs=500" - if let Some(assignment) = pair.last() { - // i.e. "['runs', '500']" - let key_value = assignment.split('=').collect::>(); - - if let Some(key) = key_value.first() { - if let Some(value) = key_value.last() { - result.push((key.to_string(), value.to_string())); - } + let key_value = line.split('=').collect::>(); // "['runs', '500']" + if let Some(key) = key_value.first() { + if let Some(value) = key_value.last() { + result.push((key.to_string(), value.to_string())); } } }); diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 020a3f71356d8..d65b7733b696b 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -108,6 +108,7 @@ impl Default for FuzzDictionaryConfig { #[cfg(test)] mod tests { use crate::{conf_parser::ConfParser, FuzzConfig}; + use solang_parser::pt::Comment; #[test] fn parse_config_default_profile() -> eyre::Result<()> { @@ -140,4 +141,34 @@ mod tests { assert!(false) } } + + #[test] + fn e2e() -> eyre::Result<()> { + use solang_parser::parse; + let code = r#" + contract FuzzTestContract { + /** + * forge-config: default.fuzz.runs = 1023 + * forge-config: default.fuzz.max-test-rejects = 521 + */ + function testFuzz(string name) public returns (string) { + return name; + } + } + "#; + + let (_, comments) = parse(code, 0).expect("Valid code"); + let comm = &comments[0]; + match comm { + Comment::DocBlock(_, text) => { + let config = FuzzConfig::parse(text)?.expect("Valid config"); + assert_eq!(config.runs, 1023); + assert_eq!(config.max_test_rejects, 521); + } + _ => { + assert!(false); // Force test to fail + } + } + Ok(()) + } } From b0a0bfc86a24d706eb32b47f1ecb58e0e43fc6a3 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sun, 16 Apr 2023 11:56:51 +0200 Subject: [PATCH 07/57] Configurations can be parsed from project compilation output --- Cargo.lock | 1 + config/Cargo.toml | 1 + config/src/fuzz.rs | 2 +- config/src/{ => inline}/conf_parser.rs | 6 +- config/src/inline/mod.rs | 110 +++++++++++++++++++++++++ config/src/invariant.rs | 2 +- config/src/lib.rs | 3 +- 7 files changed, 121 insertions(+), 4 deletions(-) rename config/src/{ => inline}/conf_parser.rs (92%) create mode 100644 config/src/inline/mod.rs diff --git a/Cargo.lock b/Cargo.lock index dc8c532a2e4d7..65934f2cea156 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2594,6 +2594,7 @@ dependencies = [ "reqwest", "semver", "serde", + "serde_json", "serde_regex", "solang-parser", "tempfile", diff --git a/config/Cargo.toml b/config/Cargo.toml index 84c85dc370f19..8716366e162f1 100644 --- a/config/Cargo.toml +++ b/config/Cargo.toml @@ -19,6 +19,7 @@ figment = { version = "0.10", features = ["toml", "env"] } number_prefix = "0.4.0" serde = { version = "1.0", features = ["derive"] } serde_regex = "1.1.0" +serde_json = "1.0.95" toml = { version = "0.7", features = ["preserve_order"] } toml_edit = "0.19" diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index d65b7733b696b..3a7d1ebb0aecc 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -4,7 +4,7 @@ use ethers_core::types::U256; use serde::{Deserialize, Serialize}; use crate::{ - conf_parser::{ConfParser, ConfParserError}, + inline::{ConfParser, ConfParserError}, Config, }; diff --git a/config/src/conf_parser.rs b/config/src/inline/conf_parser.rs similarity index 92% rename from config/src/conf_parser.rs rename to config/src/inline/conf_parser.rs index b58bb4a13bdde..551f839f64d07 100644 --- a/config/src/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -5,10 +5,14 @@ use regex::Regex; /// Errors returned by the [`ConfParser`] trait. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum ConfParserError { + /// An invalid configuration property has been provided. + /// The property cannot be mapped to the configutaion object #[error("'{0}' is not a valid config property")] InvalidConfigProperty(String), + /// An error occurred while tryin to parse an integer configuration value #[error(transparent)] ParseIntError(#[from] ParseIntError), + /// An error occurred while tryin to parse a boolean configuration value #[error(transparent)] ParseBoolError(#[from] ParseBoolError), } @@ -59,7 +63,7 @@ pub trait ConfParser { re.replace(&l, "").to_string() }) .for_each(|line| { - let key_value = line.split('=').collect::>(); // "['runs', '500']" + let key_value = line.split('=').collect::>(); // i.e. "['runs', '500']" if let Some(key) = key_value.first() { if let Some(value) = key_value.last() { result.push((key.to_string(), value.to_string())); diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs new file mode 100644 index 0000000000000..d6130626550a7 --- /dev/null +++ b/config/src/inline/mod.rs @@ -0,0 +1,110 @@ +mod conf_parser; +pub use conf_parser::{ConfParser, ConfParserError}; +use ethers_solc::{artifacts::Node, ProjectCompileOutput}; +use serde_json::Value; +use std::collections::{BTreeMap, HashMap}; + +/// Represents per-test configurations, declared inline +/// as structured comments in Solidity test files. This allows +/// to create configs directly bound to solidity test. +/// `T` is the configuration type, and is bound to the [`ConfParser`] trait. Known +/// implementations of [`ConfParser`] include [`FuzzConfig`](super::FuzzConfig) and +/// [`InvariantConfig`](super::InvariantConfig)) +pub struct InlineConfig +where + T: ConfParser + 'static, +{ + /// Maps a (contract, test-function) + /// to a specific configuration provided by the user, in the + /// test function comments. + configs: HashMap<(String, String), T>, +} + +impl InlineConfig +where + T: ConfParser + 'static, +{ + /// Returns an inline configuration, if any, for `contract_name` and `fn_name`.
+ /// - `contract_name` The name of the contract containing the annotated function. Note this + /// the actual contract name, not the solidity file name. + /// - `fn_name` The name of the function annotated with an inline configuration. + pub fn get_config>(&self, contract_name: S, fn_name: S) -> Option<&T> { + self.configs.get(&(contract_name.into(), fn_name.into())) + } +} + +impl<'a, T> TryFrom<&'a ProjectCompileOutput> for InlineConfig +where + T: ConfParser + 'static, +{ + type Error = ConfParserError; + + /// Tries to detect inline configurations from the project compile output. + /// This object is known to emit function comments, that we can try to parse + /// at our convenience, to detect inline configurations. + fn try_from(output: &'a ProjectCompileOutput) -> Result { + let mut configs: HashMap<(String, String), T> = HashMap::new(); + + for artifact in output.artifacts() { + if let Some(ast) = artifact.1.ast.as_ref() { + let contract_name: String = artifact.0; + for node in ast.nodes.iter() { + try_apply(&mut configs, &contract_name, node)?; + } + } + } + + Ok(Self { configs }) + } +} + +/// Implements a DFS over a compiler output node and its children. +/// If a configuration is found for a solidity function, it is added to +/// `map` under the (contract name, function name) key. +/// This function may result in parsing errors (see [`ConfParserError`]). +fn try_apply( + map: &mut HashMap<(String, String), T>, + contract_name: &str, + node: &Node, +) -> Result<(), ConfParserError> +where + T: ConfParser + 'static, +{ + for n in node.nodes.iter() { + if let Some((fn_name, fn_docs)) = get_fn_data(n) { + if let Some(config) = T::parse(fn_docs)? { + let key = (contract_name.into(), fn_name); + map.insert(key, config); + } + } + + try_apply(map, contract_name, n)?; + } + Ok(()) +} + +fn get_fn_data(node: &Node) -> Option<(String, String)> { + if format!("{:?}", node.node_type) == *"FunctionDefinition" { + let fn_data = &node.other; + let fn_name: String = get_fn_name(fn_data)?; + let fn_docs: String = get_fn_docs(fn_data)?; + return Some((fn_name, fn_docs)) + } + None +} + +fn get_fn_name(fn_data: &BTreeMap) -> Option { + match fn_data.get("name")? { + Value::String(fn_name) => Some(fn_name.into()), + _ => None, + } +} + +fn get_fn_docs(fn_data: &BTreeMap) -> Option { + if let Value::Object(fn_docs) = fn_data.get("documentation")? { + if let Value::String(comment) = fn_docs.get("text")? { + return Some(comment.into()) + } + } + None +} diff --git a/config/src/invariant.rs b/config/src/invariant.rs index e423c09ce9190..fa3b0bf96354b 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -1,8 +1,8 @@ //! Configuration for invariant testing use crate::{ - conf_parser::{ConfParser, ConfParserError}, fuzz::FuzzDictionaryConfig, + inline::{ConfParser, ConfParserError}, Config, }; use serde::{Deserialize, Serialize}; diff --git a/config/src/lib.rs b/config/src/lib.rs index 208dcd7598180..b94e13487b321 100644 --- a/config/src/lib.rs +++ b/config/src/lib.rs @@ -92,7 +92,8 @@ use crate::fs_permissions::PathPermission; pub use invariant::InvariantConfig; use providers::remappings::RemappingsProvider; -mod conf_parser; +mod inline; +pub use inline::{ConfParserError, InlineConfig}; /// Foundry configuration /// From 1d38733eed7044219a25330c9873d5c3809324c4 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sun, 16 Apr 2023 11:57:31 +0200 Subject: [PATCH 08/57] E2E tests for inline configuration load --- forge/tests/it/inline.rs | 33 +++++++++++++++++++ forge/tests/it/main.rs | 1 + testdata/inline/FuzzInlineConf.t.sol | 12 +++++++ testdata/inline/InvariantInlineConf.t.sol | 39 +++++++++++++++++++++++ 4 files changed, 85 insertions(+) create mode 100644 forge/tests/it/inline.rs create mode 100644 testdata/inline/FuzzInlineConf.t.sol create mode 100644 testdata/inline/InvariantInlineConf.t.sol diff --git a/forge/tests/it/inline.rs b/forge/tests/it/inline.rs new file mode 100644 index 0000000000000..713a41fbfaac0 --- /dev/null +++ b/forge/tests/it/inline.rs @@ -0,0 +1,33 @@ +#[cfg(test)] +mod tests { + use foundry_config::{FuzzConfig, InlineConfig, InvariantConfig}; + use crate::test_helpers::COMPILED; + + #[test] + fn inline_fuzz_config() { + let compiled = COMPILED.clone(); + if let Ok(conf) = InlineConfig::::try_from(&compiled) { + // Inline config defined in testdata/inline/FuzzInlineConf.t.sol + let contract_name = "FuzzInlineConf"; + let function_name = "testFailFuzz"; + let inline_config: &FuzzConfig = conf.get_config(contract_name, function_name).unwrap(); + assert_eq!(inline_config.runs, 1024); + assert_eq!(inline_config.max_test_rejects, 500); + } + } + + #[test] + fn inline_invariant_config() { + let compiled = COMPILED.clone(); + if let Ok(conf) = InlineConfig::::try_from(&compiled) { + // Inline config defined in testdata/inline/InvariantInlineConf.t.sol + let contract_name = "InvariantInlineConf"; + let function_name = "invariant_neverFalse"; + let inline_config: &InvariantConfig = conf.get_config(contract_name, function_name).unwrap(); + assert_eq!(inline_config.runs, 333); + assert_eq!(inline_config.depth, 32); + assert_eq!(inline_config.fail_on_revert, false); + assert_eq!(inline_config.call_override, true); + } + } +} diff --git a/forge/tests/it/main.rs b/forge/tests/it/main.rs index aca6b9d5b365a..fda47212122aa 100644 --- a/forge/tests/it/main.rs +++ b/forge/tests/it/main.rs @@ -4,6 +4,7 @@ mod core; mod fork; mod fs; mod fuzz; +mod inline; mod invariant; mod repros; pub mod test_helpers; diff --git a/testdata/inline/FuzzInlineConf.t.sol b/testdata/inline/FuzzInlineConf.t.sol new file mode 100644 index 0000000000000..a90cba2ee5894 --- /dev/null +++ b/testdata/inline/FuzzInlineConf.t.sol @@ -0,0 +1,12 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity >=0.8.0; + +import "ds-test/test.sol"; + +contract FuzzInlineConf is DSTest { + /// forge-config: default.fuzz.runs = 1024 + /// forge-config: default.fuzz.max-test-rejects = 500 + function testFailFuzz(uint8 x) public { + require(x > 128, "should revert"); + } +} diff --git a/testdata/inline/InvariantInlineConf.t.sol b/testdata/inline/InvariantInlineConf.t.sol new file mode 100644 index 0000000000000..90193262347d6 --- /dev/null +++ b/testdata/inline/InvariantInlineConf.t.sol @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: Unlicense +pragma solidity >=0.8.0; + +import "ds-test/test.sol"; + +contract InvariantBreaker { + bool public flag0 = true; + bool public flag1 = true; + + function set0(int256 val) public returns (bool) { + if (val % 100 == 0) { + flag0 = false; + } + return flag0; + } + + function set1(int256 val) public returns (bool) { + if (val % 10 == 0 && !flag0) { + flag1 = false; + } + return flag1; + } +} + +contract InvariantInlineConf is DSTest { + InvariantBreaker inv; + + function setUp() public { + inv = new InvariantBreaker(); + } + + /// forge-config: default.invariant.runs = 333 + /// forge-config: default.invariant.depth = 32 + /// forge-config: default.invariant.fail-on-revert = false + /// forge-config: default.invariant.call-override = true + function invariant_neverFalse() public { + require(inv.flag1(), "false."); + } +} From 7d540c0c5a34ecf1697f46b50ba8c6eef8504ef9 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 18 Apr 2023 14:38:45 +0200 Subject: [PATCH 09/57] - ConfParser: parse fn is now try_merge - TestOptions struct extended to track test specific configs - Tests --- cli/src/cmd/forge/test/mod.rs | 12 ++++-- config/src/fuzz.rs | 23 ++++++----- config/src/inline/conf_parser.rs | 12 +++--- config/src/inline/mod.rs | 35 ++++++++++------- config/src/invariant.rs | 21 ++++++----- forge/src/lib.rs | 65 +++++++++++++++++++++++++++++--- forge/src/multi_runner.rs | 2 +- forge/tests/it/inline.rs | 11 ++++-- 8 files changed, 129 insertions(+), 52 deletions(-) diff --git a/cli/src/cmd/forge/test/mod.rs b/cli/src/cmd/forge/test/mod.rs index 2773a0877111b..35110660d55e8 100644 --- a/cli/src/cmd/forge/test/mod.rs +++ b/cli/src/cmd/forge/test/mod.rs @@ -18,7 +18,7 @@ use forge::{ identifier::{EtherscanIdentifier, LocalTraceIdentifier, SignaturesIdentifier}, CallTraceDecoderBuilder, TraceKind, }, - MultiContractRunner, MultiContractRunnerBuilder, TestOptions, + MultiContractRunner, MultiContractRunnerBuilder, TestOptions, TestOptionsBuilder, }; use foundry_common::{ compile::{self, ProjectCompiler}, @@ -121,8 +121,6 @@ impl TestArgs { // Merge all configs let (mut config, mut evm_opts) = self.load_config_and_evm_opts_emit_warnings()?; - let test_options = TestOptions { fuzz: config.fuzz, invariant: config.invariant }; - let mut filter = self.filter(&config); trace!(target: "forge::test", ?filter, "using filter"); @@ -148,6 +146,12 @@ impl TestArgs { compiler.compile(&project) }?; + let test_options: TestOptions = TestOptionsBuilder::default() + .fuzz(config.fuzz) + .invariant(config.invariant) + .compile_output(&output) + .build()?; + // Determine print verbosity and executor verbosity let verbosity = evm_opts.verbosity; if self.gas_report && evm_opts.verbosity < 3 { @@ -165,7 +169,7 @@ impl TestArgs { .sender(evm_opts.sender) .with_fork(evm_opts.get_fork(&config, env.clone())) .with_cheats_config(CheatsConfig::new(&config, &evm_opts)) - .with_test_options(test_options) + .with_test_options(test_options.clone()) .build(project.paths.root, output, env, evm_opts)?; if self.debug.is_some() { diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 3a7d1ebb0aecc..180282f12c351 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -46,16 +46,17 @@ impl ConfParser for FuzzConfig { format!("forge-config:{profile}.fuzz.") } - fn parse>(text: S) -> Result, ConfParserError> + fn try_merge>(&self, text: S) -> Result where Self: Sized + 'static, { let vars: Vec<(String, String)> = Self::config_variables::(text); if vars.is_empty() { - return Ok(None) + return Ok(*self) } - let mut conf = Self::default(); + let mut conf = *self; + for pair in vars { let key = pair.0; let value = pair.1; @@ -65,7 +66,7 @@ impl ConfParser for FuzzConfig { _ => Err(ConfParserError::InvalidConfigProperty(key.to_string()))?, } } - Ok(Some(conf)) + Ok(conf) } } @@ -107,13 +108,14 @@ impl Default for FuzzDictionaryConfig { #[cfg(test)] mod tests { - use crate::{conf_parser::ConfParser, FuzzConfig}; + use crate::{inline::ConfParser, FuzzConfig}; use solang_parser::pt::Comment; #[test] fn parse_config_default_profile() -> eyre::Result<()> { let conf = "forge-config: default.fuzz.runs = 1024"; - let parsed = FuzzConfig::parse(conf)?.expect("Parsed config exists"); + let base_conf: FuzzConfig = FuzzConfig::default(); + let parsed: FuzzConfig = base_conf.try_merge(conf).expect("Valid config"); assert_eq!(parsed.runs, 1024); Ok(()) } @@ -126,7 +128,8 @@ mod tests { forge-config: default.fuzz.runs = 1024 forge-config: ci.fuzz.runs = 2048"#; - let parsed = FuzzConfig::parse(conf).unwrap().expect("Parsed config exists"); + let base_conf: FuzzConfig = FuzzConfig::default(); + let parsed: FuzzConfig = base_conf.try_merge(conf).expect("Valid config"); assert_eq!(parsed.runs, 2048); Ok(()) }); @@ -135,7 +138,8 @@ mod tests { #[test] fn unrecognized_property() { let conf = "forge-config: default.fuzz.unknownprop = 200"; - if let Err(e) = FuzzConfig::parse(conf) { + let base_config = FuzzConfig::default(); + if let Err(e) = base_config.try_merge(conf) { assert_eq!(e.to_string(), "'unknownprop' is not a valid config property"); } else { assert!(false) @@ -161,7 +165,8 @@ mod tests { let comm = &comments[0]; match comm { Comment::DocBlock(_, text) => { - let config = FuzzConfig::parse(text)?.expect("Valid config"); + let base_config = FuzzConfig::default(); + let config: FuzzConfig = base_config.try_merge(text).expect("Valid config"); assert_eq!(config.runs, 1023); assert_eq!(config.max_test_rejects, 521); } diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 551f839f64d07..fe3e95adfdf05 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -40,10 +40,9 @@ pub trait ConfParser { fn config_prefix() -> String; /// Returns - /// * `Some(Self)`in case `text` contains a valid configuration for `Self`. - /// * `None` in case `text` does NOT contain any configuration matching `config_prefix`. - /// * `Err(ConfParserError)` in case of wrong configuration. - fn parse>(text: S) -> Result, ConfParserError> + /// - An instance of `Self`, resulting from the merge of a text configuration into `self` + /// - `Err(ConfParserError)` in case of wrong configuration. + fn try_merge>(&self, text: S) -> Result where Self: Sized + 'static; @@ -110,17 +109,18 @@ mod tests { assert_eq!(vec![("runs".to_string(), "600".to_string())], vars); } + #[derive(Default)] struct TestParser; impl ConfParser for TestParser { fn config_prefix() -> String { "forge-config:default.fuzz.".to_string() } - fn parse>(_text: S) -> Result, ConfParserError> + fn try_merge>(&self, _text: S) -> Result where Self: Sized + 'static, { - Ok(None) + Ok(Self::default()) } } } diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index d6130626550a7..b787c69fd19b7 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -10,6 +10,7 @@ use std::collections::{BTreeMap, HashMap}; /// `T` is the configuration type, and is bound to the [`ConfParser`] trait. Known /// implementations of [`ConfParser`] include [`FuzzConfig`](super::FuzzConfig) and /// [`InvariantConfig`](super::InvariantConfig)) +#[derive(Default, Debug, Clone)] pub struct InlineConfig where T: ConfParser + 'static, @@ -25,31 +26,37 @@ where T: ConfParser + 'static, { /// Returns an inline configuration, if any, for `contract_name` and `fn_name`.
- /// - `contract_name` The name of the contract containing the annotated function. Note this - /// the actual contract name, not the solidity file name. + /// - `contract_name` The name of the contract containing the annotated function. Note this the + /// actual contract name, not the solidity file name. /// - `fn_name` The name of the function annotated with an inline configuration. pub fn get_config>(&self, contract_name: S, fn_name: S) -> Option<&T> { self.configs.get(&(contract_name.into(), fn_name.into())) } } -impl<'a, T> TryFrom<&'a ProjectCompileOutput> for InlineConfig +impl<'a, T> TryFrom<(&'a ProjectCompileOutput, &'a T)> for InlineConfig where T: ConfParser + 'static, { type Error = ConfParserError; - /// Tries to detect inline configurations from the project compile output. - /// This object is known to emit function comments, that we can try to parse - /// at our convenience, to detect inline configurations. - fn try_from(output: &'a ProjectCompileOutput) -> Result { + /// Tries to create an instance of `Self`, detecting inline configurations from the project + /// compile output. The second item of the input tuple is used as a base configuration. + /// The aim is to inherit from "base" all the config parameters not explicitly derived from the + /// compile output. + /// + /// NOTE: `ProjectCompileOutput` is known to emit function comments, that we can try to parse + /// at our convenience, to detect inline test configurations. + fn try_from(value: (&'a ProjectCompileOutput, &'a T)) -> Result { let mut configs: HashMap<(String, String), T> = HashMap::new(); + let compile_output = value.0; + let base_conf = value.1; - for artifact in output.artifacts() { + for artifact in compile_output.artifacts() { if let Some(ast) = artifact.1.ast.as_ref() { let contract_name: String = artifact.0; for node in ast.nodes.iter() { - try_apply(&mut configs, &contract_name, node)?; + try_apply(base_conf, &mut configs, &contract_name, node)?; } } } @@ -63,6 +70,7 @@ where /// `map` under the (contract name, function name) key. /// This function may result in parsing errors (see [`ConfParserError`]). fn try_apply( + base_conf: &T, map: &mut HashMap<(String, String), T>, contract_name: &str, node: &Node, @@ -72,13 +80,12 @@ where { for n in node.nodes.iter() { if let Some((fn_name, fn_docs)) = get_fn_data(n) { - if let Some(config) = T::parse(fn_docs)? { - let key = (contract_name.into(), fn_name); - map.insert(key, config); - } + let inline_conf = base_conf.try_merge(fn_docs)?; + let key = (contract_name.into(), fn_name); + map.insert(key, inline_conf); } - try_apply(map, contract_name, n)?; + try_apply(base_conf, map, contract_name, n)?; } Ok(()) } diff --git a/config/src/invariant.rs b/config/src/invariant.rs index fa3b0bf96354b..8a0e8f2387aa0 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -42,16 +42,17 @@ impl ConfParser for InvariantConfig { format!("forge-config:{profile}.invariant.") } - fn parse>(text: S) -> Result, ConfParserError> + fn try_merge>(&self, text: S) -> Result where Self: Sized + 'static, { let vars: Vec<(String, String)> = Self::config_variables::(text); if vars.is_empty() { - return Ok(None) + return Ok(*self) } - let mut conf = Self::default(); + let mut conf = *self; + for pair in vars { let key = pair.0; let value = pair.1; @@ -63,13 +64,13 @@ impl ConfParser for InvariantConfig { _ => Err(ConfParserError::InvalidConfigProperty(key.to_string()))?, } } - Ok(Some(conf)) + Ok(conf) } } #[cfg(test)] mod tests { - use crate::{conf_parser::ConfParser, InvariantConfig}; + use crate::{inline::ConfParser, InvariantConfig}; #[test] fn parse_config_default_profile() -> eyre::Result<()> { @@ -79,8 +80,8 @@ mod tests { forge-config: default.invariant.fail-on-revert = true forge-config: default.invariant.call-override = false "#; - - let parsed = InvariantConfig::parse(conf)?.expect("Parsed config exists"); + let base_conf: InvariantConfig = InvariantConfig::default(); + let parsed: InvariantConfig = base_conf.try_merge(conf).expect("Valid config"); assert_eq!(parsed.runs, 1024); assert_eq!(parsed.depth, 30); assert_eq!(parsed.fail_on_revert, true); @@ -100,7 +101,8 @@ mod tests { forge-config: ci.invariant.call-override = false "#; - let parsed = InvariantConfig::parse(conf).unwrap().expect("Parsed config exists"); + let base_conf: InvariantConfig = InvariantConfig::default(); + let parsed: InvariantConfig = base_conf.try_merge(conf).expect("Valid config"); assert_eq!(parsed.runs, 1024); assert_eq!(parsed.depth, 30); assert_eq!(parsed.fail_on_revert, true); @@ -112,7 +114,8 @@ mod tests { #[test] fn unrecognized_property() { let conf = "forge-config: default.invariant.unknownprop = 200"; - if let Err(e) = InvariantConfig::parse(conf) { + let base_conf: InvariantConfig = InvariantConfig::default(); + if let Err(e) = base_conf.try_merge(conf) { assert_eq!(e.to_string(), "'unknownprop' is not a valid config property"); } else { assert!(false) diff --git a/forge/src/lib.rs b/forge/src/lib.rs index 591752ac80c42..ccdde4d76f084 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -1,3 +1,5 @@ +use ethers::solc::ProjectCompileOutput; +use foundry_config::{ConfParserError, FuzzConfig, InlineConfig, InvariantConfig}; use proptest::test_runner::{RngAlgorithm, TestRng, TestRunner}; use tracing::trace; @@ -24,12 +26,18 @@ pub mod result; pub use foundry_evm::*; /// Metadata on how to run fuzz/invariant tests -#[derive(Debug, Clone, Copy, Default)] +#[derive(Debug, Clone, Default)] pub struct TestOptions { - /// The fuzz test configuration - pub fuzz: foundry_config::FuzzConfig, - /// The invariant test configuration - pub invariant: foundry_config::InvariantConfig, + /// The base "fuzz" test configuration. To be used as a fallback in case + /// no more specific configs are found for a given run. + pub fuzz: FuzzConfig, + /// The base "invariant" test configuration. To be used as a fallback in case + /// no more specific configs are found for a given run. + pub invariant: InvariantConfig, + /// Contains per-test specific "fuzz" configurations. + pub inline_fuzz: InlineConfig, + /// Contains per-test specific "invariant" configurations. + pub inline_invariant: InlineConfig, } impl TestOptions { @@ -62,3 +70,50 @@ impl TestOptions { } } } + +// TODO: Document this structure +#[derive(Default)] +pub struct TestOptionsBuilder { + fuzz: Option, + invariant: Option, + output: Option, +} + +impl TestOptionsBuilder { + #[must_use = "A base 'fuzz' config must be provided"] + pub fn fuzz(mut self, conf: FuzzConfig) -> Self { + self.fuzz = Some(conf); + self + } + + #[must_use = "A base 'invariant' config must be provided"] + pub fn invariant(mut self, conf: InvariantConfig) -> Self { + self.invariant = Some(conf); + self + } + + pub fn compile_output(mut self, output: &ProjectCompileOutput) -> Self { + self.output = Some(output.clone()); + self + } + + pub fn build(self) -> Result { + let base_fuzz = self.fuzz.unwrap_or_default(); + let base_invariant = self.invariant.unwrap_or_default(); + + match self.output { + Some(compile_output) => Ok(TestOptions { + fuzz: base_fuzz, + invariant: base_invariant, + inline_fuzz: InlineConfig::try_from((&compile_output, &base_fuzz))?, + inline_invariant: InlineConfig::try_from((&compile_output, &base_invariant))?, + }), + None => Ok(TestOptions { + fuzz: base_fuzz, + invariant: base_invariant, + inline_fuzz: InlineConfig::default(), + inline_invariant: InlineConfig::default(), + }), + } + } +} diff --git a/forge/src/multi_runner.rs b/forge/src/multi_runner.rs index 5b5637fbae722..b14d617163724 100644 --- a/forge/src/multi_runner.rs +++ b/forge/src/multi_runner.rs @@ -151,7 +151,7 @@ impl MultiContractRunner { executor, deploy_code.clone(), libs, - (filter, test_options), + (filter, test_options.clone()), )?; tracing::trace!(contract= ?identifier, "executed all tests in contract"); diff --git a/forge/tests/it/inline.rs b/forge/tests/it/inline.rs index 713a41fbfaac0..a1264d285180c 100644 --- a/forge/tests/it/inline.rs +++ b/forge/tests/it/inline.rs @@ -1,12 +1,13 @@ #[cfg(test)] mod tests { - use foundry_config::{FuzzConfig, InlineConfig, InvariantConfig}; use crate::test_helpers::COMPILED; + use foundry_config::{FuzzConfig, InlineConfig, InvariantConfig}; #[test] fn inline_fuzz_config() { let compiled = COMPILED.clone(); - if let Ok(conf) = InlineConfig::::try_from(&compiled) { + let base_fuzz = FuzzConfig::default(); + if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_fuzz)) { // Inline config defined in testdata/inline/FuzzInlineConf.t.sol let contract_name = "FuzzInlineConf"; let function_name = "testFailFuzz"; @@ -19,11 +20,13 @@ mod tests { #[test] fn inline_invariant_config() { let compiled = COMPILED.clone(); - if let Ok(conf) = InlineConfig::::try_from(&compiled) { + let base_invariant = InvariantConfig::default(); + if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_invariant)) { // Inline config defined in testdata/inline/InvariantInlineConf.t.sol let contract_name = "InvariantInlineConf"; let function_name = "invariant_neverFalse"; - let inline_config: &InvariantConfig = conf.get_config(contract_name, function_name).unwrap(); + let inline_config: &InvariantConfig = + conf.get_config(contract_name, function_name).unwrap(); assert_eq!(inline_config.runs, 333); assert_eq!(inline_config.depth, 32); assert_eq!(inline_config.fail_on_revert, false); From f3b25fc06ed7ea0a0196a82f97fba7061b351a34 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 18 Apr 2023 21:59:49 +0200 Subject: [PATCH 10/57] Since TestOptions is no more Copy => TEST_OPTS constant is now a function --- forge/tests/it/config.rs | 60 ++++++++++++++++++---------------- forge/tests/it/core.rs | 10 +++--- forge/tests/it/fork.rs | 2 +- forge/tests/it/fuzz.rs | 6 ++-- forge/tests/it/invariant.rs | 14 ++++---- forge/tests/it/test_helpers.rs | 2 +- 6 files changed, 49 insertions(+), 45 deletions(-) diff --git a/forge/tests/it/config.rs b/forge/tests/it/config.rs index 5748221658bbf..b76246278cb22 100644 --- a/forge/tests/it/config.rs +++ b/forge/tests/it/config.rs @@ -30,7 +30,7 @@ impl TestConfig { } pub fn with_filter(runner: MultiContractRunner, filter: Filter) -> Self { - Self { runner, should_fail: false, filter, opts: TEST_OPTS } + Self { runner, should_fail: false, filter, opts: test_opts() } } pub fn filter(filter: Filter) -> Self { @@ -48,7 +48,7 @@ impl TestConfig { /// Executes the test runner pub fn test(&mut self) -> BTreeMap { - self.runner.test(&self.filter, None, self.opts).unwrap() + self.runner.test(&self.filter, None, self.opts.clone()).unwrap() } #[track_caller] @@ -62,7 +62,7 @@ impl TestConfig { /// * filter matched 0 test cases /// * a test results deviates from the configured `should_fail` setting pub fn try_run(&mut self) -> eyre::Result<()> { - let suite_result = self.runner.test(&self.filter, None, self.opts).unwrap(); + let suite_result = self.runner.test(&self.filter, None, self.opts.clone()).unwrap(); if suite_result.is_empty() { eyre::bail!("empty test result"); } @@ -93,33 +93,37 @@ impl Default for TestConfig { } } -pub static TEST_OPTS: TestOptions = TestOptions { - fuzz: FuzzConfig { - runs: 256, - max_test_rejects: 65536, - seed: None, - dictionary: FuzzDictionaryConfig { - include_storage: true, - include_push_bytes: true, - dictionary_weight: 40, - max_fuzz_dictionary_addresses: 10_000, - max_fuzz_dictionary_values: 10_000, +pub fn test_opts() -> TestOptions { + TestOptions { + fuzz: FuzzConfig { + runs: 256, + max_test_rejects: 65536, + seed: None, + dictionary: FuzzDictionaryConfig { + include_storage: true, + include_push_bytes: true, + dictionary_weight: 40, + max_fuzz_dictionary_addresses: 10_000, + max_fuzz_dictionary_values: 10_000, + }, }, - }, - invariant: InvariantConfig { - runs: 256, - depth: 15, - fail_on_revert: false, - call_override: false, - dictionary: FuzzDictionaryConfig { - dictionary_weight: 80, - include_storage: true, - include_push_bytes: true, - max_fuzz_dictionary_addresses: 10_000, - max_fuzz_dictionary_values: 10_000, + invariant: InvariantConfig { + runs: 256, + depth: 15, + fail_on_revert: false, + call_override: false, + dictionary: FuzzDictionaryConfig { + dictionary_weight: 80, + include_storage: true, + include_push_bytes: true, + max_fuzz_dictionary_addresses: 10_000, + max_fuzz_dictionary_values: 10_000, + }, }, - }, -}; + inline_fuzz: Default::default(), + inline_invariant: Default::default(), + } +} pub fn manifest_root() -> PathBuf { let mut root = Path::new(env!("CARGO_MANIFEST_DIR")); diff --git a/forge/tests/it/core.rs b/forge/tests/it/core.rs index ddb042e416067..1d88cc4d8b234 100644 --- a/forge/tests/it/core.rs +++ b/forge/tests/it/core.rs @@ -10,7 +10,7 @@ use std::{collections::BTreeMap, env}; #[test] fn test_core() { let mut runner = runner(); - let results = runner.test(&Filter::new(".*", ".*", ".*core"), None, TEST_OPTS).unwrap(); + let results = runner.test(&Filter::new(".*", ".*", ".*core"), None, test_opts()).unwrap(); assert_multiple( &results, @@ -86,7 +86,7 @@ fn test_core() { #[test] fn test_logs() { let mut runner = runner(); - let results = runner.test(&Filter::new(".*", ".*", ".*logs"), None, TEST_OPTS).unwrap(); + let results = runner.test(&Filter::new(".*", ".*", ".*logs"), None, test_opts()).unwrap(); assert_multiple( &results, @@ -649,7 +649,7 @@ fn test_env_vars() { // test `setEnv` first, and confirm that it can correctly set environment variables, // so that we can use it in subsequent `env*` tests - runner.test(&Filter::new("testSetEnv", ".*", ".*"), None, TEST_OPTS).unwrap(); + runner.test(&Filter::new("testSetEnv", ".*", ".*"), None, test_opts()).unwrap(); let env_var_key = "_foundryCheatcodeSetEnvTestKey"; let env_var_val = "_foundryCheatcodeSetEnvTestVal"; let res = env::var(env_var_key); @@ -664,7 +664,7 @@ Reason: `setEnv` failed to set an environment variable `{env_var_key}={env_var_v fn test_doesnt_run_abstract_contract() { let mut runner = runner(); let results = runner - .test(&Filter::new(".*", ".*", ".*Abstract.t.sol".to_string().as_str()), None, TEST_OPTS) + .test(&Filter::new(".*", ".*", ".*Abstract.t.sol".to_string().as_str()), None, test_opts()) .unwrap(); assert!(results.get("core/Abstract.t.sol:AbstractTestBase").is_none()); assert!(results.get("core/Abstract.t.sol:AbstractTest").is_some()); @@ -673,7 +673,7 @@ fn test_doesnt_run_abstract_contract() { #[test] fn test_trace() { let mut runner = tracing_runner(); - let suite_result = runner.test(&Filter::new(".*", ".*", ".*trace"), None, TEST_OPTS).unwrap(); + let suite_result = runner.test(&Filter::new(".*", ".*", ".*trace"), None, test_opts()).unwrap(); // TODO: This trace test is very basic - it is probably a good candidate for snapshot // testing. diff --git a/forge/tests/it/fork.rs b/forge/tests/it/fork.rs index 340854920fced..337e0530585c2 100644 --- a/forge/tests/it/fork.rs +++ b/forge/tests/it/fork.rs @@ -18,7 +18,7 @@ fn test_cheats_fork_revert() { &format!(".*cheats{RE_PATH_SEPARATOR}Fork"), ), None, - TEST_OPTS, + test_opts(), ) .unwrap(); assert_eq!(suite_result.len(), 1); diff --git a/forge/tests/it/fuzz.rs b/forge/tests/it/fuzz.rs index acc4ae027e9c7..54439aaad2066 100644 --- a/forge/tests/it/fuzz.rs +++ b/forge/tests/it/fuzz.rs @@ -15,7 +15,7 @@ fn test_fuzz() { .exclude_tests(r#"invariantCounter|testIncrement\(address\)|testNeedle\(uint256\)"#) .exclude_paths("invariant"), None, - TEST_OPTS, + test_opts(), ) .unwrap(); @@ -53,12 +53,12 @@ fn test_fuzz() { fn test_fuzz_collection() { let mut runner = runner(); - let mut opts = TEST_OPTS; + let mut opts = test_opts(); opts.invariant.depth = 100; opts.invariant.runs = 1000; opts.fuzz.runs = 1000; opts.fuzz.seed = Some(U256::from(6u32)); - runner.test_options = opts; + runner.test_options = opts.clone(); let results = runner.test(&Filter::new(".*", ".*", ".*fuzz/FuzzCollection.t.sol"), None, opts).unwrap(); diff --git a/forge/tests/it/invariant.rs b/forge/tests/it/invariant.rs index 211a88de1b70c..e49d4f3cffa63 100644 --- a/forge/tests/it/invariant.rs +++ b/forge/tests/it/invariant.rs @@ -13,7 +13,7 @@ fn test_invariant() { .test( &Filter::new(".*", ".*", ".*fuzz/invariant/(target|targetAbi|common)"), None, - TEST_OPTS, + test_opts(), ) .unwrap(); @@ -79,9 +79,9 @@ fn test_invariant() { fn test_invariant_override() { let mut runner = runner(); - let mut opts = TEST_OPTS; + let mut opts = test_opts(); opts.invariant.call_override = true; - runner.test_options = opts; + runner.test_options = opts.clone(); let results = runner .test( @@ -104,10 +104,10 @@ fn test_invariant_override() { fn test_invariant_storage() { let mut runner = runner(); - let mut opts = TEST_OPTS; + let mut opts = test_opts(); opts.invariant.depth = 100; opts.fuzz.seed = Some(U256::from(6u32)); - runner.test_options = opts; + runner.test_options = opts.clone(); let results = runner .test( @@ -137,9 +137,9 @@ fn test_invariant_storage() { fn test_invariant_shrink() { let mut runner = runner(); - let mut opts = TEST_OPTS; + let mut opts = test_opts(); opts.fuzz.seed = Some(U256::from(102u32)); - runner.test_options = opts; + runner.test_options = opts.clone(); let results = runner .test( diff --git a/forge/tests/it/test_helpers.rs b/forge/tests/it/test_helpers.rs index 1b9465f6e8f2d..a423b01d223d0 100644 --- a/forge/tests/it/test_helpers.rs +++ b/forge/tests/it/test_helpers.rs @@ -83,7 +83,7 @@ pub fn fuzz_executor(executor: &Executor) -> FuzzedExecutor { executor, proptest::test_runner::TestRunner::new(cfg), CALLER, - config::TEST_OPTS.fuzz, + config::test_opts().fuzz, ) } From 0868d6736444ed6a74b53cbfea2eec4dfd72bd8c Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 18 Apr 2023 22:12:03 +0200 Subject: [PATCH 11/57] Inline config matcher uses stripped file prefixes to identify contracts --- cli/src/cmd/forge/test/mod.rs | 6 +++-- config/src/inline/mod.rs | 42 ++++++++++++++++++++--------------- forge/src/lib.rs | 12 +++++++--- forge/tests/it/inline.rs | 12 +++++----- 4 files changed, 44 insertions(+), 28 deletions(-) diff --git a/cli/src/cmd/forge/test/mod.rs b/cli/src/cmd/forge/test/mod.rs index 35110660d55e8..57453de3af1f2 100644 --- a/cli/src/cmd/forge/test/mod.rs +++ b/cli/src/cmd/forge/test/mod.rs @@ -146,11 +146,13 @@ impl TestArgs { compiler.compile(&project) }?; + let project_root = &project.paths.root; + let test_options: TestOptions = TestOptionsBuilder::default() .fuzz(config.fuzz) .invariant(config.invariant) .compile_output(&output) - .build()?; + .build(project_root)?; // Determine print verbosity and executor verbosity let verbosity = evm_opts.verbosity; @@ -170,7 +172,7 @@ impl TestArgs { .with_fork(evm_opts.get_fork(&config, env.clone())) .with_cheats_config(CheatsConfig::new(&config, &evm_opts)) .with_test_options(test_options.clone()) - .build(project.paths.root, output, env, evm_opts)?; + .build(project_root, output, env, evm_opts)?; if self.debug.is_some() { filter.args_mut().test_pattern = self.debug; diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index b787c69fd19b7..78f245478bb65 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -2,7 +2,10 @@ mod conf_parser; pub use conf_parser::{ConfParser, ConfParserError}; use ethers_solc::{artifacts::Node, ProjectCompileOutput}; use serde_json::Value; -use std::collections::{BTreeMap, HashMap}; +use std::{ + collections::{BTreeMap, HashMap}, + path::Path, +}; /// Represents per-test configurations, declared inline /// as structured comments in Solidity test files. This allows @@ -25,18 +28,20 @@ impl InlineConfig where T: ConfParser + 'static, { - /// Returns an inline configuration, if any, for `contract_name` and `fn_name`.
- /// - `contract_name` The name of the contract containing the annotated function. Note this the - /// actual contract name, not the solidity file name. + /// Returns an inline configuration, if any, for `contract_id` and `fn_name`.
+ /// - `contract_id` The identifier of the contract containing the annotated function. Note that + /// the contract id is a path relative to the project's root (i.e. someDirUnderRoot/Contract.t.sol:ContractName). + /// /// - `fn_name` The name of the function annotated with an inline configuration. - pub fn get_config>(&self, contract_name: S, fn_name: S) -> Option<&T> { - self.configs.get(&(contract_name.into(), fn_name.into())) + pub fn get_config>(&self, contract_id: S, fn_name: S) -> Option<&T> { + self.configs.get(&(contract_id.into(), fn_name.into())) } } -impl<'a, T> TryFrom<(&'a ProjectCompileOutput, &'a T)> for InlineConfig +impl<'a, T, P> TryFrom<(&'a ProjectCompileOutput, &'a T, &'a P)> for InlineConfig where T: ConfParser + 'static, + P: AsRef, { type Error = ConfParserError; @@ -47,16 +52,17 @@ where /// /// NOTE: `ProjectCompileOutput` is known to emit function comments, that we can try to parse /// at our convenience, to detect inline test configurations. - fn try_from(value: (&'a ProjectCompileOutput, &'a T)) -> Result { + fn try_from(value: (&'a ProjectCompileOutput, &'a T, &'a P)) -> Result { let mut configs: HashMap<(String, String), T> = HashMap::new(); - let compile_output = value.0; + let compile_output = value.0.clone(); let base_conf = value.1; + let root = value.2; - for artifact in compile_output.artifacts() { + for artifact in compile_output.with_stripped_file_prefixes(root).into_artifacts() { if let Some(ast) = artifact.1.ast.as_ref() { - let contract_name: String = artifact.0; + let contract_id: String = artifact.0.identifier(); for node in ast.nodes.iter() { - try_apply(base_conf, &mut configs, &contract_name, node)?; + try_apply(base_conf, &mut configs, &contract_id, node)?; } } } @@ -67,12 +73,12 @@ where /// Implements a DFS over a compiler output node and its children. /// If a configuration is found for a solidity function, it is added to -/// `map` under the (contract name, function name) key. +/// `map` under the (contract id, function name) key. /// This function may result in parsing errors (see [`ConfParserError`]). fn try_apply( base_conf: &T, map: &mut HashMap<(String, String), T>, - contract_name: &str, + contract_id: &str, node: &Node, ) -> Result<(), ConfParserError> where @@ -81,11 +87,11 @@ where for n in node.nodes.iter() { if let Some((fn_name, fn_docs)) = get_fn_data(n) { let inline_conf = base_conf.try_merge(fn_docs)?; - let key = (contract_name.into(), fn_name); + let key = (contract_id.into(), fn_name); map.insert(key, inline_conf); } - try_apply(base_conf, map, contract_name, n)?; + try_apply(base_conf, map, contract_id, n)?; } Ok(()) } @@ -95,7 +101,7 @@ fn get_fn_data(node: &Node) -> Option<(String, String)> { let fn_data = &node.other; let fn_name: String = get_fn_name(fn_data)?; let fn_docs: String = get_fn_docs(fn_data)?; - return Some((fn_name, fn_docs)) + return Some((fn_name, fn_docs)); } None } @@ -110,7 +116,7 @@ fn get_fn_name(fn_data: &BTreeMap) -> Option { fn get_fn_docs(fn_data: &BTreeMap) -> Option { if let Value::Object(fn_docs) = fn_data.get("documentation")? { if let Value::String(comment) = fn_docs.get("text")? { - return Some(comment.into()) + return Some(comment.into()); } } None diff --git a/forge/src/lib.rs b/forge/src/lib.rs index ccdde4d76f084..937a72bf3c1ef 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -1,3 +1,5 @@ +use std::path::Path; + use ethers::solc::ProjectCompileOutput; use foundry_config::{ConfParserError, FuzzConfig, InlineConfig, InvariantConfig}; use proptest::test_runner::{RngAlgorithm, TestRng, TestRunner}; @@ -97,7 +99,7 @@ impl TestOptionsBuilder { self } - pub fn build(self) -> Result { + pub fn build(self, root: impl AsRef) -> Result { let base_fuzz = self.fuzz.unwrap_or_default(); let base_invariant = self.invariant.unwrap_or_default(); @@ -105,8 +107,12 @@ impl TestOptionsBuilder { Some(compile_output) => Ok(TestOptions { fuzz: base_fuzz, invariant: base_invariant, - inline_fuzz: InlineConfig::try_from((&compile_output, &base_fuzz))?, - inline_invariant: InlineConfig::try_from((&compile_output, &base_invariant))?, + inline_fuzz: InlineConfig::try_from((&compile_output, &base_fuzz, &root))?, + inline_invariant: InlineConfig::try_from(( + &compile_output, + &base_invariant, + &root, + ))?, }), None => Ok(TestOptions { fuzz: base_fuzz, diff --git a/forge/tests/it/inline.rs b/forge/tests/it/inline.rs index a1264d285180c..86ccf0d5297e3 100644 --- a/forge/tests/it/inline.rs +++ b/forge/tests/it/inline.rs @@ -1,15 +1,16 @@ #[cfg(test)] mod tests { - use crate::test_helpers::COMPILED; + use crate::test_helpers::{COMPILED, PROJECT}; use foundry_config::{FuzzConfig, InlineConfig, InvariantConfig}; #[test] fn inline_fuzz_config() { + let root = &PROJECT.paths.root; let compiled = COMPILED.clone(); let base_fuzz = FuzzConfig::default(); - if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_fuzz)) { + if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_fuzz, root)) { // Inline config defined in testdata/inline/FuzzInlineConf.t.sol - let contract_name = "FuzzInlineConf"; + let contract_name = "inline/FuzzInlineConf.t.sol:FuzzInlineConf"; let function_name = "testFailFuzz"; let inline_config: &FuzzConfig = conf.get_config(contract_name, function_name).unwrap(); assert_eq!(inline_config.runs, 1024); @@ -19,11 +20,12 @@ mod tests { #[test] fn inline_invariant_config() { + let root = &PROJECT.paths.root; let compiled = COMPILED.clone(); let base_invariant = InvariantConfig::default(); - if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_invariant)) { + if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_invariant, root)) { // Inline config defined in testdata/inline/InvariantInlineConf.t.sol - let contract_name = "InvariantInlineConf"; + let contract_name = "inline/InvariantInlineConf.t.sol:InvariantInlineConf"; let function_name = "invariant_neverFalse"; let inline_config: &InvariantConfig = conf.get_config(contract_name, function_name).unwrap(); From 9f8672e60e3557ff251044a4ab90aced703e7a07 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 18 Apr 2023 22:31:13 +0200 Subject: [PATCH 12/57] TestOptionsBuilder docs --- forge/src/lib.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/forge/src/lib.rs b/forge/src/lib.rs index 937a72bf3c1ef..cf0c7ef6bb98f 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -73,7 +73,7 @@ impl TestOptions { } } -// TODO: Document this structure +/// Builder utility to create a [`TestOptions`] instance. #[derive(Default)] pub struct TestOptionsBuilder { fuzz: Option, @@ -82,23 +82,35 @@ pub struct TestOptionsBuilder { } impl TestOptionsBuilder { + + /// Sets a [`FuzzConfig`] to be used as base "fuzz" configuration. #[must_use = "A base 'fuzz' config must be provided"] pub fn fuzz(mut self, conf: FuzzConfig) -> Self { self.fuzz = Some(conf); self } + /// Sets a [`InvariantConfig`] to be used as base "invariant" configuration. #[must_use = "A base 'invariant' config must be provided"] pub fn invariant(mut self, conf: InvariantConfig) -> Self { self.invariant = Some(conf); self } + /// Sets a project compiler output instance. This is used to extract + /// inline test configurations that override `self.fuzz` and `self.invariant` + /// specs when necessary. pub fn compile_output(mut self, output: &ProjectCompileOutput) -> Self { self.output = Some(output.clone()); self } + /// Creates an instance of [`TestOptions`]. This takes care of creating "fuzz" and + /// "invariant" fallbacks, and extracting all inline test configs, if available. + /// + /// `root` is a reference to the user's project root dir. This is essential + /// to determine the base path of generated contract identifiers. This is to provide correct + /// matchers for inline test configs. pub fn build(self, root: impl AsRef) -> Result { let base_fuzz = self.fuzz.unwrap_or_default(); let base_invariant = self.invariant.unwrap_or_default(); From 09ff7dbcdb45cbfe14d51233559a0b6f9b19102b Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 19 Apr 2023 12:29:55 +0200 Subject: [PATCH 13/57] Inline fuzz configs are applied during fuzz test execution + E2E tests --- forge/src/lib.rs | 36 +++++++++++++++++++----- forge/src/multi_runner.rs | 6 ++-- forge/src/runner.rs | 13 +++++++-- forge/tests/it/inline.rs | 41 ++++++++++++++++++++++++++-- testdata/inline/FuzzInlineConf.t.sol | 4 +-- 5 files changed, 82 insertions(+), 18 deletions(-) diff --git a/forge/src/lib.rs b/forge/src/lib.rs index cf0c7ef6bb98f..770abba7f28ed 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -43,12 +43,35 @@ pub struct TestOptions { } impl TestOptions { - pub fn invariant_fuzzer(&self) -> TestRunner { - self.fuzzer_with_cases(self.invariant.runs) + /// Returns a fuzz runner instance. Parameters are used to select tight scoped fuzz configs + /// that apply for a contract-function pair. A fallback configuration is applied + /// if no specific setup is found for a given input. + /// + /// - `contract_id` is the name of the test contract. + /// - `test_fn` is the name of the test function declared inside the test contract. + pub fn fuzzer(&self, contract_id: S, test_fn: S) -> TestRunner + where + S: Into, + { + let fuzz = self.fuzz_config(contract_id, test_fn); + self.fuzzer_with_cases(fuzz.runs) + } + + /// Returns a fuzz configuration setup. Parameters are used to select tight scoped fuzz configs + /// that apply for a contract-function pair. A fallback configuration is applied + /// if no specific setup is found for a given input. + /// + /// - `contract_id` is the name of the test contract. + /// - `test_fn` is the name of the test function declared inside the test contract. + pub fn fuzz_config(&self, contract_id: S, test_fn: S) -> &FuzzConfig + where + S: Into, + { + self.inline_fuzz.get_config(contract_id, test_fn).unwrap_or(&self.fuzz) } - pub fn fuzzer(&self) -> TestRunner { - self.fuzzer_with_cases(self.fuzz.runs) + pub fn invariant_fuzzer(&self) -> TestRunner { + self.fuzzer_with_cases(self.invariant.runs) } pub fn fuzzer_with_cases(&self, cases: u32) -> TestRunner { @@ -82,7 +105,6 @@ pub struct TestOptionsBuilder { } impl TestOptionsBuilder { - /// Sets a [`FuzzConfig`] to be used as base "fuzz" configuration. #[must_use = "A base 'fuzz' config must be provided"] pub fn fuzz(mut self, conf: FuzzConfig) -> Self { @@ -98,7 +120,7 @@ impl TestOptionsBuilder { } /// Sets a project compiler output instance. This is used to extract - /// inline test configurations that override `self.fuzz` and `self.invariant` + /// inline test configurations that override `self.fuzz` and `self.invariant` /// specs when necessary. pub fn compile_output(mut self, output: &ProjectCompileOutput) -> Self { self.output = Some(output.clone()); @@ -107,7 +129,7 @@ impl TestOptionsBuilder { /// Creates an instance of [`TestOptions`]. This takes care of creating "fuzz" and /// "invariant" fallbacks, and extracting all inline test configs, if available. - /// + /// /// `root` is a reference to the user's project root dir. This is essential /// to determine the base path of generated contract identifiers. This is to provide correct /// matchers for inline test configs. diff --git a/forge/src/multi_runner.rs b/forge/src/multi_runner.rs index b14d617163724..3bacecfb64630 100644 --- a/forge/src/multi_runner.rs +++ b/forge/src/multi_runner.rs @@ -170,16 +170,15 @@ impl MultiContractRunner { Ok(results) } - // The _name field is unused because we only want it for tracing #[tracing::instrument( name = "contract", skip_all, err, - fields(name = %_name) + fields(name = %name) )] fn run_tests( &self, - _name: &str, + name: &str, contract: &Abi, executor: Executor, deploy_code: Bytes, @@ -187,6 +186,7 @@ impl MultiContractRunner { (filter, test_options): (&impl TestFilter, TestOptions), ) -> Result { let runner = ContractRunner::new( + name, executor, contract, deploy_code, diff --git a/forge/src/runner.rs b/forge/src/runner.rs index ebb2d875ab435..895a47c77eb67 100644 --- a/forge/src/runner.rs +++ b/forge/src/runner.rs @@ -32,9 +32,9 @@ use tracing::{error, trace}; /// A type that executes all tests of a contract #[derive(Debug, Clone)] pub struct ContractRunner<'a> { + pub name: &'a str, /// The executor used by the runner. pub executor: Executor, - /// Library contracts to be deployed before the test contract pub predeploy_libs: &'a [Bytes], /// The deployed contract's code @@ -53,6 +53,7 @@ pub struct ContractRunner<'a> { impl<'a> ContractRunner<'a> { #[allow(clippy::too_many_arguments)] pub fn new( + name: &'a str, executor: Executor, contract: &'a Abi, code: Bytes, @@ -62,6 +63,7 @@ impl<'a> ContractRunner<'a> { predeploy_libs: &'a [Bytes], ) -> Self { Self { + name, executor, contract, code, @@ -282,12 +284,17 @@ impl<'a> ContractRunner<'a> { .par_iter() .flat_map(|(func, should_fail)| { if func.is_fuzz_test() { + let fn_name = &func.name; + let runner = test_options.fuzzer(self.name, fn_name); + let fuzz_config = test_options.fuzz_config(self.name, fn_name); + + self.run_fuzz_test( func, *should_fail, - test_options.fuzzer(), + runner, setup.clone(), - test_options.fuzz, + *fuzz_config, ) } else { self.clone().run_test(func, *should_fail, setup.clone()) diff --git a/forge/tests/it/inline.rs b/forge/tests/it/inline.rs index 86ccf0d5297e3..bae6a51270b0f 100644 --- a/forge/tests/it/inline.rs +++ b/forge/tests/it/inline.rs @@ -1,8 +1,41 @@ #[cfg(test)] mod tests { - use crate::test_helpers::{COMPILED, PROJECT}; + use crate::{ + config::runner, + test_helpers::{filter::Filter, COMPILED, PROJECT}, + }; + use forge::{TestOptionsBuilder, result::{SuiteResult, TestKind, TestResult}}; use foundry_config::{FuzzConfig, InlineConfig, InvariantConfig}; + #[test] + fn inline_config_run_fuzz() { + let root = &PROJECT.paths.root; + + let opts = TestOptionsBuilder::default() + .fuzz(FuzzConfig::default()) + .invariant(InvariantConfig::default()) + .compile_output(&COMPILED) + .build(root) + .expect("Config loaded"); + + let filter = Filter::new(".*", ".*", ".*inline/FuzzInlineConf.t.sol"); + + let mut runner = runner(); + runner.test_options = opts.clone(); + + let result = runner.test(&filter, None, opts).expect("Test ran"); + let suite_result: &SuiteResult = result.get("inline/FuzzInlineConf.t.sol:FuzzInlineConf").unwrap(); + let test_result: &TestResult = suite_result.test_results.get("testInlineConfFuzz(uint8)").unwrap(); + match &test_result.kind { + TestKind::Fuzz { runs, .. } => { + assert_eq!(runs, &1024); + } + _ => { + assert!(false); // Force test to fail + } + } + } + #[test] fn inline_fuzz_config() { let root = &PROJECT.paths.root; @@ -11,7 +44,7 @@ mod tests { if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_fuzz, root)) { // Inline config defined in testdata/inline/FuzzInlineConf.t.sol let contract_name = "inline/FuzzInlineConf.t.sol:FuzzInlineConf"; - let function_name = "testFailFuzz"; + let function_name = "testInlineConfFuzz"; let inline_config: &FuzzConfig = conf.get_config(contract_name, function_name).unwrap(); assert_eq!(inline_config.runs, 1024); assert_eq!(inline_config.max_test_rejects, 500); @@ -23,7 +56,9 @@ mod tests { let root = &PROJECT.paths.root; let compiled = COMPILED.clone(); let base_invariant = InvariantConfig::default(); - if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_invariant, root)) { + if let Ok(conf) = + InlineConfig::::try_from((&compiled, &base_invariant, root)) + { // Inline config defined in testdata/inline/InvariantInlineConf.t.sol let contract_name = "inline/InvariantInlineConf.t.sol:InvariantInlineConf"; let function_name = "invariant_neverFalse"; diff --git a/testdata/inline/FuzzInlineConf.t.sol b/testdata/inline/FuzzInlineConf.t.sol index a90cba2ee5894..5a856ac57adcb 100644 --- a/testdata/inline/FuzzInlineConf.t.sol +++ b/testdata/inline/FuzzInlineConf.t.sol @@ -6,7 +6,7 @@ import "ds-test/test.sol"; contract FuzzInlineConf is DSTest { /// forge-config: default.fuzz.runs = 1024 /// forge-config: default.fuzz.max-test-rejects = 500 - function testFailFuzz(uint8 x) public { - require(x > 128, "should revert"); + function testInlineConfFuzz(uint8 x) public { + require(true, "this is not going to revert"); } } From 9fc175c02e7c376cb86891a4aee1c750e67692ed Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 19 Apr 2023 21:18:31 +0200 Subject: [PATCH 14/57] Inline invariant configs are applied during fuzz test execution + E2E tests + Docs --- cli/src/cmd/forge/test/mod.rs | 2 +- config/src/fuzz.rs | 19 +++--- config/src/inline/conf_parser.rs | 9 +-- config/src/inline/mod.rs | 38 ++++++++---- config/src/invariant.rs | 14 ++--- forge/src/lib.rs | 45 ++++++++++---- forge/src/runner.rs | 38 +++++++----- forge/tests/it/inline.rs | 71 ++++++++++++++++++----- testdata/inline/InvariantInlineConf.t.sol | 15 ++++- 9 files changed, 179 insertions(+), 72 deletions(-) diff --git a/cli/src/cmd/forge/test/mod.rs b/cli/src/cmd/forge/test/mod.rs index 57453de3af1f2..0d14a14eb593c 100644 --- a/cli/src/cmd/forge/test/mod.rs +++ b/cli/src/cmd/forge/test/mod.rs @@ -147,7 +147,7 @@ impl TestArgs { }?; let project_root = &project.paths.root; - + let test_options: TestOptions = TestOptionsBuilder::default() .fuzz(config.fuzz) .invariant(config.invariant) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 180282f12c351..b642853c0117e 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -46,13 +46,13 @@ impl ConfParser for FuzzConfig { format!("forge-config:{profile}.fuzz.") } - fn try_merge>(&self, text: S) -> Result + fn try_merge>(&self, text: S) -> Result, ConfParserError> where Self: Sized + 'static, { let vars: Vec<(String, String)> = Self::config_variables::(text); if vars.is_empty() { - return Ok(*self) + return Ok(None) } let mut conf = *self; @@ -66,7 +66,7 @@ impl ConfParser for FuzzConfig { _ => Err(ConfParserError::InvalidConfigProperty(key.to_string()))?, } } - Ok(conf) + Ok(Some(conf)) } } @@ -112,12 +112,11 @@ mod tests { use solang_parser::pt::Comment; #[test] - fn parse_config_default_profile() -> eyre::Result<()> { + fn parse_config_default_profile() { let conf = "forge-config: default.fuzz.runs = 1024"; let base_conf: FuzzConfig = FuzzConfig::default(); - let parsed: FuzzConfig = base_conf.try_merge(conf).expect("Valid config"); + let parsed: FuzzConfig = base_conf.try_merge(conf).unwrap().expect("Valid config"); assert_eq!(parsed.runs, 1024); - Ok(()) } #[test] @@ -129,7 +128,7 @@ mod tests { forge-config: ci.fuzz.runs = 2048"#; let base_conf: FuzzConfig = FuzzConfig::default(); - let parsed: FuzzConfig = base_conf.try_merge(conf).expect("Valid config"); + let parsed: FuzzConfig = base_conf.try_merge(conf).unwrap().expect("Valid config"); assert_eq!(parsed.runs, 2048); Ok(()) }); @@ -147,7 +146,7 @@ mod tests { } #[test] - fn e2e() -> eyre::Result<()> { + fn e2e() { use solang_parser::parse; let code = r#" contract FuzzTestContract { @@ -166,7 +165,8 @@ mod tests { match comm { Comment::DocBlock(_, text) => { let base_config = FuzzConfig::default(); - let config: FuzzConfig = base_config.try_merge(text).expect("Valid config"); + let config: FuzzConfig = + base_config.try_merge(text).unwrap().expect("Valid config"); assert_eq!(config.runs, 1023); assert_eq!(config.max_test_rejects, 521); } @@ -174,6 +174,5 @@ mod tests { assert!(false); // Force test to fail } } - Ok(()) } } diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index fe3e95adfdf05..4dae97fea3fb0 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -40,9 +40,10 @@ pub trait ConfParser { fn config_prefix() -> String; /// Returns - /// - An instance of `Self`, resulting from the merge of a text configuration into `self` + /// - `Some(Self)` in case some configurations are merged into self. + /// - `None` in case there are no configurations that can be applied to self. /// - `Err(ConfParserError)` in case of wrong configuration. - fn try_merge>(&self, text: S) -> Result + fn try_merge>(&self, text: S) -> Result, ConfParserError> where Self: Sized + 'static; @@ -116,11 +117,11 @@ mod tests { "forge-config:default.fuzz.".to_string() } - fn try_merge>(&self, _text: S) -> Result + fn try_merge>(&self, _text: S) -> Result, ConfParserError> where Self: Sized + 'static, { - Ok(Self::default()) + Ok(Some(Self::default())) } } } diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 78f245478bb65..fb90fcb66aff9 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -30,8 +30,8 @@ where { /// Returns an inline configuration, if any, for `contract_id` and `fn_name`.
/// - `contract_id` The identifier of the contract containing the annotated function. Note that - /// the contract id is a path relative to the project's root (i.e. someDirUnderRoot/Contract.t.sol:ContractName). - /// + /// contract id is a path relative to the project's root check `with_stripped_file_prefixes` + /// in [`ProjectCompileOutput`](ethers_solc::compile::output::ProjectCompileOutput)). /// - `fn_name` The name of the function annotated with an inline configuration. pub fn get_config>(&self, contract_id: S, fn_name: S) -> Option<&T> { self.configs.get(&(contract_id.into(), fn_name.into())) @@ -46,9 +46,7 @@ where type Error = ConfParserError; /// Tries to create an instance of `Self`, detecting inline configurations from the project - /// compile output. The second item of the input tuple is used as a base configuration. - /// The aim is to inherit from "base" all the config parameters not explicitly derived from the - /// compile output. + /// compile output. The second item of the input tuple is used as a fallback configuration. /// /// NOTE: `ProjectCompileOutput` is known to emit function comments, that we can try to parse /// at our convenience, to detect inline test configurations. @@ -61,7 +59,7 @@ where for artifact in compile_output.with_stripped_file_prefixes(root).into_artifacts() { if let Some(ast) = artifact.1.ast.as_ref() { let contract_id: String = artifact.0.identifier(); - for node in ast.nodes.iter() { + if let Some(node) = contract_root_node(&ast.nodes, &contract_id) { try_apply(base_conf, &mut configs, &contract_id, node)?; } } @@ -71,6 +69,22 @@ where } } +/// Given a list of nodes, find a "ContractDefinition" node that matches +/// the provided contract_id. +fn contract_root_node<'a>(nodes: &'a [Node], contract_id: &'a str) -> Option<&'a Node> { + for n in nodes.iter() { + if format!("{:?}", n.node_type) == *"ContractDefinition" { + let contract_data = &n.other; + if let Value::String(contract_name) = contract_data.get("name")? { + if contract_id.ends_with(contract_name) { + return Some(n) + } + } + } + } + None +} + /// Implements a DFS over a compiler output node and its children. /// If a configuration is found for a solidity function, it is added to /// `map` under the (contract id, function name) key. @@ -86,9 +100,11 @@ where { for n in node.nodes.iter() { if let Some((fn_name, fn_docs)) = get_fn_data(n) { - let inline_conf = base_conf.try_merge(fn_docs)?; - let key = (contract_id.into(), fn_name); - map.insert(key, inline_conf); + if let Some(conf) = base_conf.try_merge(fn_docs.clone())? { + // We found a config that applies for the pair contract_id, fn_name. + let key = (contract_id.into(), fn_name); + map.insert(key, conf); + } } try_apply(base_conf, map, contract_id, n)?; @@ -101,7 +117,7 @@ fn get_fn_data(node: &Node) -> Option<(String, String)> { let fn_data = &node.other; let fn_name: String = get_fn_name(fn_data)?; let fn_docs: String = get_fn_docs(fn_data)?; - return Some((fn_name, fn_docs)); + return Some((fn_name, fn_docs)) } None } @@ -116,7 +132,7 @@ fn get_fn_name(fn_data: &BTreeMap) -> Option { fn get_fn_docs(fn_data: &BTreeMap) -> Option { if let Value::Object(fn_docs) = fn_data.get("documentation")? { if let Value::String(comment) = fn_docs.get("text")? { - return Some(comment.into()); + return Some(comment.into()) } } None diff --git a/config/src/invariant.rs b/config/src/invariant.rs index 8a0e8f2387aa0..e1b9ee0d76420 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -42,13 +42,13 @@ impl ConfParser for InvariantConfig { format!("forge-config:{profile}.invariant.") } - fn try_merge>(&self, text: S) -> Result + fn try_merge>(&self, text: S) -> Result, ConfParserError> where Self: Sized + 'static, { let vars: Vec<(String, String)> = Self::config_variables::(text); if vars.is_empty() { - return Ok(*self) + return Ok(None) } let mut conf = *self; @@ -64,7 +64,7 @@ impl ConfParser for InvariantConfig { _ => Err(ConfParserError::InvalidConfigProperty(key.to_string()))?, } } - Ok(conf) + Ok(Some(conf)) } } @@ -73,7 +73,7 @@ mod tests { use crate::{inline::ConfParser, InvariantConfig}; #[test] - fn parse_config_default_profile() -> eyre::Result<()> { + fn parse_config_default_profile() { let conf = r#" forge-config: default.invariant.runs = 1024 forge-config: default.invariant.depth = 30 @@ -81,13 +81,11 @@ mod tests { forge-config: default.invariant.call-override = false "#; let base_conf: InvariantConfig = InvariantConfig::default(); - let parsed: InvariantConfig = base_conf.try_merge(conf).expect("Valid config"); + let parsed: InvariantConfig = base_conf.try_merge(conf).unwrap().expect("Valid config"); assert_eq!(parsed.runs, 1024); assert_eq!(parsed.depth, 30); assert_eq!(parsed.fail_on_revert, true); assert_eq!(parsed.call_override, false); - - Ok(()) } #[test] @@ -102,7 +100,7 @@ mod tests { "#; let base_conf: InvariantConfig = InvariantConfig::default(); - let parsed: InvariantConfig = base_conf.try_merge(conf).expect("Valid config"); + let parsed: InvariantConfig = base_conf.try_merge(conf).unwrap().expect("Valid config"); assert_eq!(parsed.runs, 1024); assert_eq!(parsed.depth, 30); assert_eq!(parsed.fail_on_revert, true); diff --git a/forge/src/lib.rs b/forge/src/lib.rs index 770abba7f28ed..82d6ed512e2df 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -43,13 +43,13 @@ pub struct TestOptions { } impl TestOptions { - /// Returns a fuzz runner instance. Parameters are used to select tight scoped fuzz configs - /// that apply for a contract-function pair. A fallback configuration is applied + /// Returns a "fuzz" test runner instance. Parameters are used to select tight scoped fuzz configs + /// that apply for a contract-function pair. A fallback configuration is applied /// if no specific setup is found for a given input. - /// - /// - `contract_id` is the name of the test contract. + /// + /// - `contract_id` is the id of the test contract, expressed as a relative path from the project root. /// - `test_fn` is the name of the test function declared inside the test contract. - pub fn fuzzer(&self, contract_id: S, test_fn: S) -> TestRunner + pub fn fuzz_runner(&self, contract_id: S, test_fn: S) -> TestRunner where S: Into, { @@ -57,11 +57,25 @@ impl TestOptions { self.fuzzer_with_cases(fuzz.runs) } - /// Returns a fuzz configuration setup. Parameters are used to select tight scoped fuzz configs - /// that apply for a contract-function pair. A fallback configuration is applied + /// Returns an "invariant" test runner instance. Parameters are used to select tight scoped fuzz configs + /// that apply for a contract-function pair. A fallback configuration is applied /// if no specific setup is found for a given input. - /// - /// - `contract_id` is the name of the test contract. + /// + /// - `contract_id` is the id of the test contract, expressed as a relative path from the project root. + /// - `test_fn` is the name of the test function declared inside the test contract. + pub fn invariant_runner(&self, contract_id: S, test_fn: S) -> TestRunner + where + S: Into, + { + let invariant = self.invariant_config(contract_id, test_fn); + self.fuzzer_with_cases(invariant.runs) + } + + /// Returns a "fuzz" configuration setup. Parameters are used to select tight scoped fuzz configs + /// that apply for a contract-function pair. A fallback configuration is applied + /// if no specific setup is found for a given input. + /// + /// - `contract_id` is the id of the test contract, expressed as a relative path from the project root. /// - `test_fn` is the name of the test function declared inside the test contract. pub fn fuzz_config(&self, contract_id: S, test_fn: S) -> &FuzzConfig where @@ -70,8 +84,17 @@ impl TestOptions { self.inline_fuzz.get_config(contract_id, test_fn).unwrap_or(&self.fuzz) } - pub fn invariant_fuzzer(&self) -> TestRunner { - self.fuzzer_with_cases(self.invariant.runs) + /// Returns an "invariant" configuration setup. Parameters are used to select tight scoped + /// invariant configs that apply for a contract-function pair. A fallback configuration is + /// applied if no specific setup is found for a given input. + /// + /// - `contract_id` is the id of the test contract, expressed as a relative path from the project root. + /// - `test_fn` is the name of the test function declared inside the test contract. + pub fn invariant_config(&self, contract_id: S, test_fn: S) -> &InvariantConfig + where + S: Into, + { + self.inline_invariant.get_config(contract_id, test_fn).unwrap_or(&self.invariant) } pub fn fuzzer_with_cases(&self, cases: u32) -> TestRunner { diff --git a/forge/src/runner.rs b/forge/src/runner.rs index 895a47c77eb67..89ec47211aacf 100644 --- a/forge/src/runner.rs +++ b/forge/src/runner.rs @@ -11,7 +11,7 @@ use foundry_common::{ contracts::{ContractsByAddress, ContractsByArtifact}, TestFunctionExt, }; -use foundry_config::FuzzConfig; +use foundry_config::{FuzzConfig, InvariantConfig}; use foundry_evm::{ decode::decode_console_logs, executor::{CallResult, DeployResult, EvmError, ExecutionErr, Executor}, @@ -285,10 +285,9 @@ impl<'a> ContractRunner<'a> { .flat_map(|(func, should_fail)| { if func.is_fuzz_test() { let fn_name = &func.name; - let runner = test_options.fuzzer(self.name, fn_name); + let runner = test_options.fuzz_runner(self.name, fn_name); let fuzz_config = test_options.fuzz_config(self.name, fn_name); - self.run_fuzz_test( func, *should_fail, @@ -307,6 +306,7 @@ impl<'a> ContractRunner<'a> { if has_invariants { let identified_contracts = load_contracts(setup.traces.clone(), known_contracts); + let functions: Vec<&Function> = self .contract .functions() @@ -315,14 +315,26 @@ impl<'a> ContractRunner<'a> { }) .collect(); - let results = self.run_invariant_test( - test_options.invariant_fuzzer(), - setup, - test_options, - functions.clone(), - known_contracts, - identified_contracts, - )?; + let mut results: Vec = vec![]; + + for func in functions.iter() { + let fn_name = &func.name; + let runner = test_options.invariant_runner(self.name, fn_name); + let invariant_config = test_options.invariant_config(self.name, fn_name); + + let invariant_results = self.run_invariant_test( + runner, + setup.clone(), + *invariant_config, + vec![func], + known_contracts, + identified_contracts.clone(), + )?; + + for test_result in invariant_results { + results.push(test_result); + } + } results.into_iter().zip(functions.iter()).for_each(|(result, function)| { match result.kind { @@ -444,7 +456,7 @@ impl<'a> ContractRunner<'a> { &mut self, runner: TestRunner, setup: TestSetup, - test_options: TestOptions, + invariant_config: InvariantConfig, functions: Vec<&Function>, known_contracts: Option<&ContractsByArtifact>, identified_contracts: ContractsByAddress, @@ -457,7 +469,7 @@ impl<'a> ContractRunner<'a> { let mut evm = InvariantExecutor::new( &mut self.executor, runner, - test_options.invariant, + invariant_config, &identified_contracts, project_contracts, ); diff --git a/forge/tests/it/inline.rs b/forge/tests/it/inline.rs index bae6a51270b0f..7023210d5b359 100644 --- a/forge/tests/it/inline.rs +++ b/forge/tests/it/inline.rs @@ -4,19 +4,15 @@ mod tests { config::runner, test_helpers::{filter::Filter, COMPILED, PROJECT}, }; - use forge::{TestOptionsBuilder, result::{SuiteResult, TestKind, TestResult}}; + use forge::{ + result::{SuiteResult, TestKind, TestResult}, + TestOptions, TestOptionsBuilder, + }; use foundry_config::{FuzzConfig, InlineConfig, InvariantConfig}; #[test] fn inline_config_run_fuzz() { - let root = &PROJECT.paths.root; - - let opts = TestOptionsBuilder::default() - .fuzz(FuzzConfig::default()) - .invariant(InvariantConfig::default()) - .compile_output(&COMPILED) - .build(root) - .expect("Config loaded"); + let opts = test_options(); let filter = Filter::new(".*", ".*", ".*inline/FuzzInlineConf.t.sol"); @@ -24,18 +20,57 @@ mod tests { runner.test_options = opts.clone(); let result = runner.test(&filter, None, opts).expect("Test ran"); - let suite_result: &SuiteResult = result.get("inline/FuzzInlineConf.t.sol:FuzzInlineConf").unwrap(); - let test_result: &TestResult = suite_result.test_results.get("testInlineConfFuzz(uint8)").unwrap(); + let suite_result: &SuiteResult = + result.get("inline/FuzzInlineConf.t.sol:FuzzInlineConf").unwrap(); + let test_result: &TestResult = + suite_result.test_results.get("testInlineConfFuzz(uint8)").unwrap(); match &test_result.kind { TestKind::Fuzz { runs, .. } => { assert_eq!(runs, &1024); } _ => { - assert!(false); // Force test to fail - } + assert!(false); // Force test to fail + } } } + #[test] + fn inline_config_run_invariant() { + const ROOT: &str = "inline/InvariantInlineConf.t.sol"; + + let opts = test_options(); + let filter = Filter::new(".*", ".*", ".*inline/InvariantInlineConf.t.sol"); + let mut runner = runner(); + runner.test_options = opts.clone(); + + let result = runner.test(&filter, None, opts).expect("Test ran"); + + let suite_result_1 = + result.get(&format!("{ROOT}:InvariantInlineConf")).expect("Result exists"); + let suite_result_2 = + result.get(&format!("{ROOT}:InvariantInlineConf2")).expect("Result exists"); + + let test_result_1 = suite_result_1.test_results.get("invariant_neverFalse()").unwrap(); + let test_result_2 = suite_result_2.test_results.get("invariant_neverFalse()").unwrap(); + + match &test_result_1.kind { + TestKind::Invariant { runs, .. } => { + assert_eq!(runs, &333); + } + _ => { + assert!(false); // Force test to fail + } + } + + match &test_result_2.kind { + TestKind::Invariant { runs, .. } => { + assert_eq!(runs, &42); + } + _ => { + assert!(false); // Force test to fail + } + } + } #[test] fn inline_fuzz_config() { let root = &PROJECT.paths.root; @@ -70,4 +105,14 @@ mod tests { assert_eq!(inline_config.call_override, true); } } + + fn test_options() -> TestOptions { + let root = &PROJECT.paths.root; + TestOptionsBuilder::default() + .fuzz(FuzzConfig::default()) + .invariant(InvariantConfig::default()) + .compile_output(&COMPILED) + .build(root) + .expect("Config loaded") + } } diff --git a/testdata/inline/InvariantInlineConf.t.sol b/testdata/inline/InvariantInlineConf.t.sol index 90193262347d6..41e534f44a90a 100644 --- a/testdata/inline/InvariantInlineConf.t.sol +++ b/testdata/inline/InvariantInlineConf.t.sol @@ -34,6 +34,19 @@ contract InvariantInlineConf is DSTest { /// forge-config: default.invariant.fail-on-revert = false /// forge-config: default.invariant.call-override = true function invariant_neverFalse() public { - require(inv.flag1(), "false."); + require(true, "this is not going to revert"); + } +} + +contract InvariantInlineConf2 is DSTest { + InvariantBreaker inv; + + function setUp() public { + inv = new InvariantBreaker(); + } + + /// forge-config: default.invariant.runs = 42 + function invariant_neverFalse() public { + require(true, "this is not going to revert"); } } From 211be7a31d8b8867fd2b02635647ab6a6ccc7332 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 19 Apr 2023 21:36:58 +0200 Subject: [PATCH 15/57] typos --- config/src/inline/conf_parser.rs | 14 +++++++------- config/src/inline/mod.rs | 14 ++++++++------ forge/src/lib.rs | 24 ++++++++++++++---------- 3 files changed, 29 insertions(+), 23 deletions(-) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 4dae97fea3fb0..6b39f2e999d02 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -6,13 +6,13 @@ use regex::Regex; #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum ConfParserError { /// An invalid configuration property has been provided. - /// The property cannot be mapped to the configutaion object + /// The property cannot be mapped to the configuration object #[error("'{0}' is not a valid config property")] InvalidConfigProperty(String), - /// An error occurred while tryin to parse an integer configuration value + /// An error occurred while trying to parse an integer configuration value #[error(transparent)] ParseIntError(#[from] ParseIntError), - /// An error occurred while tryin to parse a boolean configuration value + /// An error occurred while trying to parse a boolean configuration value #[error(transparent)] ParseBoolError(#[from] ParseBoolError), } @@ -25,12 +25,12 @@ pub enum ConfParserError { /// /// ```solidity /// contract MyTest is Test { -/// // forge-config: default.fuzz.runs = 100 -/// // forge-config: ci.fuzz.runs = 500 +/// /// forge-config: default.fuzz.runs = 100 +/// /// forge-config: ci.fuzz.runs = 500 /// function test_SimpleFuzzTest(uint256 x) public {...} /// -/// // forge-config: default.fuzz.runs = 500 -/// // forge-config: ci.fuzz.runs = 10000 +/// /// forge-config: default.fuzz.runs = 500 +/// /// forge-config: ci.fuzz.runs = 10000 /// function test_ImportantFuzzTest(uint256 x) public {...} /// } /// ``` diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index fb90fcb66aff9..733bdc1861c47 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -9,7 +9,7 @@ use std::{ /// Represents per-test configurations, declared inline /// as structured comments in Solidity test files. This allows -/// to create configs directly bound to solidity test. +/// to create configs directly bound to a solidity test. /// `T` is the configuration type, and is bound to the [`ConfParser`] trait. Known /// implementations of [`ConfParser`] include [`FuzzConfig`](super::FuzzConfig) and /// [`InvariantConfig`](super::InvariantConfig)) @@ -19,8 +19,7 @@ where T: ConfParser + 'static, { /// Maps a (contract, test-function) - /// to a specific configuration provided by the user, in the - /// test function comments. + /// to a specific configuration provided by the user. configs: HashMap<(String, String), T>, } @@ -46,10 +45,13 @@ where type Error = ConfParserError; /// Tries to create an instance of `Self`, detecting inline configurations from the project - /// compile output. The second item of the input tuple is used as a fallback configuration. + /// compile output. /// - /// NOTE: `ProjectCompileOutput` is known to emit function comments, that we can try to parse - /// at our convenience, to detect inline test configurations. + /// Param is a tuple, whose elements are: + /// 1. Solidity compiler output, essential to extract comments from compiled functions. + /// 2. A reference to a base configuration. This essentially works as a fallback. + /// 3. A root path to express contract base dirs. This is essential to match inline configs at + /// runtime. fn try_from(value: (&'a ProjectCompileOutput, &'a T, &'a P)) -> Result { let mut configs: HashMap<(String, String), T> = HashMap::new(); let compile_output = value.0.clone(); diff --git a/forge/src/lib.rs b/forge/src/lib.rs index 82d6ed512e2df..7ec105131a054 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -43,11 +43,12 @@ pub struct TestOptions { } impl TestOptions { - /// Returns a "fuzz" test runner instance. Parameters are used to select tight scoped fuzz configs - /// that apply for a contract-function pair. A fallback configuration is applied + /// Returns a "fuzz" test runner instance. Parameters are used to select tight scoped fuzz + /// configs that apply for a contract-function pair. A fallback configuration is applied /// if no specific setup is found for a given input. /// - /// - `contract_id` is the id of the test contract, expressed as a relative path from the project root. + /// - `contract_id` is the id of the test contract, expressed as a relative path from the + /// project root. /// - `test_fn` is the name of the test function declared inside the test contract. pub fn fuzz_runner(&self, contract_id: S, test_fn: S) -> TestRunner where @@ -57,11 +58,12 @@ impl TestOptions { self.fuzzer_with_cases(fuzz.runs) } - /// Returns an "invariant" test runner instance. Parameters are used to select tight scoped fuzz configs - /// that apply for a contract-function pair. A fallback configuration is applied + /// Returns an "invariant" test runner instance. Parameters are used to select tight scoped fuzz + /// configs that apply for a contract-function pair. A fallback configuration is applied /// if no specific setup is found for a given input. /// - /// - `contract_id` is the id of the test contract, expressed as a relative path from the project root. + /// - `contract_id` is the id of the test contract, expressed as a relative path from the + /// project root. /// - `test_fn` is the name of the test function declared inside the test contract. pub fn invariant_runner(&self, contract_id: S, test_fn: S) -> TestRunner where @@ -71,11 +73,12 @@ impl TestOptions { self.fuzzer_with_cases(invariant.runs) } - /// Returns a "fuzz" configuration setup. Parameters are used to select tight scoped fuzz configs - /// that apply for a contract-function pair. A fallback configuration is applied + /// Returns a "fuzz" configuration setup. Parameters are used to select tight scoped fuzz + /// configs that apply for a contract-function pair. A fallback configuration is applied /// if no specific setup is found for a given input. /// - /// - `contract_id` is the id of the test contract, expressed as a relative path from the project root. + /// - `contract_id` is the id of the test contract, expressed as a relative path from the + /// project root. /// - `test_fn` is the name of the test function declared inside the test contract. pub fn fuzz_config(&self, contract_id: S, test_fn: S) -> &FuzzConfig where @@ -88,7 +91,8 @@ impl TestOptions { /// invariant configs that apply for a contract-function pair. A fallback configuration is /// applied if no specific setup is found for a given input. /// - /// - `contract_id` is the id of the test contract, expressed as a relative path from the project root. + /// - `contract_id` is the id of the test contract, expressed as a relative path from the + /// project root. /// - `test_fn` is the name of the test function declared inside the test contract. pub fn invariant_config(&self, contract_id: S, test_fn: S) -> &InvariantConfig where From b424e8da63189f06b984c2978c376b1382eaf479 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Fri, 28 Apr 2023 09:07:39 +0200 Subject: [PATCH 16/57] Docs typo --- config/src/inline/conf_parser.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 6b39f2e999d02..5e637852f2fd0 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -37,6 +37,8 @@ pub enum ConfParserError { pub trait ConfParser { /// Returns a prefix that is common to all valid configuration lines. /// That helps the parser to extract correct values out of a text. + /// + /// An example prefix would be `forge-config:default.fuzz.`. fn config_prefix() -> String; /// Returns From abcedb7b764a608bd3fd71fde9fe5c6cc41c4113 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Fri, 28 Apr 2023 09:37:31 +0200 Subject: [PATCH 17/57] cargo +nightly fmt --- config/src/inline/conf_parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 5e637852f2fd0..0fa118c79cc98 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -37,7 +37,7 @@ pub enum ConfParserError { pub trait ConfParser { /// Returns a prefix that is common to all valid configuration lines. /// That helps the parser to extract correct values out of a text. - /// + /// /// An example prefix would be `forge-config:default.fuzz.`. fn config_prefix() -> String; From 2119106004ff09df080d9fc24ab6a6bfdaf7554d Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 29 Apr 2023 09:45:52 +0200 Subject: [PATCH 18/57] Added test for block comments --- testdata/inline/FuzzInlineConf.t.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/testdata/inline/FuzzInlineConf.t.sol b/testdata/inline/FuzzInlineConf.t.sol index 5a856ac57adcb..8ab3a16d310bd 100644 --- a/testdata/inline/FuzzInlineConf.t.sol +++ b/testdata/inline/FuzzInlineConf.t.sol @@ -4,8 +4,10 @@ pragma solidity >=0.8.0; import "ds-test/test.sol"; contract FuzzInlineConf is DSTest { - /// forge-config: default.fuzz.runs = 1024 - /// forge-config: default.fuzz.max-test-rejects = 500 + /** + * forge-config: default.fuzz.runs = 1024 + * forge-config: default.fuzz.max-test-rejects = 500 + */ function testInlineConfFuzz(uint8 x) public { require(true, "this is not going to revert"); } From a4dc0eb8cb3f4652a17aedacb3d8a2a97f9aaaea Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 29 Apr 2023 10:30:04 +0200 Subject: [PATCH 19/57] Renamed ConfParser to InlineConfigParser --- config/src/fuzz.rs | 10 +++++----- config/src/inline/conf_parser.rs | 19 +++++++++++-------- config/src/inline/mod.rs | 20 ++++++++++---------- config/src/invariant.rs | 10 +++++----- config/src/lib.rs | 2 +- forge/src/lib.rs | 4 ++-- 6 files changed, 34 insertions(+), 31 deletions(-) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index b642853c0117e..ccfb7aa2e0359 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -4,7 +4,7 @@ use ethers_core::types::U256; use serde::{Deserialize, Serialize}; use crate::{ - inline::{ConfParser, ConfParserError}, + inline::{InlineConfigParser, InlineConfigParserError}, Config, }; @@ -40,13 +40,13 @@ impl Default for FuzzConfig { } } -impl ConfParser for FuzzConfig { +impl InlineConfigParser for FuzzConfig { fn config_prefix() -> String { let profile = Config::selected_profile().to_string(); format!("forge-config:{profile}.fuzz.") } - fn try_merge>(&self, text: S) -> Result, ConfParserError> + fn try_merge>(&self, text: S) -> Result, InlineConfigParserError> where Self: Sized + 'static, { @@ -63,7 +63,7 @@ impl ConfParser for FuzzConfig { match key.as_str() { "runs" => conf.runs = value.parse()?, "max-test-rejects" => conf.max_test_rejects = value.parse()?, - _ => Err(ConfParserError::InvalidConfigProperty(key.to_string()))?, + _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } Ok(Some(conf)) @@ -108,7 +108,7 @@ impl Default for FuzzDictionaryConfig { #[cfg(test)] mod tests { - use crate::{inline::ConfParser, FuzzConfig}; + use crate::{inline::InlineConfigParser, FuzzConfig}; use solang_parser::pt::Comment; #[test] diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 0fa118c79cc98..569b98b470f30 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -2,9 +2,9 @@ use std::{num::ParseIntError, str::ParseBoolError}; use regex::Regex; -/// Errors returned by the [`ConfParser`] trait. +/// Errors returned by the [`InlineConfigParser`] trait. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] -pub enum ConfParserError { +pub enum InlineConfigParserError { /// An invalid configuration property has been provided. /// The property cannot be mapped to the configuration object #[error("'{0}' is not a valid config property")] @@ -34,7 +34,7 @@ pub enum ConfParserError { /// function test_ImportantFuzzTest(uint256 x) public {...} /// } /// ``` -pub trait ConfParser { +pub trait InlineConfigParser { /// Returns a prefix that is common to all valid configuration lines. /// That helps the parser to extract correct values out of a text. /// @@ -44,8 +44,8 @@ pub trait ConfParser { /// Returns /// - `Some(Self)` in case some configurations are merged into self. /// - `None` in case there are no configurations that can be applied to self. - /// - `Err(ConfParserError)` in case of wrong configuration. - fn try_merge>(&self, text: S) -> Result, ConfParserError> + /// - `Err(InlineConfigParserError)` in case of wrong configuration. + fn try_merge>(&self, text: S) -> Result, InlineConfigParserError> where Self: Sized + 'static; @@ -83,7 +83,7 @@ fn remove_whitespaces(s: &str) -> String { #[cfg(test)] mod tests { - use super::{ConfParser, ConfParserError}; + use super::{InlineConfigParser, InlineConfigParserError}; #[test] fn config_variables() { @@ -114,12 +114,15 @@ mod tests { #[derive(Default)] struct TestParser; - impl ConfParser for TestParser { + impl InlineConfigParser for TestParser { fn config_prefix() -> String { "forge-config:default.fuzz.".to_string() } - fn try_merge>(&self, _text: S) -> Result, ConfParserError> + fn try_merge>( + &self, + _text: S, + ) -> Result, InlineConfigParserError> where Self: Sized + 'static, { diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 733bdc1861c47..0064b43c6fc7d 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -1,5 +1,5 @@ mod conf_parser; -pub use conf_parser::{ConfParser, ConfParserError}; +pub use conf_parser::{InlineConfigParser, InlineConfigParserError}; use ethers_solc::{artifacts::Node, ProjectCompileOutput}; use serde_json::Value; use std::{ @@ -10,13 +10,13 @@ use std::{ /// Represents per-test configurations, declared inline /// as structured comments in Solidity test files. This allows /// to create configs directly bound to a solidity test. -/// `T` is the configuration type, and is bound to the [`ConfParser`] trait. Known -/// implementations of [`ConfParser`] include [`FuzzConfig`](super::FuzzConfig) and +/// `T` is the configuration type, and is bound to the [`InlineConfigParser`] trait. Known +/// implementations of [`InlineConfigParser`] include [`FuzzConfig`](super::FuzzConfig) and /// [`InvariantConfig`](super::InvariantConfig)) #[derive(Default, Debug, Clone)] pub struct InlineConfig where - T: ConfParser + 'static, + T: InlineConfigParser + 'static, { /// Maps a (contract, test-function) /// to a specific configuration provided by the user. @@ -25,7 +25,7 @@ where impl InlineConfig where - T: ConfParser + 'static, + T: InlineConfigParser + 'static, { /// Returns an inline configuration, if any, for `contract_id` and `fn_name`.
/// - `contract_id` The identifier of the contract containing the annotated function. Note that @@ -39,10 +39,10 @@ where impl<'a, T, P> TryFrom<(&'a ProjectCompileOutput, &'a T, &'a P)> for InlineConfig where - T: ConfParser + 'static, + T: InlineConfigParser + 'static, P: AsRef, { - type Error = ConfParserError; + type Error = InlineConfigParserError; /// Tries to create an instance of `Self`, detecting inline configurations from the project /// compile output. @@ -90,15 +90,15 @@ fn contract_root_node<'a>(nodes: &'a [Node], contract_id: &'a str) -> Option<&'a /// Implements a DFS over a compiler output node and its children. /// If a configuration is found for a solidity function, it is added to /// `map` under the (contract id, function name) key. -/// This function may result in parsing errors (see [`ConfParserError`]). +/// This function may result in parsing errors (see [`InlineConfigParserError`]). fn try_apply( base_conf: &T, map: &mut HashMap<(String, String), T>, contract_id: &str, node: &Node, -) -> Result<(), ConfParserError> +) -> Result<(), InlineConfigParserError> where - T: ConfParser + 'static, + T: InlineConfigParser + 'static, { for n in node.nodes.iter() { if let Some((fn_name, fn_docs)) = get_fn_data(n) { diff --git a/config/src/invariant.rs b/config/src/invariant.rs index e1b9ee0d76420..79922524b224d 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -2,7 +2,7 @@ use crate::{ fuzz::FuzzDictionaryConfig, - inline::{ConfParser, ConfParserError}, + inline::{InlineConfigParser, InlineConfigParserError}, Config, }; use serde::{Deserialize, Serialize}; @@ -36,13 +36,13 @@ impl Default for InvariantConfig { } } -impl ConfParser for InvariantConfig { +impl InlineConfigParser for InvariantConfig { fn config_prefix() -> String { let profile = Config::selected_profile().to_string(); format!("forge-config:{profile}.invariant.") } - fn try_merge>(&self, text: S) -> Result, ConfParserError> + fn try_merge>(&self, text: S) -> Result, InlineConfigParserError> where Self: Sized + 'static, { @@ -61,7 +61,7 @@ impl ConfParser for InvariantConfig { "depth" => conf.depth = value.parse()?, "fail-on-revert" => conf.fail_on_revert = value.parse()?, "call-override" => conf.call_override = value.parse()?, - _ => Err(ConfParserError::InvalidConfigProperty(key.to_string()))?, + _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } Ok(Some(conf)) @@ -70,7 +70,7 @@ impl ConfParser for InvariantConfig { #[cfg(test)] mod tests { - use crate::{inline::ConfParser, InvariantConfig}; + use crate::{inline::InlineConfigParser, InvariantConfig}; #[test] fn parse_config_default_profile() { diff --git a/config/src/lib.rs b/config/src/lib.rs index b94e13487b321..d931432d94ca4 100644 --- a/config/src/lib.rs +++ b/config/src/lib.rs @@ -93,7 +93,7 @@ pub use invariant::InvariantConfig; use providers::remappings::RemappingsProvider; mod inline; -pub use inline::{ConfParserError, InlineConfig}; +pub use inline::{InlineConfig, InlineConfigParserError}; /// Foundry configuration /// diff --git a/forge/src/lib.rs b/forge/src/lib.rs index 7ec105131a054..e528210560bba 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -1,7 +1,7 @@ use std::path::Path; use ethers::solc::ProjectCompileOutput; -use foundry_config::{ConfParserError, FuzzConfig, InlineConfig, InvariantConfig}; +use foundry_config::{FuzzConfig, InlineConfig, InlineConfigParserError, InvariantConfig}; use proptest::test_runner::{RngAlgorithm, TestRng, TestRunner}; use tracing::trace; @@ -160,7 +160,7 @@ impl TestOptionsBuilder { /// `root` is a reference to the user's project root dir. This is essential /// to determine the base path of generated contract identifiers. This is to provide correct /// matchers for inline test configs. - pub fn build(self, root: impl AsRef) -> Result { + pub fn build(self, root: impl AsRef) -> Result { let base_fuzz = self.fuzz.unwrap_or_default(); let base_invariant = self.invariant.unwrap_or_default(); From 7e3f10a5b471a9e6a95b62c13ddaaeb5eb4c0831 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 29 Apr 2023 10:47:38 +0200 Subject: [PATCH 20/57] Use NodeType enum to match condition --- config/src/inline/mod.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 0064b43c6fc7d..72d53a70693f0 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -1,6 +1,9 @@ mod conf_parser; pub use conf_parser::{InlineConfigParser, InlineConfigParserError}; -use ethers_solc::{artifacts::Node, ProjectCompileOutput}; +use ethers_solc::{ + artifacts::{ast::NodeType, Node}, + ProjectCompileOutput, +}; use serde_json::Value; use std::{ collections::{BTreeMap, HashMap}, @@ -75,7 +78,7 @@ where /// the provided contract_id. fn contract_root_node<'a>(nodes: &'a [Node], contract_id: &'a str) -> Option<&'a Node> { for n in nodes.iter() { - if format!("{:?}", n.node_type) == *"ContractDefinition" { + if let NodeType::ContractDefinition = n.node_type { let contract_data = &n.other; if let Value::String(contract_name) = contract_data.get("name")? { if contract_id.ends_with(contract_name) { @@ -115,12 +118,13 @@ where } fn get_fn_data(node: &Node) -> Option<(String, String)> { - if format!("{:?}", node.node_type) == *"FunctionDefinition" { + if let NodeType::FunctionDefinition = node.node_type { let fn_data = &node.other; let fn_name: String = get_fn_name(fn_data)?; let fn_docs: String = get_fn_docs(fn_data)?; return Some((fn_name, fn_docs)) } + None } From 9bd4c360749dfdf8a18e3df95a0ad72cbe8bbc44 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Mon, 1 May 2023 15:48:43 +0200 Subject: [PATCH 21/57] Use helper type to describe the HashMap key --- config/src/inline/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 72d53a70693f0..02957fc1734e5 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -10,6 +10,9 @@ use std::{ path::Path, }; +/// Represents a (test-contract, test-function) pair +type InlineConfigKey = (String, String); + /// Represents per-test configurations, declared inline /// as structured comments in Solidity test files. This allows /// to create configs directly bound to a solidity test. @@ -21,9 +24,9 @@ pub struct InlineConfig where T: InlineConfigParser + 'static, { - /// Maps a (contract, test-function) + /// Maps a (test-contract, test-function) pair /// to a specific configuration provided by the user. - configs: HashMap<(String, String), T>, + configs: HashMap, } impl InlineConfig From 9b534c86ab70e63ce65e6ebe4ef3351b1407db4e Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 2 May 2023 15:53:06 +0200 Subject: [PATCH 22/57] Misconfigured line number added to the error - Need UNIT TESTS --- config/src/inline/mod.rs | 52 ++++++++++++++++++++++++++++++---------- config/src/lib.rs | 2 +- forge/src/lib.rs | 4 ++-- 3 files changed, 43 insertions(+), 15 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 02957fc1734e5..23721580db8ac 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -10,6 +10,18 @@ use std::{ path::Path, }; +/// Wrapper error struct that catches config parsing +/// errors [`InlineConfigParserError`], enriching them with context information +/// reporting the misconfigured line. +#[derive(thiserror::Error, Debug)] +#[error("Inline config error detected at {line} {source}")] +pub struct InlineConfigError { + /// Specifies the misconfigured line. This is something of the form + /// `dir/TestContract.t.sol:FuzzContract:10:12:111` + line: String, + source: InlineConfigParserError, +} + /// Represents a (test-contract, test-function) pair type InlineConfigKey = (String, String); @@ -48,7 +60,7 @@ where T: InlineConfigParser + 'static, P: AsRef, { - type Error = InlineConfigParserError; + type Error = InlineConfigError; /// Tries to create an instance of `Self`, detecting inline configurations from the project /// compile output. @@ -102,16 +114,24 @@ fn try_apply( map: &mut HashMap<(String, String), T>, contract_id: &str, node: &Node, -) -> Result<(), InlineConfigParserError> +) -> Result<(), InlineConfigError> where T: InlineConfigParser + 'static, { for n in node.nodes.iter() { - if let Some((fn_name, fn_docs)) = get_fn_data(n) { - if let Some(conf) = base_conf.try_merge(fn_docs.clone())? { - // We found a config that applies for the pair contract_id, fn_name. - let key = (contract_id.into(), fn_name); - map.insert(key, conf); + if let Some((fn_name, fn_docs, docs_src_line)) = get_fn_data(n) { + match base_conf.try_merge(fn_docs.clone()) { + Ok(Some(conf)) => { + // We found a config that applies for the pair contract_id, fn_name. + let key: InlineConfigKey = (contract_id.into(), fn_name); + map.insert(key, conf); + } + Err(e) => { + // We add context information to the error, to specify the misconfigured line + let line = format!("{contract_id}:{fn_name}:{docs_src_line}"); + Err(InlineConfigError { line, source: e })? + } + _ => { /* No inline config found, do nothing */ } } } @@ -120,12 +140,12 @@ where Ok(()) } -fn get_fn_data(node: &Node) -> Option<(String, String)> { +fn get_fn_data(node: &Node) -> Option<(String, String, String)> { if let NodeType::FunctionDefinition = node.node_type { let fn_data = &node.other; let fn_name: String = get_fn_name(fn_data)?; - let fn_docs: String = get_fn_docs(fn_data)?; - return Some((fn_name, fn_docs)) + let (fn_docs, docs_src_line): (String, String) = get_fn_docs(fn_data)?; + return Some((fn_name, fn_docs, docs_src_line)) } None @@ -138,10 +158,18 @@ fn get_fn_name(fn_data: &BTreeMap) -> Option { } } -fn get_fn_docs(fn_data: &BTreeMap) -> Option { +/// Inspects compiler output for a test function and returns: +/// - `Some((String, String))` in case the function has natspec comments. First item is a textual +/// natspec representation, the second item is the natspec src line, in the form "raw:col:length". +/// - `None` in case the function has not natspec comments. +fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { if let Value::Object(fn_docs) = fn_data.get("documentation")? { if let Value::String(comment) = fn_docs.get("text")? { - return Some(comment.into()) + let src_line = fn_docs + .get("src") + .map(Value::to_string) + .unwrap_or(String::from("")); + return Some((comment.into(), src_line)) } } None diff --git a/config/src/lib.rs b/config/src/lib.rs index d931432d94ca4..374b81722d829 100644 --- a/config/src/lib.rs +++ b/config/src/lib.rs @@ -93,7 +93,7 @@ pub use invariant::InvariantConfig; use providers::remappings::RemappingsProvider; mod inline; -pub use inline::{InlineConfig, InlineConfigParserError}; +pub use inline::{InlineConfig, InlineConfigError}; /// Foundry configuration /// diff --git a/forge/src/lib.rs b/forge/src/lib.rs index e528210560bba..0718c136e21dc 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -1,7 +1,7 @@ use std::path::Path; use ethers::solc::ProjectCompileOutput; -use foundry_config::{FuzzConfig, InlineConfig, InlineConfigParserError, InvariantConfig}; +use foundry_config::{FuzzConfig, InlineConfig, InlineConfigError, InvariantConfig}; use proptest::test_runner::{RngAlgorithm, TestRng, TestRunner}; use tracing::trace; @@ -160,7 +160,7 @@ impl TestOptionsBuilder { /// `root` is a reference to the user's project root dir. This is essential /// to determine the base path of generated contract identifiers. This is to provide correct /// matchers for inline test configs. - pub fn build(self, root: impl AsRef) -> Result { + pub fn build(self, root: impl AsRef) -> Result { let base_fuzz = self.fuzz.unwrap_or_default(); let base_invariant = self.invariant.unwrap_or_default(); From 06145695c2fe2ed79f42288730ff1b7421adea92 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 2 May 2023 18:04:46 +0200 Subject: [PATCH 23/57] Added very descriptive context to the parse error + unit test --- config/src/fuzz.rs | 14 +++++++++++--- config/src/inline/conf_parser.rs | 10 ++++------ config/src/inline/mod.rs | 20 ++++++++++++++++++++ config/src/invariant.rs | 24 ++++++++++++++++++++---- 4 files changed, 55 insertions(+), 13 deletions(-) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index ccfb7aa2e0359..bf5859f94108a 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -61,9 +61,17 @@ impl InlineConfigParser for FuzzConfig { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => conf.runs = value.parse()?, - "max-test-rejects" => conf.max_test_rejects = value.parse()?, - _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, + "runs" => { + conf.runs = value + .parse() + .map_err(|_| InlineConfigParserError::ParseIntError(key, value))? + } + "max-test-rejects" => { + conf.max_test_rejects = value + .parse() + .map_err(|_| InlineConfigParserError::ParseIntError(key, value))? + } + _ => Err(InlineConfigParserError::InvalidConfigProperty(key))?, } } Ok(Some(conf)) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 569b98b470f30..df385a8a74616 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -1,5 +1,3 @@ -use std::{num::ParseIntError, str::ParseBoolError}; - use regex::Regex; /// Errors returned by the [`InlineConfigParser`] trait. @@ -10,11 +8,11 @@ pub enum InlineConfigParserError { #[error("'{0}' is not a valid config property")] InvalidConfigProperty(String), /// An error occurred while trying to parse an integer configuration value - #[error(transparent)] - ParseIntError(#[from] ParseIntError), + #[error("Unable to parse config key '{0}' = '{1}' into an integer value")] + ParseIntError(String, String), /// An error occurred while trying to parse a boolean configuration value - #[error(transparent)] - ParseBoolError(#[from] ParseBoolError), + #[error("Unable to parse config key '{0}' = '{1}' into a boolean value")] + ParseBoolError(String, String), } /// This trait is intended to parse configurations from diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 23721580db8ac..43ea793eea856 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -174,3 +174,23 @@ fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { } None } + +#[cfg(test)] +mod tests { + use std::num::ParseIntError; + + use crate::InlineConfigError; + + use super::InlineConfigParserError; + + #[test] + fn inline_config_error() { + let source = + InlineConfigParserError::ParseBoolError("key".into(), "invalid-bool-value".into()); + let line = "dir/TestContract.t.sol:FuzzContract:10:12:111".to_string(); + let error = InlineConfigError { line: line.clone(), source: source.clone() }; + + let expected = format!("Inline config error detected at {line} {source}"); + assert_eq!(error.to_string(), expected); + } +} diff --git a/config/src/invariant.rs b/config/src/invariant.rs index 79922524b224d..3174107976079 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -57,10 +57,26 @@ impl InlineConfigParser for InvariantConfig { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => conf.runs = value.parse()?, - "depth" => conf.depth = value.parse()?, - "fail-on-revert" => conf.fail_on_revert = value.parse()?, - "call-override" => conf.call_override = value.parse()?, + "runs" => { + conf.runs = value + .parse() + .map_err(|_| InlineConfigParserError::ParseIntError(key, value))? + } + "depth" => { + conf.depth = value + .parse() + .map_err(|_| InlineConfigParserError::ParseIntError(key, value))? + } + "fail-on-revert" => { + conf.fail_on_revert = value + .parse() + .map_err(|_| InlineConfigParserError::ParseBoolError(key, value))? + } + "call-override" => { + conf.call_override = value + .parse() + .map_err(|_| InlineConfigParserError::ParseBoolError(key, value))? + } _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } From 6e79594df989f4802ccd35dbee9a9aae8b5928cb Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 2 May 2023 18:15:08 +0200 Subject: [PATCH 24/57] Emphasis on the "Invalid" keyword --- config/src/inline/conf_parser.rs | 6 +++--- config/src/inline/mod.rs | 9 +++------ 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index df385a8a74616..879df70cf5490 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -5,13 +5,13 @@ use regex::Regex; pub enum InlineConfigParserError { /// An invalid configuration property has been provided. /// The property cannot be mapped to the configuration object - #[error("'{0}' is not a valid config property")] + #[error("'{0}' is not an Invalid config property")] InvalidConfigProperty(String), /// An error occurred while trying to parse an integer configuration value - #[error("Unable to parse config key '{0}' = '{1}' into an integer value")] + #[error("Invalid config value for key '{0}'. Unable to parse '{1}' into an integer value")] ParseIntError(String, String), /// An error occurred while trying to parse a boolean configuration value - #[error("Unable to parse config key '{0}' = '{1}' into a boolean value")] + #[error("Invalid config value for key '{0}'. Unable to parse '{1}' into a boolean value")] ParseBoolError(String, String), } diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 43ea793eea856..039632d744e35 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -14,7 +14,7 @@ use std::{ /// errors [`InlineConfigParserError`], enriching them with context information /// reporting the misconfigured line. #[derive(thiserror::Error, Debug)] -#[error("Inline config error detected at {line} {source}")] +#[error("Inline config Error detected at {line} {source}")] pub struct InlineConfigError { /// Specifies the misconfigured line. This is something of the form /// `dir/TestContract.t.sol:FuzzContract:10:12:111` @@ -177,11 +177,8 @@ fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { #[cfg(test)] mod tests { - use std::num::ParseIntError; - - use crate::InlineConfigError; - use super::InlineConfigParserError; + use crate::InlineConfigError; #[test] fn inline_config_error() { @@ -190,7 +187,7 @@ mod tests { let line = "dir/TestContract.t.sol:FuzzContract:10:12:111".to_string(); let error = InlineConfigError { line: line.clone(), source: source.clone() }; - let expected = format!("Inline config error detected at {line} {source}"); + let expected = format!("Inline config Error detected at {line} {source}"); assert_eq!(error.to_string(), expected); } } From 651cf782c95332c6ea4488bd80d9e810ae2d1119 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 3 May 2023 19:50:06 +0200 Subject: [PATCH 25/57] Big refactoring. Design is cleaner and more appropriate. It allows better validation flexibility. Need to fix tests --- config/src/fuzz.rs | 30 ++---- config/src/inline/conf_parser.rs | 148 ++++++++++++++------------- config/src/inline/mod.rs | 166 ++++--------------------------- config/src/inline/natspec.rs | 130 ++++++++++++++++++++++++ config/src/invariant.rs | 40 ++------ config/src/lib.rs | 2 +- forge/tests/it/inline.rs | 5 +- 7 files changed, 250 insertions(+), 271 deletions(-) create mode 100644 config/src/inline/natspec.rs diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index bf5859f94108a..5870d760412a7 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -3,10 +3,7 @@ use ethers_core::types::U256; use serde::{Deserialize, Serialize}; -use crate::{ - inline::{InlineConfigParser, InlineConfigParserError}, - Config, -}; +use crate::inline::{InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_FUZZ_KEY}; /// Contains for fuzz testing #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -41,16 +38,13 @@ impl Default for FuzzConfig { } impl InlineConfigParser for FuzzConfig { - fn config_prefix() -> String { - let profile = Config::selected_profile().to_string(); - format!("forge-config:{profile}.fuzz.") + fn config_key() -> String { + INLINE_CONFIG_FUZZ_KEY.into() } - fn try_merge>(&self, text: S) -> Result, InlineConfigParserError> - where - Self: Sized + 'static, - { - let vars: Vec<(String, String)> = Self::config_variables::(text); + fn try_merge(&self, configs: &[String]) -> Result, InlineConfigParserError> { + let vars: Vec<(String, String)> = Self::config_variables(configs); + if vars.is_empty() { return Ok(None) } @@ -61,16 +55,8 @@ impl InlineConfigParser for FuzzConfig { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => { - conf.runs = value - .parse() - .map_err(|_| InlineConfigParserError::ParseIntError(key, value))? - } - "max-test-rejects" => { - conf.max_test_rejects = value - .parse() - .map_err(|_| InlineConfigParserError::ParseIntError(key, value))? - } + "runs" => conf.runs = Self::parse_u32(key, value)?, + "max-test-rejects" => conf.max_test_rejects = Self::parse_u32(key, value)?, _ => Err(InlineConfigParserError::InvalidConfigProperty(key))?, } } diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 879df70cf5490..be601caa03204 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -1,5 +1,9 @@ use regex::Regex; +use crate::{InlineConfigError, NatSpec}; + +use super::remove_whitespaces; + /// Errors returned by the [`InlineConfigParser`] trait. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum InlineConfigParserError { @@ -32,36 +36,82 @@ pub enum InlineConfigParserError { /// function test_ImportantFuzzTest(uint256 x) public {...} /// } /// ``` -pub trait InlineConfigParser { - /// Returns a prefix that is common to all valid configuration lines. - /// That helps the parser to extract correct values out of a text. +pub trait InlineConfigParser +where + Self: Clone + Default + Sized + 'static, +{ + /// Returns a config key that is common to all valid configuration lines + /// for the current impl. This helps to extract correct values out of a text. /// - /// An example prefix would be `forge-config:default.fuzz.`. - fn config_prefix() -> String; + /// An example key would be `fuzz` of `invariant`. + fn config_key() -> String; + /// Tries to override `self` properties with values specified in the `configs` parameter. + /// /// Returns /// - `Some(Self)` in case some configurations are merged into self. /// - `None` in case there are no configurations that can be applied to self. /// - `Err(InlineConfigParserError)` in case of wrong configuration. - fn try_merge>(&self, text: S) -> Result, InlineConfigParserError> - where - Self: Sized + 'static; + fn try_merge(&self, configs: &[String]) -> Result, InlineConfigParserError>; + + /// Validates all configurations contained in a natspec, that apply + /// to the current configuration key. + /// + /// i.e. Given the `invariant` config key and a natspec comment of the form, + /// ```solidity + /// /// forge-config: default.invariant.runs = 500 + /// /// forge-config: default.invariant.depth = 500 + /// /// forge-config: ci.invariant.depth = 500 + /// /// forge-config: ci.fuzz.runs = 10 + /// ``` + /// would validate the whole `invariant` configuration. + fn validate(natspec: &NatSpec) -> Result<(), InlineConfigError> { + let config_key = Self::config_key(); + + let configs = natspec + .config_lines() + .into_iter() + .filter(|l| l.contains(&config_key)) + .collect::>(); + + Self::default().try_merge(&configs).map_err(|e| { + let line = natspec.debug_context(); + InlineConfigError { line, source: e } + })?; + + Ok(()) + } - /// Given a configuration `text` returns all available pairs (key, value) - /// matching the `config_prefix` - fn config_variables>(text: S) -> Vec<(String, String)> { + /// Given a list of `config_lines, returns all available pairs (key, value) + /// matching the current config key + /// + /// i.e. Given the `invariant` config key and a vector of config lines + /// ```rust + /// let _config_lines = vec![ + /// "forge-config: default.invariant.runs = 500", + /// "forge-config: default.invariant.depth = 500", + /// "forge-config: ci.invariant.depth = 500", + /// "forge-config: ci.fuzz.runs = 10" + /// ]; + /// ``` + /// would return the whole set of `invariant` configs. + /// ```rust + /// let _result = vec![ + /// ("runs", "500"), + /// ("depth", "500"), + /// ("depth", "500"), + /// ]; + /// ``` + fn config_variables(config_lines: &[String]) -> Vec<(String, String)> { let mut result: Vec<(String, String)> = vec![]; - let prefix = Self::config_prefix(); + let prefix = Self::config_key(); + let pattern = format!("^.*{prefix}"); + let re = Regex::new(&pattern).unwrap(); - text.as_ref() - .split('\n') - .map(remove_whitespaces) - .filter(|l| l.contains(&prefix)) - .map(|l| { - let pattern = format!("^.*{prefix}"); - let re = Regex::new(&pattern).unwrap(); - re.replace(&l, "").to_string() - }) + config_lines + .iter() + .map(|l| remove_whitespaces(l)) + .map(|l| re.replace(&l, "").to_string()) .for_each(|line| { let key_value = line.split('=').collect::>(); // i.e. "['runs', '500']" if let Some(key) = key_value.first() { @@ -73,58 +123,14 @@ pub trait InlineConfigParser { result } -} - -fn remove_whitespaces(s: &str) -> String { - s.chars().filter(|c| !c.is_whitespace()).collect() -} - -#[cfg(test)] -mod tests { - use super::{InlineConfigParser, InlineConfigParserError}; - #[test] - fn config_variables() { - let text = r#" - forge-config: default.fuzz.runs = 600 - forge-config: default.fuzz.foo = 700 - forge-config: default.fuzz.bar = 800 - invalid-prefix - "#; - - let vars = TestParser::config_variables(text); - assert_eq!( - vec![ - ("runs".to_string(), "600".to_string()), - ("foo".to_string(), "700".to_string()), - ("bar".to_string(), "800".to_string()) - ], - vars - ); - } - - #[test] - fn white_spaces_are_ignored() { - let text = "forge-config: default. fuzz.runs = 600"; - let vars = TestParser::config_variables(text); - assert_eq!(vec![("runs".to_string(), "600".to_string())], vars); + /// Tries to parse a `u32` from `value` + fn parse_u32(key: String, value: String) -> Result { + value.parse().map_err(|_| InlineConfigParserError::ParseIntError(key, value)) } - #[derive(Default)] - struct TestParser; - impl InlineConfigParser for TestParser { - fn config_prefix() -> String { - "forge-config:default.fuzz.".to_string() - } - - fn try_merge>( - &self, - _text: S, - ) -> Result, InlineConfigParserError> - where - Self: Sized + 'static, - { - Ok(Some(Self::default())) - } + /// Tries to parse a `bool` from `value` + fn parse_bool(key: String, value: String) -> Result { + value.parse().map_err(|_| InlineConfigParserError::ParseBoolError(key, value)) } } diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 039632d744e35..730e01a6fdd7d 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -1,14 +1,13 @@ mod conf_parser; pub use conf_parser::{InlineConfigParser, InlineConfigParserError}; -use ethers_solc::{ - artifacts::{ast::NodeType, Node}, - ProjectCompileOutput, -}; -use serde_json::Value; -use std::{ - collections::{BTreeMap, HashMap}, - path::Path, -}; +use std::collections::HashMap; + +mod natspec; +pub use natspec::NatSpec; + +const INLINE_CONFIG_PREFIX: &str = "forge-config:"; +pub const INLINE_CONFIG_FUZZ_KEY: &str = "fuzz"; +pub const INLINE_CONFIG_INVARIANT_KEY: &str = "invariant"; /// Wrapper error struct that catches config parsing /// errors [`InlineConfigParserError`], enriching them with context information @@ -18,8 +17,9 @@ use std::{ pub struct InlineConfigError { /// Specifies the misconfigured line. This is something of the form /// `dir/TestContract.t.sol:FuzzContract:10:12:111` - line: String, - source: InlineConfigParserError, + pub line: String, + /// The inner error + pub source: InlineConfigParserError, } /// Represents a (test-contract, test-function) pair @@ -28,151 +28,29 @@ type InlineConfigKey = (String, String); /// Represents per-test configurations, declared inline /// as structured comments in Solidity test files. This allows /// to create configs directly bound to a solidity test. -/// `T` is the configuration type, and is bound to the [`InlineConfigParser`] trait. Known -/// implementations of [`InlineConfigParser`] include [`FuzzConfig`](super::FuzzConfig) and -/// [`InvariantConfig`](super::InvariantConfig)) #[derive(Default, Debug, Clone)] -pub struct InlineConfig -where - T: InlineConfigParser + 'static, -{ +pub struct InlineConfig { /// Maps a (test-contract, test-function) pair /// to a specific configuration provided by the user. configs: HashMap, } -impl InlineConfig -where - T: InlineConfigParser + 'static, -{ - /// Returns an inline configuration, if any, for `contract_id` and `fn_name`.
- /// - `contract_id` The identifier of the contract containing the annotated function. Note that - /// contract id is a path relative to the project's root check `with_stripped_file_prefixes` - /// in [`ProjectCompileOutput`](ethers_solc::compile::output::ProjectCompileOutput)). - /// - `fn_name` The name of the function annotated with an inline configuration. - pub fn get_config>(&self, contract_id: S, fn_name: S) -> Option<&T> { +impl InlineConfig { + /// Returns an inline configuration, if any, for a test function. + /// Configuration is identified by the pair "contract", "function". + pub fn get>(&self, contract_id: S, fn_name: S) -> Option<&T> { self.configs.get(&(contract_id.into(), fn_name.into())) } -} - -impl<'a, T, P> TryFrom<(&'a ProjectCompileOutput, &'a T, &'a P)> for InlineConfig -where - T: InlineConfigParser + 'static, - P: AsRef, -{ - type Error = InlineConfigError; - - /// Tries to create an instance of `Self`, detecting inline configurations from the project - /// compile output. - /// - /// Param is a tuple, whose elements are: - /// 1. Solidity compiler output, essential to extract comments from compiled functions. - /// 2. A reference to a base configuration. This essentially works as a fallback. - /// 3. A root path to express contract base dirs. This is essential to match inline configs at - /// runtime. - fn try_from(value: (&'a ProjectCompileOutput, &'a T, &'a P)) -> Result { - let mut configs: HashMap<(String, String), T> = HashMap::new(); - let compile_output = value.0.clone(); - let base_conf = value.1; - let root = value.2; - - for artifact in compile_output.with_stripped_file_prefixes(root).into_artifacts() { - if let Some(ast) = artifact.1.ast.as_ref() { - let contract_id: String = artifact.0.identifier(); - if let Some(node) = contract_root_node(&ast.nodes, &contract_id) { - try_apply(base_conf, &mut configs, &contract_id, node)?; - } - } - } - Ok(Self { configs }) + /// Inserts an inline configuration, for a test function. + /// Configuration is identified by the pair "contract", "function". + pub fn insert>(&mut self, contract_id: S, fn_name: S, config: T) { + self.configs.insert((contract_id.into(), fn_name.into()), config); } } -/// Given a list of nodes, find a "ContractDefinition" node that matches -/// the provided contract_id. -fn contract_root_node<'a>(nodes: &'a [Node], contract_id: &'a str) -> Option<&'a Node> { - for n in nodes.iter() { - if let NodeType::ContractDefinition = n.node_type { - let contract_data = &n.other; - if let Value::String(contract_name) = contract_data.get("name")? { - if contract_id.ends_with(contract_name) { - return Some(n) - } - } - } - } - None -} - -/// Implements a DFS over a compiler output node and its children. -/// If a configuration is found for a solidity function, it is added to -/// `map` under the (contract id, function name) key. -/// This function may result in parsing errors (see [`InlineConfigParserError`]). -fn try_apply( - base_conf: &T, - map: &mut HashMap<(String, String), T>, - contract_id: &str, - node: &Node, -) -> Result<(), InlineConfigError> -where - T: InlineConfigParser + 'static, -{ - for n in node.nodes.iter() { - if let Some((fn_name, fn_docs, docs_src_line)) = get_fn_data(n) { - match base_conf.try_merge(fn_docs.clone()) { - Ok(Some(conf)) => { - // We found a config that applies for the pair contract_id, fn_name. - let key: InlineConfigKey = (contract_id.into(), fn_name); - map.insert(key, conf); - } - Err(e) => { - // We add context information to the error, to specify the misconfigured line - let line = format!("{contract_id}:{fn_name}:{docs_src_line}"); - Err(InlineConfigError { line, source: e })? - } - _ => { /* No inline config found, do nothing */ } - } - } - - try_apply(base_conf, map, contract_id, n)?; - } - Ok(()) -} - -fn get_fn_data(node: &Node) -> Option<(String, String, String)> { - if let NodeType::FunctionDefinition = node.node_type { - let fn_data = &node.other; - let fn_name: String = get_fn_name(fn_data)?; - let (fn_docs, docs_src_line): (String, String) = get_fn_docs(fn_data)?; - return Some((fn_name, fn_docs, docs_src_line)) - } - - None -} - -fn get_fn_name(fn_data: &BTreeMap) -> Option { - match fn_data.get("name")? { - Value::String(fn_name) => Some(fn_name.into()), - _ => None, - } -} - -/// Inspects compiler output for a test function and returns: -/// - `Some((String, String))` in case the function has natspec comments. First item is a textual -/// natspec representation, the second item is the natspec src line, in the form "raw:col:length". -/// - `None` in case the function has not natspec comments. -fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { - if let Value::Object(fn_docs) = fn_data.get("documentation")? { - if let Value::String(comment) = fn_docs.get("text")? { - let src_line = fn_docs - .get("src") - .map(Value::to_string) - .unwrap_or(String::from("")); - return Some((comment.into(), src_line)) - } - } - None +fn remove_whitespaces(s: &str) -> String { + s.chars().filter(|c| !c.is_whitespace()).collect() } #[cfg(test)] diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs new file mode 100644 index 0000000000000..596513eb09b19 --- /dev/null +++ b/config/src/inline/natspec.rs @@ -0,0 +1,130 @@ +use std::{collections::BTreeMap, path::Path}; + +use ethers_solc::{ + artifacts::{ast::NodeType, Node}, + ProjectCompileOutput, +}; +use serde_json::Value; + +use super::{remove_whitespaces, INLINE_CONFIG_PREFIX}; + +/// Convenient struct to hold in-line per-test configurations +pub struct NatSpec { + /// The parent contract of the natspec + pub contract: String, + /// The function annotated with the natspec + pub function: String, + /// The line the natspec appears, in the form + /// `row:col:length` i.e. `10:21:122` + pub line: String, + /// The actual natspec comment, without slashes or block + /// punctuation + pub docs: String, +} + +impl NatSpec { + /// Factory function that extracts a vector of [`NatSpec`] instances from + /// a solc compiler output. The root path is to express contract base dirs. + /// That is essential to match per-test configs at runtime. + pub fn parse

(output: &ProjectCompileOutput, root: &P) -> Vec + where + P: AsRef, + { + let mut natspecs: Vec = vec![]; + + let output = output.clone(); + for artifact in output.with_stripped_file_prefixes(root).into_artifacts() { + if let Some(ast) = artifact.1.ast.as_ref() { + let contract: String = artifact.0.identifier(); + if let Some(node) = contract_root_node(&ast.nodes, &contract) { + apply(&mut natspecs, &contract, node) + } + } + } + natspecs + } + + /// Returns a string describing the natspec + /// context, for debugging purposes 🐞
+ /// i.e. `dir/TestContract.t.sol:FuzzContract:10:12:111` + pub fn debug_context(&self) -> String { + format!("{}:{}:{}", self.contract, self.function, self.line) + } + + /// Returns a list of configuration lines that match a specific string prefix + pub fn config_lines_with_prefix>(&self, prefix: S) -> Vec { + let prefix: String = prefix.into(); + self.config_lines().into_iter().filter(|l| l.starts_with(&prefix)).collect() + } + + /// Returns a list of all the configuration lines available in the natspec + pub fn config_lines(&self) -> Vec { + self.docs + .split('\n') + .map(remove_whitespaces) + .filter(|line| line.contains(INLINE_CONFIG_PREFIX)) + .collect::>() + } +} + +/// Given a list of nodes, find a "ContractDefinition" node that matches +/// the provided contract_id. +fn contract_root_node<'a>(nodes: &'a [Node], contract_id: &'a str) -> Option<&'a Node> { + for n in nodes.iter() { + if let NodeType::ContractDefinition = n.node_type { + let contract_data = &n.other; + if let Value::String(contract_name) = contract_data.get("name")? { + if contract_id.ends_with(contract_name) { + return Some(n) + } + } + } + } + None +} + +/// Implements a DFS over a compiler output node and its children. +/// If a natspec is found it is added to `natspecs` +fn apply(natspecs: &mut Vec, contract: &str, node: &Node) { + for n in node.nodes.iter() { + if let Some((function, docs, line)) = get_fn_data(n) { + natspecs.push(NatSpec { contract: contract.into(), function, line, docs }) + } + apply(natspecs, contract, n); + } +} + +fn get_fn_data(node: &Node) -> Option<(String, String, String)> { + if let NodeType::FunctionDefinition = node.node_type { + let fn_data = &node.other; + let fn_name: String = get_fn_name(fn_data)?; + let (fn_docs, docs_src_line): (String, String) = get_fn_docs(fn_data)?; + return Some((fn_name, fn_docs, docs_src_line)) + } + + None +} + +fn get_fn_name(fn_data: &BTreeMap) -> Option { + match fn_data.get("name")? { + Value::String(fn_name) => Some(fn_name.into()), + _ => None, + } +} + +/// Inspects Solc compiler output for documentation comments: +/// - `Some((String, String))` in case the function has natspec comments. First item is a textual +/// natspec representation, the second item is the natspec src line, in the form "raw:col:length". +/// - `None` in case the function has not natspec comments. +fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { + if let Value::Object(fn_docs) = fn_data.get("documentation")? { + if let Value::String(comment) = fn_docs.get("text")? { + let src_line = fn_docs + .get("src") + .map(Value::to_string) + .unwrap_or(String::from("")); + return Some((comment.into(), src_line)) + } + } + None +} diff --git a/config/src/invariant.rs b/config/src/invariant.rs index 3174107976079..3067d8d179e55 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -2,8 +2,7 @@ use crate::{ fuzz::FuzzDictionaryConfig, - inline::{InlineConfigParser, InlineConfigParserError}, - Config, + inline::{InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_INVARIANT_KEY}, }; use serde::{Deserialize, Serialize}; @@ -37,16 +36,13 @@ impl Default for InvariantConfig { } impl InlineConfigParser for InvariantConfig { - fn config_prefix() -> String { - let profile = Config::selected_profile().to_string(); - format!("forge-config:{profile}.invariant.") + fn config_key() -> String { + INLINE_CONFIG_INVARIANT_KEY.into() } - fn try_merge>(&self, text: S) -> Result, InlineConfigParserError> - where - Self: Sized + 'static, - { - let vars: Vec<(String, String)> = Self::config_variables::(text); + fn try_merge(&self, configs: &[String]) -> Result, InlineConfigParserError> { + let vars: Vec<(String, String)> = Self::config_variables(configs); + if vars.is_empty() { return Ok(None) } @@ -57,26 +53,10 @@ impl InlineConfigParser for InvariantConfig { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => { - conf.runs = value - .parse() - .map_err(|_| InlineConfigParserError::ParseIntError(key, value))? - } - "depth" => { - conf.depth = value - .parse() - .map_err(|_| InlineConfigParserError::ParseIntError(key, value))? - } - "fail-on-revert" => { - conf.fail_on_revert = value - .parse() - .map_err(|_| InlineConfigParserError::ParseBoolError(key, value))? - } - "call-override" => { - conf.call_override = value - .parse() - .map_err(|_| InlineConfigParserError::ParseBoolError(key, value))? - } + "runs" => conf.runs = Self::parse_u32(key, value)?, + "depth" => conf.depth = Self::parse_u32(key, value)?, + "fail-on-revert" => conf.fail_on_revert = Self::parse_bool(key, value)?, + "call-override" => conf.call_override = Self::parse_bool(key, value)?, _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } diff --git a/config/src/lib.rs b/config/src/lib.rs index 374b81722d829..688ab8650de9f 100644 --- a/config/src/lib.rs +++ b/config/src/lib.rs @@ -93,7 +93,7 @@ pub use invariant::InvariantConfig; use providers::remappings::RemappingsProvider; mod inline; -pub use inline::{InlineConfig, InlineConfigError}; +pub use inline::{InlineConfig, InlineConfigError, InlineConfigParser, NatSpec}; /// Foundry configuration /// diff --git a/forge/tests/it/inline.rs b/forge/tests/it/inline.rs index 7023210d5b359..da089b9da1260 100644 --- a/forge/tests/it/inline.rs +++ b/forge/tests/it/inline.rs @@ -80,7 +80,7 @@ mod tests { // Inline config defined in testdata/inline/FuzzInlineConf.t.sol let contract_name = "inline/FuzzInlineConf.t.sol:FuzzInlineConf"; let function_name = "testInlineConfFuzz"; - let inline_config: &FuzzConfig = conf.get_config(contract_name, function_name).unwrap(); + let inline_config: &FuzzConfig = conf.get(contract_name, function_name).unwrap(); assert_eq!(inline_config.runs, 1024); assert_eq!(inline_config.max_test_rejects, 500); } @@ -97,8 +97,7 @@ mod tests { // Inline config defined in testdata/inline/InvariantInlineConf.t.sol let contract_name = "inline/InvariantInlineConf.t.sol:InvariantInlineConf"; let function_name = "invariant_neverFalse"; - let inline_config: &InvariantConfig = - conf.get_config(contract_name, function_name).unwrap(); + let inline_config: &InvariantConfig = conf.get(contract_name, function_name).unwrap(); assert_eq!(inline_config.runs, 333); assert_eq!(inline_config.depth, 32); assert_eq!(inline_config.fail_on_revert, false); From e77920aa4c71446ed9b3e043e0a9f82fb6ba88ab Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 3 May 2023 22:00:49 +0200 Subject: [PATCH 26/57] natspec unit tests --- config/src/inline/mod.rs | 2 +- config/src/inline/natspec.rs | 51 +++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 730e01a6fdd7d..b6b40d8e46043 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -5,7 +5,7 @@ use std::collections::HashMap; mod natspec; pub use natspec::NatSpec; -const INLINE_CONFIG_PREFIX: &str = "forge-config:"; +const INLINE_CONFIG_PREFIX: &str = "forge-config"; pub const INLINE_CONFIG_FUZZ_KEY: &str = "fuzz"; pub const INLINE_CONFIG_INVARIANT_KEY: &str = "invariant"; diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index 596513eb09b19..0129e17813634 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -46,7 +46,7 @@ impl NatSpec { /// Returns a string describing the natspec /// context, for debugging purposes 🐞
- /// i.e. `dir/TestContract.t.sol:FuzzContract:10:12:111` + /// i.e. `dir/TestContract.t.sol:FuzzContract:test_myFunction:10:12:111` pub fn debug_context(&self) -> String { format!("{}:{}:{}", self.contract, self.function, self.line) } @@ -128,3 +128,52 @@ fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { } None } + +#[cfg(test)] +mod tests { + use crate::NatSpec; + + #[test] + fn config_lines() { + let natspec = natspec(); + let config_lines = natspec.config_lines(); + assert_eq!( + config_lines, + vec![ + "forge-config:default.fuzz.runs=600".to_string(), + "forge-config:ci.fuzz.runs=500".to_string(), + "forge-config:default.invariant.runs=1".to_string() + ] + ) + } + + #[test] + fn config_lines_with_prefix() { + use super::INLINE_CONFIG_PREFIX; + let natspec = natspec(); + let prefix = format!("{INLINE_CONFIG_PREFIX}:default"); + let config_lines = natspec.config_lines_with_prefix(prefix); + assert_eq!( + config_lines, + vec![ + "forge-config:default.fuzz.runs=600".to_string(), + "forge-config:default.invariant.runs=1".to_string() + ] + ) + } + + fn natspec() -> NatSpec { + let conf = r#" + forge-config: default.fuzz.runs = 600 + forge-config: ci.fuzz.runs = 500 + forge-config: default.invariant.runs = 1 + "#; + + NatSpec { + contract: "dir/TestContract.t.sol:FuzzContract".to_string(), + function: "test_myFunction".to_string(), + line: "10:12:111".to_string(), + docs: conf.to_string(), + } + } +} From 8e0ef3e2e54609176557e274a163e4dafb856632 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 3 May 2023 22:43:01 +0200 Subject: [PATCH 27/57] Refactor Unit tests InvariantConfig + FuzzConfig --- Cargo.lock | 1 - config/Cargo.toml | 1 - config/src/fuzz.rs | 91 +++++++++++++------------------- config/src/inline/conf_parser.rs | 12 +++-- config/src/invariant.rs | 79 +++++++++++++-------------- 5 files changed, 84 insertions(+), 100 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 65934f2cea156..f9a0e99649025 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2596,7 +2596,6 @@ dependencies = [ "serde", "serde_json", "serde_regex", - "solang-parser", "tempfile", "thiserror", "toml 0.7.3", diff --git a/config/Cargo.toml b/config/Cargo.toml index 8716366e162f1..1bc6b324d93f2 100644 --- a/config/Cargo.toml +++ b/config/Cargo.toml @@ -47,4 +47,3 @@ path-slash = "0.2.1" pretty_assertions = "1.3.0" figment = { version = "0.10", features = ["test"] } tempfile = "3.5" -solang-parser = "0.2.3" diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 5870d760412a7..7ed1c5077e4d8 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -46,7 +46,7 @@ impl InlineConfigParser for FuzzConfig { let vars: Vec<(String, String)> = Self::config_variables(configs); if vars.is_empty() { - return Ok(None) + return Ok(None); } let mut conf = *self; @@ -103,70 +103,53 @@ impl Default for FuzzDictionaryConfig { #[cfg(test)] mod tests { use crate::{inline::InlineConfigParser, FuzzConfig}; - use solang_parser::pt::Comment; #[test] - fn parse_config_default_profile() { - let conf = "forge-config: default.fuzz.runs = 1024"; - let base_conf: FuzzConfig = FuzzConfig::default(); - let parsed: FuzzConfig = base_conf.try_merge(conf).unwrap().expect("Valid config"); - assert_eq!(parsed.runs, 1024); + fn unrecognized_property() { + let configs = &["forge-config: default.fuzz.unknownprop = 200".to_string()]; + let base_config = FuzzConfig::default(); + if let Err(e) = base_config.try_merge(configs) { + assert_eq!(e.to_string(), "'unknownprop' is an Invalid config property"); + } else { + assert!(false) + } } #[test] - fn parse_config_ci_profile() { - figment::Jail::expect_with(|jail| { - jail.set_env("FOUNDRY_PROFILE", "ci"); - let conf = r#" - forge-config: default.fuzz.runs = 1024 - forge-config: ci.fuzz.runs = 2048"#; - - let base_conf: FuzzConfig = FuzzConfig::default(); - let parsed: FuzzConfig = base_conf.try_merge(conf).unwrap().expect("Valid config"); - assert_eq!(parsed.runs, 2048); - Ok(()) - }); + fn successful_merge() { + let configs = &["forge-config: default.fuzz.runs = 42424242".to_string()]; + let base_config = FuzzConfig::default(); + let merged: FuzzConfig = base_config.try_merge(configs).expect("No errors").unwrap(); + assert_eq!(merged.runs, 42424242); } #[test] - fn unrecognized_property() { - let conf = "forge-config: default.fuzz.unknownprop = 200"; + fn merge_is_none() { + let empty_config = &[]; let base_config = FuzzConfig::default(); - if let Err(e) = base_config.try_merge(conf) { - assert_eq!(e.to_string(), "'unknownprop' is not a valid config property"); - } else { - assert!(false) - } + let merged = base_config.try_merge(empty_config).expect("No errors"); + assert!(merged.is_none()); } #[test] - fn e2e() { - use solang_parser::parse; - let code = r#" - contract FuzzTestContract { - /** - * forge-config: default.fuzz.runs = 1023 - * forge-config: default.fuzz.max-test-rejects = 521 - */ - function testFuzz(string name) public returns (string) { - return name; - } - } - "#; - - let (_, comments) = parse(code, 0).expect("Valid code"); - let comm = &comments[0]; - match comm { - Comment::DocBlock(_, text) => { - let base_config = FuzzConfig::default(); - let config: FuzzConfig = - base_config.try_merge(text).unwrap().expect("Valid config"); - assert_eq!(config.runs, 1023); - assert_eq!(config.max_test_rejects, 521); - } - _ => { - assert!(false); // Force test to fail - } - } + fn merge_is_none_unrelated_property() { + let unrelated_configs = &["forge-config: default.invariant.runs = 2".to_string()]; + let base_config = FuzzConfig::default(); + let merged = base_config.try_merge(unrelated_configs).expect("No errors"); + assert!(merged.is_none()); + } + + #[test] + fn config_variables() { + let configs = &[ + "forge-config: default.fuzz.runs = 42424242".to_string(), + "forge-config: ci.fuzz.runs = 666666".to_string(), + "forge-config: default.invariant.runs = 2".to_string(), + ]; + let variables = FuzzConfig::config_variables(configs); + assert_eq!( + variables, + vec![("runs".into(), "42424242".into()), ("runs".into(), "666666".into())] + ); } } diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index be601caa03204..05f05e9fd42a9 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -2,14 +2,14 @@ use regex::Regex; use crate::{InlineConfigError, NatSpec}; -use super::remove_whitespaces; +use super::{remove_whitespaces, INLINE_CONFIG_PREFIX}; /// Errors returned by the [`InlineConfigParser`] trait. #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum InlineConfigParserError { /// An invalid configuration property has been provided. /// The property cannot be mapped to the configuration object - #[error("'{0}' is not an Invalid config property")] + #[error("'{0}' is an Invalid config property")] InvalidConfigProperty(String), /// An error occurred while trying to parse an integer configuration value #[error("Invalid config value for key '{0}'. Unable to parse '{1}' into an integer value")] @@ -104,13 +104,15 @@ where /// ``` fn config_variables(config_lines: &[String]) -> Vec<(String, String)> { let mut result: Vec<(String, String)> = vec![]; - let prefix = Self::config_key(); - let pattern = format!("^.*{prefix}"); - let re = Regex::new(&pattern).unwrap(); + let config_key = Self::config_key(); + let profile = ".*"; + let prefix = format!("^{INLINE_CONFIG_PREFIX}:{profile}{config_key}\\."); + let re = Regex::new(&prefix).unwrap(); config_lines .iter() .map(|l| remove_whitespaces(l)) + .filter(|l| re.is_match(l)) .map(|l| re.replace(&l, "").to_string()) .for_each(|line| { let key_value = line.split('=').collect::>(); // i.e. "['runs', '500']" diff --git a/config/src/invariant.rs b/config/src/invariant.rs index 3067d8d179e55..60a86dd881869 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -69,50 +69,51 @@ mod tests { use crate::{inline::InlineConfigParser, InvariantConfig}; #[test] - fn parse_config_default_profile() { - let conf = r#" - forge-config: default.invariant.runs = 1024 - forge-config: default.invariant.depth = 30 - forge-config: default.invariant.fail-on-revert = true - forge-config: default.invariant.call-override = false - "#; - let base_conf: InvariantConfig = InvariantConfig::default(); - let parsed: InvariantConfig = base_conf.try_merge(conf).unwrap().expect("Valid config"); - assert_eq!(parsed.runs, 1024); - assert_eq!(parsed.depth, 30); - assert_eq!(parsed.fail_on_revert, true); - assert_eq!(parsed.call_override, false); + fn unrecognized_property() { + let configs = &["forge-config: default.invariant.unknownprop = 200".to_string()]; + let base_config = InvariantConfig::default(); + if let Err(e) = base_config.try_merge(configs) { + assert_eq!(e.to_string(), "'unknownprop' is an Invalid config property"); + } else { + assert!(false) + } } #[test] - fn parse_config_ci_profile() { - figment::Jail::expect_with(|jail| { - jail.set_env("FOUNDRY_PROFILE", "ci"); - let conf = r#" - forge-config: ci.invariant.runs = 1024 - forge-config: ci.invariant.depth = 30 - forge-config: ci.invariant.fail-on-revert = true - forge-config: ci.invariant.call-override = false - "#; + fn successful_merge() { + let configs = &["forge-config: default.invariant.runs = 42424242".to_string()]; + let base_config = InvariantConfig::default(); + let merged: InvariantConfig = base_config.try_merge(configs).expect("No errors").unwrap(); + assert_eq!(merged.runs, 42424242); + } - let base_conf: InvariantConfig = InvariantConfig::default(); - let parsed: InvariantConfig = base_conf.try_merge(conf).unwrap().expect("Valid config"); - assert_eq!(parsed.runs, 1024); - assert_eq!(parsed.depth, 30); - assert_eq!(parsed.fail_on_revert, true); - assert_eq!(parsed.call_override, false); - Ok(()) - }); + #[test] + fn merge_is_none() { + let empty_config = &[]; + let base_config = InvariantConfig::default(); + let merged = base_config.try_merge(empty_config).expect("No errors"); + assert!(merged.is_none()); } #[test] - fn unrecognized_property() { - let conf = "forge-config: default.invariant.unknownprop = 200"; - let base_conf: InvariantConfig = InvariantConfig::default(); - if let Err(e) = base_conf.try_merge(conf) { - assert_eq!(e.to_string(), "'unknownprop' is not a valid config property"); - } else { - assert!(false) - } + fn merge_is_none_unrelated_property() { + let unrelated_configs = &["forge-config: default.fuzz.runs = 2".to_string()]; + let base_config = InvariantConfig::default(); + let merged = base_config.try_merge(unrelated_configs).expect("No errors"); + assert!(merged.is_none()); } -} + + #[test] + fn config_variables() { + let configs = &[ + "forge-config: default.fuzz.runs = 42424242".to_string(), + "forge-config: ci.fuzz.runs = 666666".to_string(), + "forge-config: default.invariant.runs = 2".to_string(), + ]; + let variables = InvariantConfig::config_variables(configs); + assert_eq!( + variables, + vec![("runs".into(), "2".into())] + ); + } +} \ No newline at end of file From 13448dfc614b6b80b978ba009e9eafc3de0bd8fc Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 3 May 2023 22:46:53 +0200 Subject: [PATCH 28/57] Noisy comment test --- config/src/inline/natspec.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index 0129e17813634..4fdbcc5c0ba52 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -166,6 +166,12 @@ mod tests { let conf = r#" forge-config: default.fuzz.runs = 600 forge-config: ci.fuzz.runs = 500 + ========= SOME NOISY TEXT ============= + 䩹𧀫Jx닧Ʀ̳盅K擷􅟽Ɂw첊}ꏻk86ᖪk-檻ܴ렝[Dz𐤬oᘓƤ + ꣖ۻ%Ƅ㪕ς:(饁΍av/烲ڻ̛߉橞㗡𥺃̹M봓䀖ؿ̄󵼁)𯖛d􂽰񮍃 + ϊ&»ϿЏ񊈞2򕄬񠪁鞷砕eߥH󶑶J粊񁼯머?槿ᴴጅ𙏑ϖ뀓򨙺򷃅Ӽ츙4󍔹 + 醤㭊r􎜕󷾸𶚏 ܖ̹灱녗V*竅􋹲⒪苏贗񾦼=숽ؓ򗋲бݧ󫥛𛲍ʹ園Ьi + ======================================= forge-config: default.invariant.runs = 1 "#; From 6eef43e502f1c71a95b44e289849ba08a0c7f71f Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 3 May 2023 22:53:11 +0200 Subject: [PATCH 29/57] Use meaningful names --- config/src/fuzz.rs | 10 +++++----- config/src/inline/conf_parser.rs | 2 +- config/src/invariant.rs | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 7ed1c5077e4d8..f3e15e16b0676 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -43,15 +43,15 @@ impl InlineConfigParser for FuzzConfig { } fn try_merge(&self, configs: &[String]) -> Result, InlineConfigParserError> { - let vars: Vec<(String, String)> = Self::config_variables(configs); + let overrides: Vec<(String, String)> = Self::overrides(configs); - if vars.is_empty() { + if overrides.is_empty() { return Ok(None); } let mut conf = *self; - for pair in vars { + for pair in overrides { let key = pair.0; let value = pair.1; match key.as_str() { @@ -140,13 +140,13 @@ mod tests { } #[test] - fn config_variables() { + fn override_detection() { let configs = &[ "forge-config: default.fuzz.runs = 42424242".to_string(), "forge-config: ci.fuzz.runs = 666666".to_string(), "forge-config: default.invariant.runs = 2".to_string(), ]; - let variables = FuzzConfig::config_variables(configs); + let variables = FuzzConfig::overrides(configs); assert_eq!( variables, vec![("runs".into(), "42424242".into()), ("runs".into(), "666666".into())] diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 05f05e9fd42a9..3d1cf335391de 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -102,7 +102,7 @@ where /// ("depth", "500"), /// ]; /// ``` - fn config_variables(config_lines: &[String]) -> Vec<(String, String)> { + fn overrides(config_lines: &[String]) -> Vec<(String, String)> { let mut result: Vec<(String, String)> = vec![]; let config_key = Self::config_key(); let profile = ".*"; diff --git a/config/src/invariant.rs b/config/src/invariant.rs index 60a86dd881869..f6556667d173a 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -41,15 +41,15 @@ impl InlineConfigParser for InvariantConfig { } fn try_merge(&self, configs: &[String]) -> Result, InlineConfigParserError> { - let vars: Vec<(String, String)> = Self::config_variables(configs); + let overrides: Vec<(String, String)> = Self::overrides(configs); - if vars.is_empty() { + if overrides.is_empty() { return Ok(None) } let mut conf = *self; - for pair in vars { + for pair in overrides { let key = pair.0; let value = pair.1; match key.as_str() { @@ -104,13 +104,13 @@ mod tests { } #[test] - fn config_variables() { + fn override_detection() { let configs = &[ "forge-config: default.fuzz.runs = 42424242".to_string(), "forge-config: ci.fuzz.runs = 666666".to_string(), "forge-config: default.invariant.runs = 2".to_string(), ]; - let variables = InvariantConfig::config_variables(configs); + let variables = InvariantConfig::overrides(configs); assert_eq!( variables, vec![("runs".into(), "2".into())] From 35e52b8e845509c5cbe74b6a8e00803771e9f67b Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Thu, 4 May 2023 11:48:54 +0200 Subject: [PATCH 30/57] Profile validation implemented + Unit tests --- config/src/fuzz.rs | 10 ++-- config/src/inline/conf_parser.rs | 81 +++++++++++++++++++++++++++++--- config/src/invariant.rs | 20 ++++---- config/src/lib.rs | 2 +- 4 files changed, 91 insertions(+), 22 deletions(-) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index f3e15e16b0676..57b4415377c78 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -3,7 +3,9 @@ use ethers_core::types::U256; use serde::{Deserialize, Serialize}; -use crate::inline::{InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_FUZZ_KEY}; +use crate::inline::{ + parse_u32, InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_FUZZ_KEY, +}; /// Contains for fuzz testing #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -46,7 +48,7 @@ impl InlineConfigParser for FuzzConfig { let overrides: Vec<(String, String)> = Self::overrides(configs); if overrides.is_empty() { - return Ok(None); + return Ok(None) } let mut conf = *self; @@ -55,8 +57,8 @@ impl InlineConfigParser for FuzzConfig { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => conf.runs = Self::parse_u32(key, value)?, - "max-test-rejects" => conf.max_test_rejects = Self::parse_u32(key, value)?, + "runs" => conf.runs = parse_u32(key, value)?, + "max-test-rejects" => conf.max_test_rejects = parse_u32(key, value)?, _ => Err(InlineConfigParserError::InvalidConfigProperty(key))?, } } diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 3d1cf335391de..e98efa2f83261 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -11,6 +11,9 @@ pub enum InlineConfigParserError { /// The property cannot be mapped to the configuration object #[error("'{0}' is an Invalid config property")] InvalidConfigProperty(String), + /// An invalid profile has been provided + #[error("'{0}' specifies an Invalid profile. Available profiles are {1}")] + InvalidProfile(String, String), /// An error occurred while trying to parse an integer configuration value #[error("Invalid config value for key '{0}'. Unable to parse '{1}' into an integer value")] ParseIntError(String, String), @@ -65,7 +68,7 @@ where /// /// forge-config: ci.fuzz.runs = 10 /// ``` /// would validate the whole `invariant` configuration. - fn validate(natspec: &NatSpec) -> Result<(), InlineConfigError> { + fn validate_configs(natspec: &NatSpec) -> Result<(), InlineConfigError> { let config_key = Self::config_key(); let configs = natspec @@ -125,14 +128,78 @@ where result } +} - /// Tries to parse a `u32` from `value` - fn parse_u32(key: String, value: String) -> Result { - value.parse().map_err(|_| InlineConfigParserError::ParseIntError(key, value)) +/// Checks that each configuration line specified in `natspec`, contains at least one +/// of `profiles`. +/// +/// i.e. Given available profiles +/// ```rust +/// let _profiles = vec!["ci", "default"]; +/// ``` +/// A configuration like `forge-config: ciii.invariant.depth = 1` would result +/// in an error. +pub fn validate_profiles(natspec: &NatSpec, profiles: &[String]) -> Result<(), InlineConfigError> { + for config in natspec.config_lines() { + if !profiles.iter().any(|p| config.starts_with(&format!("{INLINE_CONFIG_PREFIX}:{p}."))) { + let err_line: String = natspec.debug_context(); + let profiles = format!("{profiles:?}"); + Err(InlineConfigError { + source: InlineConfigParserError::InvalidProfile(config, profiles), + line: err_line, + })? + } } + Ok(()) +} + +/// Tries to parse a `u32` from `value` +pub fn parse_u32(key: String, value: String) -> Result { + value.parse().map_err(|_| InlineConfigParserError::ParseIntError(key, value)) +} + +/// Tries to parse a `bool` from `value` +pub fn parse_bool(key: String, value: String) -> Result { + value.parse().map_err(|_| InlineConfigParserError::ParseBoolError(key, value)) +} + +#[cfg(test)] +mod tests { + use crate::{inline::conf_parser::validate_profiles, NatSpec}; + + #[test] + fn validate_profiles_error() { + let profiles = ["ci".to_string(), "default".to_string()]; + let natspec = NatSpec { + contract: Default::default(), + function: Default::default(), + line: Default::default(), + docs: r#" + forge-config: ciii.invariant.depth = 1 + forge-config: default.invariant.depth = 1 + "# + .into(), + }; + + let result = validate_profiles(&natspec, &profiles); + assert!(result.is_err()); + } + + #[test] + fn validate_profiles_success() { + let profiles = ["ci".to_string(), "default".to_string()]; + let natspec = NatSpec { + contract: Default::default(), + function: Default::default(), + line: Default::default(), + docs: r#" + forge-config: ci.invariant.depth = 1 + forge-config: default.invariant.depth = 1 + "# + .into(), + }; - /// Tries to parse a `bool` from `value` - fn parse_bool(key: String, value: String) -> Result { - value.parse().map_err(|_| InlineConfigParserError::ParseBoolError(key, value)) + let result = validate_profiles(&natspec, &profiles); + assert!(result.is_ok()); } } diff --git a/config/src/invariant.rs b/config/src/invariant.rs index f6556667d173a..effe42ba6d0fd 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -2,7 +2,10 @@ use crate::{ fuzz::FuzzDictionaryConfig, - inline::{InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_INVARIANT_KEY}, + inline::{ + parse_bool, parse_u32, InlineConfigParser, InlineConfigParserError, + INLINE_CONFIG_INVARIANT_KEY, + }, }; use serde::{Deserialize, Serialize}; @@ -53,10 +56,10 @@ impl InlineConfigParser for InvariantConfig { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => conf.runs = Self::parse_u32(key, value)?, - "depth" => conf.depth = Self::parse_u32(key, value)?, - "fail-on-revert" => conf.fail_on_revert = Self::parse_bool(key, value)?, - "call-override" => conf.call_override = Self::parse_bool(key, value)?, + "runs" => conf.runs = parse_u32(key, value)?, + "depth" => conf.depth = parse_u32(key, value)?, + "fail-on-revert" => conf.fail_on_revert = parse_bool(key, value)?, + "call-override" => conf.call_override = parse_bool(key, value)?, _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } @@ -111,9 +114,6 @@ mod tests { "forge-config: default.invariant.runs = 2".to_string(), ]; let variables = InvariantConfig::overrides(configs); - assert_eq!( - variables, - vec![("runs".into(), "2".into())] - ); + assert_eq!(variables, vec![("runs".into(), "2".into())]); } -} \ No newline at end of file +} diff --git a/config/src/lib.rs b/config/src/lib.rs index 688ab8650de9f..328869408c78e 100644 --- a/config/src/lib.rs +++ b/config/src/lib.rs @@ -93,7 +93,7 @@ pub use invariant::InvariantConfig; use providers::remappings::RemappingsProvider; mod inline; -pub use inline::{InlineConfig, InlineConfigError, InlineConfigParser, NatSpec}; +pub use inline::{validate_profiles, InlineConfig, InlineConfigError, InlineConfigParser, NatSpec}; /// Foundry configuration /// From 380375e1f1870d503d4ed113396644fe2bf9a6c5 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Thu, 4 May 2023 11:50:14 +0200 Subject: [PATCH 31/57] Given a natspec, extract current profile configs + Unit tests --- config/src/inline/mod.rs | 14 ++++++++++++-- config/src/inline/natspec.rs | 36 +++++++++++++++++++++++++++++------- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index b6b40d8e46043..7a6abd7862ce6 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -1,13 +1,23 @@ mod conf_parser; -pub use conf_parser::{InlineConfigParser, InlineConfigParserError}; +pub use conf_parser::{ + parse_bool, parse_u32, validate_profiles, InlineConfigParser, InlineConfigParserError, +}; +use once_cell::sync::Lazy; use std::collections::HashMap; mod natspec; pub use natspec::NatSpec; -const INLINE_CONFIG_PREFIX: &str = "forge-config"; +use crate::Config; + pub const INLINE_CONFIG_FUZZ_KEY: &str = "fuzz"; pub const INLINE_CONFIG_INVARIANT_KEY: &str = "invariant"; +const INLINE_CONFIG_PREFIX: &str = "forge-config"; + +static INLINE_CONFIG_PREFIX_SELECTED_PROFILE: Lazy = Lazy::new(|| { + let selected_profile = Config::selected_profile().to_string(); + format!("{INLINE_CONFIG_PREFIX}:{selected_profile}.") +}); /// Wrapper error struct that catches config parsing /// errors [`InlineConfigParserError`], enriching them with context information diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index 4fdbcc5c0ba52..f5ad1fe3da86c 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -6,7 +6,7 @@ use ethers_solc::{ }; use serde_json::Value; -use super::{remove_whitespaces, INLINE_CONFIG_PREFIX}; +use super::{remove_whitespaces, INLINE_CONFIG_PREFIX, INLINE_CONFIG_PREFIX_SELECTED_PROFILE}; /// Convenient struct to hold in-line per-test configurations pub struct NatSpec { @@ -51,6 +51,12 @@ impl NatSpec { format!("{}:{}:{}", self.contract, self.function, self.line) } + /// Returns a list of configuration lines that match the current profile + pub fn current_profile_configs(&self) -> Vec { + let prefix: &str = INLINE_CONFIG_PREFIX_SELECTED_PROFILE.as_ref(); + self.config_lines_with_prefix(prefix) + } + /// Returns a list of configuration lines that match a specific string prefix pub fn config_lines_with_prefix>(&self, prefix: S) -> Vec { let prefix: String = prefix.into(); @@ -119,11 +125,13 @@ fn get_fn_name(fn_data: &BTreeMap) -> Option { fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { if let Value::Object(fn_docs) = fn_data.get("documentation")? { if let Value::String(comment) = fn_docs.get("text")? { - let src_line = fn_docs - .get("src") - .map(Value::to_string) - .unwrap_or(String::from("")); - return Some((comment.into(), src_line)) + if comment.contains(INLINE_CONFIG_PREFIX) { + let src_line = fn_docs + .get("src") + .map(Value::to_string) + .unwrap_or(String::from("")); + return Some((comment.into(), src_line)) + } } } None @@ -147,7 +155,21 @@ mod tests { ) } - #[test] + #[test] + fn current_profile_configs() { + let natspec = natspec(); + let config_lines = natspec.current_profile_configs(); + + assert_eq!( + config_lines, + vec![ + "forge-config:default.fuzz.runs=600".to_string(), + "forge-config:default.invariant.runs=1".to_string() + ] + ); + } + + #[test] fn config_lines_with_prefix() { use super::INLINE_CONFIG_PREFIX; let natspec = natspec(); From efdbb09866dfaa3bfc86547ab9e1be7d3dfa5c63 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Thu, 4 May 2023 11:52:42 +0200 Subject: [PATCH 32/57] TestOptions instantiated with new validation rules - NEED TESTS --- forge/src/lib.rs | 94 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 81 insertions(+), 13 deletions(-) diff --git a/forge/src/lib.rs b/forge/src/lib.rs index 0718c136e21dc..c6b86d8d1d575 100644 --- a/forge/src/lib.rs +++ b/forge/src/lib.rs @@ -1,7 +1,10 @@ use std::path::Path; use ethers::solc::ProjectCompileOutput; -use foundry_config::{FuzzConfig, InlineConfig, InlineConfigError, InvariantConfig}; +use foundry_config::{ + validate_profiles, Config, FuzzConfig, InlineConfig, InlineConfigError, InlineConfigParser, + InvariantConfig, NatSpec, +}; use proptest::test_runner::{RngAlgorithm, TestRng, TestRunner}; use tracing::trace; @@ -84,7 +87,7 @@ impl TestOptions { where S: Into, { - self.inline_fuzz.get_config(contract_id, test_fn).unwrap_or(&self.fuzz) + self.inline_fuzz.get(contract_id, test_fn).unwrap_or(&self.fuzz) } /// Returns an "invariant" configuration setup. Parameters are used to select tight scoped @@ -98,7 +101,7 @@ impl TestOptions { where S: Into, { - self.inline_invariant.get_config(contract_id, test_fn).unwrap_or(&self.invariant) + self.inline_invariant.get(contract_id, test_fn).unwrap_or(&self.invariant) } pub fn fuzzer_with_cases(&self, cases: u32) -> TestRunner { @@ -123,11 +126,70 @@ impl TestOptions { } } +impl<'a, P> TryFrom<(&'a ProjectCompileOutput, &'a P, Vec, FuzzConfig, InvariantConfig)> + for TestOptions +where + P: AsRef, +{ + type Error = InlineConfigError; + + /// Tries to create an instance of `Self`, detecting inline configurations from the project + /// compile output. + /// + /// Param is a tuple, whose elements are: + /// 1. Solidity compiler output, essential to extract natspec test configs. + /// 2. Root path to express contract base dirs. This is essential to match inline configs at + /// runtime. 3. List of available configuration profiles + /// 4. Reference to a fuzz base configuration. + /// 5. Reference to an invariant base configuration. + fn try_from( + value: (&'a ProjectCompileOutput, &'a P, Vec, FuzzConfig, InvariantConfig), + ) -> Result { + let output = value.0; + let root = value.1; + let profiles = &value.2; + let base_fuzz: FuzzConfig = value.3; + let base_invariant: InvariantConfig = value.4; + + let natspecs: Vec = NatSpec::parse(output, root); + let mut inline_invariant = InlineConfig::::default(); + let mut inline_fuzz = InlineConfig::::default(); + + for natspec in natspecs { + // Perform general validation + validate_profiles(&natspec, profiles)?; + FuzzConfig::validate_configs(&natspec)?; + InvariantConfig::validate_configs(&natspec)?; + + // Apply in-line configurations for the current profile + let configs: Vec = natspec.current_profile_configs(); + let c: &str = &natspec.contract; + let f: &str = &natspec.function; + let line: String = natspec.debug_context(); + + match base_fuzz.try_merge(&configs) { + Ok(Some(conf)) => inline_fuzz.insert(c, f, conf), + Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, + _ => { /* No inline config found, do nothing */ } + } + + match base_invariant.try_merge(&configs) { + Ok(Some(conf)) => inline_invariant.insert(c, f, conf), + Err(e) => Err(InlineConfigError { line: line.clone(), source: e })?, + _ => { /* No inline config found, do nothing */ } + } + } + + Ok(Self { fuzz: base_fuzz, invariant: base_invariant, inline_fuzz, inline_invariant }) + } +} + /// Builder utility to create a [`TestOptions`] instance. #[derive(Default)] pub struct TestOptionsBuilder { fuzz: Option, invariant: Option, + profiles: Option>, output: Option, } @@ -146,6 +208,13 @@ impl TestOptionsBuilder { self } + /// Sets available configuration profiles. Profiles are useful to validate existing in-line + /// configurations. This argument is necessary in case a `compile_output`is provided. + pub fn profiles(mut self, p: Vec) -> Self { + self.profiles = Some(p); + self + } + /// Sets a project compiler output instance. This is used to extract /// inline test configurations that override `self.fuzz` and `self.invariant` /// specs when necessary. @@ -161,20 +230,19 @@ impl TestOptionsBuilder { /// to determine the base path of generated contract identifiers. This is to provide correct /// matchers for inline test configs. pub fn build(self, root: impl AsRef) -> Result { + let default_profiles = vec![Config::selected_profile().into()]; + let profiles: Vec = self.profiles.unwrap_or(default_profiles); let base_fuzz = self.fuzz.unwrap_or_default(); let base_invariant = self.invariant.unwrap_or_default(); match self.output { - Some(compile_output) => Ok(TestOptions { - fuzz: base_fuzz, - invariant: base_invariant, - inline_fuzz: InlineConfig::try_from((&compile_output, &base_fuzz, &root))?, - inline_invariant: InlineConfig::try_from(( - &compile_output, - &base_invariant, - &root, - ))?, - }), + Some(compile_output) => Ok(TestOptions::try_from(( + &compile_output, + &root, + profiles, + base_fuzz, + base_invariant, + ))?), None => Ok(TestOptions { fuzz: base_fuzz, invariant: base_invariant, From 20f023ff5e8a561fd6cc13b4d8170c4263343cb4 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Thu, 4 May 2023 12:06:07 +0200 Subject: [PATCH 33/57] Integration tests working --- forge/tests/it/inline.rs | 49 ++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/forge/tests/it/inline.rs b/forge/tests/it/inline.rs index da089b9da1260..0aa1d937fc43e 100644 --- a/forge/tests/it/inline.rs +++ b/forge/tests/it/inline.rs @@ -8,7 +8,7 @@ mod tests { result::{SuiteResult, TestKind, TestResult}, TestOptions, TestOptionsBuilder, }; - use foundry_config::{FuzzConfig, InlineConfig, InvariantConfig}; + use foundry_config::{FuzzConfig, InvariantConfig}; #[test] fn inline_config_run_fuzz() { @@ -71,38 +71,33 @@ mod tests { } } } + #[test] - fn inline_fuzz_config() { + fn build_test_options() { let root = &PROJECT.paths.root; - let compiled = COMPILED.clone(); - let base_fuzz = FuzzConfig::default(); - if let Ok(conf) = InlineConfig::::try_from((&compiled, &base_fuzz, root)) { - // Inline config defined in testdata/inline/FuzzInlineConf.t.sol - let contract_name = "inline/FuzzInlineConf.t.sol:FuzzInlineConf"; - let function_name = "testInlineConfFuzz"; - let inline_config: &FuzzConfig = conf.get(contract_name, function_name).unwrap(); - assert_eq!(inline_config.runs, 1024); - assert_eq!(inline_config.max_test_rejects, 500); - } + let profiles = vec!["default".to_string(), "ci".to_string()]; + let build_result = TestOptionsBuilder::default() + .fuzz(FuzzConfig::default()) + .invariant(InvariantConfig::default()) + .compile_output(&COMPILED) + .profiles(profiles) + .build(root); + + assert!(build_result.is_ok()); } #[test] - fn inline_invariant_config() { + fn build_test_options_invalid_profile() { let root = &PROJECT.paths.root; - let compiled = COMPILED.clone(); - let base_invariant = InvariantConfig::default(); - if let Ok(conf) = - InlineConfig::::try_from((&compiled, &base_invariant, root)) - { - // Inline config defined in testdata/inline/InvariantInlineConf.t.sol - let contract_name = "inline/InvariantInlineConf.t.sol:InvariantInlineConf"; - let function_name = "invariant_neverFalse"; - let inline_config: &InvariantConfig = conf.get(contract_name, function_name).unwrap(); - assert_eq!(inline_config.runs, 333); - assert_eq!(inline_config.depth, 32); - assert_eq!(inline_config.fail_on_revert, false); - assert_eq!(inline_config.call_override, true); - } + let profiles = vec!["profile-sheldon-cooper".to_string()]; + let build_result = TestOptionsBuilder::default() + .fuzz(FuzzConfig::default()) + .invariant(InvariantConfig::default()) + .compile_output(&COMPILED) + .profiles(profiles) + .build(root); + + assert!(build_result.is_err()); } fn test_options() -> TestOptions { From 2c69b6e9e89bc94d3ad44867bab024eb0ed22649 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Thu, 4 May 2023 12:09:25 +0200 Subject: [PATCH 34/57] Integration tests docs and typos --- forge/tests/it/inline.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/forge/tests/it/inline.rs b/forge/tests/it/inline.rs index 0aa1d937fc43e..c6c1437a0e695 100644 --- a/forge/tests/it/inline.rs +++ b/forge/tests/it/inline.rs @@ -87,16 +87,18 @@ mod tests { } #[test] - fn build_test_options_invalid_profile() { + fn build_test_options_just_one_valid_profile() { let root = &PROJECT.paths.root; - let profiles = vec!["profile-sheldon-cooper".to_string()]; + let valid_profiles = vec!["profile-sheldon-cooper".to_string()]; let build_result = TestOptionsBuilder::default() .fuzz(FuzzConfig::default()) .invariant(InvariantConfig::default()) .compile_output(&COMPILED) - .profiles(profiles) + .profiles(valid_profiles) .build(root); + // We expect an error, since COMPILED contains in-line + // per-test configs for "default" and "ci" profiles assert!(build_result.is_err()); } From f506d388d4f19da47be1e925577be29bab7e2afe Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Thu, 4 May 2023 18:03:09 +0200 Subject: [PATCH 35/57] Utility function to get all available profiles in config - unit tests --- cli/src/cmd/forge/test/mod.rs | 5 ++- config/src/utils.rs | 72 +++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/cli/src/cmd/forge/test/mod.rs b/cli/src/cmd/forge/test/mod.rs index 0d14a14eb593c..66796c65ce603 100644 --- a/cli/src/cmd/forge/test/mod.rs +++ b/cli/src/cmd/forge/test/mod.rs @@ -25,7 +25,7 @@ use foundry_common::{ evm::EvmArgs, get_contract_name, get_file_name, }; -use foundry_config::{figment, Config}; +use foundry_config::{figment, get_available_profiles, Config}; use regex::Regex; use std::{collections::BTreeMap, path::PathBuf, sync::mpsc::channel, thread, time::Duration}; use tracing::trace; @@ -147,11 +147,14 @@ impl TestArgs { }?; let project_root = &project.paths.root; + let toml = config.get_config_path(); + let profiles = get_available_profiles(toml)?; let test_options: TestOptions = TestOptionsBuilder::default() .fuzz(config.fuzz) .invariant(config.invariant) .compile_output(&output) + .profiles(profiles) .build(project_root)?; // Determine print verbosity and executor verbosity diff --git a/config/src/utils.rs b/config/src/utils.rs index d51a4d20dc15a..c84aaa22c2359 100644 --- a/config/src/utils.rs +++ b/config/src/utils.rs @@ -9,6 +9,7 @@ use std::{ path::{Path, PathBuf}, str::FromStr, }; +use toml_edit::{Document, Item}; /// Loads the config for the current project workspace pub fn load_config() -> Config { @@ -168,6 +169,38 @@ pub(crate) fn get_dir_remapping(dir: impl AsRef) -> Option { } } +/// Returns all available `profile` keys in a given `.toml` file +/// +/// i.e. The toml below would return would return `["default", "ci", "local"]` +/// ```toml +/// [profile.default] +/// ... +/// [profile.ci] +/// ... +/// [profile.local] +/// ``` +pub fn get_available_profiles(toml_path: impl AsRef) -> eyre::Result> { + let doc = toml(toml_path)?; + let mut result = vec![Config::DEFAULT_PROFILE.to_string()]; + + if let Some(Item::Table(profiles)) = doc.as_table().get(Config::PROFILE_SECTION) { + for (_, (profile, _)) in profiles.iter().enumerate() { + let p = profile.to_string(); + if !result.contains(&p) { + result.push(p); + } + } + } + + Ok(result) +} + +fn toml(path: impl AsRef) -> eyre::Result { + let path = path.as_ref().to_owned(); + let doc: Document = std::fs::read_to_string(path)?.parse()?; + Ok(doc) +} + /// Deserialize stringified percent. The value must be between 0 and 100 inclusive. pub(crate) fn deserialize_stringified_percent<'de, D>(deserializer: D) -> Result where @@ -209,3 +242,42 @@ where }; Ok(num) } + +#[cfg(test)] +mod tests { + use std::path::Path; + + use crate::get_available_profiles; + + #[test] + fn get_profiles_from_toml() { + figment::Jail::expect_with(|jail| { + jail.create_file( + "foundry.toml", + r#" + [foo.baz] + libs = ['node_modules', 'lib'] + + [profile.default] + libs = ['node_modules', 'lib'] + + [profile.ci] + libs = ['node_modules', 'lib'] + + [profile.local] + libs = ['node_modules', 'lib'] + "#, + )?; + + let path = Path::new("./foundry.toml"); + let profiles = get_available_profiles(path).unwrap(); + + assert_eq!( + profiles, + vec!["default".to_string(), "ci".to_string(), "local".to_string()] + ); + + Ok(()) + }); + } +} From b8d43eadf9ef40e0595a0c578ec2ee4e4244c5ff Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Thu, 4 May 2023 18:15:26 +0200 Subject: [PATCH 36/57] try update PR --- config/src/utils.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/src/utils.rs b/config/src/utils.rs index c84aaa22c2359..4c529c76e0bee 100644 --- a/config/src/utils.rs +++ b/config/src/utils.rs @@ -245,9 +245,8 @@ where #[cfg(test)] mod tests { - use std::path::Path; - use crate::get_available_profiles; + use std::path::Path; #[test] fn get_profiles_from_toml() { From 9a0476240263a1ed5b459a9c3c9918d0c41d3911 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:01:04 +0200 Subject: [PATCH 37/57] Punctuation in config/src/inline/conf_parser.rs Co-authored-by: evalir --- config/src/inline/conf_parser.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index e98efa2f83261..ab80ede210f7f 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -130,8 +130,7 @@ where } } -/// Checks that each configuration line specified in `natspec`, contains at least one -/// of `profiles`. +/// Checks if all configuration lines specified in `natspec` use a valid profile. /// /// i.e. Given available profiles /// ```rust From ccb368b34b3188b8c68a935bdebabb02949d3dbb Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:01:45 +0200 Subject: [PATCH 38/57] Punctuation in config/src/inline/conf_parser.rs Co-authored-by: evalir --- config/src/inline/conf_parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index ab80ede210f7f..610ce1038478a 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -57,7 +57,7 @@ where /// - `Err(InlineConfigParserError)` in case of wrong configuration. fn try_merge(&self, configs: &[String]) -> Result, InlineConfigParserError>; - /// Validates all configurations contained in a natspec, that apply + /// Validates all configurations contained in a natspec that apply /// to the current configuration key. /// /// i.e. Given the `invariant` config key and a natspec comment of the form, From 95212c906da31036bae4174cc31332c7716d1863 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:08:16 +0200 Subject: [PATCH 39/57] review: docs in cli/src/cmd/forge/test/mod.rs --- cli/src/cmd/forge/test/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cli/src/cmd/forge/test/mod.rs b/cli/src/cmd/forge/test/mod.rs index 66796c65ce603..0736b1488b28e 100644 --- a/cli/src/cmd/forge/test/mod.rs +++ b/cli/src/cmd/forge/test/mod.rs @@ -146,6 +146,8 @@ impl TestArgs { compiler.compile(&project) }?; + // Create test options from general project settings + // and compiler output let project_root = &project.paths.root; let toml = config.get_config_path(); let profiles = get_available_profiles(toml)?; From 222dd313d3972119073c0669d389153c61f77d17 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:09:52 +0200 Subject: [PATCH 40/57] review: naming convention in InlineConfigParser --- config/src/fuzz.rs | 10 +++++----- config/src/inline/conf_parser.rs | 20 +++++++++++--------- config/src/inline/mod.rs | 4 ++-- config/src/invariant.rs | 14 +++++++------- 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 57b4415377c78..3b3d97eca60b8 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -4,7 +4,7 @@ use ethers_core::types::U256; use serde::{Deserialize, Serialize}; use crate::inline::{ - parse_u32, InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_FUZZ_KEY, + parse_config_u32, InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_FUZZ_KEY, }; /// Contains for fuzz testing @@ -45,7 +45,7 @@ impl InlineConfigParser for FuzzConfig { } fn try_merge(&self, configs: &[String]) -> Result, InlineConfigParserError> { - let overrides: Vec<(String, String)> = Self::overrides(configs); + let overrides: Vec<(String, String)> = Self::get_config_overrides(configs); if overrides.is_empty() { return Ok(None) @@ -57,8 +57,8 @@ impl InlineConfigParser for FuzzConfig { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => conf.runs = parse_u32(key, value)?, - "max-test-rejects" => conf.max_test_rejects = parse_u32(key, value)?, + "runs" => conf.runs = parse_config_u32(key, value)?, + "max-test-rejects" => conf.max_test_rejects = parse_config_u32(key, value)?, _ => Err(InlineConfigParserError::InvalidConfigProperty(key))?, } } @@ -148,7 +148,7 @@ mod tests { "forge-config: ci.fuzz.runs = 666666".to_string(), "forge-config: default.invariant.runs = 2".to_string(), ]; - let variables = FuzzConfig::overrides(configs); + let variables = FuzzConfig::get_config_overrides(configs); assert_eq!( variables, vec![("runs".into(), "42424242".into()), ("runs".into(), "666666".into())] diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index 610ce1038478a..ad63f8da0d7da 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -16,10 +16,10 @@ pub enum InlineConfigParserError { InvalidProfile(String, String), /// An error occurred while trying to parse an integer configuration value #[error("Invalid config value for key '{0}'. Unable to parse '{1}' into an integer value")] - ParseIntError(String, String), + ParseInt(String, String), /// An error occurred while trying to parse a boolean configuration value #[error("Invalid config value for key '{0}'. Unable to parse '{1}' into a boolean value")] - ParseBoolError(String, String), + ParseBool(String, String), } /// This trait is intended to parse configurations from @@ -105,7 +105,7 @@ where /// ("depth", "500"), /// ]; /// ``` - fn overrides(config_lines: &[String]) -> Vec<(String, String)> { + fn get_config_overrides(config_lines: &[String]) -> Vec<(String, String)> { let mut result: Vec<(String, String)> = vec![]; let config_key = Self::config_key(); let profile = ".*"; @@ -152,14 +152,16 @@ pub fn validate_profiles(natspec: &NatSpec, profiles: &[String]) -> Result<(), I Ok(()) } -/// Tries to parse a `u32` from `value` -pub fn parse_u32(key: String, value: String) -> Result { - value.parse().map_err(|_| InlineConfigParserError::ParseIntError(key, value)) +/// Tries to parse a `u32` from `value`. The `key` argument is used to give details +/// in the case of an error. +pub fn parse_config_u32(key: String, value: String) -> Result { + value.parse().map_err(|_| InlineConfigParserError::ParseInt(key, value)) } -/// Tries to parse a `bool` from `value` -pub fn parse_bool(key: String, value: String) -> Result { - value.parse().map_err(|_| InlineConfigParserError::ParseBoolError(key, value)) +/// Tries to parse a `bool` from `value`. The `key` argument is used to give details +/// in the case of an error. +pub fn parse_config_bool(key: String, value: String) -> Result { + value.parse().map_err(|_| InlineConfigParserError::ParseBool(key, value)) } #[cfg(test)] diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 7a6abd7862ce6..223bc2fadb100 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -1,6 +1,6 @@ mod conf_parser; pub use conf_parser::{ - parse_bool, parse_u32, validate_profiles, InlineConfigParser, InlineConfigParserError, + parse_config_bool, parse_config_u32, validate_profiles, InlineConfigParser, InlineConfigParserError, }; use once_cell::sync::Lazy; use std::collections::HashMap; @@ -71,7 +71,7 @@ mod tests { #[test] fn inline_config_error() { let source = - InlineConfigParserError::ParseBoolError("key".into(), "invalid-bool-value".into()); + InlineConfigParserError::ParseBool("key".into(), "invalid-bool-value".into()); let line = "dir/TestContract.t.sol:FuzzContract:10:12:111".to_string(); let error = InlineConfigError { line: line.clone(), source: source.clone() }; diff --git a/config/src/invariant.rs b/config/src/invariant.rs index effe42ba6d0fd..cdaf357513ed8 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -3,7 +3,7 @@ use crate::{ fuzz::FuzzDictionaryConfig, inline::{ - parse_bool, parse_u32, InlineConfigParser, InlineConfigParserError, + parse_config_bool, parse_config_u32, InlineConfigParser, InlineConfigParserError, INLINE_CONFIG_INVARIANT_KEY, }, }; @@ -44,7 +44,7 @@ impl InlineConfigParser for InvariantConfig { } fn try_merge(&self, configs: &[String]) -> Result, InlineConfigParserError> { - let overrides: Vec<(String, String)> = Self::overrides(configs); + let overrides: Vec<(String, String)> = Self::get_config_overrides(configs); if overrides.is_empty() { return Ok(None) @@ -56,10 +56,10 @@ impl InlineConfigParser for InvariantConfig { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => conf.runs = parse_u32(key, value)?, - "depth" => conf.depth = parse_u32(key, value)?, - "fail-on-revert" => conf.fail_on_revert = parse_bool(key, value)?, - "call-override" => conf.call_override = parse_bool(key, value)?, + "runs" => conf.runs = parse_config_u32(key, value)?, + "depth" => conf.depth = parse_config_u32(key, value)?, + "fail-on-revert" => conf.fail_on_revert = parse_config_bool(key, value)?, + "call-override" => conf.call_override = parse_config_bool(key, value)?, _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } @@ -113,7 +113,7 @@ mod tests { "forge-config: ci.fuzz.runs = 666666".to_string(), "forge-config: default.invariant.runs = 2".to_string(), ]; - let variables = InvariantConfig::overrides(configs); + let variables = InvariantConfig::get_config_overrides(configs); assert_eq!(variables, vec![("runs".into(), "2".into())]); } } From 187ea1bb2f29e331b846bdfaf3d8b708a2ee8503 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:12:01 +0200 Subject: [PATCH 41/57] review: test renaming suggestion Co-authored-by: evalir --- config/src/inline/conf_parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index ad63f8da0d7da..d29f40f4bb00e 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -169,7 +169,7 @@ mod tests { use crate::{inline::conf_parser::validate_profiles, NatSpec}; #[test] - fn validate_profiles_error() { + fn can_reject_invalid_profiles() { let profiles = ["ci".to_string(), "default".to_string()]; let natspec = NatSpec { contract: Default::default(), From 0d5d19d54151989f4d3b5f756f2b67985d4ded86 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:12:18 +0200 Subject: [PATCH 42/57] review: test renaming suggestion Co-authored-by: evalir --- config/src/inline/conf_parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index d29f40f4bb00e..b3ca200d9f1ef 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -187,7 +187,7 @@ mod tests { } #[test] - fn validate_profiles_success() { + fn can_accept_valid_profiles() { let profiles = ["ci".to_string(), "default".to_string()]; let natspec = NatSpec { contract: Default::default(), From 3ccaf97d7665668edfcc7e60cb2580a48e9e4da2 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:13:28 +0200 Subject: [PATCH 43/57] review: test renaming suggestion Co-authored-by: evalir --- config/src/inline/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 223bc2fadb100..a56de965005c0 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -69,7 +69,7 @@ mod tests { use crate::InlineConfigError; #[test] - fn inline_config_error() { + fn can_format_inline_config_errors() { let source = InlineConfigParserError::ParseBool("key".into(), "invalid-bool-value".into()); let line = "dir/TestContract.t.sol:FuzzContract:10:12:111".to_string(); From 2472b37e42b18aedbbfaffe2607052ec82aa7a16 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:15:34 +0200 Subject: [PATCH 44/57] review: docs punctuation Co-authored-by: evalir --- config/src/inline/natspec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index f5ad1fe3da86c..4c57d59ddd3fa 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -45,7 +45,7 @@ impl NatSpec { } /// Returns a string describing the natspec - /// context, for debugging purposes 🐞
+ /// context, for debugging purposes 🐞 /// i.e. `dir/TestContract.t.sol:FuzzContract:test_myFunction:10:12:111` pub fn debug_context(&self) -> String { format!("{}:{}:{}", self.contract, self.function, self.line) From 9afae065c96b3d1ab40de96a4062efef8cb032d2 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:16:41 +0200 Subject: [PATCH 45/57] review: docs Co-authored-by: evalir --- config/src/inline/natspec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index 4c57d59ddd3fa..9f059ce2babe3 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -118,7 +118,7 @@ fn get_fn_name(fn_data: &BTreeMap) -> Option { } } -/// Inspects Solc compiler output for documentation comments: +/// Inspects Solc compiler output for documentation comments. Returns: /// - `Some((String, String))` in case the function has natspec comments. First item is a textual /// natspec representation, the second item is the natspec src line, in the form "raw:col:length". /// - `None` in case the function has not natspec comments. From 8682f262162c2f5aad8727684c3c7efe526e18de Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:24:47 +0200 Subject: [PATCH 46/57] review: function internal utils function renaming + docs --- config/src/utils.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/config/src/utils.rs b/config/src/utils.rs index 4c529c76e0bee..ec686c436edd3 100644 --- a/config/src/utils.rs +++ b/config/src/utils.rs @@ -180,7 +180,7 @@ pub(crate) fn get_dir_remapping(dir: impl AsRef) -> Option { /// [profile.local] /// ``` pub fn get_available_profiles(toml_path: impl AsRef) -> eyre::Result> { - let doc = toml(toml_path)?; + let doc = read_toml(toml_path)?; let mut result = vec![Config::DEFAULT_PROFILE.to_string()]; if let Some(Item::Table(profiles)) = doc.as_table().get(Config::PROFILE_SECTION) { @@ -195,7 +195,9 @@ pub fn get_available_profiles(toml_path: impl AsRef) -> eyre::Result) -> eyre::Result { +/// Returns a [`toml_edit::Document`] loaded from the provided `path`. +/// Can raise an error in case of I/O or parsing errors. +fn read_toml(path: impl AsRef) -> eyre::Result { let path = path.as_ref().to_owned(); let doc: Document = std::fs::read_to_string(path)?.parse()?; Ok(doc) From 4184a1c0843a0df7c749ffd954157cf749727bb1 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:53:58 +0200 Subject: [PATCH 47/57] review: get_fn_docs unit tests --- config/src/inline/mod.rs | 6 +++--- config/src/inline/natspec.rs | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index a56de965005c0..1afa57360dab2 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -1,6 +1,7 @@ mod conf_parser; pub use conf_parser::{ - parse_config_bool, parse_config_u32, validate_profiles, InlineConfigParser, InlineConfigParserError, + parse_config_bool, parse_config_u32, validate_profiles, InlineConfigParser, + InlineConfigParserError, }; use once_cell::sync::Lazy; use std::collections::HashMap; @@ -70,8 +71,7 @@ mod tests { #[test] fn can_format_inline_config_errors() { - let source = - InlineConfigParserError::ParseBool("key".into(), "invalid-bool-value".into()); + let source = InlineConfigParserError::ParseBool("key".into(), "invalid-bool-value".into()); let line = "dir/TestContract.t.sol:FuzzContract:10:12:111".to_string(); let error = InlineConfigError { line: line.clone(), source: source.clone() }; diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index 9f059ce2babe3..be5785459f832 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -81,7 +81,7 @@ fn contract_root_node<'a>(nodes: &'a [Node], contract_id: &'a str) -> Option<&'a let contract_data = &n.other; if let Value::String(contract_name) = contract_data.get("name")? { if contract_id.ends_with(contract_name) { - return Some(n) + return Some(n); } } } @@ -105,7 +105,7 @@ fn get_fn_data(node: &Node) -> Option<(String, String, String)> { let fn_data = &node.other; let fn_name: String = get_fn_name(fn_data)?; let (fn_docs, docs_src_line): (String, String) = get_fn_docs(fn_data)?; - return Some((fn_name, fn_docs, docs_src_line)) + return Some((fn_name, fn_docs, docs_src_line)); } None @@ -130,7 +130,7 @@ fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { .get("src") .map(Value::to_string) .unwrap_or(String::from("")); - return Some((comment.into(), src_line)) + return Some((comment.into(), src_line)); } } } @@ -139,7 +139,9 @@ fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { #[cfg(test)] mod tests { - use crate::NatSpec; + use crate::{inline::natspec::get_fn_docs, NatSpec}; + use serde_json::{json, Value}; + use std::collections::BTreeMap; #[test] fn config_lines() { @@ -184,6 +186,15 @@ mod tests { ) } + #[test] + fn can_handle_unavailable_src_line_with_fallback() { + let mut fn_data: BTreeMap = BTreeMap::new(); + let doc_withouth_src_field = json!({ "text": "forge-config:default.fuzz.runs=600" }); + fn_data.insert("documentation".into(), doc_withouth_src_field); + let (_, src_line) = get_fn_docs(&fn_data).expect("Some docs"); + assert_eq!(src_line, "".to_string()); + } + fn natspec() -> NatSpec { let conf = r#" forge-config: default.fuzz.runs = 600 From 9d38aa77001fefb9d8f1512fee62efef472ee4b2 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 12:55:13 +0200 Subject: [PATCH 48/57] review: test renaming suggestion Co-authored-by: evalir --- config/src/invariant.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/src/invariant.rs b/config/src/invariant.rs index cdaf357513ed8..daddd490d0373 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -99,7 +99,7 @@ mod tests { } #[test] - fn merge_is_none_unrelated_property() { + fn can_merge_unrelated_properties_into_config() { let unrelated_configs = &["forge-config: default.fuzz.runs = 2".to_string()]; let base_config = InvariantConfig::default(); let merged = base_config.try_merge(unrelated_configs).expect("No errors"); From c61f98ea10b3f2aa2b32fb995a7436f110f6e462 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 13:03:19 +0200 Subject: [PATCH 49/57] review: clarify intent --- config/src/fuzz.rs | 9 +++++---- config/src/invariant.rs | 13 +++++++------ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 3b3d97eca60b8..37a204ec52425 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -51,18 +51,19 @@ impl InlineConfigParser for FuzzConfig { return Ok(None) } - let mut conf = *self; + // self is Copy. We clone it with dereference. + let mut conf_clone = *self; for pair in overrides { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => conf.runs = parse_config_u32(key, value)?, - "max-test-rejects" => conf.max_test_rejects = parse_config_u32(key, value)?, + "runs" => conf_clone.runs = parse_config_u32(key, value)?, + "max-test-rejects" => conf_clone.max_test_rejects = parse_config_u32(key, value)?, _ => Err(InlineConfigParserError::InvalidConfigProperty(key))?, } } - Ok(Some(conf)) + Ok(Some(conf_clone)) } } diff --git a/config/src/invariant.rs b/config/src/invariant.rs index daddd490d0373..1c6388bc20b85 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -50,20 +50,21 @@ impl InlineConfigParser for InvariantConfig { return Ok(None) } - let mut conf = *self; + // self is Copy. We clone it with dereference. + let mut conf_clone = *self; for pair in overrides { let key = pair.0; let value = pair.1; match key.as_str() { - "runs" => conf.runs = parse_config_u32(key, value)?, - "depth" => conf.depth = parse_config_u32(key, value)?, - "fail-on-revert" => conf.fail_on_revert = parse_config_bool(key, value)?, - "call-override" => conf.call_override = parse_config_bool(key, value)?, + "runs" => conf_clone.runs = parse_config_u32(key, value)?, + "depth" => conf_clone.depth = parse_config_u32(key, value)?, + "fail-on-revert" => conf_clone.fail_on_revert = parse_config_bool(key, value)?, + "call-override" => conf_clone.call_override = parse_config_bool(key, value)?, _ => Err(InlineConfigParserError::InvalidConfigProperty(key.to_string()))?, } } - Ok(Some(conf)) + Ok(Some(conf_clone)) } } From 2be61d691a8c5713fe16d69c33ca4878f93cf08c Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sat, 6 May 2023 13:13:11 +0200 Subject: [PATCH 50/57] review: document functions --- config/src/inline/natspec.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index be5785459f832..07077c48bcc94 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -100,6 +100,13 @@ fn apply(natspecs: &mut Vec, contract: &str, node: &Node) { } } +/// Given a compilation output node, if it is a function definition +/// that also contains a natspec then return a tuple of: +/// - Function name +/// - Natspec text +/// - Natspec position with format "row:col:length" +/// +/// Return None otherwise. fn get_fn_data(node: &Node) -> Option<(String, String, String)> { if let NodeType::FunctionDefinition = node.node_type { let fn_data = &node.other; @@ -111,6 +118,7 @@ fn get_fn_data(node: &Node) -> Option<(String, String, String)> { None } +/// Given a dictionary of function data returns the name of the function. fn get_fn_name(fn_data: &BTreeMap) -> Option { match fn_data.get("name")? { Value::String(fn_name) => Some(fn_name.into()), From fa06e626d4cf29273ce1bf6a9e24774fc3cad18d Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sun, 7 May 2023 12:28:27 +0200 Subject: [PATCH 51/57] review: applied case typos --- config/src/fuzz.rs | 2 +- config/src/inline/conf_parser.rs | 4 ++-- config/src/invariant.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/config/src/fuzz.rs b/config/src/fuzz.rs index 37a204ec52425..b43ae10e156cd 100644 --- a/config/src/fuzz.rs +++ b/config/src/fuzz.rs @@ -112,7 +112,7 @@ mod tests { let configs = &["forge-config: default.fuzz.unknownprop = 200".to_string()]; let base_config = FuzzConfig::default(); if let Err(e) = base_config.try_merge(configs) { - assert_eq!(e.to_string(), "'unknownprop' is an Invalid config property"); + assert_eq!(e.to_string(), "'unknownprop' is an invalid config property"); } else { assert!(false) } diff --git a/config/src/inline/conf_parser.rs b/config/src/inline/conf_parser.rs index b3ca200d9f1ef..a64cd67299184 100644 --- a/config/src/inline/conf_parser.rs +++ b/config/src/inline/conf_parser.rs @@ -9,10 +9,10 @@ use super::{remove_whitespaces, INLINE_CONFIG_PREFIX}; pub enum InlineConfigParserError { /// An invalid configuration property has been provided. /// The property cannot be mapped to the configuration object - #[error("'{0}' is an Invalid config property")] + #[error("'{0}' is an invalid config property")] InvalidConfigProperty(String), /// An invalid profile has been provided - #[error("'{0}' specifies an Invalid profile. Available profiles are {1}")] + #[error("'{0}' specifies an invalid profile. Available profiles are: {1}")] InvalidProfile(String, String), /// An error occurred while trying to parse an integer configuration value #[error("Invalid config value for key '{0}'. Unable to parse '{1}' into an integer value")] diff --git a/config/src/invariant.rs b/config/src/invariant.rs index 1c6388bc20b85..ad49e40fffb84 100644 --- a/config/src/invariant.rs +++ b/config/src/invariant.rs @@ -77,7 +77,7 @@ mod tests { let configs = &["forge-config: default.invariant.unknownprop = 200".to_string()]; let base_config = InvariantConfig::default(); if let Err(e) = base_config.try_merge(configs) { - assert_eq!(e.to_string(), "'unknownprop' is an Invalid config property"); + assert_eq!(e.to_string(), "'unknownprop' is an invalid config property"); } else { assert!(false) } From 616d1078841a430a353dd83909b4350afe5322d8 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Sun, 7 May 2023 21:39:40 +0200 Subject: [PATCH 52/57] FIX CI: Available profiles fallback to vec![current_profile] in case the foundry.toml path cannot be resolved --- config/src/utils.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/config/src/utils.rs b/config/src/utils.rs index ec686c436edd3..abbf3f727cca2 100644 --- a/config/src/utils.rs +++ b/config/src/utils.rs @@ -180,9 +180,14 @@ pub(crate) fn get_dir_remapping(dir: impl AsRef) -> Option { /// [profile.local] /// ``` pub fn get_available_profiles(toml_path: impl AsRef) -> eyre::Result> { - let doc = read_toml(toml_path)?; let mut result = vec![Config::DEFAULT_PROFILE.to_string()]; + if !toml_path.as_ref().exists() { + return Ok(result) + } + + let doc = read_toml(toml_path)?; + if let Some(Item::Table(profiles)) = doc.as_table().get(Config::PROFILE_SECTION) { for (_, (profile, _)) in profiles.iter().enumerate() { let p = profile.to_string(); From b016d3dd1c3d2da5e9b08df01fe2100bab94f342 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Mon, 8 May 2023 17:21:17 +0200 Subject: [PATCH 53/57] cargo +nightly fmt --- config/src/inline/natspec.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index 07077c48bcc94..0a2ab409be843 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -81,7 +81,7 @@ fn contract_root_node<'a>(nodes: &'a [Node], contract_id: &'a str) -> Option<&'a let contract_data = &n.other; if let Value::String(contract_name) = contract_data.get("name")? { if contract_id.ends_with(contract_name) { - return Some(n); + return Some(n) } } } @@ -105,14 +105,14 @@ fn apply(natspecs: &mut Vec, contract: &str, node: &Node) { /// - Function name /// - Natspec text /// - Natspec position with format "row:col:length" -/// +/// /// Return None otherwise. fn get_fn_data(node: &Node) -> Option<(String, String, String)> { if let NodeType::FunctionDefinition = node.node_type { let fn_data = &node.other; let fn_name: String = get_fn_name(fn_data)?; let (fn_docs, docs_src_line): (String, String) = get_fn_docs(fn_data)?; - return Some((fn_name, fn_docs, docs_src_line)); + return Some((fn_name, fn_docs, docs_src_line)) } None @@ -138,7 +138,7 @@ fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { .get("src") .map(Value::to_string) .unwrap_or(String::from("")); - return Some((comment.into(), src_line)); + return Some((comment.into(), src_line)) } } } From 2360a1c42de63828ac87ef6093854f5ac5a4428c Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Mon, 8 May 2023 21:48:15 +0200 Subject: [PATCH 54/57] review: case typo --- config/src/inline/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index 1afa57360dab2..fec2aa0e6b998 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -24,7 +24,7 @@ static INLINE_CONFIG_PREFIX_SELECTED_PROFILE: Lazy = Lazy::new(|| { /// errors [`InlineConfigParserError`], enriching them with context information /// reporting the misconfigured line. #[derive(thiserror::Error, Debug)] -#[error("Inline config Error detected at {line} {source}")] +#[error("Inline config error detected at {line} {source}")] pub struct InlineConfigError { /// Specifies the misconfigured line. This is something of the form /// `dir/TestContract.t.sol:FuzzContract:10:12:111` @@ -75,7 +75,7 @@ mod tests { let line = "dir/TestContract.t.sol:FuzzContract:10:12:111".to_string(); let error = InlineConfigError { line: line.clone(), source: source.clone() }; - let expected = format!("Inline config Error detected at {line} {source}"); + let expected = format!("Inline config error detected at {line} {source}"); assert_eq!(error.to_string(), expected); } } From ed942367e1d5da2f0454450eb1b8c10a650cf7b2 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 9 May 2023 07:08:56 +0200 Subject: [PATCH 55/57] review: remove double quotes from src line --- config/src/inline/natspec.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index 0a2ab409be843..b2a9c560bf627 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -134,10 +134,12 @@ fn get_fn_docs(fn_data: &BTreeMap) -> Option<(String, String)> { if let Value::Object(fn_docs) = fn_data.get("documentation")? { if let Value::String(comment) = fn_docs.get("text")? { if comment.contains(INLINE_CONFIG_PREFIX) { - let src_line = fn_docs + let mut src_line = fn_docs .get("src") - .map(Value::to_string) + .map(|src| src.to_string()) .unwrap_or(String::from("")); + + src_line.retain(|c| c != '"'); return Some((comment.into(), src_line)) } } @@ -203,6 +205,15 @@ mod tests { assert_eq!(src_line, "".to_string()); } + #[test] + fn can_handle_available_src_line() { + let mut fn_data: BTreeMap = BTreeMap::new(); + let doc_withouth_src_field = json!({ "text": "forge-config:default.fuzz.runs=600", "src": "73:21:12" }); + fn_data.insert("documentation".into(), doc_withouth_src_field); + let (_, src_line) = get_fn_docs(&fn_data).expect("Some docs"); + assert_eq!(src_line, "73:21:12".to_string()); + } + fn natspec() -> NatSpec { let conf = r#" forge-config: default.fuzz.runs = 600 From fcee8ec1c8c14450a312085e64fa0df716fe1c49 Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Tue, 9 May 2023 07:38:07 +0200 Subject: [PATCH 56/57] review: removed duplicated error msg; removed row:col:len detail (it was not accurate) --- config/src/inline/mod.rs | 2 +- config/src/inline/natspec.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index fec2aa0e6b998..ad81fbf24a58a 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -24,7 +24,7 @@ static INLINE_CONFIG_PREFIX_SELECTED_PROFILE: Lazy = Lazy::new(|| { /// errors [`InlineConfigParserError`], enriching them with context information /// reporting the misconfigured line. #[derive(thiserror::Error, Debug)] -#[error("Inline config error detected at {line} {source}")] +#[error("Inline config error detected at {line}")] pub struct InlineConfigError { /// Specifies the misconfigured line. This is something of the form /// `dir/TestContract.t.sol:FuzzContract:10:12:111` diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index b2a9c560bf627..e6db1e50f9751 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -46,9 +46,9 @@ impl NatSpec { /// Returns a string describing the natspec /// context, for debugging purposes 🐞 - /// i.e. `dir/TestContract.t.sol:FuzzContract:test_myFunction:10:12:111` + /// i.e. `test/Counter.t.sol:CounterTest:testSetNumber` pub fn debug_context(&self) -> String { - format!("{}:{}:{}", self.contract, self.function, self.line) + format!("{}:{}", self.contract, self.function) } /// Returns a list of configuration lines that match the current profile From eef9149badc6f082e486578c3c4be1201e1ab3df Mon Sep 17 00:00:00 2001 From: Andrea Simeoni Date: Wed, 10 May 2023 23:33:49 +0200 Subject: [PATCH 57/57] fix CI --- config/src/inline/mod.rs | 4 ++-- config/src/inline/natspec.rs | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/src/inline/mod.rs b/config/src/inline/mod.rs index ad81fbf24a58a..a96f816ade6fd 100644 --- a/config/src/inline/mod.rs +++ b/config/src/inline/mod.rs @@ -72,10 +72,10 @@ mod tests { #[test] fn can_format_inline_config_errors() { let source = InlineConfigParserError::ParseBool("key".into(), "invalid-bool-value".into()); - let line = "dir/TestContract.t.sol:FuzzContract:10:12:111".to_string(); + let line = "dir/TestContract.t.sol:FuzzContract".to_string(); let error = InlineConfigError { line: line.clone(), source: source.clone() }; - let expected = format!("Inline config error detected at {line} {source}"); + let expected = format!("Inline config error detected at {line}"); assert_eq!(error.to_string(), expected); } } diff --git a/config/src/inline/natspec.rs b/config/src/inline/natspec.rs index e6db1e50f9751..ae85089c1aa97 100644 --- a/config/src/inline/natspec.rs +++ b/config/src/inline/natspec.rs @@ -208,7 +208,8 @@ mod tests { #[test] fn can_handle_available_src_line() { let mut fn_data: BTreeMap = BTreeMap::new(); - let doc_withouth_src_field = json!({ "text": "forge-config:default.fuzz.runs=600", "src": "73:21:12" }); + let doc_withouth_src_field = + json!({ "text": "forge-config:default.fuzz.runs=600", "src": "73:21:12" }); fn_data.insert("documentation".into(), doc_withouth_src_field); let (_, src_line) = get_fn_docs(&fn_data).expect("Some docs"); assert_eq!(src_line, "73:21:12".to_string());