From cef146edbd754e2e63018133a8a8901b7e425adf Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 8 Mar 2022 19:14:15 +1000 Subject: [PATCH] lint(clippy): warn on manual printing to stdout or stderr (#3767) Most logging should use `tracing::trace!()` or `tracing::debug!()` instead. --- .cargo/config.toml | 10 ++++++---- zebra-chain/src/parameters/tests.rs | 5 ----- zebra-consensus/examples/get-params-path.rs | 1 + zebra-consensus/src/primitives/halo2/tests.rs | 2 +- zebra-state/src/service/finalized_state/tests/prop.rs | 1 + zebra-test/src/command.rs | 1 + zebra-test/src/net.rs | 2 ++ zebra-test/tests/command.rs | 1 + zebra-utils/src/bin/zebra-checkpoints/main.rs | 1 + zebrad/build.rs | 3 ++- zebrad/src/application.rs | 1 + zebrad/src/commands/generate.rs | 1 + zebrad/src/commands/version.rs | 1 + zebrad/tests/acceptance.rs | 4 +++- 14 files changed, 22 insertions(+), 12 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index ab452214..4e644a32 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -38,6 +38,12 @@ rustflags = [ "-Wclippy::dbg_macro", "-Wclippy::todo", + # Manual debugging output. + # Use tracing::trace!() or tracing::debug!() instead. + "-Wclippy::print_stdout", + "-Wclippy::print_stderr", + "-Wclippy::dbg_macro", + # Code styles we want to accept "-Aclippy::try_err", @@ -59,10 +65,6 @@ rustflags = [ #"-Wclippy::cast_precision_loss", # 25 non-test warnings, 10 test warnings #"-Wclippy::cast_sign_loss", # 6 non-test warnings, 15 test warnings - # disable these lints using a zebra-test config.toml, but warn for other crates - #"-Wclippy::print_stdout", - #"-Wclippy::print_stderr", - # fix hidden lifetime parameters #"-Wrust_2018_idioms", ] diff --git a/zebra-chain/src/parameters/tests.rs b/zebra-chain/src/parameters/tests.rs index 4debc6f9..166ab31a 100644 --- a/zebra-chain/src/parameters/tests.rs +++ b/zebra-chain/src/parameters/tests.rs @@ -14,11 +14,6 @@ use NetworkUpgrade::*; fn activation_bijective() { zebra_test::init(); - if std::env::var_os("TEST_FAKE_ACTIVATION_HEIGHTS").is_some() { - eprintln!("Skipping activation_bijective() since $TEST_FAKE_ACTIVATION_HEIGHTS is set"); - return; - } - let mainnet_activations = NetworkUpgrade::activation_list(Mainnet); let mainnet_heights: HashSet<&block::Height> = mainnet_activations.keys().collect(); assert_eq!(MAINNET_ACTIVATION_HEIGHTS.len(), mainnet_heights.len()); diff --git a/zebra-consensus/examples/get-params-path.rs b/zebra-consensus/examples/get-params-path.rs index e17c38c0..68c5de97 100644 --- a/zebra-consensus/examples/get-params-path.rs +++ b/zebra-consensus/examples/get-params-path.rs @@ -3,6 +3,7 @@ // Modified from: // https://github.com/zcash/librustzcash/blob/c48bb4def2e122289843ddb3cb2984c325c03ca0/zcash_proofs/examples/get-params-path.rs +#[allow(clippy::print_stdout)] fn main() { let path = zebra_consensus::groth16::Groth16Parameters::directory(); if let Some(path) = path.to_str() { diff --git a/zebra-consensus/src/primitives/halo2/tests.rs b/zebra-consensus/src/primitives/halo2/tests.rs index 438b9053..ce9b1e2a 100644 --- a/zebra-consensus/src/primitives/halo2/tests.rs +++ b/zebra-consensus/src/primitives/halo2/tests.rs @@ -23,7 +23,7 @@ use zebra_chain::{ use crate::primitives::halo2::*; -#[allow(dead_code)] +#[allow(dead_code, clippy::print_stdout)] fn generate_test_vectors() { let proving_key = ProvingKey::build(); diff --git a/zebra-state/src/service/finalized_state/tests/prop.rs b/zebra-state/src/service/finalized_state/tests/prop.rs index d56fbfa7..51536a74 100644 --- a/zebra-state/src/service/finalized_state/tests/prop.rs +++ b/zebra-state/src/service/finalized_state/tests/prop.rs @@ -48,6 +48,7 @@ fn blocks_with_v5_transactions() -> Result<()> { /// /// This test requires setting the TEST_FAKE_ACTIVATION_HEIGHTS. #[test] +#[allow(clippy::print_stderr)] fn all_upgrades_and_wrong_commitments_with_fake_activation_heights() -> Result<()> { zebra_test::init(); diff --git a/zebra-test/src/command.rs b/zebra-test/src/command.rs index 623d9f39..17047180 100644 --- a/zebra-test/src/command.rs +++ b/zebra-test/src/command.rs @@ -276,6 +276,7 @@ impl TestChild { /// Note: the timeout is only checked after each full line is received from /// the child. #[instrument(skip(self, lines))] + #[allow(clippy::print_stdout)] pub fn expect_line_matching( &mut self, lines: &mut L, diff --git a/zebra-test/src/net.rs b/zebra-test/src/net.rs index d92ff8f9..46415af4 100644 --- a/zebra-test/src/net.rs +++ b/zebra-test/src/net.rs @@ -18,6 +18,7 @@ const ZEBRA_SKIP_IPV6_TESTS: &str = "ZEBRA_SKIP_IPV6_TESTS"; /// Should we skip Zebra tests which need reliable, fast network connectivity? // // TODO: separate "good and reliable" from "any network"? +#[allow(clippy::print_stderr)] pub fn zebra_skip_network_tests() -> bool { if env::var_os(ZEBRA_SKIP_NETWORK_TESTS).is_some() { // This message is captured by the test runner, use @@ -34,6 +35,7 @@ pub fn zebra_skip_network_tests() -> bool { /// /// Since `zebra_skip_network_tests` only disables tests which need reliable network connectivity, /// we allow IPv6 tests even when `ZEBRA_SKIP_NETWORK_TESTS` is set. +#[allow(clippy::print_stderr)] pub fn zebra_skip_ipv6_tests() -> bool { if env::var_os(ZEBRA_SKIP_IPV6_TESTS).is_some() { eprintln!("Skipping IPv6 network test because '$ZEBRA_SKIP_IPV6_TESTS' is set."); diff --git a/zebra-test/tests/command.rs b/zebra-test/tests/command.rs index 4a482301..9086ac0f 100644 --- a/zebra-test/tests/command.rs +++ b/zebra-test/tests/command.rs @@ -11,6 +11,7 @@ use zebra_test::{command::TestDirExt, prelude::Stdio}; /// (This message is captured by the test runner, use `cargo test -- --nocapture` to see it.) /// /// The command's stdout and stderr are ignored. +#[allow(clippy::print_stderr)] fn is_command_available(cmd: &str, args: &[&str]) -> bool { let status = Command::new(cmd) .args(args) diff --git a/zebra-utils/src/bin/zebra-checkpoints/main.rs b/zebra-utils/src/bin/zebra-checkpoints/main.rs index f8523404..60b0e05a 100644 --- a/zebra-utils/src/bin/zebra-checkpoints/main.rs +++ b/zebra-utils/src/bin/zebra-checkpoints/main.rs @@ -65,6 +65,7 @@ fn cmd_output(cmd: &mut std::process::Command) -> Result { Ok(s) } +#[allow(clippy::print_stdout)] fn main() -> Result<()> { init_tracing(); diff --git a/zebrad/build.rs b/zebrad/build.rs index 66c3c403..d136b69a 100644 --- a/zebrad/build.rs +++ b/zebrad/build.rs @@ -28,6 +28,7 @@ fn disable_non_reproducible(_config: &mut Config) { */ } +#[allow(clippy::print_stderr)] fn main() { let mut config = Config::default(); disable_non_reproducible(&mut config); @@ -48,7 +49,7 @@ fn main() { Err(e) => { eprintln!( "git error in vergen build script: skipping git env vars: {:?}", - e + e, ); *config.git_mut().enabled_mut() = false; vergen(config).expect("non-git vergen should succeed"); diff --git a/zebrad/src/application.rs b/zebrad/src/application.rs index d098513a..10f74313 100644 --- a/zebrad/src/application.rs +++ b/zebrad/src/application.rs @@ -192,6 +192,7 @@ impl Application for ZebradApp { /// If you would like to add additional components to your application /// beyond the default ones provided by the framework, this is the place /// to do so. + #[allow(clippy::print_stderr)] fn register_components(&mut self, command: &Self::Cmd) -> Result<(), FrameworkError> { use crate::components::{ metrics::MetricsEndpoint, tokio::TokioComponent, tracing::TracingEndpoint, diff --git a/zebrad/src/commands/generate.rs b/zebrad/src/commands/generate.rs index e1f6ae8e..427aeccd 100644 --- a/zebrad/src/commands/generate.rs +++ b/zebrad/src/commands/generate.rs @@ -13,6 +13,7 @@ pub struct GenerateCmd { impl Runnable for GenerateCmd { /// Start the application. + #[allow(clippy::print_stdout)] fn run(&self) { let default_config = ZebradConfig::default(); let mut output = r"# Default configuration for zebrad. diff --git a/zebrad/src/commands/version.rs b/zebrad/src/commands/version.rs index c248ae9d..047b9b12 100644 --- a/zebrad/src/commands/version.rs +++ b/zebrad/src/commands/version.rs @@ -11,6 +11,7 @@ pub struct VersionCmd {} impl Runnable for VersionCmd { /// Print version message + #[allow(clippy::print_stdout)] fn run(&self) { println!("{} {}", ZebradCmd::name(), ZebradCmd::version()); } diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index 95e96338..aac81a08 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1261,6 +1261,7 @@ fn cached_mandatory_checkpoint_test_config() -> Result { /// /// Returns an error if the child exits or the fixed timeout elapses /// before `STOP_AT_HEIGHT_REGEX` is found. +#[allow(clippy::print_stderr)] fn create_cached_database_height( network: Network, height: Height, @@ -1268,7 +1269,8 @@ fn create_cached_database_height( checkpoint_sync: bool, stop_regex: &str, ) -> Result<()> { - println!("Creating cached database"); + eprintln!("creating cached database"); + // 16 hours let timeout = Duration::from_secs(60 * 60 * 16);