Skip to content

Commit

Permalink
Autofix Automatic/Suggested fixes with --fix
Browse files Browse the repository at this point in the history
  • Loading branch information
evanrittenhouse committed Jun 20, 2023
1 parent 64bd955 commit 1f707ff
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 106 deletions.
16 changes: 13 additions & 3 deletions crates/ruff/src/autofix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use nohash_hasher::IntSet;
use ruff_text_size::{TextLen, TextRange, TextSize};
use rustc_hash::FxHashMap;

use ruff_diagnostics::{Diagnostic, Edit, Fix, IsolationLevel};
use ruff_diagnostics::{Applicability, Diagnostic, Edit, Fix, IsolationLevel};
use ruff_python_ast::source_code::Locator;

use crate::autofix::source_map::SourceMap;
Expand All @@ -26,10 +26,20 @@ 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,
base_applicability: &Applicability,
) -> Option<FixResult> {
let mut with_fixes = diagnostics
.iter()
.filter(|diag| diag.fix.is_some())
.filter(|diag| {
if let Some(fix) = &diag.fix {
&fix.applicability() >= base_applicability
} else {
false
}
})
.peekable();

if with_fixes.peek().is_none() {
Expand Down
5 changes: 3 additions & 2 deletions crates/ruff/src/linter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_hash::FxHashMap;
use rustpython_parser::lexer::LexResult;
use rustpython_parser::ParseError;

use ruff_diagnostics::Diagnostic;
use ruff_diagnostics::{Applicability, Diagnostic};
use ruff_python_ast::imports::ImportMap;
use ruff_python_ast::source_code::{Indexer, Locator, SourceFileBuilder, Stylist};
use ruff_python_stdlib::path::is_python_stub_file;
Expand Down Expand Up @@ -399,6 +399,7 @@ pub fn lint_fix<'a>(
noqa: flags::Noqa,
settings: &Settings,
source_kind: &mut SourceKind,
base_applicability: &Applicability,
) -> Result<FixerResult<'a>> {
let mut transformed = Cow::Borrowed(contents);

Expand Down Expand Up @@ -469,7 +470,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, base_applicability)
{
if iterations < MAX_ITERATIONS {
// Count the number of fixed errors.
Expand Down
6 changes: 4 additions & 2 deletions crates/ruff/src/settings/flags.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use ruff_diagnostics::Applicability;

#[derive(Debug, Copy, Clone, Hash, is_macro::Is)]
pub enum FixMode {
Generate,
Apply,
Diff,
Apply(Applicability),
Diff(Applicability),
}

#[derive(Debug, Copy, Clone, Hash, result_like::BoolLike)]
Expand Down
8 changes: 6 additions & 2 deletions crates/ruff/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,12 @@ fn test_contents(source_kind: &mut SourceKind, path: &Path, settings: &Settings)
code: fixed_contents,
source_map,
..
}) = fix_file(&diagnostics, &Locator::new(&contents))
{
}) = fix_file(
&diagnostics,
&Locator::new(&contents),
// We treat Suggested/Automatic applicabilities as fixable, so take the superset.
&ruff_diagnostics::Applicability::Suggested,
) {
if iterations < max_iterations() {
iterations += 1;
} else {
Expand Down
189 changes: 99 additions & 90 deletions crates/ruff_cli/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,52 +157,56 @@ pub(crate) fn lint_path(
error: parse_error,
},
fixed,
) = if matches!(autofix, flags::FixMode::Apply | flags::FixMode::Diff) {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
&contents,
path,
package,
noqa,
&settings.lib,
&mut source_kind,
) {
if !fixed.is_empty() {
match autofix {
flags::FixMode::Apply => match &source_kind {
SourceKind::Python(_) => {
write(path, transformed.as_bytes())?;
}
SourceKind::Jupyter(notebook) => {
notebook.write(path)?;
) = match autofix {
flags::FixMode::Apply(base_applicability) | flags::FixMode::Diff(base_applicability) => {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
&contents,
path,
package,
noqa,
&settings.lib,
&mut source_kind,
&base_applicability,
) {
if !fixed.is_empty() {
match autofix {
flags::FixMode::Apply(_) => match &source_kind {
SourceKind::Python(_) => {
write(path, transformed.as_bytes())?;
}
SourceKind::Jupyter(notebook) => {
notebook.write(path)?;
}
},
flags::FixMode::Diff(_) => {
let mut stdout = io::stdout().lock();
TextDiff::from_lines(contents.as_str(), &transformed)
.unified_diff()
.header(&fs::relativize_path(path), &fs::relativize_path(path))
.to_writer(&mut stdout)?;
stdout.write_all(b"\n")?;
stdout.flush()?;
}
},
flags::FixMode::Diff => {
let mut stdout = io::stdout().lock();
TextDiff::from_lines(contents.as_str(), &transformed)
.unified_diff()
.header(&fs::relativize_path(path), &fs::relativize_path(path))
.to_writer(&mut stdout)?;
stdout.write_all(b"\n")?;
stdout.flush()?;
flags::FixMode::Generate => {}
}
flags::FixMode::Generate => {}
}
(result, fixed)
} else {
// If we fail to autofix, lint the original source code.
let result = lint_only(&contents, path, package, &settings.lib, noqa);
let fixed = FxHashMap::default();
(result, fixed)
}
(result, fixed)
} else {
// If we fail to autofix, lint the original source code.
}
flags::FixMode::Generate => {
let result = lint_only(&contents, path, package, &settings.lib, noqa);
let fixed = FxHashMap::default();
(result, fixed)
}
} else {
let result = lint_only(&contents, path, package, &settings.lib, noqa);
let fixed = FxHashMap::default();
(result, fixed)
};

let imports = imports.unwrap_or_default();
Expand Down Expand Up @@ -253,53 +257,74 @@ pub(crate) fn lint_stdin(
autofix: flags::FixMode,
) -> Result<Diagnostics> {
let mut source_kind = SourceKind::Python(contents.to_string());

// Lint the inputs.
let (
LinterResult {
data: (messages, imports),
error: parse_error,
},
fixed,
) = if matches!(autofix, flags::FixMode::Apply | flags::FixMode::Diff) {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
contents,
path.unwrap_or_else(|| Path::new("-")),
package,
noqa,
settings,
&mut source_kind,
) {
match autofix {
flags::FixMode::Apply => {
// Write the contents to stdout, regardless of whether any errors were fixed.
io::stdout().write_all(transformed.as_bytes())?;
}
flags::FixMode::Diff => {
// But only write a diff if it's non-empty.
if !fixed.is_empty() {
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));
}
) = match autofix {
flags::FixMode::Apply(base_applicability) | flags::FixMode::Diff(base_applicability) => {
if let Ok(FixerResult {
result,
transformed,
fixed,
}) = lint_fix(
contents,
path.unwrap_or_else(|| Path::new("-")),
package,
noqa,
settings,
&mut source_kind,
&base_applicability,
) {
match autofix {
flags::FixMode::Apply(_) => {
// Write the contents to stdout, regardless of whether any errors were fixed.
io::stdout().write_all(transformed.as_bytes())?;
}
flags::FixMode::Diff(_) => {
// But only write a diff if it's non-empty.
if !fixed.is_empty() {
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()?;
}
}
flags::FixMode::Generate => {}
}
flags::FixMode::Generate => {}
}

(result, fixed)
} else {
// If we fail to autofix, lint the original source code.
(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,
noqa,
);
let fixed = FxHashMap::default();

// Write the contents to stdout anyway.
if autofix.is_apply() {
io::stdout().write_all(contents.as_bytes())?;
}

(result, fixed)
}
}
flags::FixMode::Generate => {
let result = lint_only(
contents,
path.unwrap_or_else(|| Path::new("-")),
Expand All @@ -308,24 +333,8 @@ pub(crate) fn lint_stdin(
noqa,
);
let fixed = FxHashMap::default();

// Write the contents to stdout anyway.
if autofix.is_apply() {
io::stdout().write_all(contents.as_bytes())?;
}

(result, fixed)
}
} else {
let result = lint_only(
contents,
path.unwrap_or_else(|| Path::new("-")),
package,
settings,
noqa,
);
let fixed = FxHashMap::default();
(result, fixed)
};

let imports = imports.unwrap_or_default();
Expand Down
7 changes: 4 additions & 3 deletions crates/ruff_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use ruff::logging::{set_up_logging, LogLevel};
use ruff::settings::types::SerializationFormat;
use ruff::settings::{flags, CliSettings};
use ruff::{fs, warn_user_once};
use ruff_diagnostics::Applicability;
use ruff_python_formatter::format_module;

use crate::args::{Args, CheckArgs, Command};
Expand Down Expand Up @@ -198,9 +199,9 @@ pub fn check(args: CheckArgs, log_level: LogLevel) -> Result<ExitStatus> {
// - If `--diff` or `--fix-only` are set, don't print any violations (only
// fixes).
let autofix = if cli.diff {
flags::FixMode::Diff
flags::FixMode::Diff(Applicability::Automatic)
} else if fix || fix_only {
flags::FixMode::Apply
flags::FixMode::Apply(Applicability::Automatic)
} else {
flags::FixMode::Generate
};
Expand Down Expand Up @@ -339,7 +340,7 @@ pub fn check(args: CheckArgs, 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!(autofix, flags::FixMode::Apply | flags::FixMode::Diff)) {
if !(is_stdin && matches!(autofix, flags::FixMode::Apply(_) | flags::FixMode::Diff(_))) {
if cli.statistics {
printer.write_statistics(&diagnostics)?;
} else {
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff_diagnostics/src/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,21 @@ use crate::edit::Edit;
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,
Automatic = 2,

/// 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,
Suggested = 1,

/// 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,
Manual = -1,

/// The applicability of the fix is unknown.
#[default]
Unspecified,
Unspecified = 0,
}

/// Indicates the level of isolation required to apply a fix.
Expand Down

0 comments on commit 1f707ff

Please sign in to comment.