diff --git a/Cargo.lock b/Cargo.lock index dea2c7c4d0ab..bd7a9fd7808b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3347,6 +3347,7 @@ dependencies = [ "serde", "serde_json", "serde_regex", + "solang-parser", "tempfile", "thiserror", "toml 0.8.11", diff --git a/Cargo.toml b/Cargo.toml index 860175871679..10465a1fe308 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -46,6 +46,7 @@ debug = 1 # Solc and artifacts foundry-compilers.opt-level = 3 solang-parser.opt-level = 3 +lalrpop-util.opt-level = 3 serde_json.opt-level = 3 # EVM diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index b3f4c514a766..4f42da66d33e 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -18,6 +18,8 @@ alloy-chains = { workspace = true, features = ["serde"] } alloy-primitives = { workspace = true, features = ["serde"] } revm-primitives = { workspace = true, default-features = false, features = ["std"] } +solang-parser.workspace = true + dirs-next = "2" dunce = "1" eyre.workspace = true diff --git a/crates/config/src/inline/mod.rs b/crates/config/src/inline/mod.rs index af2cbe7e3cf1..9989d5b76c55 100644 --- a/crates/config/src/inline/mod.rs +++ b/crates/config/src/inline/mod.rs @@ -3,7 +3,7 @@ pub use conf_parser::{parse_config_bool, parse_config_u32, validate_profiles, In pub use error::{InlineConfigError, InlineConfigParserError}; pub use natspec::NatSpec; use once_cell::sync::Lazy; -use std::{borrow::Cow, collections::HashMap}; +use std::collections::HashMap; mod conf_parser; mod error; @@ -25,22 +25,14 @@ static INLINE_CONFIG_PREFIX_SELECTED_PROFILE: Lazy = Lazy::new(|| { pub struct InlineConfig { /// Maps a (test-contract, test-function) pair /// to a specific configuration provided by the user. - configs: HashMap, T>, + configs: HashMap<(String, String), 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: C, fn_name: F) -> Option<&T> - where - C: Into, - F: Into, - { - // TODO use borrow - let key = InlineConfigKey { - contract: Cow::Owned(contract_id.into()), - function: Cow::Owned(fn_name.into()), - }; + pub fn get(&self, contract_id: &str, fn_name: &str) -> Option<&T> { + let key = (contract_id.to_string(), fn_name.to_string()); self.configs.get(&key) } @@ -51,21 +43,11 @@ impl InlineConfig { C: Into, F: Into, { - let key = InlineConfigKey { - contract: Cow::Owned(contract_id.into()), - function: Cow::Owned(fn_name.into()), - }; + let key = (contract_id.into(), fn_name.into()); self.configs.insert(key, config); } } -/// Represents a (test-contract, test-function) pair -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -struct InlineConfigKey<'a> { - contract: Cow<'a, str>, - function: Cow<'a, str>, -} - pub(crate) fn remove_whitespaces(s: &str) -> String { s.chars().filter(|c| !c.is_whitespace()).collect() } diff --git a/crates/config/src/inline/natspec.rs b/crates/config/src/inline/natspec.rs index 11557fd7a829..3b62d40cc1c2 100644 --- a/crates/config/src/inline/natspec.rs +++ b/crates/config/src/inline/natspec.rs @@ -4,10 +4,11 @@ use foundry_compilers::{ ProjectCompileOutput, }; use serde_json::Value; +use solang_parser::pt; use std::{collections::BTreeMap, path::Path}; /// Convenient struct to hold in-line per-test configurations -#[derive(Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct NatSpec { /// The parent contract of the natspec pub contract: String, @@ -28,14 +29,28 @@ impl NatSpec { pub fn parse(output: &ProjectCompileOutput, root: &Path) -> Vec { let mut natspecs: Vec = vec![]; + let solc = SolcParser::new(); + let solang = SolangParser::new(); for (id, artifact) in output.artifact_ids() { - let Some(ast) = &artifact.ast else { continue }; - let path = id.source.as_path(); - let path = path.strip_prefix(root).unwrap_or(path); - // id.identifier + let abs_path = id.source.as_path(); + let path = abs_path.strip_prefix(root).unwrap_or(abs_path); + let contract_name = id.name.split('.').next().unwrap(); + // `id.identifier` but with the stripped path. let contract = format!("{}:{}", path.display(), id.name); - let Some(node) = contract_root_node(&ast.nodes, &contract) else { continue }; - apply(&mut natspecs, &contract, node) + + let mut used_solc_ast = false; + if let Some(ast) = &artifact.ast { + if let Some(node) = solc.contract_root_node(&ast.nodes, &contract) { + solc.parse(&mut natspecs, &contract, node); + used_solc_ast = true; + } + } + + if !used_solc_ast { + if let Ok(src) = std::fs::read_to_string(abs_path) { + solang.parse(&mut natspecs, &src, &contract, contract_name); + } + } } natspecs @@ -63,89 +78,246 @@ impl NatSpec { /// Returns a list of all the configuration lines available in the natspec pub fn config_lines(&self) -> impl Iterator + '_ { - self.docs.lines().map(remove_whitespaces).filter(|line| line.contains(INLINE_CONFIG_PREFIX)) + self.docs.lines().filter(|line| line.contains(INLINE_CONFIG_PREFIX)).map(remove_whitespaces) } } -/// 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) +struct SolcParser { + _private: (), +} + +impl SolcParser { + fn new() -> Self { + Self { _private: () } + } + + /// Given a list of nodes, find a "ContractDefinition" node that matches + /// the provided contract_id. + fn contract_root_node<'a>(&self, nodes: &'a [Node], contract_id: &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 } - 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 }) + /// Implements a DFS over a compiler output node and its children. + /// If a natspec is found it is added to `natspecs` + fn parse(&self, natspecs: &mut Vec, contract: &str, node: &Node) { + for n in node.nodes.iter() { + if let Some((function, docs, line)) = self.get_fn_data(n) { + natspecs.push(NatSpec { contract: contract.into(), function, line, docs }) + } + self.parse(natspecs, contract, n); + } + } + + /// 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(&self, node: &Node) -> Option<(String, String, String)> { + if let NodeType::FunctionDefinition = node.node_type { + let fn_data = &node.other; + let fn_name: String = self.get_fn_name(fn_data)?; + let (fn_docs, docs_src_line): (String, String) = self.get_fn_docs(fn_data)?; + return Some((fn_name, fn_docs, docs_src_line)) + } + + None + } + + /// Given a dictionary of function data returns the name of the function. + fn get_fn_name(&self, 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. 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(&self, 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 mut src_line = fn_docs + .get("src") + .map(|src| src.to_string()) + .unwrap_or_else(|| String::from("")); + + src_line.retain(|c| c != '"'); + return Some((comment.into(), src_line)) + } + } } - apply(natspecs, contract, n); + None } } -/// 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; - 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 +struct SolangParser { + _private: (), } -/// 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()), - _ => None, +impl SolangParser { + fn new() -> Self { + Self { _private: () } } -} -/// 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. -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 mut src_line = fn_docs - .get("src") - .map(|src| src.to_string()) - .unwrap_or_else(|| String::from("")); - - src_line.retain(|c| c != '"'); - return Some((comment.into(), src_line)) + fn parse( + &self, + natspecs: &mut Vec, + src: &str, + contract_id: &str, + contract_name: &str, + ) { + // Fast path to avoid parsing the file. + if !src.contains(INLINE_CONFIG_PREFIX) { + return; + } + + let Ok((pt, comments)) = solang_parser::parse(src, 0) else { return }; + let mut prev_end = 0; + for item in &pt.0 { + let pt::SourceUnitPart::ContractDefinition(c) = item else { continue }; + let Some(id) = c.name.as_ref() else { continue }; + if id.name != contract_name { + continue + }; + for part in &c.parts { + let pt::ContractPart::FunctionDefinition(f) = part else { continue }; + let start = f.loc.start(); + // Parse doc comments in between the previous function and the current one. + let docs = solang_parser::doccomment::parse_doccomments(&comments, prev_end, start); + let docs = docs + .into_iter() + .flat_map(|doc| doc.into_comments()) + .filter(|doc| doc.value.contains(INLINE_CONFIG_PREFIX)); + for doc in docs { + natspecs.push(NatSpec { + contract: contract_id.to_string(), + function: f.name.as_ref().map(|id| id.to_string()).unwrap_or_default(), + line: "0:0:0".to_string(), + docs: doc.value, + }); + } + prev_end = f.loc.end(); } + prev_end = c.loc.end(); } } - None } #[cfg(test)] mod tests { - use crate::{inline::natspec::get_fn_docs, NatSpec}; - use serde_json::{json, Value}; - use std::collections::BTreeMap; + use super::*; + use serde_json::json; + + #[test] + fn parse_solang() { + let src = " +contract C { /// forge-config: default.fuzz.runs = 600 + +\t\t\t\t /// forge-config: default.fuzz.runs = 601 + + function f1() {} + /** forge-config: default.fuzz.runs = 700 */ +function f2() {} /** forge-config: default.fuzz.runs = 800 */ function f3() {} + +/** + * forge-config: default.fuzz.runs = 1024 + * forge-config: default.fuzz.max-test-rejects = 500 + */ + function f4() {} +} +"; + let mut natspecs = vec![]; + let solang = SolangParser::new(); + let id = || "path.sol:C".to_string(); + let default_line = || "0:0:0".to_string(); + solang.parse(&mut natspecs, src, &id(), "C"); + assert_eq!( + natspecs, + [ + // f1 + NatSpec { + contract: id(), + function: "f1".to_string(), + line: default_line(), + docs: "forge-config: default.fuzz.runs = 600\nforge-config: default.fuzz.runs = 601".to_string(), + }, + // f2 + NatSpec { + contract: id(), + function: "f2".to_string(), + line: default_line(), + docs: "forge-config: default.fuzz.runs = 700".to_string(), + }, + // f3 + NatSpec { + contract: id(), + function: "f3".to_string(), + line: default_line(), + docs: "forge-config: default.fuzz.runs = 800".to_string(), + }, + // f4 + NatSpec { + contract: id(), + function: "f4".to_string(), + line: default_line(), + docs: "forge-config: default.fuzz.runs = 1024\nforge-config: default.fuzz.max-test-rejects = 500".to_string(), + }, + ] + ); + } + + #[test] + fn parse_solang_2() { + let src = r#" +// SPDX-License-Identifier: MIT OR Apache-2.0 +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 testInlineConfFuzz(uint8 x) public { + require(true, "this is not going to revert"); + } +} + "#; + let mut natspecs = vec![]; + let solang = SolangParser::new(); + let id = || "inline/FuzzInlineConf.t.sol:FuzzInlineConf".to_string(); + let default_line = || "0:0:0".to_string(); + solang.parse(&mut natspecs, src, &id(), "FuzzInlineConf"); + assert_eq!( + natspecs, + [ + NatSpec { + contract: id(), + function: "testInlineConfFuzz".to_string(), + line: default_line(), + docs: "forge-config: default.fuzz.runs = 1024\nforge-config: default.fuzz.max-test-rejects = 500".to_string(), + }, + ] + ); + } #[test] fn config_lines() { @@ -195,7 +367,7 @@ mod tests { let mut fn_data: BTreeMap = BTreeMap::new(); let doc_without_src_field = json!({ "text": "forge-config:default.fuzz.runs=600" }); fn_data.insert("documentation".into(), doc_without_src_field); - let (_, src_line) = get_fn_docs(&fn_data).expect("Some docs"); + let (_, src_line) = SolcParser::new().get_fn_docs(&fn_data).expect("Some docs"); assert_eq!(src_line, "".to_string()); } @@ -205,7 +377,7 @@ mod tests { let doc_without_src_field = json!({ "text": "forge-config:default.fuzz.runs=600", "src": "73:21:12" }); fn_data.insert("documentation".into(), doc_without_src_field); - let (_, src_line) = get_fn_docs(&fn_data).expect("Some docs"); + let (_, src_line) = SolcParser::new().get_fn_docs(&fn_data).expect("Some docs"); assert_eq!(src_line, "73:21:12".to_string()); } diff --git a/crates/forge/src/lib.rs b/crates/forge/src/lib.rs index a430204e158d..d163f819cc62 100644 --- a/crates/forge/src/lib.rs +++ b/crates/forge/src/lib.rs @@ -91,10 +91,7 @@ impl TestOptions { /// - `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 - S: Into, - { + pub fn fuzz_runner(&self, contract_id: &str, test_fn: &str) -> TestRunner { let fuzz_config = self.fuzz_config(contract_id, test_fn).clone(); let failure_persist_path = fuzz_config .failure_persist_dir @@ -116,10 +113,7 @@ impl TestOptions { /// - `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, - { + pub fn invariant_runner(&self, contract_id: &str, test_fn: &str) -> TestRunner { let invariant = self.invariant_config(contract_id, test_fn); self.fuzzer_with_cases(invariant.runs, None) } @@ -131,10 +125,7 @@ impl TestOptions { /// - `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 - S: Into, - { + pub fn fuzz_config(&self, contract_id: &str, test_fn: &str) -> &FuzzConfig { self.inline_fuzz.get(contract_id, test_fn).unwrap_or(&self.fuzz) } @@ -145,10 +136,7 @@ impl TestOptions { /// - `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, - { + pub fn invariant_config(&self, contract_id: &str, test_fn: &str) -> &InvariantConfig { self.inline_invariant.get(contract_id, test_fn).unwrap_or(&self.invariant) } diff --git a/crates/forge/tests/it/inline.rs b/crates/forge/tests/it/inline.rs index 3e7b4616380a..0a2f4d59301e 100644 --- a/crates/forge/tests/it/inline.rs +++ b/crates/forge/tests/it/inline.rs @@ -4,10 +4,7 @@ use crate::{ config::runner, test_helpers::{COMPILED, PROJECT}, }; -use forge::{ - result::{SuiteResult, TestKind, TestResult}, - TestOptions, TestOptionsBuilder, -}; +use forge::{result::TestKind, TestOptions, TestOptionsBuilder}; use foundry_config::{FuzzConfig, InvariantConfig}; use foundry_test_utils::Filter; @@ -16,17 +13,11 @@ async fn inline_config_run_fuzz() { let filter = Filter::new(".*", ".*", ".*inline/FuzzInlineConf.t.sol"); let mut runner = runner(); let result = runner.test_collect(&filter); - 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); - } - _ => { - unreachable!() - } + let suite_result = result.get("inline/FuzzInlineConf.t.sol:FuzzInlineConf").unwrap(); + let test_result = suite_result.test_results.get("testInlineConfFuzz(uint8)").unwrap(); + match test_result.kind { + TestKind::Fuzz { runs, .. } => assert_eq!(runs, 1024), + _ => unreachable!(), } } @@ -44,24 +35,15 @@ async fn inline_config_run_invariant() { 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); - } - _ => { - unreachable!() - } + match test_result_1.kind { + TestKind::Invariant { runs, .. } => assert_eq!(runs, 333), + _ => unreachable!(), } - match &test_result_2.kind { - TestKind::Invariant { runs, .. } => { - assert_eq!(runs, &42); - } - _ => { - unreachable!() - } + let test_result_2 = suite_result_2.test_results.get("invariant_neverFalse()").unwrap(); + match test_result_2.kind { + TestKind::Invariant { runs, .. } => assert_eq!(runs, 42), + _ => unreachable!(), } }