chore(common::shell): finish implementation + enforce in clippy (foundry-rs#9268)

* enforce for script and verify crates

* complete and enforce common shell

* permit eprintln! due to circular dependency outside of common path

* avoid code duplication

Author: zerosnacks

Cargo.lock

@@ -3,10 +3,10 @@ msrv = "1.80"
 # so it is safe to ignore it as well
 ignore-interior-mutability = ["bytes::Bytes", "alloy_primitives::Bytes"]
 
-# disallowed-macros = [
-#     # See `foundry_common::shell`
-#     { path = "std::print", reason = "use `sh_print` or similar macros instead" },
-#     { path = "std::eprint", reason = "use `sh_eprint` or similar macros instead" },
-#     { path = "std::println", reason = "use `sh_println` or similar macros instead" },
-#     { path = "std::eprintln", reason = "use `sh_eprintln` or similar macros instead" },
-# ]
+disallowed-macros = [
+    # See `foundry_common::shell`
+    { path = "std::print", reason = "use `sh_print` or similar macros instead" },
+    { path = "std::eprint", reason = "use `sh_eprint` or similar macros instead" },
+    { path = "std::println", reason = "use `sh_println` or similar macros instead" },
+    { path = "std::eprintln", reason = "use `sh_eprintln` or similar macros instead" },
+]

clippy.toml

@@ -205,6 +205,7 @@ async fn can_check_blob_fields_on_genesis() {
     assert_eq!(block.header.excess_blob_gas, Some(0));
 }
 
+#[allow(clippy::disallowed_macros)]
 #[tokio::test(flavor = "multi_thread")]
 async fn can_correctly_estimate_blob_gas_with_recommended_fillers() {
     let node_config = NodeConfig::test().with_hardfork(Some(EthereumHardfork::Cancun.into()));
@@ -247,6 +248,7 @@ async fn can_correctly_estimate_blob_gas_with_recommended_fillers() {
     );
 }
 
+#[allow(clippy::disallowed_macros)]
 #[tokio::test(flavor = "multi_thread")]
 async fn can_correctly_estimate_blob_gas_with_recommended_fillers_with_signer() {
     let node_config = NodeConfig::test().with_hardfork(Some(EthereumHardfork::Cancun.into()));

crates/anvil/tests/it/eip4844.rs

@@ -247,6 +247,7 @@ async fn test_subscriptions() {
     assert_eq!(blocks, vec![1, 2, 3])
 }
 
+#[allow(clippy::disallowed_macros)]
 #[tokio::test(flavor = "multi_thread")]
 async fn test_sub_new_heads_fast() {
     let (api, handle) = spawn(NodeConfig::test()).await;

crates/anvil/tests/it/pubsub.rs

@@ -1,6 +1,7 @@
 #[macro_use]
 extern crate tracing;
 
+use alloy_dyn_abi::DynSolValue;
 use alloy_primitives::{eip191_hash_message, hex, keccak256, Address, B256};
 use alloy_provider::Provider;
 use alloy_rpc_types::{BlockId, BlockNumberOrTag::Latest};
@@ -12,7 +13,7 @@ use foundry_cli::{handler, utils};
 use foundry_common::{
     abi::get_event,
     ens::{namehash, ProviderEnsExt},
-    fmt::{format_uint_exp, print_tokens},
+    fmt::{format_tokens, format_tokens_raw, format_uint_exp},
     fs,
     selectors::{
         decode_calldata, decode_event_topic, decode_function_selector, decode_selectors,
@@ -135,11 +136,11 @@ async fn main_args(args: CastArgs) -> Result<()> {
         }
         CastSubcommand::ParseUnits { value, unit } => {
             let value = stdin::unwrap_line(value)?;
-            println!("{}", SimpleCast::parse_units(&value, unit)?);
+            sh_println!("{}", SimpleCast::parse_units(&value, unit)?)?;
         }
         CastSubcommand::FormatUnits { value, unit } => {
             let value = stdin::unwrap_line(value)?;
-            println!("{}", SimpleCast::format_units(&value, unit)?);
+            sh_println!("{}", SimpleCast::format_units(&value, unit)?)?;
         }
         CastSubcommand::FromWei { value, unit } => {
             let value = stdin::unwrap_line(value)?;
@@ -189,7 +190,7 @@ async fn main_args(args: CastArgs) -> Result<()> {
         // ABI encoding & decoding
         CastSubcommand::AbiDecode { sig, calldata, input } => {
             let tokens = SimpleCast::abi_decode(&sig, &calldata, input)?;
-            print_tokens(&tokens, shell::is_json())
+            print_tokens(&tokens);
         }
         CastSubcommand::AbiEncode { sig, packed, args } => {
             if !packed {
@@ -200,14 +201,14 @@ async fn main_args(args: CastArgs) -> Result<()> {
         }
         CastSubcommand::CalldataDecode { sig, calldata } => {
             let tokens = SimpleCast::calldata_decode(&sig, &calldata, true)?;
-            print_tokens(&tokens, shell::is_json())
+            print_tokens(&tokens);
         }
         CastSubcommand::CalldataEncode { sig, args } => {
             sh_println!("{}", SimpleCast::calldata_encode(sig, &args)?)?;
         }
         CastSubcommand::StringDecode { data } => {
             let tokens = SimpleCast::calldata_decode("Any(string)", &data, true)?;
-            print_tokens(&tokens, shell::is_json())
+            print_tokens(&tokens);
         }
         CastSubcommand::Interface(cmd) => cmd.run().await?,
         CastSubcommand::CreationCode(cmd) => cmd.run().await?,
@@ -482,7 +483,7 @@ async fn main_args(args: CastArgs) -> Result<()> {
             };
 
             let tokens = SimpleCast::calldata_decode(sig, &calldata, true)?;
-            print_tokens(&tokens, shell::is_json())
+            print_tokens(&tokens);
         }
         CastSubcommand::FourByteEvent { topic } => {
             let topic = stdin::unwrap_line(topic)?;
@@ -618,5 +619,22 @@ async fn main_args(args: CastArgs) -> Result<()> {
             sh_println!("{}", SimpleCast::decode_eof(&eof)?)?
         }
     };
+
+    /// Prints slice of tokens using [`format_tokens`] or [`format_tokens_raw`] depending whether
+    /// the shell is in JSON mode.
+    ///
+    /// This is included here to avoid a cyclic dependency between `fmt` and `common`.
+    fn print_tokens(tokens: &[DynSolValue]) {
+        if shell::is_json() {
+            let tokens: Vec<String> = format_tokens_raw(tokens).collect();
+            let _ = sh_println!("{}", serde_json::to_string_pretty(&tokens).unwrap());
+        } else {
+            let tokens = format_tokens(tokens);
+            tokens.for_each(|t| {
+                let _ = sh_println!("{t}");
+            });
+        }
+    }
+
     Ok(())
 }

crates/cast/bin/main.rs

@@ -103,6 +103,7 @@ impl Cheatcodes<'static> {
 }
 
 #[cfg(test)]
+#[allow(clippy::disallowed_macros)]
 mod tests {
     use super::*;
     use std::{fs, path::Path};

crates/cheatcodes/spec/src/lib.rs

@@ -620,11 +620,11 @@ fn prompt(
 
     match rx.recv_timeout(timeout) {
         Ok(res) => res.map_err(|err| {
-            println!();
+            let _ = sh_println!();
             err.to_string().into()
         }),
         Err(_) => {
-            println!();
+            let _ = sh_eprintln!();
             Err("Prompt timed out".into())
         }
     }

crates/cheatcodes/src/fs.rs

@@ -6,8 +6,12 @@
 #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))]
 #![allow(elided_lifetimes_in_paths)] // Cheats context uses 3 lifetimes
 
+#[macro_use]
+extern crate foundry_common;
+
 #[macro_use]
 pub extern crate foundry_cheatcodes_spec as spec;
+
 #[macro_use]
 extern crate tracing;
 

crates/cheatcodes/src/lib.rs

@@ -128,9 +128,8 @@ pub fn needs_setup(abi: &JsonAbi) -> bool {
 
     for setup_fn in setup_fns.iter() {
         if setup_fn.name != "setUp" {
-            println!(
-                "{} Found invalid setup function \"{}\" did you mean \"setUp()\"?",
-                "Warning:".yellow().bold(),
+            let _ = sh_warn!(
+                "Found invalid setup function \"{}\" did you mean \"setUp()\"?",
                 setup_fn.signature()
             );
         }
@@ -450,19 +449,19 @@ pub async fn print_traces(
 ) -> Result<()> {
     let traces = result.traces.as_mut().expect("No traces found");
 
-    println!("Traces:");
+    sh_println!("Traces:")?;
     for (_, arena) in traces {
         decode_trace_arena(arena, decoder).await?;
-        println!("{}", render_trace_arena_with_bytecodes(arena, verbose));
+        sh_println!("{}", render_trace_arena_with_bytecodes(arena, verbose))?;
     }
-    println!();
+    sh_println!()?;
 
     if result.success {
-        println!("{}", "Transaction successfully executed.".green());
+        sh_println!("{}", "Transaction successfully executed.".green())?;
     } else {
-        println!("{}", "Transaction failed.".red());
+        sh_err!("Transaction failed.")?;
     }
 
-    println!("Gas used: {}", result.gas_used);
+    sh_println!("Gas used: {}", result.gas_used)?;
     Ok(())
 }

crates/cli/src/utils/cmd.rs

@@ -132,18 +132,6 @@ pub fn format_tokens_raw(tokens: &[DynSolValue]) -> impl Iterator<Item = String>
     tokens.iter().map(format_token_raw)
 }
 
-/// Prints slice of tokens using [`format_tokens`] or [`format_tokens_raw`] depending on `json`
-/// parameter.
-pub fn print_tokens(tokens: &[DynSolValue], json: bool) {
-    if json {
-        let tokens: Vec<String> = format_tokens_raw(tokens).collect();
-        println!("{}", serde_json::to_string_pretty(&tokens).unwrap());
-    } else {
-        let tokens = format_tokens(tokens);
-        tokens.for_each(|t| println!("{t}"));
-    }
-}
-
 /// Pretty-prints the given value into a string suitable for user output.
 pub fn format_token(value: &DynSolValue) -> String {
     DynValueDisplay::new(value, false).to_string()

crates/common/fmt/src/dynamic.rs

@@ -4,9 +4,7 @@ mod console;
 pub use console::{console_format, ConsoleFmt, FormatSpec};
 
 mod dynamic;
-pub use dynamic::{
-    format_token, format_token_raw, format_tokens, format_tokens_raw, parse_tokens, print_tokens,
-};
+pub use dynamic::{format_token, format_token_raw, format_tokens, format_tokens_raw, parse_tokens};
 
 mod exp;
 pub use exp::{format_int_exp, format_uint_exp, to_exp_notation};

crates/common/fmt/src/lib.rs

@@ -146,9 +146,9 @@ pub fn find_source(
             Ok(source)
         } else {
             let implementation = metadata.implementation.unwrap();
-            println!(
+            sh_println!(
                 "Contract at {address} is a proxy, trying to fetch source at {implementation}..."
-            );
+            )?;
             match find_source(client, implementation).await {
                 impl_source @ Ok(_) => impl_source,
                 Err(e) => {

crates/common/src/abi.rs

@@ -127,7 +127,7 @@ impl ProjectCompiler {
     pub fn compile<C: Compiler>(mut self, project: &Project<C>) -> Result<ProjectCompileOutput<C>> {
         // TODO: Avoid process::exit
         if !project.paths.has_input_files() && self.files.is_empty() {
-            println!("Nothing to compile");
+            sh_println!("Nothing to compile")?;
             // nothing to do here
             std::process::exit(0);
         }
@@ -182,10 +182,10 @@ impl ProjectCompiler {
 
         if !quiet {
             if output.is_unchanged() {
-                println!("No files changed, compilation skipped");
+                sh_println!("No files changed, compilation skipped")?;
             } else {
                 // print the compiler output / warnings
-                println!("{output}");
+                sh_println!("{output}")?;
             }
 
             self.handle_output(&output);
@@ -206,20 +206,22 @@ impl ProjectCompiler {
                 artifacts.entry(version).or_default().push(name);
             }
             for (version, names) in artifacts {
-                println!(
+                let _ = sh_println!(
                     "  compiler version: {}.{}.{}",
-                    version.major, version.minor, version.patch
+                    version.major,
+                    version.minor,
+                    version.patch
                 );
                 for name in names {
-                    println!("    - {name}");
+                    let _ = sh_println!("    - {name}");
                 }
             }
         }
 
         if print_sizes {
             // add extra newline if names were already printed
             if print_names {
-                println!();
+                let _ = sh_println!();
             }
 
             let mut size_report = SizeReport { contracts: BTreeMap::new() };
@@ -252,7 +254,7 @@ impl ProjectCompiler {
                     .insert(name, ContractInfo { runtime_size, init_size, is_dev_contract });
             }
 
-            println!("{size_report}");
+            let _ = sh_println!("{size_report}");
 
             // TODO: avoid process::exit
             // exit with error if any contract exceeds the size limit, excluding test contracts.

crates/common/src/compile.rs

@@ -123,7 +123,9 @@ pub trait ProviderEnsExt<T: Transport + Clone, N: Network, P: Provider<T, N>> {
             .call()
             .await
             .map_err(EnsError::Resolve)
-            .inspect_err(|e| eprintln!("{e:?}"))?
+            .inspect_err(|e| {
+                let _ = sh_eprintln!("{e:?}");
+            })?
             ._0;
         Ok(addr)
     }

crates/common/src/ens.rs

@@ -492,24 +492,20 @@ pub struct SelectorImportResponse {
 impl SelectorImportResponse {
     /// Print info about the functions which were uploaded or already known
     pub fn describe(&self) {
-        self.result
-            .function
-            .imported
-            .iter()
-            .for_each(|(k, v)| println!("Imported: Function {k}: {v}"));
-        self.result.event.imported.iter().for_each(|(k, v)| println!("Imported: Event {k}: {v}"));
-        self.result
-            .function
-            .duplicated
-            .iter()
-            .for_each(|(k, v)| println!("Duplicated: Function {k}: {v}"));
-        self.result
-            .event
-            .duplicated
-            .iter()
-            .for_each(|(k, v)| println!("Duplicated: Event {k}: {v}"));
-
-        println!("Selectors successfully uploaded to OpenChain");
+        self.result.function.imported.iter().for_each(|(k, v)| {
+            let _ = sh_println!("Imported: Function {k}: {v}");
+        });
+        self.result.event.imported.iter().for_each(|(k, v)| {
+            let _ = sh_println!("Imported: Event {k}: {v}");
+        });
+        self.result.function.duplicated.iter().for_each(|(k, v)| {
+            let _ = sh_println!("Duplicated: Function {k}: {v}");
+        });
+        self.result.event.duplicated.iter().for_each(|(k, v)| {
+            let _ = sh_println!("Duplicated: Event {k}: {v}");
+        });
+
+        let _ = sh_println!("Selectors successfully uploaded to OpenChain");
     }
 }
 
@@ -579,6 +575,7 @@ pub fn parse_signatures(tokens: Vec<String>) -> ParsedSignatures {
 }
 
 #[cfg(test)]
+#[allow(clippy::disallowed_macros)]
 #[allow(clippy::needless_return)]
 mod tests {
     use super::*;

crates/common/src/selectors.rs

@@ -72,7 +72,7 @@ impl Spinner {
 
         let indicator = self.indicator[self.idx % self.indicator.len()].green();
         let indicator = Paint::new(format!("[{indicator}]")).bold();
-        print!("\r\x33[2K\r{indicator} {}", self.message);
+        let _ = sh_print!("\r\x33[2K\r{indicator} {}", self.message);
         io::stdout().flush().unwrap();
 
         self.idx = self.idx.wrapping_add(1);
@@ -112,11 +112,11 @@ impl SpinnerReporter {
                         Ok(SpinnerMsg::Msg(msg)) => {
                             spinner.message(msg);
                             // new line so past messages are not overwritten
-                            println!();
+                            let _ = sh_println!();
                         }
                         Ok(SpinnerMsg::Shutdown(ack)) => {
                             // end with a newline
-                            println!();
+                            let _ = sh_println!();
                             let _ = ack.send(());
                             break
                         }