From 3232c786b9c67ba4765bc986d6f420f997d1b630 Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 14 Nov 2024 11:49:45 +0100 Subject: [PATCH 1/3] add --sizes and --names JSON compatibility + generalize report kind --- crates/common/Cargo.toml | 2 +- crates/common/src/compile.rs | 87 +++++++++++++++++++++++++------- crates/common/src/lib.rs | 1 + crates/common/src/reports.rs | 24 +++++++++ crates/forge/bin/cmd/build.rs | 3 +- crates/forge/bin/cmd/test/mod.rs | 3 +- crates/forge/src/gas_report.rs | 31 ++++-------- 7 files changed, 106 insertions(+), 45 deletions(-) create mode 100644 crates/common/src/reports.rs diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml index af95e94bcc35..8fa745a13982 100644 --- a/crates/common/Cargo.toml +++ b/crates/common/Cargo.toml @@ -54,7 +54,7 @@ num-format.workspace = true reqwest.workspace = true semver.workspace = true serde_json.workspace = true -serde.workspace = true +serde = { workspace = true, features = ["derive"] } thiserror.workspace = true tokio.workspace = true tracing.workspace = true diff --git a/crates/common/src/compile.rs b/crates/common/src/compile.rs index 4c7e7ba0fcef..252e81ecd9bb 100644 --- a/crates/common/src/compile.rs +++ b/crates/common/src/compile.rs @@ -1,6 +1,11 @@ //! Support for compiling [foundry_compilers::Project] -use crate::{term::SpinnerReporter, TestFunctionExt}; +use crate::{ + reports::{report_kind, ReportKind}, + shell, + term::SpinnerReporter, + TestFunctionExt, +}; use comfy_table::{presets::ASCII_MARKDOWN, Attribute, Cell, CellAlignment, Color, Table}; use eyre::Result; use foundry_block_explorers::contract::Metadata; @@ -181,11 +186,13 @@ impl ProjectCompiler { } if !quiet { - if output.is_unchanged() { - sh_println!("No files changed, compilation skipped")?; - } else { - // print the compiler output / warnings - sh_println!("{output}")?; + if !shell::is_json() { + if output.is_unchanged() { + sh_println!("No files changed, compilation skipped")?; + } else { + // print the compiler output / warnings + sh_println!("{output}")?; + } } self.handle_output(&output); @@ -205,22 +212,27 @@ impl ProjectCompiler { for (name, (_, version)) in output.versioned_artifacts() { artifacts.entry(version).or_default().push(name); } - for (version, names) in artifacts { - let _ = sh_println!( - " compiler version: {}.{}.{}", - version.major, - version.minor, - version.patch - ); - for name in names { - let _ = sh_println!(" - {name}"); + + if shell::is_json() { + let _ = sh_println!("{}", serde_json::to_string(&artifacts).unwrap()); + } else { + for (version, names) in artifacts { + let _ = sh_println!( + " compiler version: {}.{}.{}", + version.major, + version.minor, + version.patch + ); + for name in names { + let _ = sh_println!(" - {name}"); + } } } } if print_sizes { // add extra newline if names were already printed - if print_names { + if print_names && !shell::is_json() { let _ = sh_println!(); } @@ -316,6 +328,44 @@ impl SizeReport { impl Display for SizeReport { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { + match report_kind() { + ReportKind::Markdown => { + let table = self.format_table_output(); + writeln!(f, "{table}")?; + writeln!(f, "\n")?; + } + ReportKind::JSON => { + writeln!(f, "{}", self.format_json_output())?; + } + } + + Ok(()) + } +} + +impl SizeReport { + fn format_json_output(&self) -> String { + let contracts = self + .contracts + .iter() + .filter(|(_, c)| !c.is_dev_contract && (c.runtime_size > 0 || c.init_size > 0)) + .map(|(name, contract)| { + ( + name.clone(), + serde_json::json!({ + "runtime_size": contract.runtime_size, + "init_size": contract.init_size, + "runtime_margin": CONTRACT_RUNTIME_SIZE_LIMIT as isize - contract.runtime_size as isize, + "init_margin": CONTRACT_INITCODE_SIZE_LIMIT as isize - contract.init_size as isize, + }), + ) + }) + .collect::>(); + + serde_json::to_string(&contracts).unwrap() + } + + fn format_table_output(&self) -> Table { let mut table = Table::new(); table.load_preset(ASCII_MARKDOWN); table.set_header([ @@ -366,8 +416,7 @@ impl Display for SizeReport { ]); } - writeln!(f, "{table}")?; - Ok(()) + table } } @@ -476,7 +525,7 @@ pub fn etherscan_project( /// Configures the reporter and runs the given closure. pub fn with_compilation_reporter(quiet: bool, f: impl FnOnce() -> O) -> O { #[allow(clippy::collapsible_else_if)] - let reporter = if quiet { + let reporter = if quiet || shell::is_json() { Report::new(NoReporter::default()) } else { if std::io::stdout().is_terminal() { diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs index 68559d081b0a..259e8fee2dac 100644 --- a/crates/common/src/lib.rs +++ b/crates/common/src/lib.rs @@ -26,6 +26,7 @@ pub mod errors; pub mod evm; pub mod fs; pub mod provider; +pub mod reports; pub mod retry; pub mod selectors; pub mod serde_helpers; diff --git a/crates/common/src/reports.rs b/crates/common/src/reports.rs new file mode 100644 index 000000000000..d835ab66e29b --- /dev/null +++ b/crates/common/src/reports.rs @@ -0,0 +1,24 @@ +use serde::{Deserialize, Serialize}; + +use crate::shell; + +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +pub enum ReportKind { + Markdown, + JSON, +} + +impl Default for ReportKind { + fn default() -> Self { + Self::Markdown + } +} + +/// Determine the kind of report to generate based on the current shell. +pub fn report_kind() -> ReportKind { + if shell::is_json() { + ReportKind::JSON + } else { + ReportKind::Markdown + } +} diff --git a/crates/forge/bin/cmd/build.rs b/crates/forge/bin/cmd/build.rs index f2a1891e2634..4b288a699218 100644 --- a/crates/forge/bin/cmd/build.rs +++ b/crates/forge/bin/cmd/build.rs @@ -102,12 +102,11 @@ impl BuildArgs { .print_names(self.names) .print_sizes(self.sizes) .ignore_eip_3860(self.ignore_eip_3860) - .quiet(format_json) .bail(!format_json); let output = compiler.compile(&project)?; - if format_json { + if format_json && !self.names && !self.sizes { sh_println!("{}", serde_json::to_string_pretty(&output.output())?)?; } diff --git a/crates/forge/bin/cmd/test/mod.rs b/crates/forge/bin/cmd/test/mod.rs index 77ae5d2631f6..613d570785b9 100644 --- a/crates/forge/bin/cmd/test/mod.rs +++ b/crates/forge/bin/cmd/test/mod.rs @@ -5,7 +5,7 @@ use clap::{Parser, ValueHint}; use eyre::{Context, OptionExt, Result}; use forge::{ decode::decode_console_logs, - gas_report::{GasReport, GasReportKind}, + gas_report::GasReport, multi_runner::matches_contract, result::{SuiteResult, TestOutcome, TestStatus}, traces::{ @@ -583,7 +583,6 @@ impl TestArgs { config.gas_reports.clone(), config.gas_reports_ignore.clone(), config.gas_reports_include_tests, - if shell::is_json() { GasReportKind::JSON } else { GasReportKind::Markdown }, ) }); diff --git a/crates/forge/src/gas_report.rs b/crates/forge/src/gas_report.rs index e3e44994b91c..089bcdbf95f3 100644 --- a/crates/forge/src/gas_report.rs +++ b/crates/forge/src/gas_report.rs @@ -6,31 +6,22 @@ use crate::{ }; use alloy_primitives::map::HashSet; use comfy_table::{presets::ASCII_MARKDOWN, *}; -use foundry_common::{calc, TestFunctionExt}; +use foundry_common::{ + calc, + reports::{report_kind, ReportKind}, + TestFunctionExt, +}; use foundry_evm::traces::CallKind; use serde::{Deserialize, Serialize}; use serde_json::json; use std::{collections::BTreeMap, fmt::Display}; -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] -pub enum GasReportKind { - Markdown, - JSON, -} - -impl Default for GasReportKind { - fn default() -> Self { - Self::Markdown - } -} - /// Represents the gas report for a set of contracts. #[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct GasReport { /// Whether to report any contracts. report_any: bool, - /// What kind of report to generate. - report_type: GasReportKind, + /// Contracts to generate the report for. report_for: HashSet, /// Contracts to ignore when generating the report. @@ -47,13 +38,11 @@ impl GasReport { report_for: impl IntoIterator, ignore: impl IntoIterator, include_tests: bool, - report_kind: GasReportKind, ) -> Self { let report_for = report_for.into_iter().collect::>(); let ignore = ignore.into_iter().collect::>(); let report_any = report_for.is_empty() || report_for.contains("*"); - let report_type = report_kind; - Self { report_any, report_type, report_for, ignore, include_tests, ..Default::default() } + Self { report_any, report_for, ignore, include_tests, ..Default::default() } } /// Whether the given contract should be reported. @@ -158,8 +147,8 @@ impl GasReport { impl Display for GasReport { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - match self.report_type { - GasReportKind::Markdown => { + match report_kind() { + ReportKind::Markdown => { for (name, contract) in &self.contracts { if contract.functions.is_empty() { trace!(name, "gas report contract without functions"); @@ -171,7 +160,7 @@ impl Display for GasReport { writeln!(f, "\n")?; } } - GasReportKind::JSON => { + ReportKind::JSON => { writeln!(f, "{}", &self.format_json_output())?; } } From 3bb23dfe7e082b2611ace3ca4332e8f7fd8ffa5e Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Thu, 14 Nov 2024 12:38:07 +0100 Subject: [PATCH 2/3] add additional json output tests --- crates/common/src/compile.rs | 8 +++--- crates/forge/src/gas_report.rs | 14 +++++++--- crates/forge/tests/cli/build.rs | 45 +++++++++++++++++++++++++++++++++ crates/forge/tests/cli/cmd.rs | 19 ++++++++++++++ 4 files changed, 80 insertions(+), 6 deletions(-) diff --git a/crates/common/src/compile.rs b/crates/common/src/compile.rs index 252e81ecd9bb..507f328307c5 100644 --- a/crates/common/src/compile.rs +++ b/crates/common/src/compile.rs @@ -236,7 +236,8 @@ impl ProjectCompiler { let _ = sh_println!(); } - let mut size_report = SizeReport { contracts: BTreeMap::new() }; + let mut size_report = + SizeReport { report_kind: report_kind(), contracts: BTreeMap::new() }; let artifacts: BTreeMap<_, _> = output .artifact_ids() @@ -290,6 +291,8 @@ const CONTRACT_INITCODE_SIZE_LIMIT: usize = 49152; /// Contracts with info about their size pub struct SizeReport { + /// What kind of report to generate. + report_kind: ReportKind, /// `contract name -> info` pub contracts: BTreeMap, } @@ -328,11 +331,10 @@ impl SizeReport { impl Display for SizeReport { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - match report_kind() { + match self.report_kind { ReportKind::Markdown => { let table = self.format_table_output(); writeln!(f, "{table}")?; - writeln!(f, "\n")?; } ReportKind::JSON => { writeln!(f, "{}", self.format_json_output())?; diff --git a/crates/forge/src/gas_report.rs b/crates/forge/src/gas_report.rs index 089bcdbf95f3..7e87af077029 100644 --- a/crates/forge/src/gas_report.rs +++ b/crates/forge/src/gas_report.rs @@ -21,7 +21,8 @@ use std::{collections::BTreeMap, fmt::Display}; pub struct GasReport { /// Whether to report any contracts. report_any: bool, - + /// What kind of report to generate. + report_kind: ReportKind, /// Contracts to generate the report for. report_for: HashSet, /// Contracts to ignore when generating the report. @@ -42,7 +43,14 @@ impl GasReport { let report_for = report_for.into_iter().collect::>(); let ignore = ignore.into_iter().collect::>(); let report_any = report_for.is_empty() || report_for.contains("*"); - Self { report_any, report_for, ignore, include_tests, ..Default::default() } + Self { + report_any, + report_kind: report_kind(), + report_for, + ignore, + include_tests, + ..Default::default() + } } /// Whether the given contract should be reported. @@ -147,7 +155,7 @@ impl GasReport { impl Display for GasReport { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - match report_kind() { + match self.report_kind { ReportKind::Markdown => { for (name, contract) in &self.contracts { if contract.functions.is_empty() { diff --git a/crates/forge/tests/cli/build.rs b/crates/forge/tests/cli/build.rs index 4b257fe8d20a..dd2545995753 100644 --- a/crates/forge/tests/cli/build.rs +++ b/crates/forge/tests/cli/build.rs @@ -82,6 +82,20 @@ forgetest!(initcode_size_exceeds_limit, |prj, cmd| { ... "# ]); + + cmd.forge_fuse().args(["build", "--sizes", "--json"]).assert_failure().stdout_eq( + str![[r#" +{ + "HugeContract":{ + "runtime_size":202, + "init_size":49359, + "runtime_margin":24374, + "init_margin":-207 + } +} +"#]] + .is_json(), + ); }); forgetest!(initcode_size_limit_can_be_ignored, |prj, cmd| { @@ -95,6 +109,23 @@ forgetest!(initcode_size_limit_can_be_ignored, |prj, cmd| { ... "# ]); + + cmd.forge_fuse() + .args(["build", "--sizes", "--ignore-eip-3860", "--json"]) + .assert_success() + .stdout_eq( + str![[r#" +{ + "HugeContract": { + "runtime_size": 202, + "init_size": 49359, + "runtime_margin": 24374, + "init_margin": -207 + } +} +"#]] + .is_json(), + ); }); // tests build output is as expected @@ -118,6 +149,20 @@ forgetest_init!(build_sizes_no_forge_std, |prj, cmd| { ... "# ]); + + cmd.forge_fuse().args(["build", "--sizes", "--json"]).assert_success().stdout_eq( + str![[r#" +{ + "Counter": { + "runtime_size": 247, + "init_size": 277, + "runtime_margin": 24329, + "init_margin": 48875 + } +} +"#]] + .is_json(), + ); }); // tests that skip key in config can be used to skip non-compilable contract diff --git a/crates/forge/tests/cli/cmd.rs b/crates/forge/tests/cli/cmd.rs index d4f3973a337a..a131460c7079 100644 --- a/crates/forge/tests/cli/cmd.rs +++ b/crates/forge/tests/cli/cmd.rs @@ -2969,6 +2969,20 @@ Compiler run successful! "#]]); + + cmd.forge_fuse().args(["build", "--sizes", "--json"]).assert_success().stdout_eq( + str![[r#" +{ + "Counter": { + "runtime_size": 247, + "init_size": 277, + "runtime_margin": 24329, + "init_margin": 48875 + } +} +"#]] + .is_json(), + ); }); // checks that build --names includes all contracts even if unchanged @@ -2984,6 +2998,11 @@ Compiler run successful! ... "#]]); + + cmd.forge_fuse() + .args(["build", "--names", "--json"]) + .assert_success() + .stdout_eq(str![[r#""{...}""#]].is_json()); }); // From 5d55efd9af12682d49bf4b1454ea796d6e617bfa Mon Sep 17 00:00:00 2001 From: zerosnacks Date: Fri, 15 Nov 2024 15:46:49 +0100 Subject: [PATCH 3/3] fix feedback nit --- crates/common/src/reports.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/common/src/reports.rs b/crates/common/src/reports.rs index d835ab66e29b..adbdc11bf66a 100644 --- a/crates/common/src/reports.rs +++ b/crates/common/src/reports.rs @@ -2,18 +2,13 @@ use serde::{Deserialize, Serialize}; use crate::shell; -#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] +#[derive(Default, Clone, Debug, Serialize, Deserialize, PartialEq, Eq)] pub enum ReportKind { + #[default] Markdown, JSON, } -impl Default for ReportKind { - fn default() -> Self { - Self::Markdown - } -} - /// Determine the kind of report to generate based on the current shell. pub fn report_kind() -> ReportKind { if shell::is_json() {