Update fix summary message in check --diff to include unsafe fix hints (#7790)

Requires #7769 

Updates the CLI display for `ruff check --fix` to hint availability of
unsafe fixes.

 ```
❯ ruff check example.py --select F601,T201 --diff --no-cache
No errors fixed (1 fix available with `--unsafe-fixes`).
```

```
❯ ruff check example.py --select F601,T201,W292 --diff --no-cache
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
 x = {'a': 1, 'a': 1}
-print(('foo'))
+print(('foo'))
\ No newline at end of file

Would fix 1 error (1 additional fix available with `--unsafe-fixes`).
```
```
❯ ruff check example.py --select F601,T201,W292 --diff --no-cache
--unsafe-fixes
--- example.py
+++ example.py
@@ -1,2 +1,2 @@
-x = {'a': 1}
-print(('foo'))
+x = {'a': 1, 'a': 1}
+print(('foo'))
\ No newline at end of file

Would fix 2 errors.
```
This commit is contained in:
Zanie Blue 2023-10-10 10:50:35 -05:00 committed by GitHub
parent d412e8ef74
commit 2b95d3832b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 136 additions and 66 deletions

View file

@ -102,12 +102,15 @@ impl Printer {
fn write_summary_text(&self, writer: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> { fn write_summary_text(&self, writer: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> {
if self.log_level >= LogLevel::Default { if self.log_level >= LogLevel::Default {
if self.flags.intersects(Flags::SHOW_VIOLATIONS) { let fixables = FixableStatistics::try_from(diagnostics, self.unsafe_fixes);
let fixed = diagnostics let fixed = diagnostics
.fixed .fixed
.values() .values()
.flat_map(std::collections::HashMap::values) .flat_map(std::collections::HashMap::values)
.sum::<usize>(); .sum::<usize>();
if self.flags.intersects(Flags::SHOW_VIOLATIONS) {
let remaining = diagnostics.messages.len(); let remaining = diagnostics.messages.len();
let total = fixed + remaining; let total = fixed + remaining;
if fixed > 0 { if fixed > 0 {
@ -121,15 +124,66 @@ impl Printer {
writeln!(writer, "Found {remaining} error{s}.")?; writeln!(writer, "Found {remaining} error{s}.")?;
} }
if let Some(fixables) = FixableSummary::try_from(diagnostics, self.unsafe_fixes) { if let Some(fixables) = fixables {
writeln!(writer, "{fixables}")?; let fix_prefix = format!("[{}]", "*".cyan());
if self.unsafe_fixes.is_enabled() {
writeln!(
writer,
"{fix_prefix} {} fixable with the --fix option.",
fixables.applicable
)?;
} else {
if fixables.applicable > 0 && fixables.unapplicable > 0 {
let es = if fixables.unapplicable == 1 { "" } else { "es" };
writeln!(writer,
"{fix_prefix} {} fixable with the `--fix` option ({} hidden fix{es} can be enabled with the `--unsafe-fixes` option).",
fixables.applicable, fixables.unapplicable
)?;
} else if fixables.applicable > 0 {
// Only applicable fixes
writeln!(
writer,
"{fix_prefix} {} fixable with the `--fix` option.",
fixables.applicable,
)?;
} else {
// Only unapplicable fixes
let es = if fixables.unapplicable == 1 { "" } else { "es" };
writeln!(writer,
"{} hidden fix{es} can be enabled with the `--unsafe-fixes` option.",
fixables.unapplicable
)?;
}
}
}
} else {
// Check if there are unapplied fixes
let unapplied = {
if let Some(fixables) = fixables {
fixables.unapplicable
} else {
0
}
};
if unapplied > 0 {
let es = if unapplied == 1 { "" } else { "es" };
if fixed > 0 {
let s = if fixed == 1 { "" } else { "s" };
if self.fix_mode.is_apply() {
writeln!(writer, "Fixed {fixed} error{s} ({unapplied} additional fix{es} available with `--unsafe-fixes`).")?;
} else {
writeln!(writer, "Would fix {fixed} error{s} ({unapplied} additional fix{es} available with `--unsafe-fixes`).")?;
}
} else {
if self.fix_mode.is_apply() {
writeln!(writer, "No errors fixed ({unapplied} fix{es} available with `--unsafe-fixes`).")?;
} else {
writeln!(writer, "No errors would be fixed ({unapplied} fix{es} available with `--unsafe-fixes`).")?;
}
} }
} else { } else {
let fixed = diagnostics
.fixed
.values()
.flat_map(std::collections::HashMap::values)
.sum::<usize>();
if fixed > 0 { if fixed > 0 {
let s = if fixed == 1 { "" } else { "s" }; let s = if fixed == 1 { "" } else { "s" };
if self.fix_mode.is_apply() { if self.fix_mode.is_apply() {
@ -140,6 +194,7 @@ impl Printer {
} }
} }
} }
}
Ok(()) Ok(())
} }
@ -170,7 +225,7 @@ impl Printer {
} }
let context = EmitterContext::new(&diagnostics.notebook_indexes); let context = EmitterContext::new(&diagnostics.notebook_indexes);
let fixables = FixableSummary::try_from(diagnostics, self.unsafe_fixes); let fixables = FixableStatistics::try_from(diagnostics, self.unsafe_fixes);
match self.format { match self.format {
SerializationFormat::Json => { SerializationFormat::Json => {
@ -354,7 +409,7 @@ impl Printer {
); );
} }
let fixables = FixableSummary::try_from(diagnostics, self.unsafe_fixes); let fixables = FixableStatistics::try_from(diagnostics, self.unsafe_fixes);
if !diagnostics.messages.is_empty() { if !diagnostics.messages.is_empty() {
if self.log_level >= LogLevel::Default { if self.log_level >= LogLevel::Default {
@ -388,13 +443,13 @@ fn num_digits(n: usize) -> usize {
} }
/// Return `true` if the [`Printer`] should indicate that a rule is fixable. /// Return `true` if the [`Printer`] should indicate that a rule is fixable.
fn show_fix_status(fix_mode: flags::FixMode, fixables: Option<&FixableSummary>) -> bool { fn show_fix_status(fix_mode: flags::FixMode, fixables: Option<&FixableStatistics>) -> bool {
// If we're in application mode, avoid indicating that a rule is fixable. // 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 // If the specific violation were truly fixable, it would've been fixed in
// this pass! (We're occasionally unable to determine whether a specific // this pass! (We're occasionally unable to determine whether a specific
// violation is fixable without trying to fix it, so if fix is not // violation is fixable without trying to fix it, so if fix is not
// enabled, we may inadvertently indicate that a rule is fixable.) // enabled, we may inadvertently indicate that a rule is fixable.)
(!fix_mode.is_apply()) && fixables.is_some_and(FixableSummary::any_applicable_fixes) (!fix_mode.is_apply()) && fixables.is_some_and(FixableStatistics::any_applicable_fixes)
} }
fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>) -> Result<()> { fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>) -> Result<()> {
@ -438,15 +493,14 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FxHashMap<String, FixTable>
Ok(()) Ok(())
} }
/// Summarizes [applicable][ruff_diagnostics::Applicability] fixes. /// Statistics for [applicable][ruff_diagnostics::Applicability] fixes.
#[derive(Debug)] #[derive(Debug)]
struct FixableSummary { struct FixableStatistics {
applicable: u32, applicable: u32,
unapplicable: u32, unapplicable: u32,
unsafe_fixes: UnsafeFixes,
} }
impl FixableSummary { impl FixableStatistics {
fn try_from(diagnostics: &Diagnostics, unsafe_fixes: UnsafeFixes) -> Option<Self> { fn try_from(diagnostics: &Diagnostics, unsafe_fixes: UnsafeFixes) -> Option<Self> {
let mut applicable = 0; let mut applicable = 0;
let mut unapplicable = 0; let mut unapplicable = 0;
@ -467,7 +521,6 @@ impl FixableSummary {
Some(Self { Some(Self {
applicable, applicable,
unapplicable, unapplicable,
unsafe_fixes,
}) })
} }
} }
@ -476,41 +529,3 @@ impl FixableSummary {
self.applicable > 0 self.applicable > 0
} }
} }
impl Display for FixableSummary {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let fix_prefix = format!("[{}]", "*".cyan());
if self.unsafe_fixes.is_enabled() {
write!(
f,
"{fix_prefix} {} fixable with the --fix option.",
self.applicable
)
} else {
if self.applicable > 0 && self.unapplicable > 0 {
let es = if self.unapplicable == 1 { "" } else { "es" };
write!(
f,
"{fix_prefix} {} fixable with the `--fix` option ({} hidden fix{es} can be enabled with the `--unsafe-fixes` option).",
self.applicable, self.unapplicable
)
} else if self.applicable > 0 {
// Only applicable fixes
write!(
f,
"{fix_prefix} {} fixable with the `--fix` option.",
self.applicable,
)
} else {
// Only unapplicable fixes
let es = if self.unapplicable == 1 { "" } else { "es" };
write!(
f,
"{} hidden fix{es} can be enabled with the `--unsafe-fixes` option.",
self.unapplicable
)
}
}
}
}

View file

@ -1035,6 +1035,35 @@ fn fix_applies_unsafe_fixes_with_opt_in() {
"###); "###);
} }
#[test]
fn fix_only_unsafe_fixes_available() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format",
"text",
"--isolated",
"--no-cache",
"--select",
"F601",
"--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 -----
-:1:14: F601 Dictionary key literal `'a'` repeated
Found 1 error.
1 hidden fix can be enabled with the `--unsafe-fixes` option.
"###);
}
#[test] #[test]
fn fix_only_flag_applies_safe_fixes_by_default() { fn fix_only_flag_applies_safe_fixes_by_default() {
assert_cmd_snapshot!( assert_cmd_snapshot!(
@ -1058,7 +1087,7 @@ fn fix_only_flag_applies_safe_fixes_by_default() {
print('foo') print('foo')
----- stderr ----- ----- stderr -----
Fixed 1 error. Fixed 1 error (1 additional fix available with `--unsafe-fixes`).
"###); "###);
} }
@ -1116,7 +1145,7 @@ fn diff_shows_safe_fixes_by_default() {
----- stderr ----- ----- stderr -----
Would fix 1 error. Would fix 1 error (1 additional fix available with `--unsafe-fixes`).
"### "###
); );
} }
@ -1153,3 +1182,29 @@ fn diff_shows_unsafe_fixes_with_opt_in() {
"### "###
); );
} }
#[test]
fn diff_only_unsafe_fixes_available() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args([
"-",
"--output-format",
"text",
"--isolated",
"--no-cache",
"--select",
"F601",
"--diff",
])
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
@r###"
success: false
exit_code: 1
----- stdout -----
----- stderr -----
No errors would be fixed (1 fix available with `--unsafe-fixes`).
"###
);
}