From ead33a596ffae934217f7873c0cb385e01898e31 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Wed, 26 Feb 2025 12:08:11 +0100 Subject: [PATCH 1/6] handle multiple contracts of the same name by including their stripped path --- crates/common/src/compile.rs | 69 +++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 25 deletions(-) diff --git a/crates/common/src/compile.rs b/crates/common/src/compile.rs index 36a81f2cd541..214c7e5817c5 100644 --- a/crates/common/src/compile.rs +++ b/crates/common/src/compile.rs @@ -36,6 +36,9 @@ use std::{ /// settings. #[must_use = "ProjectCompiler does nothing unless you call a `compile*` method"] pub struct ProjectCompiler { + /// The root of the project. + project_root: PathBuf, + /// Whether we are going to verify the contracts after compilation. verify: Option, @@ -70,6 +73,7 @@ impl ProjectCompiler { #[inline] pub fn new() -> Self { Self { + project_root: PathBuf::new(), verify: None, print_names: None, print_sizes: None, @@ -135,6 +139,8 @@ impl ProjectCompiler { mut self, project: &Project, ) -> Result> { + self.project_root = project.root().clone(); + // TODO: Avoid process::exit if !project.paths.has_input_files() && self.files.is_empty() { sh_println!("Nothing to compile")?; @@ -250,32 +256,45 @@ impl ProjectCompiler { let mut size_report = SizeReport { report_kind: report_kind(), contracts: BTreeMap::new() }; - let artifacts: BTreeMap<_, _> = output - .artifact_ids() - .filter(|(id, _)| { - // filter out forge-std specific contracts - !id.source.to_string_lossy().contains("/forge-std/src/") - }) - .map(|(id, artifact)| (id.name, artifact)) - .collect(); - - for (name, artifact) in artifacts { - let runtime_size = contract_size(artifact, false).unwrap_or_default(); - let init_size = contract_size(artifact, true).unwrap_or_default(); - - let is_dev_contract = artifact - .abi - .as_ref() - .map(|abi| { - abi.functions().any(|f| { - f.test_function_kind().is_known() || - matches!(f.name.as_str(), "IS_TEST" | "IS_SCRIPT") + let mut artifacts: BTreeMap> = BTreeMap::new(); + for (id, artifact) in output.artifact_ids().filter(|(id, _)| { + // filter out forge-std specific contracts + !id.source.to_string_lossy().contains("/forge-std/src/") + }) { + artifacts.entry(id.name.clone()).or_default().push((id.path.clone(), artifact)); + } + + for (name, artifact_list) in artifacts { + for (path, artifact) in &artifact_list { + let runtime_size = contract_size(*artifact, false).unwrap_or_default(); + let init_size = contract_size(*artifact, true).unwrap_or_default(); + + let is_dev_contract = artifact + .abi + .as_ref() + .map(|abi| { + abi.functions().any(|f| { + f.test_function_kind().is_known() || + matches!(f.name.as_str(), "IS_TEST" | "IS_SCRIPT") + }) }) - }) - .unwrap_or(false); - size_report - .contracts - .insert(name, ContractInfo { runtime_size, init_size, is_dev_contract }); + .unwrap_or(false); + + let unique_name = if artifact_list.len() > 1 { + format!( + "{} ({})", + name, + path.strip_prefix(&self.project_root).unwrap_or(path).display() + ) + } else { + name.clone() + }; + + size_report.contracts.insert( + unique_name, + ContractInfo { runtime_size, init_size, is_dev_contract }, + ); + } } let _ = sh_println!("{size_report}"); From 21a4d927eb9c8fa2a9beec22cf10711689d18e0c Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Wed, 26 Feb 2025 12:15:04 +0100 Subject: [PATCH 2/6] add test case --- crates/forge/tests/cli/build.rs | 45 +++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/crates/forge/tests/cli/build.rs b/crates/forge/tests/cli/build.rs index c586db4a6652..3c4dd588e7ad 100644 --- a/crates/forge/tests/cli/build.rs +++ b/crates/forge/tests/cli/build.rs @@ -177,6 +177,51 @@ forgetest_init!(build_sizes_no_forge_std, |prj, cmd| { ); }); +// tests build output --sizes handles multiple contracts with the same name +forgetest_init!(build_sizes_multiple_contracts, |prj, cmd| { + prj.add_source( + "a/Counter", + r" +contract Counter { + uint256 public count; + function increment() public { + count++; + } +} +", + ) + .unwrap(); + + prj.add_source( + "b/Counter", + r" +contract Counter { + uint256 public count; + function decrement() public { + count--; + } +} +", + ) + .unwrap(); + + cmd.args(["build", "--sizes"]).assert_success().stdout_eq(str![[r#" +... + +╭------------------------------------------+------------------+-------------------+--------------------+---------------------╮ +| Contract | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) | ++============================================================================================================================+ +| Counter (out/Counter.sol/Counter.json) | 481 | 509 | 24,095 | 48,643 | +|------------------------------------------+------------------+-------------------+--------------------+---------------------| +| Counter (out/a/Counter.sol/Counter.json) | 344 | 372 | 24,232 | 48,780 | +|------------------------------------------+------------------+-------------------+--------------------+---------------------| +| Counter (out/b/Counter.sol/Counter.json) | 291 | 319 | 24,285 | 48,833 | +╰------------------------------------------+------------------+-------------------+--------------------+---------------------╯ + + +"#]]); +}); + // tests that skip key in config can be used to skip non-compilable contract forgetest_init!(test_can_skip_contract, |prj, cmd| { prj.add_source( From 64d91b9bd77298e0d042492b05d99b3d686026e8 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Wed, 26 Feb 2025 12:19:16 +0100 Subject: [PATCH 3/6] include dummy Foo contract to show it doesnt show the unique path for non-clashing names --- crates/forge/tests/cli/build.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/forge/tests/cli/build.rs b/crates/forge/tests/cli/build.rs index 3c4dd588e7ad..fc3cc155f13d 100644 --- a/crates/forge/tests/cli/build.rs +++ b/crates/forge/tests/cli/build.rs @@ -179,6 +179,15 @@ forgetest_init!(build_sizes_no_forge_std, |prj, cmd| { // tests build output --sizes handles multiple contracts with the same name forgetest_init!(build_sizes_multiple_contracts, |prj, cmd| { + prj.add_source( + "Foo", + r" +contract Foo { +} +", + ) + .unwrap(); + prj.add_source( "a/Counter", r" @@ -216,6 +225,8 @@ contract Counter { | Counter (out/a/Counter.sol/Counter.json) | 344 | 372 | 24,232 | 48,780 | |------------------------------------------+------------------+-------------------+--------------------+---------------------| | Counter (out/b/Counter.sol/Counter.json) | 291 | 319 | 24,285 | 48,833 | +|------------------------------------------+------------------+-------------------+--------------------+---------------------| +| Foo | 62 | 88 | 24,514 | 49,064 | ╰------------------------------------------+------------------+-------------------+--------------------+---------------------╯ From 7e9020c8ec65045e133917f4f0ba1748ec1b417a Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Wed, 26 Feb 2025 15:31:29 +0100 Subject: [PATCH 4/6] show path to source rather than path to artifact --- crates/common/src/compile.rs | 2 +- crates/forge/tests/cli/build.rs | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/common/src/compile.rs b/crates/common/src/compile.rs index 214c7e5817c5..e7e37d2dbd55 100644 --- a/crates/common/src/compile.rs +++ b/crates/common/src/compile.rs @@ -261,7 +261,7 @@ impl ProjectCompiler { // filter out forge-std specific contracts !id.source.to_string_lossy().contains("/forge-std/src/") }) { - artifacts.entry(id.name.clone()).or_default().push((id.path.clone(), artifact)); + artifacts.entry(id.name.clone()).or_default().push((id.source.clone(), artifact)); } for (name, artifact_list) in artifacts { diff --git a/crates/forge/tests/cli/build.rs b/crates/forge/tests/cli/build.rs index fc3cc155f13d..944ed605c61a 100644 --- a/crates/forge/tests/cli/build.rs +++ b/crates/forge/tests/cli/build.rs @@ -217,17 +217,17 @@ contract Counter { cmd.args(["build", "--sizes"]).assert_success().stdout_eq(str![[r#" ... -╭------------------------------------------+------------------+-------------------+--------------------+---------------------╮ -| Contract | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) | -+============================================================================================================================+ -| Counter (out/Counter.sol/Counter.json) | 481 | 509 | 24,095 | 48,643 | -|------------------------------------------+------------------+-------------------+--------------------+---------------------| -| Counter (out/a/Counter.sol/Counter.json) | 344 | 372 | 24,232 | 48,780 | -|------------------------------------------+------------------+-------------------+--------------------+---------------------| -| Counter (out/b/Counter.sol/Counter.json) | 291 | 319 | 24,285 | 48,833 | -|------------------------------------------+------------------+-------------------+--------------------+---------------------| -| Foo | 62 | 88 | 24,514 | 49,064 | -╰------------------------------------------+------------------+-------------------+--------------------+---------------------╯ +╭-----------------------------+------------------+-------------------+--------------------+---------------------╮ +| Contract | Runtime Size (B) | Initcode Size (B) | Runtime Margin (B) | Initcode Margin (B) | ++===============================================================================================================+ +| Counter (src/Counter.sol) | 481 | 509 | 24,095 | 48,643 | +|-----------------------------+------------------+-------------------+--------------------+---------------------| +| Counter (src/a/Counter.sol) | 344 | 372 | 24,232 | 48,780 | +|-----------------------------+------------------+-------------------+--------------------+---------------------| +| Counter (src/b/Counter.sol) | 291 | 319 | 24,285 | 48,833 | +|-----------------------------+------------------+-------------------+--------------------+---------------------| +| Foo | 62 | 88 | 24,514 | 49,064 | +╰-----------------------------+------------------+-------------------+--------------------+---------------------╯ "#]]); From 57736b7ff92495589aa0ea7d2948155177022fd9 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Wed, 26 Feb 2025 15:39:17 +0100 Subject: [PATCH 5/6] add json test --- crates/forge/tests/cli/build.rs | 67 +++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/crates/forge/tests/cli/build.rs b/crates/forge/tests/cli/build.rs index 944ed605c61a..055a87e6223f 100644 --- a/crates/forge/tests/cli/build.rs +++ b/crates/forge/tests/cli/build.rs @@ -233,6 +233,73 @@ contract Counter { "#]]); }); +// tests build output --sizes --json handles multiple contracts with the same name +forgetest_init!(build_sizes_multiple_contracts_json, |prj, cmd| { + prj.add_source( + "Foo", + r" +contract Foo { +} +", + ) + .unwrap(); + + prj.add_source( + "a/Counter", + r" +contract Counter { + uint256 public count; + function increment() public { + count++; + } +} +", + ) + .unwrap(); + + prj.add_source( + "b/Counter", + r" +contract Counter { + uint256 public count; + function decrement() public { + count--; + } +} +", + ) + .unwrap(); + + cmd.args(["build", "--sizes", "--json"]).assert_success().stdout_eq(str![[r#" +{ + "Counter (src/Counter.sol)":{ + "runtime_size":481, + "init_size":509, + "runtime_margin":24095, + "init_margin":48643 + }, + "Counter (src/a/Counter.sol)":{ + "runtime_size":344, + "init_size":372, + "runtime_margin":24232, + "init_margin":48780 + }, + "Counter (src/b/Counter.sol)":{ + "runtime_size":291, + "init_size":319, + "runtime_margin":24285, + "init_margin":48833 + }, + "Foo":{ + "runtime_size":62, + "init_size":88, + "runtime_margin":24514, + "init_margin":49064 + } +} +"#]].is_json()); +}); + // tests that skip key in config can be used to skip non-compilable contract forgetest_init!(test_can_skip_contract, |prj, cmd| { prj.add_source( From 389cbbecc840773fef921c61dbfa502f7c5047a6 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Wed, 26 Feb 2025 15:41:57 +0100 Subject: [PATCH 6/6] fix fmt --- crates/forge/tests/cli/build.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/forge/tests/cli/build.rs b/crates/forge/tests/cli/build.rs index 055a87e6223f..26db462134e1 100644 --- a/crates/forge/tests/cli/build.rs +++ b/crates/forge/tests/cli/build.rs @@ -270,7 +270,8 @@ contract Counter { ) .unwrap(); - cmd.args(["build", "--sizes", "--json"]).assert_success().stdout_eq(str![[r#" + cmd.args(["build", "--sizes", "--json"]).assert_success().stdout_eq( + str![[r#" { "Counter (src/Counter.sol)":{ "runtime_size":481, @@ -297,7 +298,9 @@ contract Counter { "init_margin":49064 } } -"#]].is_json()); +"#]] + .is_json(), + ); }); // tests that skip key in config can be used to skip non-compilable contract