From 748af798223bd24e95394795109a0e683b42690c Mon Sep 17 00:00:00 2001 From: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Date: Wed, 30 Oct 2024 11:29:58 +0100 Subject: [PATCH] fix(`--gas-report`): add back signatures, even if empty, avoid nesting multiple selectors (#9229) * add back signatures, even if empty, flatten multiple selectors per feedback https://github.com/foundry-rs/foundry/pull/9216#issuecomment-2445386251 * avoid manually serializing `gas_info`, already implements serialize --- crates/forge/src/gas_report.rs | 37 +++------------- crates/forge/tests/cli/cmd.rs | 78 +++++++++++++++++----------------- 2 files changed, 43 insertions(+), 72 deletions(-) diff --git a/crates/forge/src/gas_report.rs b/crates/forge/src/gas_report.rs index 6c53bd9e6e75..ac2d0db00262 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,11 @@ 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, gas_info) + }) }) .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 } } }