Notify user if autofix introduces syntax error (#2507)

This commit is contained in:
Charlie Marsh 2023-02-02 20:02:09 -05:00 committed by GitHub
parent cb0f226962
commit bc81cea4f4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 96 additions and 43 deletions

View file

@ -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,

View file

@ -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<Vec<Message>>, 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<LexResult> = rustpython_helpers::tokenize(&contents);
let tokens: Vec<LexResult> = 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,
));
}