Skip to content

Commit

Permalink
chore: enable clippy::print_stdout and clippy::print_stderr (denoland…
Browse files Browse the repository at this point in the history
…#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.
  • Loading branch information
dsherret committed May 9, 2024
1 parent e6dc4df commit 47f7bed
Show file tree
Hide file tree
Showing 52 changed files with 308 additions and 182 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions bench_util/profiling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
18 changes: 14 additions & 4 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(
"⚠️ {}",
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
9 changes: 6 additions & 3 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
3 changes: 3 additions & 0 deletions cli/bench/main.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
2 changes: 2 additions & 0 deletions cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -259,6 +260,7 @@ fn setup_panic_hook() {
}));
}

#[allow(clippy::print_stderr)]
fn exit_with_message(message: &str, code: i32) -> ! {
eprintln!(
"{}: {}",
Expand Down Expand Up @@ -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.",
Expand All @@ -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!(
"⚠️ {}",
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 3 additions & 1 deletion cli/mainrt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand All @@ -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!(
"{}: {}",
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions cli/ops/jupyter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
4 changes: 3 additions & 1 deletion cli/tools/bench/reporters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ impl JsonReporter {
}
}

#[allow(clippy::print_stdout)]
impl BenchReporter for JsonReporter {
fn report_group_summary(&mut self) {}
#[cold]
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -118,6 +119,7 @@ impl ConsoleReporter {
}
}

#[allow(clippy::print_stdout)]
impl BenchReporter for ConsoleReporter {
#[cold]
fn report_plan(&mut self, plan: &BenchPlan) {
Expand Down
5 changes: 4 additions & 1 deletion cli/tools/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,10 @@ async fn bundle_action(
);
}
} else {
println!("{}", bundle_output.code);
#[allow(clippy::print_stdout)]
{
println!("{}", bundle_output.code);
}
}
Ok(())
}
Expand Down
5 changes: 4 additions & 1 deletion cli/tools/coverage/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ struct SummaryCoverageReporter {
file_reports: Vec<(CoverageReport, String)>,
}

#[allow(clippy::print_stdout)]
impl SummaryCoverageReporter {
pub fn new() -> SummaryCoverageReporter {
SummaryCoverageReporter {
Expand Down Expand Up @@ -166,6 +167,7 @@ impl SummaryCoverageReporter {
}
}

#[allow(clippy::print_stdout)]
impl CoverageReporter for SummaryCoverageReporter {
fn report(
&mut self,
Expand Down Expand Up @@ -312,6 +314,7 @@ impl DetailedCoverageReporter {
}
}

#[allow(clippy::print_stdout)]
impl CoverageReporter for DetailedCoverageReporter {
fn report(
&mut self,
Expand Down Expand Up @@ -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);
}
}

Expand Down
5 changes: 3 additions & 2 deletions cli/tools/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down Expand Up @@ -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");
}
Expand Down
1 change: 1 addition & 0 deletions cli/tools/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions cli/tools/jupyter/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down Expand Up @@ -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(())
}
Loading

0 comments on commit 47f7bed

Please sign in to comment.