From 924e35b1c35b83c9dc0243091d1807ad8c9550e8 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 3 Feb 2023 11:13:44 -0500 Subject: [PATCH] Add `print_stdout` and `print_stderr` to Clippy enforcement (#2542) --- .cargo/config.toml | 6 +- flake8_to_ruff/src/main.rs | 6 +- ruff_cli/src/commands.rs | 45 ++++++++++----- ruff_cli/src/commands/linter.rs | 11 +++- ruff_cli/src/diagnostics.rs | 34 +++++++----- ruff_cli/src/main.rs | 21 +++++-- ruff_cli/src/updates.rs | 5 +- ruff_dev/src/generate_cli_help.rs | 1 + ruff_dev/src/generate_json_schema.rs | 2 + ruff_dev/src/generate_options.rs | 1 + ruff_dev/src/generate_rules_table.rs | 1 + ruff_dev/src/print_ast.rs | 1 + ruff_dev/src/print_cst.rs | 1 + ruff_dev/src/print_tokens.rs | 1 + ruff_dev/src/round_trip.rs | 1 + src/flake8_to_ruff/converter.rs | 10 +++- src/linter.rs | 55 +++++++++++-------- .../rules/invalid_literal_comparisons.rs | 17 +++--- 18 files changed, 145 insertions(+), 74 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 3f21162803..31fb7bc8ca 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -9,9 +9,9 @@ rustflags = [ # `https://github.com/EmbarkStudios/rust-ecosystem/issues/22#issuecomment-947011395` "-Dunsafe_code", "-Wclippy::pedantic", - # "-Wclippy::print_stdout", - # "-Wclippy::print_stderr", - # "-Wclippy::dbg_macro", + "-Wclippy::print_stdout", + "-Wclippy::print_stderr", + "-Wclippy::dbg_macro", "-Wclippy::char_lit_as_u8", "-Aclippy::collapsible_else_if", "-Aclippy::collapsible_if", diff --git a/flake8_to_ruff/src/main.rs b/flake8_to_ruff/src/main.rs index c50784d6e5..6f99cb250a 100644 --- a/flake8_to_ruff/src/main.rs +++ b/flake8_to_ruff/src/main.rs @@ -47,7 +47,11 @@ fn main() -> Result<()> { // Create Ruff's pyproject.toml section. let pyproject = flake8_to_ruff::convert(&config, &external_config, args.plugin)?; - println!("{}", toml::to_string_pretty(&pyproject)?); + + #[allow(clippy::print_stdout)] + { + println!("{}", toml::to_string_pretty(&pyproject)?); + } Ok(()) } diff --git a/ruff_cli/src/commands.rs b/ruff_cli/src/commands.rs index 23e0039723..d69d1f8072 100644 --- a/ruff_cli/src/commands.rs +++ b/ruff_cli/src/commands.rs @@ -1,5 +1,6 @@ use std::fs::remove_dir_all; -use std::io::{self, Read}; +use std::io::Write; +use std::io::{self, BufWriter, Read}; use std::path::{Path, PathBuf}; use std::time::Instant; @@ -11,6 +12,9 @@ use log::{debug, error}; use path_absolutize::path_dedot; #[cfg(not(target_family = "wasm"))] use rayon::prelude::*; +use serde::Serialize; +use walkdir::WalkDir; + use ruff::cache::CACHE_DIR_NAME; use ruff::linter::add_noqa_to_path; use ruff::logging::LogLevel; @@ -19,8 +23,6 @@ use ruff::registry::{Linter, Rule, RuleNamespace}; use ruff::resolver::PyprojectDiscovery; use ruff::settings::flags; use ruff::{fix, fs, packaging, resolver, warn_user_once, AutofixAvailability, IOError}; -use serde::Serialize; -use walkdir::WalkDir; use crate::args::{HelpFormat, Overrides}; use crate::cache; @@ -230,8 +232,10 @@ pub fn show_settings( }; let path = entry.path(); let settings = resolver.resolve(path, pyproject_strategy); - println!("Resolved settings for: {path:?}"); - println!("{settings:#?}"); + + let mut stdout = BufWriter::new(io::stdout().lock()); + write!(stdout, "Resolved settings for: {path:?}")?; + write!(stdout, "{settings:#?}")?; Ok(()) } @@ -251,12 +255,13 @@ pub fn show_files( } // Print the list of files. + let mut stdout = BufWriter::new(io::stdout().lock()); for entry in paths .iter() .flatten() .sorted_by(|a, b| a.path().cmp(b.path())) { - println!("{}", entry.path().to_string_lossy()); + writeln!(stdout, "{}", entry.path().to_string_lossy())?; } Ok(()) @@ -272,34 +277,39 @@ struct Explanation<'a> { /// Explain a `Rule` to the user. pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> { let (linter, _) = Linter::parse_code(rule.code()).unwrap(); + let mut stdout = BufWriter::new(io::stdout().lock()); match format { HelpFormat::Text => { - println!("{}\n", rule.as_ref()); - println!("Code: {} ({})\n", rule.code(), linter.name()); + writeln!(stdout, "{}\n", rule.as_ref())?; + writeln!(stdout, "Code: {} ({})\n", rule.code(), linter.name())?; if let Some(autofix) = rule.autofixable() { - println!( + writeln!( + stdout, "{}", match autofix.available { AutofixAvailability::Sometimes => "Autofix is sometimes available.\n", AutofixAvailability::Always => "Autofix is always available.\n", } - ); + )?; } - println!("Message formats:\n"); + + writeln!(stdout, "Message formats:\n")?; + for format in rule.message_formats() { - println!("* {format}"); + writeln!(stdout, "* {format}")?; } } HelpFormat::Json => { - println!( + writeln!( + stdout, "{}", serde_json::to_string_pretty(&Explanation { code: rule.code(), linter: linter.name(), summary: rule.message_formats()[0], })? - ); + )?; } }; Ok(()) @@ -307,6 +317,7 @@ pub fn rule(rule: &Rule, format: HelpFormat) -> Result<()> { /// Clear any caches in the current directory or any subdirectories. pub fn clean(level: LogLevel) -> Result<()> { + let mut stderr = BufWriter::new(io::stderr().lock()); for entry in WalkDir::new(&*path_dedot::CWD) .into_iter() .filter_map(Result::ok) @@ -315,7 +326,11 @@ pub fn clean(level: LogLevel) -> Result<()> { let cache = entry.path().join(CACHE_DIR_NAME); if cache.is_dir() { if level >= LogLevel::Default { - eprintln!("Removing cache at: {}", fs::relativize_path(&cache).bold()); + writeln!( + stderr, + "Removing cache at: {}", + fs::relativize_path(&cache).bold() + )?; } remove_dir_all(&cache)?; } diff --git a/ruff_cli/src/commands/linter.rs b/ruff_cli/src/commands/linter.rs index 2620912f81..a941a1e759 100644 --- a/ruff_cli/src/commands/linter.rs +++ b/ruff_cli/src/commands/linter.rs @@ -19,7 +19,11 @@ pub fn linter(format: HelpFormat) { .join("/"), prefix => prefix.to_string(), }; - println!("{:>4} {}", prefix, linter.name()); + + #[allow(clippy::print_stdout)] + { + println!("{:>4} {}", prefix, linter.name()); + } } } @@ -39,7 +43,10 @@ pub fn linter(format: HelpFormat) { }) .collect(); - println!("{}", serde_json::to_string_pretty(&linters).unwrap()); + #[allow(clippy::print_stdout)] + { + println!("{}", serde_json::to_string_pretty(&linters).unwrap()); + } } } } diff --git a/ruff_cli/src/diagnostics.rs b/ruff_cli/src/diagnostics.rs index 616f262c6e..cc1c32c6a6 100644 --- a/ruff_cli/src/diagnostics.rs +++ b/ruff_cli/src/diagnostics.rs @@ -105,14 +105,17 @@ pub fn lint_path( if let Some(err) = parse_error { // Notify the user of any parse errors. - eprintln!( - "{}{} {}{}{} {err}", - "error".red().bold(), - ":".bold(), - "Failed to parse ".bold(), - fs::relativize_path(path).bold(), - ":".bold() - ); + #[allow(clippy::print_stderr)] + { + eprintln!( + "{}{} {}{}{} {err}", + "error".red().bold(), + ":".bold(), + "Failed to parse ".bold(), + fs::relativize_path(path).bold(), + ":".bold() + ); + } // Purge the cache. cache::del(path, package.as_ref(), settings, autofix.into()); @@ -207,12 +210,15 @@ pub fn lint_stdin( }; if let Some(err) = parse_error { - eprintln!( - "{}{} Failed to parse {}: {err}", - "error".red().bold(), - ":".bold(), - path.map_or_else(|| "-".into(), fs::relativize_path).bold() - ); + #[allow(clippy::print_stderr)] + { + eprintln!( + "{}{} Failed to parse {}: {err}", + "error".red().bold(), + ":".bold(), + path.map_or_else(|| "-".into(), fs::relativize_path).bold() + ); + } } Ok(Diagnostics { messages, fixed }) diff --git a/ruff_cli/src/main.rs b/ruff_cli/src/main.rs index 4b156e71db..597728ccd1 100644 --- a/ruff_cli/src/main.rs +++ b/ruff_cli/src/main.rs @@ -53,16 +53,19 @@ fn inner_main() -> Result { { let default_panic_hook = std::panic::take_hook(); std::panic::set_hook(Box::new(move |info| { - eprintln!( - r#" + #[allow(clippy::print_stderr)] + { + eprintln!( + r#" {}: `ruff` crashed. This indicates a bug in `ruff`. If you could open an issue at: https://github.com/charliermarsh/ruff/issues/new?title=%5BPanic%5D quoting the executed command, along with the relevant file contents and `pyproject.toml` settings, we'd be very appreciative! "#, - "error".red().bold(), - ); + "error".red().bold(), + ); + } default_panic_hook(info); })); } @@ -157,7 +160,10 @@ fn check(args: CheckArgs, log_level: LogLevel) -> Result { } let modifications = commands::add_noqa(&cli.files, &pyproject_strategy, &overrides)?; if modifications > 0 && log_level >= LogLevel::Default { - eprintln!("Added {modifications} noqa directives."); + #[allow(clippy::print_stderr)] + { + eprintln!("Added {modifications} noqa directives."); + } } return Ok(ExitCode::SUCCESS); } @@ -287,7 +293,10 @@ pub fn main() -> ExitCode { match inner_main() { Ok(code) => code, Err(err) => { - eprintln!("{}{} {err:?}", "error".red().bold(), ":".bold()); + #[allow(clippy::print_stderr)] + { + eprintln!("{}{} {err:?}", "error".red().bold(), ":".bold()); + } ExitCode::FAILURE } } diff --git a/ruff_cli/src/updates.rs b/ruff_cli/src/updates.rs index 41fa51f46e..621e77e31f 100644 --- a/ruff_cli/src/updates.rs +++ b/ruff_cli/src/updates.rs @@ -68,7 +68,10 @@ pub fn check_for_updates() -> Result<()> { pkg_name = CARGO_PKG_NAME.green() ); - println!("\n{msg}\n{cmd}"); + #[allow(clippy::print_stdout)] + { + println!("\n{msg}\n{cmd}"); + } } Ok(()) diff --git a/ruff_dev/src/generate_cli_help.rs b/ruff_dev/src/generate_cli_help.rs index 704cde435e..df79cd0c16 100644 --- a/ruff_dev/src/generate_cli_help.rs +++ b/ruff_dev/src/generate_cli_help.rs @@ -1,4 +1,5 @@ //! Generate CLI help. +#![allow(clippy::print_stdout, clippy::print_stderr)] use crate::utils::replace_readme_section; use anyhow::Result; diff --git a/ruff_dev/src/generate_json_schema.rs b/ruff_dev/src/generate_json_schema.rs index ceeeedb792..6f5490d8e3 100644 --- a/ruff_dev/src/generate_json_schema.rs +++ b/ruff_dev/src/generate_json_schema.rs @@ -1,3 +1,5 @@ +#![allow(clippy::print_stdout, clippy::print_stderr)] + use std::fs; use std::path::PathBuf; diff --git a/ruff_dev/src/generate_options.rs b/ruff_dev/src/generate_options.rs index 7576b096b3..3e641ff90c 100644 --- a/ruff_dev/src/generate_options.rs +++ b/ruff_dev/src/generate_options.rs @@ -1,4 +1,5 @@ //! Generate a Markdown-compatible listing of configuration options. +#![allow(clippy::print_stdout, clippy::print_stderr)] use anyhow::Result; use itertools::Itertools; diff --git a/ruff_dev/src/generate_rules_table.rs b/ruff_dev/src/generate_rules_table.rs index df1277086c..bce90da1ea 100644 --- a/ruff_dev/src/generate_rules_table.rs +++ b/ruff_dev/src/generate_rules_table.rs @@ -1,4 +1,5 @@ //! Generate a Markdown-compatible table of supported lint rules. +#![allow(clippy::print_stdout, clippy::print_stderr)] use anyhow::Result; use itertools::Itertools; diff --git a/ruff_dev/src/print_ast.rs b/ruff_dev/src/print_ast.rs index 7d0e0d4769..bd32ed687c 100644 --- a/ruff_dev/src/print_ast.rs +++ b/ruff_dev/src/print_ast.rs @@ -1,4 +1,5 @@ //! Print the AST for a given Python file. +#![allow(clippy::print_stdout, clippy::print_stderr)] use std::fs; use std::path::PathBuf; diff --git a/ruff_dev/src/print_cst.rs b/ruff_dev/src/print_cst.rs index 943d9dc6df..671227377f 100644 --- a/ruff_dev/src/print_cst.rs +++ b/ruff_dev/src/print_cst.rs @@ -1,4 +1,5 @@ //! Print the `LibCST` CST for a given Python file. +#![allow(clippy::print_stdout, clippy::print_stderr)] use std::fs; use std::path::PathBuf; diff --git a/ruff_dev/src/print_tokens.rs b/ruff_dev/src/print_tokens.rs index 1b3d2f2135..3c730cedf8 100644 --- a/ruff_dev/src/print_tokens.rs +++ b/ruff_dev/src/print_tokens.rs @@ -1,4 +1,5 @@ //! Print the token stream for a given Python file. +#![allow(clippy::print_stdout, clippy::print_stderr)] use std::fs; use std::path::PathBuf; diff --git a/ruff_dev/src/round_trip.rs b/ruff_dev/src/round_trip.rs index f196a368d0..632b47392c 100644 --- a/ruff_dev/src/round_trip.rs +++ b/ruff_dev/src/round_trip.rs @@ -1,4 +1,5 @@ //! Run round-trip source code generation on a given Python file. +#![allow(clippy::print_stdout, clippy::print_stderr)] use std::fs; use std::path::PathBuf; diff --git a/src/flake8_to_ruff/converter.rs b/src/flake8_to_ruff/converter.rs index a6d4cb47d7..34f96ab51f 100644 --- a/src/flake8_to_ruff/converter.rs +++ b/src/flake8_to_ruff/converter.rs @@ -65,11 +65,17 @@ pub fn convert( let plugins = plugins.unwrap_or_else(|| { let from_options = plugin::infer_plugins_from_options(flake8); if !from_options.is_empty() { - eprintln!("Inferred plugins from settings: {from_options:#?}"); + #[allow(clippy::print_stderr)] + { + eprintln!("Inferred plugins from settings: {from_options:#?}"); + } } let from_codes = plugin::infer_plugins_from_codes(&referenced_codes); if !from_codes.is_empty() { - eprintln!("Inferred plugins from referenced codes: {from_codes:#?}"); + #[allow(clippy::print_stderr)] + { + eprintln!("Inferred plugins from referenced codes: {from_codes:#?}"); + } } from_options.into_iter().chain(from_codes).collect() }); diff --git a/src/linter.rs b/src/linter.rs index e7f0066bd4..d43c865b56 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -236,14 +236,17 @@ pub fn add_noqa_to_path(path: &Path, settings: &Settings) -> Result { // Log any parse errors. if let Some(err) = error { - eprintln!( - "{}{} {}{}{} {err:?}", - "error".red().bold(), - ":".bold(), - "Failed to parse ".bold(), - fs::relativize_path(path).bold(), - ":".bold() - ); + #[allow(clippy::print_stderr)] + { + eprintln!( + "{}{} {}{}{} {err:?}", + "error".red().bold(), + ":".bold(), + "Failed to parse ".bold(), + fs::relativize_path(path).bold(), + ":".bold() + ); + } } // Add any missing `# noqa` pragmas. @@ -372,8 +375,10 @@ pub fn lint_fix<'a>( // longer parseable on a subsequent pass, then we've introduced a // syntax error. Return the original code. if parseable && result.error.is_some() { - eprintln!( - r#" + #[allow(clippy::print_stderr)] + { + eprintln!( + r#" {}: Autofix introduced a syntax error. Reverting all changes. This indicates a bug in `{}`. If you could open an issue at: @@ -382,11 +387,12 @@ This indicates a bug in `{}`. If you could open an issue at: ...quoting the contents of `{}`, along with the `pyproject.toml` settings and executed command, we'd be very appreciative! "#, - "error".red().bold(), - CARGO_PKG_NAME, - CARGO_PKG_REPOSITORY, - fs::relativize_path(path), - ); + "error".red().bold(), + CARGO_PKG_NAME, + CARGO_PKG_REPOSITORY, + fs::relativize_path(path), + ); + } return Err(anyhow!("Autofix introduced a syntax error")); } } @@ -407,8 +413,10 @@ This indicates a bug in `{}`. If you could open an issue at: continue; } - eprintln!( - r#" + #[allow(clippy::print_stderr)] + { + eprintln!( + r#" {}: Failed to converge after {} iterations. This indicates a bug in `{}`. If you could open an issue at: @@ -417,12 +425,13 @@ This indicates a bug in `{}`. If you could open an issue at: ...quoting the contents of `{}`, along with the `pyproject.toml` settings and executed command, we'd be very appreciative! "#, - "error".red().bold(), - MAX_ITERATIONS, - CARGO_PKG_NAME, - CARGO_PKG_REPOSITORY, - fs::relativize_path(path), - ); + "error".red().bold(), + MAX_ITERATIONS, + CARGO_PKG_NAME, + CARGO_PKG_REPOSITORY, + fs::relativize_path(path), + ); + } } // Convert to messages. diff --git a/src/rules/pyflakes/rules/invalid_literal_comparisons.rs b/src/rules/pyflakes/rules/invalid_literal_comparisons.rs index add31f1190..02b069c7ac 100644 --- a/src/rules/pyflakes/rules/invalid_literal_comparisons.rs +++ b/src/rules/pyflakes/rules/invalid_literal_comparisons.rs @@ -1,3 +1,11 @@ +use itertools::izip; +use log::error; +use once_cell::unsync::Lazy; +use rustpython_ast::{Cmpop, Expr}; +use serde::{Deserialize, Serialize}; + +use ruff_macros::derive_message_formats; + use crate::ast::helpers; use crate::ast::operations::locate_cmpops; use crate::ast::types::Range; @@ -6,11 +14,6 @@ use crate::define_violation; use crate::fix::Fix; use crate::registry::Diagnostic; use crate::violation::AlwaysAutofixableViolation; -use itertools::izip; -use once_cell::unsync::Lazy; -use ruff_macros::derive_message_formats; -use rustpython_ast::{Cmpop, Expr}; -use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum IsCmpop { @@ -75,7 +78,7 @@ pub fn invalid_literal_comparison( Cmpop::Is => Some("==".to_string()), Cmpop::IsNot => Some("!=".to_string()), node => { - eprintln!("Failed to fix invalid comparison: {node:?}"); + error!("Failed to fix invalid comparison: {node:?}"); None } } { @@ -89,7 +92,7 @@ pub fn invalid_literal_comparison( )); } } else { - eprintln!("Failed to fix invalid comparison due to missing op"); + error!("Failed to fix invalid comparison due to missing op"); } } checker.diagnostics.push(diagnostic);