Python
/
ruff
mirror of https://github.com/astral-sh/ruff
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Update CLI to respect fix applicability

This commit is contained in:
Evan Rittenhouse 2023-10-02 17:05:01 -05:00 committed by Zanie
parent 90c259beb9
commit d01969c974
17 changed files with 599 additions and 269 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
crates/ruff_cli/src/args.rs
View File

@ -72,7 +72,6 @@ pub enum Command {
// The `Parser` derive is for ruff_dev, for ruff_cli `Args` would be sufficient
#[derive(Clone, Debug, clap::Parser)]
#[allow(clippy::struct_excessive_bools)]
pub struct CheckCommand {
/// List of files or directories to check.
pub files: Vec<PathBuf>,
@ -80,7 +79,10 @@ pub struct CheckCommand {
/// Use `--no-fix` to disable.
#[arg(long, overrides_with("no_fix"))]
fix: bool,
#[clap(long, overrides_with("fix"), hide = true)]
/// Attempt to automatically fix both automatic and suggested lint violations.
#[arg(long, overrides_with_all(["fix", "no_fix"]))]
fix_suggested: bool,
#[clap(long, overrides_with_all(["fix", "fix_suggested"]), hide = true)]
no_fix: bool,
/// Show violations with source code.
/// Use `--no-show-source` to disable.
@ -497,6 +499,7 @@ impl CheckCommand {
cache_dir: self.cache_dir,
fix: resolve_bool_arg(self.fix, self.no_fix),
fix_only: resolve_bool_arg(self.fix_only, self.no_fix_only),
fix_suggested: Some(self.fix_suggested),
force_exclude: resolve_bool_arg(self.force_exclude, self.no_force_exclude),
output_format: self.output_format.or(self.format),
show_fixes: resolve_bool_arg(self.show_fixes, self.no_show_fixes),
@ -599,6 +602,7 @@ pub struct CliOverrides {
pub cache_dir: Option<PathBuf>,
pub fix: Option<bool>,
pub fix_only: Option<bool>,
pub fix_suggested: Option<bool>,
pub force_exclude: Option<bool>,
pub output_format: Option<SerializationFormat>,
pub show_fixes: Option<bool>,
@ -624,6 +628,9 @@ impl ConfigurationTransformer for CliOverrides {
if let Some(fix_only) = &self.fix_only {
config.fix_only = Some(*fix_only);
}
if self.fix_suggested.is_some() {
config.fix_suggested = self.fix_suggested;
}
config.lint.rule_selections.push(RuleSelection {
select: self.select.clone(),
ignore: self

6
crates/ruff_cli/src/cache.rs
View File

@ -409,7 +409,7 @@ mod tests {
&settings.linter,
Some(&cache),
flags::Noqa::Enabled,
flags::FixMode::Generate,
flags::FixMode::Generate(flags::SuggestedFixes::Apply),
)
.unwrap();
if diagnostics
@ -454,7 +454,7 @@ mod tests {
&settings.linter,
Some(&cache),
flags::Noqa::Enabled,
flags::FixMode::Generate,
flags::FixMode::Generate(flags::SuggestedFixes::Apply),
)
.unwrap();
}
@ -711,7 +711,7 @@ mod tests {
&self.settings.linter,
Some(cache),
flags::Noqa::Enabled,
flags::FixMode::Generate,
flags::FixMode::Generate(flags::SuggestedFixes::Apply),
)
}
}

2
crates/ruff_cli/src/commands/check.rs
View File

@ -284,7 +284,7 @@ mod test {
&CliOverrides::default(),
flags::Cache::Disabled,
flags::Noqa::Disabled,
flags::FixMode::Generate,
flags::FixMode::Generate(flags::SuggestedFixes::Apply),
)
.unwrap();
let mut output = Vec::new();

292
crates/ruff_cli/src/diagnostics.rs
View File

@ -241,96 +241,110 @@ pub(crate) fn lint_path(
error: parse_error,
},
fixed,
) = if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(path, package, noqa, settings, &source_kind, source_type)
{
if !fixed.is_empty() {
match fix_mode {
flags::FixMode::Apply => match transformed.as_ref() {
SourceKind::Python(transformed) => {
write(path, transformed.as_bytes())?;
}
SourceKind::IpyNotebook(notebook) => {
let mut writer = BufWriter::new(File::create(path)?);
notebook.write(&mut writer)?;
}
},
flags::FixMode::Diff => {
match transformed.as_ref() {
) = match fix_mode {
flags::FixMode::Apply(fix_suggested) | flags::FixMode::Diff(fix_suggested) => {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
path,
package,
noqa,
settings,
&source_kind,
source_type,
fix_suggested,
) {
if !fixed.is_empty() {
match fix_mode {
flags::FixMode::Apply(_) => match transformed.as_ref() {
SourceKind::Python(transformed) => {
let mut stdout = io::stdout().lock();
TextDiff::from_lines(source_kind.source_code(), transformed)
.unified_diff()
.header(&fs::relativize_path(path), &fs::relativize_path(path))
.to_writer(&mut stdout)?;
stdout.write_all(b"\n")?;
stdout.flush()?;
write(path, transformed.as_bytes())?;
}
SourceKind::IpyNotebook(dest_notebook) => {
// We need to load the notebook again, since we might've
// mutated it.
let src_notebook = source_kind.as_ipy_notebook().unwrap();
let mut stdout = io::stdout().lock();
// Cell indices are 1-based.
for ((idx, src_cell), dest_cell) in (1u32..)
.zip(src_notebook.cells().iter())
.zip(dest_notebook.cells().iter())
{
let (Cell::Code(src_code_cell), Cell::Code(dest_code_cell)) =
(src_cell, dest_cell)
else {
continue;
};
TextDiff::from_lines(
&src_code_cell.source.to_string(),
&dest_code_cell.source.to_string(),
)
.unified_diff()
// Jupyter notebook cells don't necessarily have a newline
// at the end. For example,
//
// ```python
// print("hello")
// ```
//
// For a cell containing the above code, there'll only be one line,
// and it won't have a newline at the end. If it did, there'd be
// two lines, and the second line would be empty:
//
// ```python
// print("hello")
//
// ```
.missing_newline_hint(false)
.header(
&format!("{}:cell {}", &fs::relativize_path(path), idx),
&format!("{}:cell {}", &fs::relativize_path(path), idx),
)
.to_writer(&mut stdout)?;
SourceKind::IpyNotebook(notebook) => {
let mut writer = BufWriter::new(File::create(path)?);
notebook.write(&mut writer)?;
}
},
flags::FixMode::Diff(_) => {
match transformed.as_ref() {
SourceKind::Python(transformed) => {
let mut stdout = io::stdout().lock();
TextDiff::from_lines(source_kind.source_code(), transformed)
.unified_diff()
.header(
&fs::relativize_path(path),
&fs::relativize_path(path),
)
.to_writer(&mut stdout)?;
stdout.write_all(b"\n")?;
stdout.flush()?;
}
SourceKind::IpyNotebook(dest_notebook) => {
// We need to load the notebook again, since we might've
// mutated it.
let src_notebook = source_kind.as_ipy_notebook().unwrap();
let mut stdout = io::stdout().lock();
for ((idx, src_cell), dest_cell) in src_notebook
.cells()
.iter()
.enumerate()
.zip(dest_notebook.cells().iter())
{
let (Cell::Code(src_code_cell), Cell::Code(dest_code_cell)) =
(src_cell, dest_cell)
else {
continue;
};
TextDiff::from_lines(
&src_code_cell.source.to_string(),
&dest_code_cell.source.to_string(),
)
.unified_diff()
// Jupyter notebook cells don't necessarily have a newline
// at the end. For example,
//
// ```python
// print("hello")
// ```
//
// For a cell containing the above code, there'll only be one line,
// and it won't have a newline at the end. If it did, there'd be
// two lines, and the second line would be empty:
//
// ```python
// print("hello")
//
// ```
.missing_newline_hint(false)
.header(
&format!("{}:cell {}", &fs::relativize_path(path), idx),
&format!("{}:cell {}", &fs::relativize_path(path), idx),
)
.to_writer(&mut stdout)?;
}
stdout.write_all(b"\n")?;
stdout.flush()?;
}
stdout.write_all(b"\n")?;
stdout.flush()?;
}
}
flags::FixMode::Generate(_) => {}
}
flags::FixMode::Generate => {}
}
(result, fixed)
} else {
// If we fail to fix, lint the original source code.
let result = lint_only(path, package, settings, noqa, &source_kind, source_type);
let fixed = FxHashMap::default();
(result, fixed)
}
(result, fixed)
} else {
// If we fail to fix, lint the original source code.
}
flags::FixMode::Generate(_) => {
let result = lint_only(path, package, settings, noqa, &source_kind, source_type);
let fixed = FxHashMap::default();
(result, fixed)
}
} else {
let result = lint_only(path, package, settings, noqa, &source_kind, source_type);
let fixed = FxHashMap::default();
(result, fixed)
};
let imports = imports.unwrap_or_default();
@ -392,7 +406,7 @@ pub(crate) fn lint_stdin(
};
// Extract the sources from the file.
let source_kind = match SourceKind::from_source_code(contents, source_type) {
let source_kind = match SourceKind::from_source_code(contents.clone(), source_type) {
Ok(Some(source_kind)) => source_kind,
Ok(None) => return Ok(Diagnostics::default()),
Err(err) => {
@ -407,49 +421,70 @@ pub(crate) fn lint_stdin(
error: parse_error,
},
fixed,
) = if matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff) {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
path.unwrap_or_else(|| Path::new("-")),
package,
noqa,
&settings.linter,
&source_kind,
source_type,
) {
match fix_mode {
flags::FixMode::Apply => {
// Write the contents to stdout, regardless of whether any errors were fixed.
transformed.write(&mut io::stdout().lock())?;
}
flags::FixMode::Diff => {
// But only write a diff if it's non-empty.
if !fixed.is_empty() {
let text_diff = TextDiff::from_lines(
source_kind.source_code(),
transformed.source_code(),
);
let mut unified_diff = text_diff.unified_diff();
if let Some(path) = path {
unified_diff
.header(&fs::relativize_path(path), &fs::relativize_path(path));
}
let mut stdout = io::stdout().lock();
unified_diff.to_writer(&mut stdout)?;
stdout.write_all(b"\n")?;
stdout.flush()?;
) = match fix_mode {
flags::FixMode::Apply(fix_suggested) | flags::FixMode::Diff(fix_suggested) => {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
path.unwrap_or_else(|| Path::new("-")),
package,
noqa,
&settings.linter,
&source_kind,
source_type,
fix_suggested,
) {
match fix_mode {
flags::FixMode::Apply(_) => {
// Write the contents to stdout, regardless of whether any errors were fixed.
io::stdout().write_all(transformed.source_code().as_bytes())?;
}
}
flags::FixMode::Generate => {}
}
flags::FixMode::Diff(_) => {
// But only write a diff if it's non-empty.
if !fixed.is_empty() {
let text_diff = TextDiff::from_lines(
source_kind.source_code(),
transformed.source_code(),
);
let mut unified_diff = text_diff.unified_diff();
if let Some(path) = path {
unified_diff
.header(&fs::relativize_path(path), &fs::relativize_path(path));
}
(result, fixed)
} else {
// If we fail to fix, lint the original source code.
let mut stdout = io::stdout().lock();
unified_diff.to_writer(&mut stdout)?;
stdout.write_all(b"\n")?;
stdout.flush()?;
}
}
flags::FixMode::Generate(_) => {}
}
(result, fixed)
} else {
// If we fail to fix, lint the original source code.
let result = lint_only(
path.unwrap_or_else(|| Path::new("-")),
package,
&settings.linter,
noqa,
&source_kind,
source_type,
);
let fixed = FxHashMap::default();
// Write the contents to stdout anyway.
if fix_mode.is_apply() {
io::stdout().write_all(contents.as_bytes())?;
}
(result, fixed)
}
}
flags::FixMode::Generate(_) => {
let result = lint_only(
path.unwrap_or_else(|| Path::new("-")),
package,
@ -459,25 +494,8 @@ pub(crate) fn lint_stdin(
source_type,
);
let fixed = FxHashMap::default();
// Write the contents to stdout anyway.
if fix_mode.is_apply() {
source_kind.write(&mut io::stdout().lock())?;
}
(result, fixed)
}
} else {
let result = lint_only(
path.unwrap_or_else(|| Path::new("-")),
package,
&settings.linter,
noqa,
&source_kind,
source_type,
);
let fixed = FxHashMap::default();
(result, fixed)
};
let imports = imports.unwrap_or_default();

31
crates/ruff_cli/src/lib.rs
View File

@ -10,7 +10,7 @@ use log::warn;
use notify::{recommended_watcher, RecursiveMode, Watcher};
use ruff_linter::logging::{set_up_logging, LogLevel};
use ruff_linter::settings::flags;
use ruff_linter::settings::flags::{FixMode, SuggestedFixes};
use ruff_linter::settings::types::SerializationFormat;
use ruff_linter::{fs, warn_user, warn_user_once};
use ruff_workspace::Settings;
@ -228,6 +228,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
let Settings {
fix,
fix_only,
fix_suggested,
output_format,
show_fixes,
show_source,
@ -237,16 +238,28 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
// Fix rules are as follows:
// - By default, generate all fixes, but don't apply them to the filesystem.
// - If `--fix` or `--fix-only` is set, always apply fixes to the filesystem (or
// print them to stdout, if we're reading from stdin).
// print them to stdout, if we're reading from stdin) for [`Applicability::Automatic`] rules
// only.
// - If `--diff` or `--fix-only` are set, don't print any violations (only
// fixes).
let fix_mode = if cli.diff {
flags::FixMode::Diff
} else if fix || fix_only {
flags::FixMode::Apply
// fixes) for [`Applicability::Automatic`] rules only.
// - If `--fix--suggested` is set, the above rules will apply to both [`Applicability::Suggested`] and
// [`Applicability::Automatic`] fixes.
// TODO: can't fix this until @zanieb changes the CLI
let fix_suggested = if fix_suggested {
SuggestedFixes::Apply
} else {
flags::FixMode::Generate
SuggestedFixes::Disable
};
let fix_mode = if cli.diff {
FixMode::Diff(fix_suggested)
} else if fix || fix_only {
FixMode::Apply(fix_suggested)
} else {
// We'll always generate all fixes, regardless of [`Applicability`], in `generate` mode
FixMode::Generate(SuggestedFixes::Apply)
};
let cache = !cli.no_cache;
let noqa = !cli.ignore_noqa;
let mut printer_flags = PrinterFlags::empty();
@ -382,7 +395,7 @@ pub fn check(args: CheckCommand, log_level: LogLevel) -> Result<ExitStatus> {
// Always try to print violations (the printer itself may suppress output),
// unless we're writing fixes via stdin (in which case, the transformed
// source code goes to stdout).
if !(is_stdin && matches!(fix_mode, flags::FixMode::Apply | flags::FixMode::Diff)) {
if !(is_stdin && matches!(fix_mode, FixMode::Apply(_) | FixMode::Diff(_))) {
if cli.statistics {
printer.write_statistics(&diagnostics, &mut writer)?;
} else {

107
crates/ruff_cli/src/printer.rs
View File

@ -7,6 +7,7 @@ use anyhow::Result;
use bitflags::bitflags;
use colored::Colorize;
use itertools::{iterate, Itertools};
use ruff_diagnostics::Applicability;
use rustc_hash::FxHashMap;
use serde::Serialize;
@ -19,7 +20,7 @@ use ruff_linter::message::{
};
use ruff_linter::notify_user;
use ruff_linter::registry::{AsRule, Rule};
use ruff_linter::settings::flags;
use ruff_linter::settings::flags::{self, SuggestedFixes};
use ruff_linter::settings::types::SerializationFormat;
use crate::diagnostics::Diagnostics;
@ -118,19 +119,10 @@ impl Printer {
writeln!(writer, "Found {remaining} error{s}.")?;
}
if show_fix_status(self.fix_mode) {
let num_fixable = diagnostics
.messages
.iter()
.filter(|message| message.fix.is_some())
.count();
if num_fixable > 0 {
writeln!(
writer,
"[{}] {num_fixable} potentially fixable with the --fix option.",
"*".cyan(),
)?;
}
let fixables = FixableStatistics::new(diagnostics, self.fix_mode.suggested_fixes());
if !fixables.is_empty() {
writeln!(writer, "{}", fixables.violation_string())?;
}
} else {
let fixed = diagnostics
@ -178,6 +170,7 @@ impl Printer {
}
let context = EmitterContext::new(&diagnostics.notebook_indexes);
let fixables = FixableStatistics::new(diagnostics, self.fix_mode.suggested_fixes());
match self.format {
SerializationFormat::Json => {
@ -191,9 +184,9 @@ impl Printer {
}
SerializationFormat::Text => {
TextEmitter::default()
.with_show_fix_status(show_fix_status(self.fix_mode))
.with_show_fix_diff(self.flags.intersects(Flags::SHOW_FIX_DIFF))
.with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.with_show_fix_status(show_fix_status(self.fix_mode, fixables))
.with_show_fix_diff(self.flags.contains(Flags::SHOW_FIX_DIFF))
.with_show_source(self.flags.contains(Flags::SHOW_SOURCE))
.emit(writer, &diagnostics.messages, &context)?;
if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
@ -209,7 +202,7 @@ impl Printer {
SerializationFormat::Grouped => {
GroupedEmitter::default()
.with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.with_show_fix_status(show_fix_status(self.fix_mode))
.with_show_fix_status(show_fix_status(self.fix_mode, fixables))
.emit(writer, &diagnostics.messages, &context)?;
if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
@ -359,6 +352,8 @@ impl Printer {
);
}
let fixables = FixableStatistics::new(diagnostics, self.fix_mode.suggested_fixes());
if !diagnostics.messages.is_empty() {
if self.log_level >= LogLevel::Default {
writeln!(writer)?;
@ -366,7 +361,7 @@ impl Printer {
let context = EmitterContext::new(&diagnostics.notebook_indexes);
TextEmitter::default()
.with_show_fix_status(show_fix_status(self.fix_mode))
.with_show_fix_status(show_fix_status(self.fix_mode, fixables))
.with_show_source(self.flags.intersects(Flags::SHOW_SOURCE))
.emit(writer, &diagnostics.messages, &context)?;
}
@ -390,13 +385,13 @@ fn num_digits(n: usize) -> usize {
}
/// Return `true` if the [`Printer`] should indicate that a rule is fixable.
const fn show_fix_status(fix_mode: flags::FixMode) -> bool {
fn show_fix_status(fix_mode: flags::FixMode, fixables: FixableStatistics) -> bool {
// If we're in application mode, avoid indicating that a rule is fixable.
// If the specific violation were truly fixable, it would've been fixed in
// this pass! (We're occasionally unable to determine whether a specific
// violation is fixable without trying to fix it, so if fix is not
// enabled, we may inadvertently indicate that a rule is fixable.)
!fix_mode.is_apply()
(!fix_mode.is_apply()) && fixables.fixes_are_applicable()
}
fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>) -> Result<()> {
@ -439,3 +434,73 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>
}
Ok(())
}
/// Contains the number of [`Applicability::Automatic`] and [`Applicability::Suggested`] fixes
struct FixableStatistics<'a> {
automatic: u32,
suggested: u32,
apply_suggested: &'a SuggestedFixes,
}
impl<'a> FixableStatistics<'a> {
fn new(diagnostics: &Diagnostics, apply_suggested: &'a SuggestedFixes) -> Self {
let mut automatic = 0;
let mut suggested = 0;
for message in &diagnostics.messages {
if let Some(fix) = &message.fix {
if fix.applicability() == Applicability::Suggested {
suggested += 1;
} else if fix.applicability() == Applicability::Automatic {
automatic += 1;
}
}
}
Self {
automatic,
suggested,
apply_suggested,
}
}
fn fixes_are_applicable(&self) -> bool {
match self.apply_suggested {
SuggestedFixes::Apply => self.automatic > 0 || self.suggested > 0,
SuggestedFixes::Disable => self.automatic > 0,
}
}
/// Returns [`true`] if there aren't any fixes to be displayed
fn is_empty(&self) -> bool {
self.automatic == 0 && self.suggested == 0
}
/// Build the displayed fix status message depending on the types of the remaining fixes.
fn violation_string(&self) -> String {
let prefix = format!("[{}]", "*".cyan());
let mut fix_status = prefix;
if self.automatic > 0 {
fix_status = format!(
"{fix_status} {} potentially fixable with the --fix option.",
self.automatic
);
}
if self.suggested > 0 {
let (line_break, extra_prefix) = if self.automatic > 0 {
("\n", format!("[{}]", "*".cyan()))
} else {
("", String::new())
};
let total = self.automatic + self.suggested;
fix_status = format!(
"{fix_status}{line_break}{extra_prefix} {total} potentially fixable with the --fix-suggested option."
);
}
fix_status
}
}

307
crates/ruff_cli/tests/integration_test.rs
View File

@ -241,81 +241,9 @@ fn stdin_fix_jupyter() {
success: true
exit_code: 0
----- stdout -----
{
"cells": [
{
"cell_type": "code",
"execution_count": 1,
"id": "dccc687c-96e2-4604-b957-a8a89b5bec06",
"metadata": {},
"outputs": [],
"source": []
},
{
"cell_type": "markdown",
"id": "19e1b029-f516-4662-a9b9-623b93edac1a",
"metadata": {},
"source": [
"Foo"
]
},
{
"cell_type": "code",
"execution_count": 2,
"id": "cdce7b92-b0fb-4c02-86f6-e233b26fa84f",
"metadata": {},
"outputs": [],
"source": []
},
{
"cell_type": "code",
"execution_count": 3,
"id": "e40b33d2-7fe4-46c5-bdf0-8802f3052565",
"metadata": {},
"outputs": [
{
"name": "stdout",
"output_type": "stream",
"text": [
"1\n"
]
}
],
"source": [
"print(1)"
]
},
{
"cell_type": "code",
"execution_count": null,
"id": "a1899bc8-d46f-4ec0-b1d1-e1ca0f04bf60",
"metadata": {},
"outputs": [],
"source": []
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3 (ipykernel)",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.2"
}
},
"nbformat": 4,
"nbformat_minor": 5
}
print(1)
----- stderr -----
"###);
}
@ -869,3 +797,232 @@ fn check_input_from_argfile() -> Result<()> {
Ok(())
}
#[test]
fn displays_fix_applicability_levels() -> Result<()> {
// `--fix` should only apply automatic fixes, but should tell the user about `--fix --unautomatic` if
// there are remaining unautomatic fixes.
// TODO: this should be a failure but we don't have a way to track that
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format=text",
"--isolated",
"--select",
"F601,UP034",
"--no-cache",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
-:1:14: F601 [*] Dictionary key literal `'a'` repeated
-:2:7: UP034 [*] Avoid extraneous parentheses
Found 2 errors.
[*] 1 potentially fixable with the --fix option.
[*] 2 potentially fixable with the --fix-suggested option.
----- stderr -----
"###);
Ok(())
}
#[test]
fn displays_remaining_suggested_fixes() -> Result<()> {
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
.args(["-", "--output-format", "text", "--no-cache", "--isolated", "--select", "F601"])
.pass_stdin("x = {'a': 1, 'a': 1}\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
-:1:14: F601 [*] Dictionary key literal `'a'` repeated
Found 1 error.
[*] 1 potentially fixable with the --fix-suggested option.
----- stderr -----
"###);
Ok(())
}
#[test]
fn fix_applies_automatic_fixes_only() -> Result<()> {
// `--fix` should only apply automatic fixes. Since we're runnnig in `stdin` mode, output shouldn't
// be printed.
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format",
"text",
"--isolated","--no-cache",
"--select",
"F601,UP034",
"--fix",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
x = {'a': 1, 'a': 1}
print('foo')
----- stderr -----
"###);
Ok(())
}
#[test]
fn fix_applies_automatic_and_suggested_fixes_with_fix_suggested() -> Result<()> {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format",
"text",
"--isolated","--no-cache",
"--select",
"F601,UP034",
"--fix",
"--fix-suggested",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
-:1:14: F601 [*] Dictionary key literal `'a'` repeated
-:2:7: UP034 [*] Avoid extraneous parentheses
Found 2 errors.
[*] 1 potentially fixable with the --fix option.
[*] 2 potentially fixable with the --fix-suggested option.
----- stderr -----
"###);
Ok(())
}
#[test]
fn diff_shows_automatic_and_suggested_fixes_with_fix_suggested() -> Result<()> {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format",
"text",
"--isolated","--no-cache",
"--select",
"F601,UP034",
"--diff",
"--fix-suggested",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
@@ -1,2 +1,2 @@
-x = {'a': 1, 'a': 1}
-print(('foo'))
+x = {'a': 1}
+print('foo')
----- stderr -----
"###
);
Ok(())
}
#[test]
fn diff_shows_automatic_fixes_only() -> Result<()> {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format",
"text",
"--isolated","--no-cache",
"--select",
"F601,UP034",
"--diff",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
@@ -1,2 +1,2 @@
x = {'a': 1, 'a': 1}
-print(('foo'))
+print('foo')
----- stderr -----
"###
);
Ok(())
}
#[test]
fn fix_only_applies_automatic_and_suggested_fixes_with_fix_suggested() -> Result<()> {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format",
"text",
"--isolated","--no-cache",
"--select",
"F601,UP034",
"--fix-only",
"--fix-suggested",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: true
exit_code: 0
----- stdout -----
x = {'a': 1}
print('foo')
----- stderr -----
"###);
Ok(())
}
#[test]
fn fix_only_applies_automatic_fixes_only() -> Result<()> {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format",
"text",
"--isolated","--no-cache",
"--select",
"F601,UP034",
"--fix-only",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: true
exit_code: 0
----- stdout -----
x = {'a': 1, 'a': 1}
print('foo')
----- stderr -----
"###);
Ok(())
}

32
crates/ruff_diagnostics/src/fix.rs
View File

@ -5,27 +5,30 @@ use ruff_text_size::{Ranged, TextSize};
use crate::edit::Edit;
/// Indicates confidence in the correctness of a suggested fix.
/// Indicates confidence in the correctness of a suggested fix. Rust internally allows comparison
/// of enum values based on their order (see Rust's [enum
/// documentation](https://doc.rust-lang.org/reference/items/enumerations.html)). This allows us to
/// apply [`Fix`]es based on their [`Applicability`].
#[derive(Default, Copy, Clone, Debug, Hash, PartialEq, Eq, PartialOrd, Ord)]
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
pub enum Applicability {
/// The fix is definitely what the user intended, or maintains the exact meaning of the code.
/// This fix should be automatically applied.
Automatic,
/// The fix has a good chance of being incorrect or the code be incomplete.
/// The fix may result in invalid code if it is applied.
/// The fix should only be manually applied by the user.
Manual,
/// The applicability of the fix is unknown or not relevant.
#[default]
Unspecified,
/// The fix may be what the user intended, but it is uncertain.
/// The fix should result in valid code if it is applied.
/// The fix can be applied with user opt-in.
Suggested,
/// The fix has a good chance of being incorrect or the code be incomplete.
/// The fix may result in invalid code if it is applied.
/// The fix should only be manually applied by the user.
Manual,
/// The applicability of the fix is unknown.
#[default]
Unspecified,
/// The fix is definitely what the user intended, or maintains the exact meaning of the code.
/// This fix should be automatically applied.
Automatic,
}
/// Indicates the level of isolation required to apply a fix.
@ -162,4 +165,9 @@ impl Fix {
self.isolation_level = isolation;
self
}
/// Return [`true`] if this [`Fix`] should be applied with a given [`Applicability`].
pub fn is_applied(&self, applicability: Applicability) -> bool {
self.applicability >= applicability
}
}

22
crates/ruff_linter/src/fix/mod.rs
View File

@ -4,11 +4,12 @@ use std::collections::BTreeSet;
use ruff_text_size::{Ranged, TextLen, TextRange, TextSize};
use rustc_hash::{FxHashMap, FxHashSet};
use ruff_diagnostics::{Diagnostic, Edit, Fix, IsolationLevel, SourceMap};
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, IsolationLevel, SourceMap};
use ruff_source_file::Locator;
use crate::linter::FixTable;
use crate::registry::{AsRule, Rule};
use crate::settings::flags::SuggestedFixes;
pub(crate) mod codemods;
pub(crate) mod edits;
@ -24,10 +25,25 @@ pub(crate) struct FixResult {
}
/// Auto-fix errors in a file, and write the fixed source code to disk.
pub(crate) fn fix_file(diagnostics: &[Diagnostic], locator: &Locator) -> Option<FixResult> {
pub(crate) fn fix_file(
diagnostics: &[Diagnostic],
locator: &Locator,
fix_suggested: SuggestedFixes,
) -> Option<FixResult> {
let mut with_fixes = diagnostics
.iter()
.filter(|diag| diag.fix.is_some())
.filter(|diag| {
let Some(ref fix) = diag.fix else {
return false;
};
// change this
if matches!(fix_suggested, SuggestedFixes::Apply) {
fix.applicability() >= Applicability::Suggested
} else {
fix.applicability() == Applicability::Automatic
}
})
.peekable();
if with_fixes.peek().is_none() {

4
crates/ruff_linter/src/linter.rs
View File

@ -415,6 +415,7 @@ fn diagnostics_to_messages(
/// Generate `Diagnostic`s from source code content, iteratively fixing
/// until stable.
#[allow(clippy::too_many_arguments)]
pub fn lint_fix<'a>(
path: &Path,
package: Option<&Path>,
@ -422,6 +423,7 @@ pub fn lint_fix<'a>(
settings: &LinterSettings,
source_kind: &'a SourceKind,
source_type: PySourceType,
fix_suggested: flags::SuggestedFixes,
) -> Result<FixerResult<'a>> {
let mut transformed = Cow::Borrowed(source_kind);
@ -494,7 +496,7 @@ pub fn lint_fix<'a>(
code: fixed_contents,
fixes: applied,
source_map,
}) = fix_file(&result.data.0, &locator)
}) = fix_file(&result.data.0, &locator, fix_suggested)
{
if iterations < MAX_ITERATIONS {
// Count the number of fixed errors.

20
crates/ruff_linter/src/settings/flags.rs
View File

@ -1,8 +1,24 @@
#[derive(Debug, Copy, Clone, Hash, is_macro::Is)]
pub enum FixMode {
Generate,
Generate(SuggestedFixes),
Apply(SuggestedFixes),
Diff(SuggestedFixes),
}
impl FixMode {
pub fn suggested_fixes(&self) -> &SuggestedFixes {
match self {
FixMode::Generate(suggested) => suggested,
FixMode::Apply(suggested) => suggested,
FixMode::Diff(suggested) => suggested,
}
}
}
#[derive(Debug, Copy, Clone, Hash, is_macro::Is)]
pub enum SuggestedFixes {
Apply,
Diff,
Disable,
}
#[derive(Debug, Copy, Clone, Hash, result_like::BoolLike)]

7
crates/ruff_linter/src/test.rs
View File

@ -155,8 +155,11 @@ pub(crate) fn test_contents<'a>(
code: fixed_contents,
source_map,
..
}) = fix_file(&diagnostics, &Locator::new(transformed.source_code()))
{
}) = fix_file(
&diagnostics,
&Locator::new(transformed.source_code()),
flags::SuggestedFixes::Apply,
) {
if iterations < max_iterations() {
iterations += 1;
} else {

4
crates/ruff_workspace/src/configuration.rs
View File

@ -63,6 +63,7 @@ pub struct Configuration {
pub cache_dir: Option<PathBuf>,
pub extend: Option<PathBuf>,
pub fix: Option<bool>,
pub fix_suggested: Option<bool>,
pub fix_only: Option<bool>,
pub output_format: Option<SerializationFormat>,
pub preview: Option<PreviewMode>,
@ -136,6 +137,7 @@ impl Configuration {
.clone()
.unwrap_or_else(|| cache_dir(project_root)),
fix: self.fix.unwrap_or(false),
fix_suggested: self.fix_suggested.unwrap_or(false),
fix_only: self.fix_only.unwrap_or(false),
output_format: self.output_format.unwrap_or_default(),
show_fixes: self.show_fixes.unwrap_or(false),
@ -365,6 +367,7 @@ impl Configuration {
}),
fix: options.fix,
fix_only: options.fix_only,
fix_suggested: options.fix_suggested,
output_format: options.output_format.or_else(|| {
options
.format
@ -418,6 +421,7 @@ impl Configuration {
include: self.include.or(config.include),
fix: self.fix.or(config.fix),
fix_only: self.fix_only.or(config.fix_only),
fix_suggested: self.fix_suggested.or(config.fix_suggested),
output_format: self.output_format.or(config.output_format),
force_exclude: self.force_exclude.or(config.force_exclude),
line_length: self.line_length.or(config.line_length),

9
crates/ruff_workspace/src/options.rs
View File

@ -89,9 +89,18 @@ pub struct Options {
/// Enable fix behavior by-default when running `ruff` (overridden
/// by the `--fix` and `--no-fix` command-line flags).
/// Only includes automatic fixes unless `--fix-suggested` is provided.
#[option(default = "false", value_type = "bool", example = "fix = true")]
pub fix: Option<bool>,
/// Enable application of suggested fixes.
#[option(
default = "false",
value_type = "bool",
example = "fix-suggested = true"
)]
pub fix_suggested: Option<bool>,
/// Like `fix`, but disables reporting on leftover violation. Implies `fix`.
#[option(default = "false", value_type = "bool", example = "fix-only = true")]
pub fix_only: Option<bool>,

3
crates/ruff_workspace/src/settings.rs
View File

@ -17,6 +17,8 @@ pub struct Settings {
#[cache_key(ignore)]
pub fix: bool,
#[cache_key(ignore)]
pub fix_suggested: bool,
#[cache_key(ignore)]
pub fix_only: bool,
#[cache_key(ignore)]
pub output_format: SerializationFormat,
@ -36,6 +38,7 @@ impl Default for Settings {
Self {
cache_dir: cache_dir(project_root),
fix: false,
fix_suggested: false,
fix_only: false,
output_format: SerializationFormat::default(),
show_fixes: false,

2
docs/configuration.md
View File

@ -194,6 +194,8 @@ Arguments:
Options:
--fix
Attempt to automatically fix lint violations. Use `--no-fix` to disable
--fix-suggested
Attempt to automatically fix both automatic and suggested lint violations
--show-source
Show violations with source code. Use `--no-show-source` to disable
--show-fixes

9
ruff.schema.json generated
View File

@ -128,7 +128,7 @@
}
},
"fix": {
"description": "Enable fix behavior by-default when running `ruff` (overridden by the `--fix` and `--no-fix` command-line flags).",
"description": "Enable fix behavior by-default when running `ruff` (overridden by the `--fix` and `--no-fix` command-line flags). Only includes automatic fixes unless `--fix-suggested` is provided.",
"type": [
"boolean",
"null"
@ -141,6 +141,13 @@
"null"
]
},
"fix-suggested": {
"description": "Enable application of suggested fixes.",
"type": [
"boolean",
"null"
]
},
"fixable": {
"description": "A list of rule codes or prefixes to consider fixable. By default, all rules are considered fixable.",
"type": [