Add a `--fix-only` command-line and `pyproject.toml` option (#1375)

This commit is contained in:
Charlie Marsh 2022-12-25 18:49:56 -05:00 committed by GitHub
parent ec80d1cd85
commit d9355c989a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 134 additions and 31 deletions

View File

@ -1798,6 +1798,23 @@ fix = true
--- ---
#### [`fix-only`](#fix-only)
Like `fix`, but disables reporting on leftover violation. Implies `fix`.
**Default value**: `false`
**Type**: `bool`
**Example usage**:
```toml
[tool.ruff]
fix-only = true
```
---
#### [`fixable`](#fixable) #### [`fixable`](#fixable)
A list of check code prefixes to consider autofix-able. A list of check code prefixes to consider autofix-able.

View File

@ -295,6 +295,7 @@ mod tests {
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
format: None, format: None,
force_exclude: None, force_exclude: None,
@ -350,6 +351,7 @@ mod tests {
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
format: None, format: None,
force_exclude: None, force_exclude: None,
@ -405,6 +407,7 @@ mod tests {
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
format: None, format: None,
force_exclude: None, force_exclude: None,
@ -460,6 +463,7 @@ mod tests {
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
format: None, format: None,
force_exclude: None, force_exclude: None,
@ -515,6 +519,7 @@ mod tests {
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
format: None, format: None,
force_exclude: None, force_exclude: None,
@ -578,6 +583,7 @@ mod tests {
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
format: None, format: None,
force_exclude: None, force_exclude: None,
@ -669,6 +675,7 @@ mod tests {
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
format: None, format: None,
force_exclude: None, force_exclude: None,

View File

@ -93,6 +93,13 @@
"null" "null"
] ]
}, },
"fix-only": {
"description": "Like `fix`, but disables reporting on leftover violation. Implies `fix`.",
"type": [
"boolean",
"null"
]
},
"fixable": { "fixable": {
"description": "A list of check code prefixes to consider autofix-able.", "description": "A list of check code prefixes to consider autofix-able.",
"type": [ "type": [

View File

@ -43,6 +43,12 @@ pub struct Cli {
fix: bool, fix: bool,
#[clap(long, overrides_with("fix"), hide = true)] #[clap(long, overrides_with("fix"), hide = true)]
no_fix: bool, no_fix: bool,
/// Fix any fixable lint errors, but don't report on leftover violations.
/// Implies `--fix`.
#[arg(long, overrides_with("no_fix_only"))]
fix_only: bool,
#[clap(long, overrides_with("fix_only"), hide = true)]
no_fix_only: bool,
/// Disable cache reads. /// Disable cache reads.
#[arg(short, long)] #[arg(short, long)]
pub no_cache: bool, pub no_cache: bool,
@ -181,6 +187,7 @@ impl Cli {
unfixable: self.unfixable, unfixable: self.unfixable,
// TODO(charlie): Included in `pyproject.toml`, but not inherited. // TODO(charlie): Included in `pyproject.toml`, but not inherited.
fix: resolve_bool_arg(self.fix, self.no_fix), fix: resolve_bool_arg(self.fix, self.no_fix),
fix_only: resolve_bool_arg(self.fix_only, self.no_fix_only),
format: self.format, format: self.format,
force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude), force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude),
cache_dir: self.cache_dir, cache_dir: self.cache_dir,
@ -240,6 +247,7 @@ pub struct Overrides {
pub unfixable: Option<Vec<CheckCodePrefix>>, pub unfixable: Option<Vec<CheckCodePrefix>>,
// TODO(charlie): Captured in pyproject.toml as a default, but not part of `Settings`. // TODO(charlie): Captured in pyproject.toml as a default, but not part of `Settings`.
pub fix: Option<bool>, pub fix: Option<bool>,
pub fix_only: Option<bool>,
pub format: Option<SerializationFormat>, pub format: Option<SerializationFormat>,
pub force_exclude: Option<bool>, pub force_exclude: Option<bool>,
pub cache_dir: Option<PathBuf>, pub cache_dir: Option<PathBuf>,

View File

@ -20,7 +20,7 @@ use ::ruff::autofix::fixer;
use ::ruff::cli::{extract_log_level, Cli, Overrides}; use ::ruff::cli::{extract_log_level, Cli, Overrides};
use ::ruff::commands; use ::ruff::commands;
use ::ruff::logging::{set_up_logging, LogLevel}; use ::ruff::logging::{set_up_logging, LogLevel};
use ::ruff::printer::Printer; use ::ruff::printer::{Printer, Violations};
use ::ruff::resolver::{resolve_settings, FileDiscovery, PyprojectDiscovery, Relativity}; use ::ruff::resolver::{resolve_settings, FileDiscovery, PyprojectDiscovery, Relativity};
use ::ruff::settings::configuration::Configuration; use ::ruff::settings::configuration::Configuration;
use ::ruff::settings::types::SerializationFormat; use ::ruff::settings::types::SerializationFormat;
@ -112,17 +112,24 @@ fn inner_main() -> Result<ExitCode> {
PyprojectDiscovery::Hierarchical(settings) => settings.respect_gitignore, PyprojectDiscovery::Hierarchical(settings) => settings.respect_gitignore,
}, },
}; };
let (fix, format) = match &pyproject_strategy { let (fix, fix_only, format) = match &pyproject_strategy {
PyprojectDiscovery::Fixed(settings) => (settings.fix, settings.format), PyprojectDiscovery::Fixed(settings) => (settings.fix, settings.fix_only, settings.format),
PyprojectDiscovery::Hierarchical(settings) => (settings.fix, settings.format), PyprojectDiscovery::Hierarchical(settings) => {
(settings.fix, settings.fix_only, settings.format)
}
}; };
let autofix = if fix { let autofix = if fix || fix_only {
fixer::Mode::Apply fixer::Mode::Apply
} else if matches!(format, SerializationFormat::Json) { } else if matches!(format, SerializationFormat::Json) {
fixer::Mode::Generate fixer::Mode::Generate
} else { } else {
fixer::Mode::None fixer::Mode::None
}; };
let violations = if fix_only {
Violations::Hide
} else {
Violations::Show
};
let cache = !cli.no_cache; let cache = !cli.no_cache;
if let Some(code) = cli.explain { if let Some(code) = cli.explain {
@ -138,13 +145,13 @@ fn inner_main() -> Result<ExitCode> {
return Ok(ExitCode::SUCCESS); return Ok(ExitCode::SUCCESS);
} }
let printer = Printer::new(&format, &log_level); let printer = Printer::new(&format, &log_level, &autofix, &violations);
if cli.watch { if cli.watch {
if matches!(autofix, fixer::Mode::Generate | fixer::Mode::Apply) { if matches!(autofix, fixer::Mode::Generate | fixer::Mode::Apply) {
eprintln!("Warning: --fix is not enabled in watch mode."); eprintln!("Warning: --fix is not enabled in watch mode.");
} }
if cli.add_noqa { if cli.add_noqa {
eprintln!("Warning: --no-qa is not enabled in watch mode."); eprintln!("Warning: --add-noqa is not enabled in watch mode.");
} }
if cli.autoformat { if cli.autoformat {
eprintln!("Warning: --autoformat is not enabled in watch mode."); eprintln!("Warning: --autoformat is not enabled in watch mode.");
@ -240,7 +247,7 @@ fn inner_main() -> Result<ExitCode> {
// unless we're writing fixes via stdin (in which case, the transformed // unless we're writing fixes via stdin (in which case, the transformed
// source code goes to stdout). // source code goes to stdout).
if !(is_stdin && matches!(autofix, fixer::Mode::Apply)) { if !(is_stdin && matches!(autofix, fixer::Mode::Apply)) {
printer.write_once(&diagnostics, autofix)?; printer.write_once(&diagnostics)?;
} }
// Check for updates if we're in a non-silent log level. // Check for updates if we're in a non-silent log level.
@ -249,7 +256,7 @@ fn inner_main() -> Result<ExitCode> {
drop(updates::check_for_updates()); drop(updates::check_for_updates());
} }
if !diagnostics.messages.is_empty() && !cli.exit_zero { if !diagnostics.messages.is_empty() && !cli.exit_zero && !fix_only {
return Ok(ExitCode::FAILURE); return Ok(ExitCode::FAILURE);
} }
} }

View File

@ -18,6 +18,12 @@ use crate::message::Message;
use crate::settings::types::SerializationFormat; use crate::settings::types::SerializationFormat;
use crate::tell_user; use crate::tell_user;
/// Enum to control whether lint violations are shown to the user.
pub enum Violations {
Show,
Hide,
}
#[derive(Serialize)] #[derive(Serialize)]
struct ExpandedMessage<'a> { struct ExpandedMessage<'a> {
code: &'a CheckCode, code: &'a CheckCode,
@ -31,11 +37,23 @@ struct ExpandedMessage<'a> {
pub struct Printer<'a> { pub struct Printer<'a> {
format: &'a SerializationFormat, format: &'a SerializationFormat,
log_level: &'a LogLevel, log_level: &'a LogLevel,
autofix: &'a fixer::Mode,
violations: &'a Violations,
} }
impl<'a> Printer<'a> { impl<'a> Printer<'a> {
pub fn new(format: &'a SerializationFormat, log_level: &'a LogLevel) -> Self { pub fn new(
Self { format, log_level } format: &'a SerializationFormat,
log_level: &'a LogLevel,
autofix: &'a fixer::Mode,
violations: &'a Violations,
) -> Self {
Self {
format,
log_level,
autofix,
violations,
}
} }
pub fn write_to_user(&self, message: &str) { pub fn write_to_user(&self, message: &str) {
@ -44,35 +62,55 @@ impl<'a> Printer<'a> {
} }
} }
fn post_text(&self, diagnostics: &Diagnostics, autofix: fixer::Mode) { fn post_text(&self, diagnostics: &Diagnostics) {
if self.log_level >= &LogLevel::Default { if self.log_level >= &LogLevel::Default {
let fixed = diagnostics.fixed; match self.violations {
let remaining = diagnostics.messages.len(); Violations::Show => {
let total = fixed + remaining; let fixed = diagnostics.fixed;
if fixed > 0 { let remaining = diagnostics.messages.len();
println!("Found {total} error(s) ({fixed} fixed, {remaining} remaining)."); let total = fixed + remaining;
} else if remaining > 0 { if fixed > 0 {
println!("Found {remaining} error(s)."); println!("Found {total} error(s) ({fixed} fixed, {remaining} remaining).");
} } else if remaining > 0 {
println!("Found {remaining} error(s).");
}
if !matches!(autofix, fixer::Mode::Apply) { if !matches!(self.autofix, fixer::Mode::Apply) {
let num_fixable = diagnostics let num_fixable = diagnostics
.messages .messages
.iter() .iter()
.filter(|message| message.kind.fixable()) .filter(|message| message.kind.fixable())
.count(); .count();
if num_fixable > 0 { if num_fixable > 0 {
println!("{num_fixable} potentially fixable with the --fix option."); println!("{num_fixable} potentially fixable with the --fix option.");
}
}
}
Violations::Hide => {
let fixed = diagnostics.fixed;
if fixed > 0 {
println!("Fixed {fixed} error(s).");
}
} }
} }
} }
} }
pub fn write_once(&self, diagnostics: &Diagnostics, autofix: fixer::Mode) -> Result<()> { pub fn write_once(&self, diagnostics: &Diagnostics) -> Result<()> {
if matches!(self.log_level, LogLevel::Silent) { if matches!(self.log_level, LogLevel::Silent) {
return Ok(()); return Ok(());
} }
if matches!(self.violations, Violations::Hide) {
if matches!(
self.format,
SerializationFormat::Text | SerializationFormat::Grouped
) {
self.post_text(diagnostics);
}
return Ok(());
}
match self.format { match self.format {
SerializationFormat::Json => { SerializationFormat::Json => {
println!( println!(
@ -142,7 +180,7 @@ impl<'a> Printer<'a> {
print_message(message); print_message(message);
} }
self.post_text(diagnostics, autofix); self.post_text(diagnostics);
} }
SerializationFormat::Grouped => { SerializationFormat::Grouped => {
// Group by filename. // Group by filename.
@ -182,7 +220,7 @@ impl<'a> Printer<'a> {
println!(); println!();
} }
self.post_text(diagnostics, autofix); self.post_text(diagnostics);
} }
SerializationFormat::Github => { SerializationFormat::Github => {
// Generate error workflow command in GitHub Actions format // Generate error workflow command in GitHub Actions format

View File

@ -33,6 +33,7 @@ pub struct Configuration {
pub extend_select: Vec<Vec<CheckCodePrefix>>, pub extend_select: Vec<Vec<CheckCodePrefix>>,
pub external: Option<Vec<String>>, pub external: Option<Vec<String>>,
pub fix: Option<bool>, pub fix: Option<bool>,
pub fix_only: Option<bool>,
pub fixable: Option<Vec<CheckCodePrefix>>, pub fixable: Option<Vec<CheckCodePrefix>>,
pub format: Option<SerializationFormat>, pub format: Option<SerializationFormat>,
pub force_exclude: Option<bool>, pub force_exclude: Option<bool>,
@ -107,6 +108,7 @@ impl Configuration {
extend_select: vec![options.extend_select.unwrap_or_default()], extend_select: vec![options.extend_select.unwrap_or_default()],
external: options.external, external: options.external,
fix: options.fix, fix: options.fix,
fix_only: options.fix_only,
fixable: options.fixable, fixable: options.fixable,
format: options.format, format: options.format,
force_exclude: options.force_exclude, force_exclude: options.force_exclude,
@ -179,6 +181,7 @@ impl Configuration {
.collect(), .collect(),
external: self.external.or(config.external), external: self.external.or(config.external),
fix: self.fix.or(config.fix), fix: self.fix.or(config.fix),
fix_only: self.fix_only.or(config.fix_only),
fixable: self.fixable.or(config.fixable), fixable: self.fixable.or(config.fixable),
format: self.format.or(config.format), format: self.format.or(config.format),
force_exclude: self.force_exclude.or(config.force_exclude), force_exclude: self.force_exclude.or(config.force_exclude),
@ -226,6 +229,9 @@ impl Configuration {
if let Some(fix) = overrides.fix { if let Some(fix) = overrides.fix {
self.fix = Some(fix); self.fix = Some(fix);
} }
if let Some(fix_only) = overrides.fix_only {
self.fix_only = Some(fix_only);
}
if let Some(fixable) = overrides.fixable { if let Some(fixable) = overrides.fixable {
self.fixable = Some(fixable); self.fixable = Some(fixable);
} }

View File

@ -40,6 +40,7 @@ pub struct Settings {
pub extend_exclude: GlobSet, pub extend_exclude: GlobSet,
pub external: FxHashSet<String>, pub external: FxHashSet<String>,
pub fix: bool, pub fix: bool,
pub fix_only: bool,
pub fixable: FxHashSet<CheckCode>, pub fixable: FxHashSet<CheckCode>,
pub format: SerializationFormat, pub format: SerializationFormat,
pub force_exclude: bool, pub force_exclude: bool,
@ -122,6 +123,7 @@ impl Settings {
extend_exclude: resolve_globset(config.extend_exclude)?, extend_exclude: resolve_globset(config.extend_exclude)?,
external: FxHashSet::from_iter(config.external.unwrap_or_default()), external: FxHashSet::from_iter(config.external.unwrap_or_default()),
fix: config.fix.unwrap_or(false), fix: config.fix.unwrap_or(false),
fix_only: config.fix_only.unwrap_or(false),
fixable: resolve_codes( fixable: resolve_codes(
[CheckCodeSpec { [CheckCodeSpec {
select: &config.fixable.unwrap_or_else(|| CATEGORIES.to_vec()), select: &config.fixable.unwrap_or_else(|| CATEGORIES.to_vec()),
@ -202,6 +204,7 @@ impl Settings {
extend_exclude: GlobSet::empty(), extend_exclude: GlobSet::empty(),
external: FxHashSet::default(), external: FxHashSet::default(),
fix: false, fix: false,
fix_only: false,
fixable: FxHashSet::from_iter([check_code]), fixable: FxHashSet::from_iter([check_code]),
format: SerializationFormat::Text, format: SerializationFormat::Text,
force_exclude: false, force_exclude: false,
@ -236,6 +239,7 @@ impl Settings {
extend_exclude: GlobSet::empty(), extend_exclude: GlobSet::empty(),
external: FxHashSet::default(), external: FxHashSet::default(),
fix: false, fix: false,
fix_only: false,
fixable: FxHashSet::from_iter(check_codes), fixable: FxHashSet::from_iter(check_codes),
format: SerializationFormat::Text, format: SerializationFormat::Text,
force_exclude: false, force_exclude: false,

View File

@ -132,6 +132,9 @@ pub struct Options {
/// Enable autofix behavior by-default when running `ruff` (overridden /// Enable autofix behavior by-default when running `ruff` (overridden
/// by the `--fix` and `--no-fix` command-line flags). /// by the `--fix` and `--no-fix` command-line flags).
pub fix: Option<bool>, pub fix: Option<bool>,
#[option(default = "false", value_type = "bool", example = "fix-only = true")]
/// Like `fix`, but disables reporting on leftover violation. Implies `fix`.
pub fix_only: Option<bool>,
#[option( #[option(
default = r#"["A", "ANN", "ARG", "B", "BLE", "C", "D", "E", "ERA", "F", "FBT", "I", "ICN", "N", "PGH", "PLC", "PLE", "PLR", "PLW", "Q", "RET", "RUF", "S", "T", "TID", "UP", "W", "YTT"]"#, default = r#"["A", "ANN", "ARG", "B", "BLE", "C", "D", "E", "ERA", "F", "FBT", "I", "ICN", "N", "PGH", "PLC", "PLE", "PLR", "PLW", "Q", "RET", "RUF", "S", "T", "TID", "UP", "W", "YTT"]"#,
value_type = "Vec<CheckCodePrefix>", value_type = "Vec<CheckCodePrefix>",

View File

@ -128,6 +128,7 @@ mod tests {
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
format: None, format: None,
force_exclude: None, force_exclude: None,
@ -177,6 +178,7 @@ line-length = 79
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
force_exclude: None, force_exclude: None,
format: None, format: None,
@ -226,6 +228,7 @@ exclude = ["foo.py"]
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
force_exclude: None, force_exclude: None,
format: None, format: None,
@ -275,6 +278,7 @@ select = ["E501"]
extend_select: None, extend_select: None,
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
force_exclude: None, force_exclude: None,
format: None, format: None,
@ -325,6 +329,7 @@ ignore = ["E501"]
extend_select: Some(vec![CheckCodePrefix::RUF100]), extend_select: Some(vec![CheckCodePrefix::RUF100]),
external: None, external: None,
fix: None, fix: None,
fix_only: None,
fixable: None, fixable: None,
force_exclude: None, force_exclude: None,
format: None, format: None,
@ -403,6 +408,7 @@ other-attribute = 1
allowed_confusables: Some(vec!['', 'ρ', '']), allowed_confusables: Some(vec!['', 'ρ', '']),
line_length: Some(88), line_length: Some(88),
fix: None, fix: None,
fix_only: None,
exclude: None, exclude: None,
extend: None, extend: None,
extend_exclude: Some(vec![ extend_exclude: Some(vec![