Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions crates/ruff/src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ bitflags! {
const SHOW_VIOLATIONS = 1 << 0;
/// Whether to show a summary of the fixed violations when emitting diagnostics.
const SHOW_FIX_SUMMARY = 1 << 1;
/// Whether to show a diff of each fixed violation when emitting diagnostics.
const SHOW_FIX_DIFF = 1 << 2;
}
}

Expand Down Expand Up @@ -260,9 +258,9 @@ impl Printer {
OutputFormat::Concise | OutputFormat::Full => {
TextEmitter::default()
.with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref()))
.with_show_fix_diff(self.flags.intersects(Flags::SHOW_FIX_DIFF))
.with_show_fix_diff(self.format == OutputFormat::Full && preview)
.with_show_source(self.format == OutputFormat::Full)
.with_unsafe_fixes(self.unsafe_fixes)
.with_fix_applicability(self.unsafe_fixes.required_applicability())
.with_preview(preview)
.emit(writer, &diagnostics.inner, &context)?;

Expand Down Expand Up @@ -464,7 +462,7 @@ impl Printer {
TextEmitter::default()
.with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref()))
.with_show_source(preview)
.with_unsafe_fixes(self.unsafe_fixes)
.with_fix_applicability(self.unsafe_fixes.required_applicability())
.emit(writer, &diagnostics.inner, &context)?;
}
writer.flush()?;
Expand Down
30 changes: 30 additions & 0 deletions crates/ruff/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5830,3 +5830,33 @@ nested_optional: Optional[Optional[Optional[str]]] = None
",
);
}

#[test]
fn show_fixes_in_full_output_with_preview_enabled() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args(["check", "--no-cache", "--output-format", "full"])
.args(["--select", "F401"])
.arg("--preview")
.arg("-")
.pass_stdin("import math"),
@r"
success: false
exit_code: 1
----- stdout -----
F401 [*] `math` imported but unused
--> -:1:8
|
1 | import math
| ^^^^
|
help: Remove unused import: `math`
- import math

Found 1 error.
[*] 1 fixable with the `--fix` option.

----- stderr -----
",
);
}
7 changes: 7 additions & 0 deletions crates/ruff_db/src/diagnostic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,13 @@ impl Diagnostic {
self.fix().is_some()
}

/// Returns `true` if the diagnostic is [`fixable`](Diagnostic::fixable) and applies at the
/// configured applicability level.
pub fn fix_applies(&self, config: &DisplayDiagnosticConfig) -> bool {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: fix_applicable / is_fix_applicable

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was also thinking about this but landed on not saying something :) I think my best idea was has_applicable_fix

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I think the is_applicable verbiage makes more sense on the fix type)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(@ntBre un-resolving because I commented as you were resolving. Just for visibility — I don't have strong feelings)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah diagnostic.has_applicable_fix does read nicely, thanks!

self.fix()
.is_some_and(|fix| fix.applies(config.fix_applicability))
}

/// Returns the offset of the parent statement for this diagnostic if it exists.
///
/// This is primarily used for checking noqa/secondary code suppressions.
Expand Down
4 changes: 1 addition & 3 deletions crates/ruff_db/src/diagnostic/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,9 +254,7 @@ impl<'a> ResolvedDiagnostic<'a> {
id,
message: diag.inner.message.as_str().to_string(),
annotations,
is_fixable: diag
.fix()
.is_some_and(|fix| fix.applies(config.fix_applicability)),
is_fixable: diag.fix_applies(config),
}
}

Expand Down
8 changes: 3 additions & 5 deletions crates/ruff_db/src/diagnostic/render/concise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,9 @@ impl<'a> ConciseRenderer<'a> {
)?;
}
if self.config.show_fix_status {
if let Some(fix) = diag.fix() {
// Do not display an indicator for inapplicable fixes
if fix.applies(self.config.fix_applicability) {
write!(f, "[{fix}] ", fix = fmt_styled("*", stylesheet.separator))?;
}
// Do not display an indicator for inapplicable fixes
if diag.fix_applies(self.config) {
write!(f, "[{fix}] ", fix = fmt_styled("*", stylesheet.separator))?;
}
}
} else {
Expand Down
12 changes: 8 additions & 4 deletions crates/ruff_db/src/diagnostic/render/full.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl<'a> FullRenderer<'a> {
writeln!(f, "{}", renderer.render(diag.to_annotate()))?;
}

if self.config.show_fix_diff {
if self.config.show_fix_diff && diag.fix_applies(self.config) {
if let Some(diff) = Diff::from_diagnostic(diag, &stylesheet, self.resolver) {
write!(f, "{diff}")?;
}
Expand Down Expand Up @@ -697,6 +697,8 @@ print()
fn notebook_output_with_diff() {
let (mut env, diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full);
env.show_fix_diff(true);
env.fix_applicability(Applicability::DisplayOnly);

insta::assert_snapshot!(env.render_diagnostics(&diagnostics), @r"
error[unused-import][*]: `os` imported but unused
--> notebook.ipynb:cell 1:2:8
Expand Down Expand Up @@ -726,7 +728,7 @@ print()
2 |
3 | print('hello world')

error[unused-variable]: Local variable `x` is assigned to but never used
error[unused-variable][*]: Local variable `x` is assigned to but never used
--> notebook.ipynb:cell 3:4:5
|
2 | def foo():
Expand All @@ -749,6 +751,7 @@ print()
fn notebook_output_with_diff_spanning_cells() {
let (mut env, mut diagnostics) = create_notebook_diagnostics(DiagnosticFormat::Full);
env.show_fix_diff(true);
env.fix_applicability(Applicability::DisplayOnly);

// Move all of the edits from the later diagnostics to the first diagnostic to simulate a
// single diagnostic with edits in different cells.
Expand All @@ -761,7 +764,7 @@ print()
*fix = Fix::unsafe_edits(edits.remove(0), edits);

insta::assert_snapshot!(env.render(&diagnostic), @r"
error[unused-import]: `os` imported but unused
error[unused-import][*]: `os` imported but unused
--> notebook.ipynb:cell 1:2:8
|
1 | # cell 1
Expand Down Expand Up @@ -924,6 +927,7 @@ line 10
env.add("example.py", contents);
env.format(DiagnosticFormat::Full);
env.show_fix_diff(true);
env.fix_applicability(Applicability::DisplayOnly);

let mut diagnostic = env.err().primary("example.py", "3", "3", "label").build();
diagnostic.help("Start of diff:");
Expand All @@ -936,7 +940,7 @@ line 10
)));

insta::assert_snapshot!(env.render(&diagnostic), @r"
error[test-diagnostic]: main diagnostic message
error[test-diagnostic][*]: main diagnostic message
--> example.py:3:1
|
1 | line 1
Expand Down
14 changes: 6 additions & 8 deletions crates/ruff_linter/src/message/text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use std::io::Write;
use ruff_db::diagnostic::{
Diagnostic, DiagnosticFormat, DisplayDiagnosticConfig, DisplayDiagnostics,
};
use ruff_diagnostics::Applicability;

use crate::message::{Emitter, EmitterContext};
use crate::settings::types::UnsafeFixes;

pub struct TextEmitter {
config: DisplayDiagnosticConfig,
Expand Down Expand Up @@ -46,10 +46,8 @@ impl TextEmitter {
}

#[must_use]
pub fn with_unsafe_fixes(mut self, unsafe_fixes: UnsafeFixes) -> Self {
self.config = self
.config
.fix_applicability(unsafe_fixes.required_applicability());
pub fn with_fix_applicability(mut self, applicability: Applicability) -> Self {
self.config = self.config.fix_applicability(applicability);
self
}

Expand Down Expand Up @@ -86,13 +84,13 @@ impl Emitter for TextEmitter {
#[cfg(test)]
mod tests {
use insta::assert_snapshot;
use ruff_diagnostics::Applicability;

use crate::message::TextEmitter;
use crate::message::tests::{
capture_emitter_notebook_output, capture_emitter_output, create_diagnostics,
create_notebook_diagnostics, create_syntax_error_diagnostics,
};
use crate::settings::types::UnsafeFixes;

#[test]
fn default() {
Expand All @@ -117,7 +115,7 @@ mod tests {
let mut emitter = TextEmitter::default()
.with_show_fix_status(true)
.with_show_source(true)
.with_unsafe_fixes(UnsafeFixes::Enabled);
.with_fix_applicability(Applicability::Unsafe);
let content = capture_emitter_output(&mut emitter, &create_diagnostics());

assert_snapshot!(content);
Expand All @@ -128,7 +126,7 @@ mod tests {
let mut emitter = TextEmitter::default()
.with_show_fix_status(true)
.with_show_source(true)
.with_unsafe_fixes(UnsafeFixes::Enabled);
.with_fix_applicability(Applicability::Unsafe);
let (messages, notebook_indexes) = create_notebook_diagnostics();
let content = capture_emitter_notebook_output(&mut emitter, &messages, &notebook_indexes);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/eradicate/mod.rs
---
ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:1:1
|
1 | #import os
Expand All @@ -16,7 +16,7 @@ help: Remove commented-out code
3 | a = 4
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:2:1
|
1 | #import os
Expand All @@ -33,7 +33,7 @@ help: Remove commented-out code
4 | #foo(1, 2, 3)
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:3:1
|
1 | #import os
Expand All @@ -52,7 +52,7 @@ help: Remove commented-out code
5 |
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:5:1
|
3 | #a = 3
Expand All @@ -72,7 +72,7 @@ help: Remove commented-out code
7 | content = 1 # print('hello')
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:13:5
|
11 | # This is a real comment.
Expand All @@ -91,7 +91,7 @@ help: Remove commented-out code
15 | #import os # noqa: ERA001
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:21:5
|
19 | class A():
Expand All @@ -109,7 +109,7 @@ help: Remove commented-out code
23 | dictionary = {
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:26:5
|
24 | dictionary = {
Expand All @@ -129,7 +129,7 @@ help: Remove commented-out code
28 |
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:27:5
|
25 | # "key1": 123, # noqa: ERA001
Expand All @@ -148,7 +148,7 @@ help: Remove commented-out code
29 | #import os # noqa
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:32:1
|
30 | #import os # noqa
Expand All @@ -168,7 +168,7 @@ help: Remove commented-out code
34 | # try: print()
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:33:1
|
32 | # case 1:
Expand All @@ -187,7 +187,7 @@ help: Remove commented-out code
35 | # except:
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:34:1
|
32 | # case 1:
Expand All @@ -207,7 +207,7 @@ help: Remove commented-out code
36 | # except Foo:
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:35:1
|
33 | # try:
Expand All @@ -227,7 +227,7 @@ help: Remove commented-out code
37 | # except Exception as e: print(e)
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:36:1
|
34 | # try: # with comment
Expand All @@ -247,7 +247,7 @@ help: Remove commented-out code
38 |
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:37:1
|
35 | # try: print()
Expand All @@ -266,7 +266,7 @@ help: Remove commented-out code
39 |
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:38:1
|
36 | # except:
Expand All @@ -284,7 +284,7 @@ help: Remove commented-out code
40 | # Script tag without an opening tag (Error)
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:44:1
|
43 | # requires-python = ">=3.11"
Expand All @@ -303,7 +303,7 @@ help: Remove commented-out code
46 | # ]
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:47:1
|
45 | # "requests<3",
Expand All @@ -322,7 +322,7 @@ help: Remove commented-out code
49 | # Script tag (OK)
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:75:1
|
73 | # /// script
Expand All @@ -342,7 +342,7 @@ help: Remove commented-out code
77 | # ]
note: This is a display-only fix and is likely to be incorrect

ERA001 Found commented-out code
ERA001 [*] Found commented-out code
--> ERA001.py:78:1
|
76 | # "requests<3",
Expand Down
Loading
Loading