From 78aafb4b34a3f269f58802488e34b483affbe4c8 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 7 Oct 2022 17:36:17 -0400 Subject: [PATCH] Warn the user if an explicitly selected check code is ignored (#356) --- src/cli.rs | 136 ++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 3 +- src/main.rs | 104 ++++++++++-------------------------- src/settings.rs | 32 ++++++------ 4 files changed, 181 insertions(+), 94 deletions(-) create mode 100644 src/cli.rs diff --git a/src/cli.rs b/src/cli.rs new file mode 100644 index 0000000000..7e71aa05b1 --- /dev/null +++ b/src/cli.rs @@ -0,0 +1,136 @@ +use std::fmt; +use std::path::PathBuf; + +use clap::{command, Parser}; +use log::warn; +use regex::Regex; + +use crate::checks::CheckCode; +use crate::printer::SerializationFormat; +use crate::pyproject::StrCheckCodePair; +use crate::settings::PythonVersion; +use crate::RawSettings; + +#[derive(Debug, Parser)] +#[command(author, about = "ruff: An extremely fast Python linter.")] +#[command(version)] +pub struct Cli { + #[arg(required = true)] + pub files: Vec, + /// Enable verbose logging. + #[arg(short, long)] + pub verbose: bool, + /// Disable all logging (but still exit with status code "1" upon detecting errors). + #[arg(short, long)] + pub quiet: bool, + /// Exit with status code "0", even upon detecting errors. + #[arg(short, long)] + pub exit_zero: bool, + /// Run in watch mode by re-running whenever files change. + #[arg(short, long)] + pub watch: bool, + /// Attempt to automatically fix lint errors. + #[arg(short, long)] + pub fix: bool, + /// Disable cache reads. + #[arg(short, long)] + pub no_cache: bool, + /// List of error codes to enable. + #[arg(long, value_delimiter = ',')] + pub select: Vec, + /// Like --select, but adds additional error codes on top of the selected ones. + #[arg(long, value_delimiter = ',')] + pub extend_select: Vec, + /// List of error codes to ignore. + #[arg(long, value_delimiter = ',')] + pub ignore: Vec, + /// Like --ignore, but adds additional error codes on top of the ignored ones. + #[arg(long, value_delimiter = ',')] + pub extend_ignore: Vec, + /// List of paths, used to exclude files and/or directories from checks. + #[arg(long, value_delimiter = ',')] + pub exclude: Vec, + /// Like --exclude, but adds additional files and directories on top of the excluded ones. + #[arg(long, value_delimiter = ',')] + pub extend_exclude: Vec, + /// List of mappings from file pattern to code to exclude + #[arg(long, value_delimiter = ',')] + pub per_file_ignores: Vec, + /// Output serialization format for error messages. + #[arg(long, value_enum, default_value_t=SerializationFormat::Text)] + pub format: SerializationFormat, + /// See the files ruff will be run against with the current settings. + #[arg(long)] + pub show_files: bool, + /// See ruff's settings. + #[arg(long)] + pub show_settings: bool, + /// Enable automatic additions of noqa directives to failing lines. + #[arg(long)] + pub add_noqa: bool, + /// Regular expression matching the name of dummy variables. + #[arg(long)] + pub dummy_variable_rgx: Option, + /// The minimum Python version that should be supported. + #[arg(long)] + pub target_version: Option, + /// Round-trip auto-formatting. + // TODO(charlie): This should be a sub-command. + #[arg(long, hide = true)] + pub autoformat: bool, +} + +pub enum Warnable { + Select, + ExtendSelect, +} + +impl fmt::Display for Warnable { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + match self { + Warnable::Select => fmt.write_str("--select"), + Warnable::ExtendSelect => fmt.write_str("--extend-select"), + } + } +} + +/// Warn the user if they attempt to enable a code that won't be respected. +pub fn warn_on( + flag: Warnable, + codes: &Vec, + cli_ignore: &Vec, + cli_extend_ignore: &Vec, + pyproject_settings: &RawSettings, + pyproject_path: &Option, +) { + for code in codes { + if !cli_ignore.is_empty() { + if cli_ignore.contains(code) { + warn!("{code:?} was passed to {flag}, but ignored via --ignore") + } + } else if pyproject_settings.ignore.contains(code) { + if let Some(path) = pyproject_path { + warn!( + "{code:?} was passed to {flag}, but ignored by the `ignore` field in {}", + path.to_string_lossy() + ) + } else { + warn!("{code:?} was passed to {flag}, but ignored by the default `ignore` field",) + } + } + if !cli_extend_ignore.is_empty() { + if cli_extend_ignore.contains(code) { + warn!("{code:?} was passed to {flag}, but ignored via --extend-ignore") + } + } else if pyproject_settings.extend_ignore.contains(code) { + if let Some(path) = pyproject_path { + warn!( + "{code:?} was passed to {flag}, but ignored by the `extend_ignore` field in {}", + path.to_string_lossy() + ) + } else { + warn!("{code:?} was passed to {flag}, but ignored by the default `extend_ignore` field") + } + } + } +} diff --git a/src/lib.rs b/src/lib.rs index 82932f8de8..4beb99d634 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,6 +15,7 @@ pub mod cache; pub mod check_ast; mod check_lines; pub mod checks; +pub mod cli; pub mod code_gen; pub mod fs; pub mod linter; @@ -41,7 +42,7 @@ pub fn check(path: &Path, contents: &str) -> Result> { None => debug!("Unable to find pyproject.toml; using default settings..."), }; - let settings = Settings::from_raw(RawSettings::from_pyproject(pyproject, project_root)?); + let settings = Settings::from_raw(RawSettings::from_pyproject(&pyproject, &project_root)?); // Tokenize once. let tokens: Vec = tokenize(contents); diff --git a/src/main.rs b/src/main.rs index 8ef7a1a7a9..25cdb1f78a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,17 +5,17 @@ use std::sync::mpsc::channel; use std::time::Instant; use anyhow::Result; -use clap::{command, Parser}; +use clap::Parser; use colored::Colorize; use log::{debug, error}; use notify::{raw_watcher, RecursiveMode, Watcher}; use rayon::prelude::*; -use regex::Regex; use walkdir::DirEntry; use ruff::cache; use ruff::checks::CheckCode; use ruff::checks::CheckKind; +use ruff::cli::{warn_on, Cli, Warnable}; use ruff::fs::iter_python_files; use ruff::linter::add_noqa_to_path; use ruff::linter::autoformat_path; @@ -23,84 +23,15 @@ use ruff::linter::lint_path; use ruff::logging::set_up_logging; use ruff::message::Message; use ruff::printer::{Printer, SerializationFormat}; -use ruff::pyproject::{self, StrCheckCodePair}; +use ruff::pyproject::{self}; +use ruff::settings::CurrentSettings; use ruff::settings::RawSettings; -use ruff::settings::{CurrentSettings, PythonVersion}; use ruff::settings::{FilePattern, PerFileIgnore, Settings}; use ruff::tell_user; const CARGO_PKG_NAME: &str = env!("CARGO_PKG_NAME"); const CARGO_PKG_VERSION: &str = env!("CARGO_PKG_VERSION"); -#[derive(Debug, Parser)] -#[command(author, about = "ruff: An extremely fast Python linter.")] -#[command(version)] -struct Cli { - #[arg(required = true)] - files: Vec, - /// Enable verbose logging. - #[arg(short, long)] - verbose: bool, - /// Disable all logging (but still exit with status code "1" upon detecting errors). - #[arg(short, long)] - quiet: bool, - /// Exit with status code "0", even upon detecting errors. - #[arg(short, long)] - exit_zero: bool, - /// Run in watch mode by re-running whenever files change. - #[arg(short, long)] - watch: bool, - /// Attempt to automatically fix lint errors. - #[arg(short, long)] - fix: bool, - /// Disable cache reads. - #[arg(short, long)] - no_cache: bool, - /// List of error codes to enable. - #[arg(long, value_delimiter = ',')] - select: Vec, - /// Like --select, but adds additional error codes on top of the selected ones. - #[arg(long, value_delimiter = ',')] - extend_select: Vec, - /// List of error codes to ignore. - #[arg(long, value_delimiter = ',')] - ignore: Vec, - /// Like --ignore, but adds additional error codes on top of the ignored ones. - #[arg(long, value_delimiter = ',')] - extend_ignore: Vec, - /// List of paths, used to exclude files and/or directories from checks. - #[arg(long, value_delimiter = ',')] - exclude: Vec, - /// Like --exclude, but adds additional files and directories on top of the excluded ones. - #[arg(long, value_delimiter = ',')] - extend_exclude: Vec, - /// List of mappings from file pattern to code to exclude - #[arg(long, value_delimiter = ',')] - per_file_ignores: Vec, - /// Output serialization format for error messages. - #[arg(long, value_enum, default_value_t=SerializationFormat::Text)] - format: SerializationFormat, - /// See the files ruff will be run against with the current settings. - #[arg(long)] - show_files: bool, - /// See ruff's settings. - #[arg(long)] - show_settings: bool, - /// Enable automatic additions of noqa directives to failing lines. - #[arg(long)] - add_noqa: bool, - /// Regular expression matching the name of dummy variables. - #[arg(long)] - dummy_variable_rgx: Option, - /// The minimum Python version that should be supported. - #[arg(long)] - target_version: Option, - /// Round-trip auto-formatting. - // TODO(charlie): This should be a sub-command. - #[arg(long, hide = true)] - autoformat: bool, -} - #[cfg(feature = "update-informer")] fn check_for_updates() { use update_informer::{registry, Check}; @@ -125,8 +56,11 @@ fn check_for_updates() { } } -fn show_settings(settings: RawSettings) { - println!("{:#?}", CurrentSettings::from_settings(settings)); +fn show_settings(settings: RawSettings, project_root: Option, pyproject: Option) { + println!( + "{:#?}", + CurrentSettings::from_settings(settings, project_root, pyproject) + ); } fn show_files(files: &[PathBuf], settings: &Settings) { @@ -292,7 +226,7 @@ fn inner_main() -> Result { .map(|pair| PerFileIgnore::new(pair, &project_root)) .collect(); - let mut settings = RawSettings::from_pyproject(pyproject, project_root)?; + let mut settings = RawSettings::from_pyproject(&pyproject, &project_root)?; if !exclude.is_empty() { settings.exclude = exclude; } @@ -303,9 +237,25 @@ fn inner_main() -> Result { settings.per_file_ignores = per_file_ignores; } if !cli.select.is_empty() { + warn_on( + Warnable::Select, + &cli.select, + &cli.ignore, + &cli.extend_ignore, + &settings, + &pyproject, + ); settings.select = cli.select; } if !cli.extend_select.is_empty() { + warn_on( + Warnable::ExtendSelect, + &cli.extend_select, + &cli.ignore, + &cli.extend_ignore, + &settings, + &pyproject, + ); settings.extend_select = cli.extend_select; } if !cli.ignore.is_empty() { @@ -326,7 +276,7 @@ fn inner_main() -> Result { return Ok(ExitCode::FAILURE); } if cli.show_settings { - show_settings(settings); + show_settings(settings, project_root, pyproject); return Ok(ExitCode::SUCCESS); } diff --git a/src/settings.rs b/src/settings.rs index 6f6b0c8e35..5d768e8860 100644 --- a/src/settings.rs +++ b/src/settings.rs @@ -1,4 +1,3 @@ -use serde::{Deserialize, Serialize}; use std::collections::BTreeSet; use std::hash::{Hash, Hasher}; use std::path::{Path, PathBuf}; @@ -8,6 +7,7 @@ use anyhow::{anyhow, Result}; use glob::Pattern; use once_cell::sync::Lazy; use regex::Regex; +use serde::{Deserialize, Serialize}; use crate::checks::{CheckCode, DEFAULT_CHECK_CODES}; use crate::fs; @@ -94,8 +94,6 @@ pub struct RawSettings { pub ignore: Vec, pub line_length: usize, pub per_file_ignores: Vec, - pub project_root: Option, - pub pyproject: Option, pub select: Vec, pub target_version: PythonVersion, } @@ -129,10 +127,10 @@ static DEFAULT_DUMMY_VARIABLE_RGX: Lazy = impl RawSettings { pub fn from_pyproject( - pyproject: Option, - project_root: Option, + pyproject: &Option, + project_root: &Option, ) -> Result { - let config = load_config(&pyproject)?; + let config = load_config(pyproject)?; Ok(RawSettings { dummy_variable_rgx: match config.dummy_variable_rgx { Some(pattern) => Regex::new(&pattern) @@ -145,14 +143,14 @@ impl RawSettings { .map(|paths| { paths .iter() - .map(|path| FilePattern::from_user(path, &project_root)) + .map(|path| FilePattern::from_user(path, project_root)) .collect() }) .unwrap_or_else(|| DEFAULT_EXCLUDE.clone()), extend_exclude: config .extend_exclude .iter() - .map(|path| FilePattern::from_user(path, &project_root)) + .map(|path| FilePattern::from_user(path, project_root)) .collect(), extend_ignore: config.extend_ignore, select: config @@ -164,10 +162,8 @@ impl RawSettings { per_file_ignores: config .per_file_ignores .into_iter() - .map(|pair| PerFileIgnore::new(pair, &project_root)) + .map(|pair| PerFileIgnore::new(pair, project_root)) .collect(), - project_root, - pyproject, }) } } @@ -278,14 +274,18 @@ pub struct CurrentSettings { pub ignore: Vec, pub line_length: usize, pub per_file_ignores: Vec, - pub project_root: Option, - pub pyproject: Option, pub select: Vec, pub target_version: PythonVersion, + pub project_root: Option, + pub pyproject: Option, } impl CurrentSettings { - pub fn from_settings(settings: RawSettings) -> Self { + pub fn from_settings( + settings: RawSettings, + project_root: Option, + pyproject: Option, + ) -> Self { Self { dummy_variable_rgx: settings.dummy_variable_rgx, exclude: settings @@ -303,10 +303,10 @@ impl CurrentSettings { ignore: settings.ignore, line_length: settings.line_length, per_file_ignores: settings.per_file_ignores, - project_root: settings.project_root, - pyproject: settings.pyproject, select: settings.select, target_version: settings.target_version, + project_root, + pyproject, } } }