diff --git a/ruff_cli/src/diagnostics.rs b/ruff_cli/src/diagnostics.rs index dae3399d06..14d39e8170 100644 --- a/ruff_cli/src/diagnostics.rs +++ b/ruff_cli/src/diagnostics.rs @@ -75,21 +75,28 @@ pub fn lint_path( }, fixed, ) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) { - let (result, transformed, fixed) = lint_fix(&contents, path, package, &settings.lib)?; - if fixed > 0 { - if matches!(autofix, fix::FixMode::Apply) { - write(path, transformed)?; - } else if matches!(autofix, fix::FixMode::Diff) { - let mut stdout = io::stdout().lock(); - TextDiff::from_lines(&contents, &transformed) - .unified_diff() - .header(&fs::relativize_path(path), &fs::relativize_path(path)) - .to_writer(&mut stdout)?; - stdout.write_all(b"\n")?; - stdout.flush()?; + if let Ok((result, transformed, fixed)) = lint_fix(&contents, path, package, &settings.lib) + { + if fixed > 0 { + if matches!(autofix, fix::FixMode::Apply) { + write(path, transformed)?; + } else if matches!(autofix, fix::FixMode::Diff) { + let mut stdout = io::stdout().lock(); + TextDiff::from_lines(&contents, &transformed) + .unified_diff() + .header(&fs::relativize_path(path), &fs::relativize_path(path)) + .to_writer(&mut stdout)?; + stdout.write_all(b"\n")?; + stdout.flush()?; + } } + (result, fixed) + } else { + // If we fail to autofix, lint the original source code. + let result = lint_only(&contents, path, package, &settings.lib, autofix.into()); + let fixed = 0; + (result, fixed) } - (result, fixed) } else { let result = lint_only(&contents, path, package, &settings.lib, autofix.into()); let fixed = 0; @@ -143,33 +150,50 @@ pub fn lint_stdin( }, fixed, ) = if matches!(autofix, fix::FixMode::Apply | fix::FixMode::Diff) { - let (result, transformed, fixed) = lint_fix( + if let Ok((result, transformed, fixed)) = lint_fix( contents, path.unwrap_or_else(|| Path::new("-")), package, settings, - )?; + ) { + if matches!(autofix, fix::FixMode::Apply) { + // Write the contents to stdout, regardless of whether any errors were fixed. + io::stdout().write_all(transformed.as_bytes())?; + } else if matches!(autofix, fix::FixMode::Diff) { + // But only write a diff if it's non-empty. + if fixed > 0 { + let text_diff = TextDiff::from_lines(contents, &transformed); + let mut unified_diff = text_diff.unified_diff(); + if let Some(path) = path { + unified_diff.header(&fs::relativize_path(path), &fs::relativize_path(path)); + } - if matches!(autofix, fix::FixMode::Apply) { - // Write the contents to stdout, regardless of whether any errors were fixed. - io::stdout().write_all(transformed.as_bytes())?; - } else if matches!(autofix, fix::FixMode::Diff) { - // But only write a diff if it's non-empty. - if fixed > 0 { - let text_diff = TextDiff::from_lines(contents, &transformed); - 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()?; } - - let mut stdout = io::stdout().lock(); - unified_diff.to_writer(&mut stdout)?; - stdout.write_all(b"\n")?; - stdout.flush()?; } - } - (result, fixed) + (result, fixed) + } else { + // If we fail to autofix, lint the original source code. + let result = lint_only( + contents, + path.unwrap_or_else(|| Path::new("-")), + package, + settings, + autofix.into(), + ); + let fixed = 0; + + // Write the contents to stdout anyway. + if matches!(autofix, fix::FixMode::Apply) { + io::stdout().write_all(contents.as_bytes())?; + } + + (result, fixed) + } } else { let result = lint_only( contents, diff --git a/src/linter.rs b/src/linter.rs index 02c1bb4420..8829fd957d 100644 --- a/src/linter.rs +++ b/src/linter.rs @@ -1,6 +1,6 @@ use std::path::Path; -use anyhow::Result; +use anyhow::{anyhow, Result}; use colored::Colorize; use rustpython_parser::error::ParseError; use rustpython_parser::lexer::LexResult; @@ -320,7 +320,7 @@ pub fn lint_fix( package: Option<&Path>, settings: &Settings, ) -> Result<(LinterResult>, String, usize)> { - let mut contents = contents.to_string(); + let mut transformed = contents.to_string(); // Track the number of fixed errors across iterations. let mut fixed = 0; @@ -328,16 +328,19 @@ pub fn lint_fix( // As an escape hatch, bail after 100 iterations. let mut iterations = 0; + // Track whether the _initial_ source code was parseable. + let mut parseable = false; + // Continuously autofix until the source code stabilizes. loop { // Tokenize once. - let tokens: Vec = rustpython_helpers::tokenize(&contents); + let tokens: Vec = rustpython_helpers::tokenize(&transformed); // Map row and column locations to byte slices (lazily). - let locator = Locator::new(&contents); + let locator = Locator::new(&transformed); // Detect the current code style (lazily). - let stylist = Stylist::from_contents(&contents, &locator); + let stylist = Stylist::from_contents(&transformed, &locator); // Extra indices from the code. let indexer: Indexer = tokens.as_slice().into(); @@ -350,7 +353,7 @@ pub fn lint_fix( let result = check_path( path, package, - &contents, + &transformed, tokens, &locator, &stylist, @@ -361,6 +364,32 @@ pub fn lint_fix( flags::Noqa::Enabled, ); + if iterations == 0 { + parseable = result.error.is_none(); + } else { + // If the source code was parseable on the first pass, but is no + // 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#" +{}: Autofix introduced a syntax error. Reverting all changes. + +This indicates a bug in `{}`. If you could open an issue at: + + {}/issues/new?title=%5BAutofix%20error%5D + +...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), + ); + return Err(anyhow!("Autofix introduced a syntax error")); + } + } + // Apply autofix. if let Some((fixed_contents, applied)) = fix_file(&result.data, &locator) { if iterations < MAX_ITERATIONS { @@ -368,7 +397,7 @@ pub fn lint_fix( fixed += applied; // Store the fixed contents. - contents = fixed_contents.to_string(); + transformed = fixed_contents.to_string(); // Increment the iteration count. iterations += 1; @@ -381,11 +410,11 @@ pub fn lint_fix( r#" {}: Failed to converge after {} iterations. -This likely indicates a bug in `{}`. If you could open an issue at: +This indicates a bug in `{}`. If you could open an issue at: -{}/issues/new?title=%5BInfinite%20loop%5D + {}/issues/new?title=%5BInfinite%20loop%5D -quoting the contents of `{}`, along with the `pyproject.toml` settings and executed command, we'd be very appreciative! +...quoting the contents of `{}`, along with the `pyproject.toml` settings and executed command, we'd be very appreciative! "#, "error".red().bold(), MAX_ITERATIONS, @@ -411,7 +440,7 @@ quoting the contents of `{}`, along with the `pyproject.toml` settings and execu }) .collect() }), - contents, + transformed, fixed, )); }