chore: enable clippy::print_stdout and clippy::print_stderr (denoland…

…#23732) 1. Generally we should prefer to use the `log` crate. 2. I very often accidentally commit `eprintln`s. When we should use `println` or `eprintln`, it's not too bad to be a bit more verbose and ignore the lint rule.
crowlKats · May 9, 2024 · 47f7bed · 47f7bed
1 parent e6dc4df
commit 47f7bed
Show file tree

Hide file tree

Showing 52 changed files with 308 additions and 182 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/bench_util/profiling.rs b/bench_util/profiling.rs
@@ -39,6 +39,7 @@ macro_rules! bench_or_profile {
   };
 }
 
+#[allow(clippy::print_stdout)]
 pub fn run_profiles(opts: &TestOpts, tests: Vec<TestDescAndFn>) {
   let tests = filter_tests(opts, tests);
   // let decs = tests.iter().map(|t| t.desc.clone()).collect();

diff --git a/cli/args/flags.rs b/cli/args/flags.rs
@@ -3889,6 +3889,7 @@ fn eval_parse(flags: &mut Flags, matches: &mut ArgMatches) {
   // TODO(@satyarohith): remove this flag in 2.0.
   let as_typescript = matches.get_flag("ts");
 
+  #[allow(clippy::print_stderr)]
   if as_typescript {
     eprintln!(
       "⚠️ {}",
@@ -4218,6 +4219,8 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) {
   let no_run = matches.get_flag("no-run");
   let trace_leaks =
     matches.get_flag("trace-ops") || matches.get_flag("trace-leaks");
+
+  #[allow(clippy::print_stderr)]
   if trace_leaks && matches.get_flag("trace-ops") {
     // We can't change this to use the log crate because its not configured
     // yet at this point since the flags haven't been parsed. This flag is
@@ -4267,10 +4270,17 @@ fn test_parse(flags: &mut Flags, matches: &mut ArgMatches) {
     // yet at this point since the flags haven't been parsed. This flag is
     // deprecated though so it's not worth changing the code to use the log
     // crate here and this is only done for testing anyway.
-    eprintln!(
-      "⚠️ {}",
-      crate::colors::yellow("The `--jobs` flag is deprecated and will be removed in Deno 2.0.\nUse the `--parallel` flag with possibly the `DENO_JOBS` environment variable instead.\nLearn more at: https://docs.deno.com/runtime/manual/basics/env_variables"),
-    );
+    #[allow(clippy::print_stderr)]
+    {
+      eprintln!(
+        "⚠️ {}",
+        crate::colors::yellow(concat!(
+          "The `--jobs` flag is deprecated and will be removed in Deno 2.0.\n",
+          "Use the `--parallel` flag with possibly the `DENO_JOBS` environment variable instead.\n",
+          "Learn more at: https://docs.deno.com/runtime/manual/basics/env_variables"
+        )),
+      );
+    }
     if let Some(value) = matches.remove_one::<NonZeroUsize>("jobs") {
       Some(value)
     } else {

diff --git a/cli/args/mod.rs b/cli/args/mod.rs
@@ -747,9 +747,12 @@ impl CliOptions {
         format!("for: {}", insecure_allowlist.join(", "))
       };
       let msg =
-        format!("DANGER: TLS certificate validation is disabled {domains}");
-      // use eprintln instead of log::warn so this always gets shown
-      eprintln!("{}", colors::yellow(msg));
+        format!("DANGER: TLS certificate validation is disabled {}", domains);
+      #[allow(clippy::print_stderr)]
+      {
+        // use eprintln instead of log::warn so this always gets shown
+        eprintln!("{}", colors::yellow(msg));
+      }
     }
 
     let maybe_lockfile = maybe_lockfile.filter(|_| !force_global_cache);

diff --git a/cli/bench/main.rs b/cli/bench/main.rs
@@ -1,5 +1,8 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
+#![allow(clippy::print_stdout)]
+#![allow(clippy::print_stderr)]
+
 use deno_core::error::AnyError;
 use deno_core::serde_json;
 use deno_core::serde_json::Value;

diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs
@@ -739,6 +739,8 @@ async fn fetch_no_follow<'a>(
   Ok(FetchOnceResult::Code(body, result_headers))
 }
 
+#[allow(clippy::print_stdout)]
+#[allow(clippy::print_stderr)]
 #[cfg(test)]
 mod tests {
   use crate::cache::GlobalHttpCache;

diff --git a/cli/main.rs b/cli/main.rs
@@ -235,6 +235,7 @@ async fn run_subcommand(flags: Flags) -> Result<i32, AnyError> {
   handle.await?
 }
 
+#[allow(clippy::print_stderr)]
 fn setup_panic_hook() {
   // This function does two things inside of the panic hook:
   // - Tokio does not exit the process when a task panics, so we define a custom
@@ -259,6 +260,7 @@ fn setup_panic_hook() {
   }));
 }
 
+#[allow(clippy::print_stderr)]
 fn exit_with_message(message: &str, code: i32) -> ! {
   eprintln!(
     "{}: {}",
@@ -289,6 +291,7 @@ fn exit_for_error(error: AnyError) -> ! {
   exit_with_message(&error_string, error_code);
 }
 
+#[allow(clippy::print_stderr)]
 pub(crate) fn unstable_exit_cb(feature: &str, api_name: &str) {
   eprintln!(
     "Unstable API '{api_name}'. The `--unstable-{}` flag must be provided.",
@@ -298,6 +301,7 @@ pub(crate) fn unstable_exit_cb(feature: &str, api_name: &str) {
 }
 
 // TODO(bartlomieju): remove when `--unstable` flag is removed.
+#[allow(clippy::print_stderr)]
 pub(crate) fn unstable_warn_cb(feature: &str, api_name: &str) {
   eprintln!(
     "⚠️  {}",
@@ -369,7 +373,9 @@ fn resolve_flags_and_init(
 
   // TODO(bartlomieju): remove when `--unstable` flag is removed.
   if flags.unstable_config.legacy_flag_enabled {
+    #[allow(clippy::print_stderr)]
     if matches!(flags.subcommand, DenoSubcommand::Check(_)) {
+      // can't use log crate because that's not setup yet
       eprintln!(
         "⚠️  {}",
         colors::yellow(

diff --git a/cli/mainrt.rs b/cli/mainrt.rs
@@ -36,6 +36,7 @@ use std::env::current_exe;
 
 use crate::args::Flags;
 
+#[allow(clippy::print_stderr)]
 pub(crate) fn unstable_exit_cb(feature: &str, api_name: &str) {
   eprintln!(
     "Unstable API '{api_name}'. The `--unstable-{}` flag must be provided.",
@@ -44,6 +45,7 @@ pub(crate) fn unstable_exit_cb(feature: &str, api_name: &str) {
   std::process::exit(70);
 }
 
+#[allow(clippy::print_stderr)]
 fn exit_with_message(message: &str, code: i32) -> ! {
   eprintln!(
     "{}: {}",
@@ -57,7 +59,7 @@ fn unwrap_or_exit<T>(result: Result<T, AnyError>) -> T {
   match result {
     Ok(value) => value,
     Err(error) => {
-      let mut error_string = format!("{error:?}");
+      let mut error_string = format!("{:?}", error);
 
       if let Some(e) = error.downcast_ref::<JsError>() {
         error_string = format_js_error(e);

diff --git a/cli/ops/jupyter.rs b/cli/ops/jupyter.rs
@@ -76,13 +76,13 @@ pub fn op_print(
 
   if is_err {
     if let Err(err) = sender.send(StdioMsg::Stderr(msg.into())) {
-      eprintln!("Failed to send stderr message: {}", err);
+      log::error!("Failed to send stderr message: {}", err);
     }
     return Ok(());
   }
 
   if let Err(err) = sender.send(StdioMsg::Stdout(msg.into())) {
-    eprintln!("Failed to send stdout message: {}", err);
+    log::error!("Failed to send stdout message: {}", err);
   }
   Ok(())
 }
diff --git a/cli/tools/bench/reporters.rs b/cli/tools/bench/reporters.rs
@@ -50,6 +50,7 @@ impl JsonReporter {
   }
 }
 
+#[allow(clippy::print_stdout)]
 impl BenchReporter for JsonReporter {
   fn report_group_summary(&mut self) {}
   #[cold]
@@ -58,7 +59,7 @@ impl BenchReporter for JsonReporter {
   fn report_end(&mut self, _report: &BenchReport) {
     match write_json_to_stdout(self) {
       Ok(_) => (),
-      Err(e) => println!("{e}"),
+      Err(e) => println!("{}", e),
     }
   }
 
@@ -118,6 +119,7 @@ impl ConsoleReporter {
   }
 }
 
+#[allow(clippy::print_stdout)]
 impl BenchReporter for ConsoleReporter {
   #[cold]
   fn report_plan(&mut self, plan: &BenchPlan) {

diff --git a/cli/tools/bundle.rs b/cli/tools/bundle.rs
@@ -125,7 +125,10 @@ async fn bundle_action(
       );
     }
   } else {
-    println!("{}", bundle_output.code);
+    #[allow(clippy::print_stdout)]
+    {
+      println!("{}", bundle_output.code);
+    }
   }
   Ok(())
 }

diff --git a/cli/tools/coverage/reporter.rs b/cli/tools/coverage/reporter.rs
@@ -103,6 +103,7 @@ struct SummaryCoverageReporter {
   file_reports: Vec<(CoverageReport, String)>,
 }
 
+#[allow(clippy::print_stdout)]
 impl SummaryCoverageReporter {
   pub fn new() -> SummaryCoverageReporter {
     SummaryCoverageReporter {
@@ -166,6 +167,7 @@ impl SummaryCoverageReporter {
   }
 }
 
+#[allow(clippy::print_stdout)]
 impl CoverageReporter for SummaryCoverageReporter {
   fn report(
     &mut self,
@@ -312,6 +314,7 @@ impl DetailedCoverageReporter {
   }
 }
 
+#[allow(clippy::print_stdout)]
 impl CoverageReporter for DetailedCoverageReporter {
   fn report(
     &mut self,
@@ -416,7 +419,7 @@ impl CoverageReporter for HtmlCoverageReporter {
     )
     .unwrap();
 
-    println!("HTML coverage report has been generated at {}", root_report);
+    log::info!("HTML coverage report has been generated at {}", root_report);
   }
 }
 

diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs
@@ -396,8 +396,8 @@ async fn format_source_files(
         }
         Err(e) => {
           let _g = output_lock.lock();
-          eprintln!("Error formatting: {}", file_path.to_string_lossy());
-          eprintln!("   {e}");
+          log::error!("Error formatting: {}", file_path.to_string_lossy());
+          log::error!("   {e}");
         }
       }
       Ok(())
@@ -495,6 +495,7 @@ fn format_stdin(fmt_options: FmtOptions, ext: &str) -> Result<(), AnyError> {
   let file_path = PathBuf::from(format!("_stdin.{ext}"));
   let formatted_text = format_file(&file_path, &source, &fmt_options.options)?;
   if fmt_options.check {
+    #[allow(clippy::print_stdout)]
     if formatted_text.is_some() {
       println!("Not formatted stdin");
     }

diff --git a/cli/tools/info.rs b/cli/tools/info.rs
@@ -97,6 +97,7 @@ pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> {
   Ok(())
 }
 
+#[allow(clippy::print_stdout)]
 fn print_cache_info(
   factory: &CliFactory,
   json: bool,

diff --git a/cli/tools/jupyter/install.rs b/cli/tools/jupyter/install.rs
@@ -27,13 +27,13 @@ pub fn status() -> Result<(), AnyError> {
   if let Some(specs) = json_output.get("kernelspecs") {
     if let Some(specs_obj) = specs.as_object() {
       if specs_obj.contains_key("deno") {
-        println!("✅ Deno kernel already installed");
+        log::info!("✅ Deno kernel already installed");
         return Ok(());
       }
     }
   }
 
-  println!("ℹ️ Deno kernel is not yet installed, run `deno jupyter --install` to set it up");
+  log::warn!("ℹ️ Deno kernel is not yet installed, run `deno jupyter --install` to set it up");
   Ok(())
 }
 
@@ -108,6 +108,6 @@ pub fn install() -> Result<(), AnyError> {
   }
 
   let _ = std::fs::remove_dir(temp_dir);
-  println!("✅ Deno kernelspec installed successfully.");
+  log::info!("✅ Deno kernelspec installed successfully.");
   Ok(())
 }