From 6b65f87f0cb33fbe4da9076a509d31cfff41981a Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Wed, 30 Oct 2024 10:11:09 +0100 Subject: [PATCH 1/2] add back signatures, even if empty, flatten multiple selectors per feedback https://github.com/foundry-rs/foundry/pull/9216#issuecomment-2445386251 --- crates/forge/src/gas_report.rs | 46 ++++++-------------- crates/forge/tests/cli/cmd.rs | 78 +++++++++++++++++----------------- 2 files changed, 52 insertions(+), 72 deletions(-) diff --git a/crates/forge/src/gas_report.rs b/crates/forge/src/gas_report.rs index 6c53bd9e6e75..c39d15ce3940 100644 --- a/crates/forge/src/gas_report.rs +++ b/crates/forge/src/gas_report.rs @@ -181,17 +181,6 @@ impl Display for GasReport { impl GasReport { fn format_json_output(&self) -> String { - #[inline] - fn format_gas_info(gas_info: &GasInfo) -> serde_json::Value { - json!({ - "calls": gas_info.calls, - "min": gas_info.min, - "mean": gas_info.mean, - "median": gas_info.median, - "max": gas_info.max, - }) - } - serde_json::to_string( &self .contracts @@ -205,27 +194,20 @@ impl GasReport { let functions = contract .functions .iter() - .map(|(fname, sigs)| { - // If there is only one signature, display the gas info directly. - let function_value = if sigs.len() == 1 { - format_gas_info(sigs.values().next().unwrap()) - } else { - // If there are multiple signatures, e.g. overloads like: - // - `foo(uint256)` - // - `foo(int256)` - // display the gas info as a map with the signature as the key. - let signatures = sigs - .iter() - .map(|(sig, gas_info)| { - let display_name = sig.replace(':', ""); - (display_name, format_gas_info(gas_info)) - }) - .collect::>(); - - json!(signatures) - }; - - (fname.to_string(), function_value) + .flat_map(|(_, sigs)| { + sigs.iter().map(|(sig, gas_info)| { + let display_name = sig.replace(':', ""); + ( + display_name, + json!({ + "calls": gas_info.calls, + "min": gas_info.min, + "mean": gas_info.mean, + "median": gas_info.median, + "max": gas_info.max, + }), + ) + }) }) .collect::>(); diff --git a/crates/forge/tests/cli/cmd.rs b/crates/forge/tests/cli/cmd.rs index 48b73ac626ec..f34d0e2dec5d 100644 --- a/crates/forge/tests/cli/cmd.rs +++ b/crates/forge/tests/cli/cmd.rs @@ -1601,7 +1601,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 255 }, "functions": { - "foo": { + "foo()": { "calls": 1, "min": 45387, "mean": 45387, @@ -1617,7 +1617,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 256 }, "functions": { - "baz": { + "baz()": { "calls": 1, "min": 260712, "mean": 260712, @@ -1633,7 +1633,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 255 }, "functions": { - "bar": { + "bar()": { "calls": 1, "min": 64984, "mean": 64984, @@ -1685,7 +1685,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 255 }, "functions": { - "foo": { + "foo()": { "calls": 1, "min": 45387, "mean": 45387, @@ -1701,7 +1701,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 256 }, "functions": { - "baz": { + "baz()": { "calls": 1, "min": 260712, "mean": 260712, @@ -1717,7 +1717,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 255 }, "functions": { - "bar": { + "bar()": { "calls": 1, "min": 64984, "mean": 64984, @@ -1769,7 +1769,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 255 }, "functions": { - "foo": { + "foo()": { "calls": 1, "min": 45387, "mean": 45387, @@ -1785,7 +1785,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 256 }, "functions": { - "baz": { + "baz()": { "calls": 1, "min": 260712, "mean": 260712, @@ -1801,7 +1801,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 255 }, "functions": { - "bar": { + "bar()": { "calls": 1, "min": 64984, "mean": 64984, @@ -1860,7 +1860,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 255 }, "functions": { - "foo": { + "foo()": { "calls": 1, "min": 45387, "mean": 45387, @@ -1876,7 +1876,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 256 }, "functions": { - "baz": { + "baz()": { "calls": 1, "min": 260712, "mean": 260712, @@ -1892,7 +1892,7 @@ forgetest!(gas_report_all_contracts, |prj, cmd| { "size": 255 }, "functions": { - "bar": { + "bar()": { "calls": 1, "min": 64984, "mean": 64984, @@ -1935,7 +1935,7 @@ forgetest!(gas_report_some_contracts, |prj, cmd| { "size": 255 }, "functions": { - "foo": { + "foo()": { "calls": 1, "min": 45387, "mean": 45387, @@ -1973,7 +1973,7 @@ forgetest!(gas_report_some_contracts, |prj, cmd| { "size": 255 }, "functions": { - "bar": { + "bar()": { "calls": 1, "min": 64984, "mean": 64984, @@ -2014,7 +2014,7 @@ forgetest!(gas_report_some_contracts, |prj, cmd| { "size": 256 }, "functions": { - "baz": { + "baz()": { "calls": 1, "min": 260712, "mean": 260712, @@ -2069,7 +2069,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| { "size": 256 }, "functions": { - "baz": { + "baz()": { "calls": 1, "min": 260712, "mean": 260712, @@ -2085,7 +2085,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| { "size": 255 }, "functions": { - "bar": { + "bar()": { "calls": 1, "min": 64984, "mean": 64984, @@ -2136,7 +2136,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| { "size": 255 }, "functions": { - "foo": { + "foo()": { "calls": 1, "min": 45387, "mean": 45387, @@ -2152,7 +2152,7 @@ forgetest!(gas_report_ignore_some_contracts, |prj, cmd| { "size": 256 }, "functions": { - "baz": { + "baz()": { "calls": 1, "min": 260712, "mean": 260712, @@ -2231,7 +2231,7 @@ warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'. "size": 255 }, "functions": { - "foo": { + "foo()": { "calls": 1, "min": 45387, "mean": 45387, @@ -2247,7 +2247,7 @@ warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'. "size": 256 }, "functions": { - "baz": { + "baz()": { "calls": 1, "min": 260712, "mean": 260712, @@ -2263,7 +2263,7 @@ warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'. "size": 255 }, "functions": { - "bar": { + "bar()": { "calls": 1, "min": 64984, "mean": 64984, @@ -2283,7 +2283,7 @@ warning: ContractThree is listed in both 'gas_reports' and 'gas_reports_ignore'. "#]]); }); -forgetest!(gas_report_multiple_selectors, |prj, cmd| { +forgetest!(gas_report_flatten_multiple_selectors, |prj, cmd| { prj.insert_ds_test(); prj.add_source( "Counter.sol", @@ -2351,35 +2351,33 @@ contract CounterTest is DSTest { "size": 250 }, "functions": { - "a": { + "a()": { "calls": 1, "min": 2261, "mean": 2261, "median": 2261, "max": 2261 }, - "b": { + "b()": { "calls": 1, "min": 2305, "mean": 2305, "median": 2305, "max": 2305 }, - "setNumber": { - "setNumber(int256)": { - "calls": 2, - "min": 23648, - "mean": 33604, - "median": 33604, - "max": 43560 - }, - "setNumber(uint256)": { - "calls": 2, - "min": 23604, - "mean": 33560, - "median": 33560, - "max": 43516 - } + "setNumber(int256)": { + "calls": 2, + "min": 23648, + "mean": 33604, + "median": 33604, + "max": 43560 + }, + "setNumber(uint256)": { + "calls": 2, + "min": 23604, + "mean": 33560, + "median": 33560, + "max": 43516 } } } From 2ae4f8f8cfbd15053703701d9e339100515f1361 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Wed, 30 Oct 2024 11:16:10 +0100 Subject: [PATCH 2/2] avoid manually serializing `gas_info`, already implements serialize --- crates/forge/src/gas_report.rs | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/crates/forge/src/gas_report.rs b/crates/forge/src/gas_report.rs index c39d15ce3940..ac2d0db00262 100644 --- a/crates/forge/src/gas_report.rs +++ b/crates/forge/src/gas_report.rs @@ -197,16 +197,7 @@ impl GasReport { .flat_map(|(_, sigs)| { sigs.iter().map(|(sig, gas_info)| { let display_name = sig.replace(':', ""); - ( - display_name, - json!({ - "calls": gas_info.calls, - "min": gas_info.min, - "mean": gas_info.mean, - "median": gas_info.median, - "max": gas_info.max, - }), - ) + (display_name, gas_info) }) }) .collect::>();