Skip to content

Commit 970f8b4

Browse files
zaniebkonstin
authored andcommitted
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. ```
1 parent e22937e commit 970f8b4

File tree

2 files changed

+136
-66
lines changed

2 files changed

+136
-66
lines changed

crates/ruff_cli/src/printer.rs

Lines changed: 79 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,15 @@ impl Printer {
102102

103103
fn write_summary_text(&self, writer: &mut dyn Write, diagnostics: &Diagnostics) -> Result<()> {
104104
if self.log_level >= LogLevel::Default {
105+
let fixables = FixableStatistics::try_from(diagnostics, self.unsafe_fixes);
106+
107+
let fixed = diagnostics
108+
.fixed
109+
.values()
110+
.flat_map(std::collections::HashMap::values)
111+
.sum::<usize>();
112+
105113
if self.flags.intersects(Flags::SHOW_VIOLATIONS) {
106-
let fixed = diagnostics
107-
.fixed
108-
.values()
109-
.flat_map(std::collections::HashMap::values)
110-
.sum::<usize>();
111114
let remaining = diagnostics.messages.len();
112115
let total = fixed + remaining;
113116
if fixed > 0 {
@@ -121,21 +124,73 @@ impl Printer {
121124
writeln!(writer, "Found {remaining} error{s}.")?;
122125
}
123126

124-
if let Some(fixables) = FixableSummary::try_from(diagnostics, self.unsafe_fixes) {
125-
writeln!(writer, "{fixables}")?;
127+
if let Some(fixables) = fixables {
128+
let fix_prefix = format!("[{}]", "*".cyan());
129+
130+
if self.unsafe_fixes.is_enabled() {
131+
writeln!(
132+
writer,
133+
"{fix_prefix} {} fixable with the --fix option.",
134+
fixables.applicable
135+
)?;
136+
} else {
137+
if fixables.applicable > 0 && fixables.unapplicable > 0 {
138+
let es = if fixables.unapplicable == 1 { "" } else { "es" };
139+
writeln!(writer,
140+
"{fix_prefix} {} fixable with the `--fix` option ({} hidden fix{es} can be enabled with the `--unsafe-fixes` option).",
141+
fixables.applicable, fixables.unapplicable
142+
)?;
143+
} else if fixables.applicable > 0 {
144+
// Only applicable fixes
145+
writeln!(
146+
writer,
147+
"{fix_prefix} {} fixable with the `--fix` option.",
148+
fixables.applicable,
149+
)?;
150+
} else {
151+
// Only unapplicable fixes
152+
let es = if fixables.unapplicable == 1 { "" } else { "es" };
153+
writeln!(writer,
154+
"{} hidden fix{es} can be enabled with the `--unsafe-fixes` option.",
155+
fixables.unapplicable
156+
)?;
157+
}
158+
}
126159
}
127160
} else {
128-
let fixed = diagnostics
129-
.fixed
130-
.values()
131-
.flat_map(std::collections::HashMap::values)
132-
.sum::<usize>();
133-
if fixed > 0 {
134-
let s = if fixed == 1 { "" } else { "s" };
135-
if self.fix_mode.is_apply() {
136-
writeln!(writer, "Fixed {fixed} error{s}.")?;
161+
// Check if there are unapplied fixes
162+
let unapplied = {
163+
if let Some(fixables) = fixables {
164+
fixables.unapplicable
137165
} else {
138-
writeln!(writer, "Would fix {fixed} error{s}.")?;
166+
0
167+
}
168+
};
169+
170+
if unapplied > 0 {
171+
let es = if unapplied == 1 { "" } else { "es" };
172+
if fixed > 0 {
173+
let s = if fixed == 1 { "" } else { "s" };
174+
if self.fix_mode.is_apply() {
175+
writeln!(writer, "Fixed {fixed} error{s} ({unapplied} additional fix{es} available with `--unsafe-fixes`).")?;
176+
} else {
177+
writeln!(writer, "Would fix {fixed} error{s} ({unapplied} additional fix{es} available with `--unsafe-fixes`).")?;
178+
}
179+
} else {
180+
if self.fix_mode.is_apply() {
181+
writeln!(writer, "No errors fixed ({unapplied} fix{es} available with `--unsafe-fixes`).")?;
182+
} else {
183+
writeln!(writer, "No errors would be fixed ({unapplied} fix{es} available with `--unsafe-fixes`).")?;
184+
}
185+
}
186+
} else {
187+
if fixed > 0 {
188+
let s = if fixed == 1 { "" } else { "s" };
189+
if self.fix_mode.is_apply() {
190+
writeln!(writer, "Fixed {fixed} error{s}.")?;
191+
} else {
192+
writeln!(writer, "Would fix {fixed} error{s}.")?;
193+
}
139194
}
140195
}
141196
}
@@ -170,7 +225,7 @@ impl Printer {
170225
}
171226

172227
let context = EmitterContext::new(&diagnostics.notebook_indexes);
173-
let fixables = FixableSummary::try_from(diagnostics, self.unsafe_fixes);
228+
let fixables = FixableStatistics::try_from(diagnostics, self.unsafe_fixes);
174229

175230
match self.format {
176231
SerializationFormat::Json => {
@@ -354,7 +409,7 @@ impl Printer {
354409
);
355410
}
356411

357-
let fixables = FixableSummary::try_from(diagnostics, self.unsafe_fixes);
412+
let fixables = FixableStatistics::try_from(diagnostics, self.unsafe_fixes);
358413

359414
if !diagnostics.messages.is_empty() {
360415
if self.log_level >= LogLevel::Default {
@@ -388,13 +443,13 @@ fn num_digits(n: usize) -> usize {
388443
}
389444

390445
/// Return `true` if the [`Printer`] should indicate that a rule is fixable.
391-
fn show_fix_status(fix_mode: flags::FixMode, fixables: Option<&FixableSummary>) -> bool {
446+
fn show_fix_status(fix_mode: flags::FixMode, fixables: Option<&FixableStatistics>) -> bool {
392447
// If we're in application mode, avoid indicating that a rule is fixable.
393448
// If the specific violation were truly fixable, it would've been fixed in
394449
// this pass! (We're occasionally unable to determine whether a specific
395450
// violation is fixable without trying to fix it, so if fix is not
396451
// enabled, we may inadvertently indicate that a rule is fixable.)
397-
(!fix_mode.is_apply()) && fixables.is_some_and(FixableSummary::any_applicable_fixes)
452+
(!fix_mode.is_apply()) && fixables.is_some_and(FixableStatistics::any_applicable_fixes)
398453
}
399454

400455
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>
438493
Ok(())
439494
}
440495

441-
/// Summarizes [applicable][ruff_diagnostics::Applicability] fixes.
496+
/// Statistics for [applicable][ruff_diagnostics::Applicability] fixes.
442497
#[derive(Debug)]
443-
struct FixableSummary {
498+
struct FixableStatistics {
444499
applicable: u32,
445500
unapplicable: u32,
446-
unsafe_fixes: UnsafeFixes,
447501
}
448502

449-
impl FixableSummary {
503+
impl FixableStatistics {
450504
fn try_from(diagnostics: &Diagnostics, unsafe_fixes: UnsafeFixes) -> Option<Self> {
451505
let mut applicable = 0;
452506
let mut unapplicable = 0;
@@ -467,7 +521,6 @@ impl FixableSummary {
467521
Some(Self {
468522
applicable,
469523
unapplicable,
470-
unsafe_fixes,
471524
})
472525
}
473526
}
@@ -476,41 +529,3 @@ impl FixableSummary {
476529
self.applicable > 0
477530
}
478531
}
479-
480-
impl Display for FixableSummary {
481-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
482-
let fix_prefix = format!("[{}]", "*".cyan());
483-
484-
if self.unsafe_fixes.is_enabled() {
485-
write!(
486-
f,
487-
"{fix_prefix} {} fixable with the --fix option.",
488-
self.applicable
489-
)
490-
} else {
491-
if self.applicable > 0 && self.unapplicable > 0 {
492-
let es = if self.unapplicable == 1 { "" } else { "es" };
493-
write!(
494-
f,
495-
"{fix_prefix} {} fixable with the `--fix` option ({} hidden fix{es} can be enabled with the `--unsafe-fixes` option).",
496-
self.applicable, self.unapplicable
497-
)
498-
} else if self.applicable > 0 {
499-
// Only applicable fixes
500-
write!(
501-
f,
502-
"{fix_prefix} {} fixable with the `--fix` option.",
503-
self.applicable,
504-
)
505-
} else {
506-
// Only unapplicable fixes
507-
let es = if self.unapplicable == 1 { "" } else { "es" };
508-
write!(
509-
f,
510-
"{} hidden fix{es} can be enabled with the `--unsafe-fixes` option.",
511-
self.unapplicable
512-
)
513-
}
514-
}
515-
}
516-
}

crates/ruff_cli/tests/integration_test.rs

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1035,6 +1035,35 @@ fn fix_applies_unsafe_fixes_with_opt_in() {
10351035
"###);
10361036
}
10371037

1038+
#[test]
1039+
fn fix_only_unsafe_fixes_available() {
1040+
assert_cmd_snapshot!(
1041+
Command::new(get_cargo_bin(BIN_NAME))
1042+
.args([
1043+
"-",
1044+
"--output-format",
1045+
"text",
1046+
"--isolated",
1047+
"--no-cache",
1048+
"--select",
1049+
"F601",
1050+
"--fix",
1051+
])
1052+
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
1053+
@r###"
1054+
success: false
1055+
exit_code: 1
1056+
----- stdout -----
1057+
x = {'a': 1, 'a': 1}
1058+
print(('foo'))
1059+
1060+
----- stderr -----
1061+
-:1:14: F601 Dictionary key literal `'a'` repeated
1062+
Found 1 error.
1063+
1 hidden fix can be enabled with the `--unsafe-fixes` option.
1064+
"###);
1065+
}
1066+
10381067
#[test]
10391068
fn fix_only_flag_applies_safe_fixes_by_default() {
10401069
assert_cmd_snapshot!(
@@ -1058,7 +1087,7 @@ fn fix_only_flag_applies_safe_fixes_by_default() {
10581087
print('foo')
10591088
10601089
----- stderr -----
1061-
Fixed 1 error.
1090+
Fixed 1 error (1 additional fix available with `--unsafe-fixes`).
10621091
"###);
10631092
}
10641093

@@ -1116,7 +1145,7 @@ fn diff_shows_safe_fixes_by_default() {
11161145
11171146
11181147
----- stderr -----
1119-
Would fix 1 error.
1148+
Would fix 1 error (1 additional fix available with `--unsafe-fixes`).
11201149
"###
11211150
);
11221151
}
@@ -1153,3 +1182,29 @@ fn diff_shows_unsafe_fixes_with_opt_in() {
11531182
"###
11541183
);
11551184
}
1185+
1186+
#[test]
1187+
fn diff_only_unsafe_fixes_available() {
1188+
assert_cmd_snapshot!(
1189+
Command::new(get_cargo_bin(BIN_NAME))
1190+
.args([
1191+
"-",
1192+
"--output-format",
1193+
"text",
1194+
"--isolated",
1195+
"--no-cache",
1196+
"--select",
1197+
"F601",
1198+
"--diff",
1199+
])
1200+
.pass_stdin("x = {'a': 1, 'a': 1}\nprint(('foo'))\n"),
1201+
@r###"
1202+
success: false
1203+
exit_code: 1
1204+
----- stdout -----
1205+
1206+
----- stderr -----
1207+
No errors would be fixed (1 fix available with `--unsafe-fixes`).
1208+
"###
1209+
);
1210+
}

0 commit comments

Comments
 (0)