Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
12 changes: 9 additions & 3 deletions crates/ruff/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,7 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<Exi
// TODO: this should reference the global preview mode once https://github.com/astral-sh/ruff/issues/8232
// is resolved.
let preview = pyproject_config.settings.linter.preview;
let prefer_rule_codes = pyproject_config.settings.output_prefer_rule_codes;

if cli.watch {
// Configure the file watcher.
Expand All @@ -400,7 +401,7 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<Exi
fix_mode,
unsafe_fixes,
)?;
printer.write_continuously(&mut writer, &diagnostics, preview)?;
printer.write_continuously(&mut writer, &diagnostics, preview, prefer_rule_codes)?;

// In watch mode, we may need to re-resolve the configuration.
// TODO(charlie): Re-compute other derivative values, like the `printer`.
Expand All @@ -427,7 +428,7 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<Exi
fix_mode,
unsafe_fixes,
)?;
printer.write_continuously(&mut writer, &diagnostics, preview)?;
printer.write_continuously(&mut writer, &diagnostics, preview, prefer_rule_codes)?;
}
} else {
// Generate lint violations.
Expand Down Expand Up @@ -462,7 +463,12 @@ pub fn check(args: CheckCommand, global_options: GlobalConfigArgs) -> Result<Exi
if cli.statistics {
printer.write_statistics(&diagnostics, &mut summary_writer)?;
} else {
printer.write_once(&diagnostics, &mut summary_writer, preview)?;
printer.write_once(
&diagnostics,
&mut summary_writer,
preview,
prefer_rule_codes,
)?;
}

if !cli.exit_zero {
Expand Down
17 changes: 13 additions & 4 deletions crates/ruff/src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ impl Printer {
diagnostics: &Diagnostics,
writer: &mut dyn Write,
preview: PreviewMode,
prefer_rule_codes: bool,
) -> Result<()> {
if matches!(self.log_level, LogLevel::Silent) {
return Ok(());
Expand All @@ -223,7 +224,7 @@ impl Printer {
if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
if !diagnostics.fixed.is_empty() {
writeln!(writer)?;
print_fix_summary(writer, &diagnostics.fixed, preview)?;
print_fix_summary(writer, &diagnostics.fixed, preview, prefer_rule_codes)?;
writeln!(writer)?;
}
}
Expand All @@ -237,6 +238,7 @@ impl Printer {

let config = DisplayDiagnosticConfig::new("ruff")
.preview(preview.is_enabled())
.prefer_rule_codes(prefer_rule_codes)
.hide_severity(true)
.color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize())
.with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref()))
Expand All @@ -251,7 +253,7 @@ impl Printer {
if self.flags.intersects(Flags::SHOW_FIX_SUMMARY) {
if !diagnostics.fixed.is_empty() {
writeln!(writer)?;
print_fix_summary(writer, &diagnostics.fixed, preview)?;
print_fix_summary(writer, &diagnostics.fixed, preview, prefer_rule_codes)?;
writeln!(writer)?;
}
}
Expand Down Expand Up @@ -383,6 +385,7 @@ impl Printer {
writer: &mut dyn Write,
diagnostics: &Diagnostics,
preview: PreviewMode,
prefer_rule_codes: bool,
) -> Result<()> {
if matches!(self.log_level, LogLevel::Silent) {
return Ok(());
Expand Down Expand Up @@ -410,6 +413,7 @@ impl Printer {
let context = EmitterContext::new(&diagnostics.notebook_indexes);
let config = DisplayDiagnosticConfig::new("ruff")
.preview(preview.is_enabled())
.prefer_rule_codes(prefer_rule_codes)
.hide_severity(true)
.color(!cfg!(test) && colored::control::SHOULD_COLORIZE.should_colorize())
.with_show_fix_status(show_fix_status(self.fix_mode, fixables.as_ref()))
Expand Down Expand Up @@ -445,7 +449,12 @@ fn show_fix_status(fix_mode: flags::FixMode, fixables: Option<&FixableStatistics
(!fix_mode.is_apply()) && fixables.is_some_and(FixableStatistics::any_applicable_fixes)
}

fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap, preview: PreviewMode) -> Result<()> {
fn print_fix_summary(
writer: &mut dyn Write,
fixed: &FixMap,
preview: PreviewMode,
prefer_rule_codes: bool,
) -> Result<()> {
let total = fixed
.values()
.map(|table| table.counts().sum::<usize>())
Expand Down Expand Up @@ -475,7 +484,7 @@ fn print_fix_summary(writer: &mut dyn Write, fixed: &FixMap, preview: PreviewMod
":".cyan()
)?;
for (code, name, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) {
if is_human_readable_names_enabled(preview) {
if is_human_readable_names_enabled(preview) && !prefer_rule_codes {
writeln!(
writer,
" {count:>num_digits$} × {name} ({code})",
Expand Down
25 changes: 25 additions & 0 deletions crates/ruff/tests/cli/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4273,6 +4273,31 @@ class Foo:
);
}

#[test]
fn prefer_rule_codes_in_output() {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.args(STDIN_BASE_OPTIONS)
.args([
"--preview",
"--config",
"output-prefer-rule-codes = true",
"--select=A001",
"-",
])
.pass_stdin("print = 1\n"),
@"
success: false
exit_code: 1
----- stdout -----
-:1:1: A001 Variable `print` is shadowing a Python builtin
Found 1 error.

----- stderr -----
"
);
}

#[test_case::test_case("concise")]
#[test_case::test_case("full")]
#[test_case::test_case("json")]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ cache_dir = "[TMP]/foo/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = concise
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ cache_dir = "[TMP]/.ruff_cache"
fix = false
fix_only = false
output_format = full
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ cache_dir = "[TMP]/subdir/.ruff_cache"
fix = false
fix_only = false
output_format = full
output_prefer_rule_codes = false
show_fixes = false
unsafe_fixes = hint

Expand Down
16 changes: 16 additions & 0 deletions crates/ruff_db/src/diagnostic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1447,6 +1447,8 @@ pub struct DisplayDiagnosticConfig {
merge_window: usize,
/// Whether to use preview formatting for Ruff diagnostics.
preview: bool,
/// Whether to prefer rule codes over human-readable rule names in Ruff diagnostic output.
prefer_rule_codes: bool,
/// Whether to hide the real `Severity` of diagnostics.
///
/// This is intended for temporary use by Ruff, which only has a single `error` severity at the
Expand All @@ -1471,6 +1473,7 @@ impl DisplayDiagnosticConfig {
context: 2,
merge_window: 2,
preview: false,
prefer_rule_codes: false,
hide_severity: false,
show_fix_status: false,
fix_applicability: Applicability::Safe,
Expand Down Expand Up @@ -1535,6 +1538,19 @@ impl DisplayDiagnosticConfig {
self.preview
}

/// Whether to prefer rule codes over human-readable rule names, even in preview mode.
pub fn prefer_rule_codes(self, yes: bool) -> DisplayDiagnosticConfig {
DisplayDiagnosticConfig {
prefer_rule_codes: yes,
..self
}
}

/// Whether rule codes are explicitly preferred over human-readable rule names.
pub fn is_prefer_rule_codes_enabled(&self) -> bool {
self.prefer_rule_codes
}

/// Whether to hide a diagnostic's severity or not.
pub fn hide_severity(self, yes: bool) -> DisplayDiagnosticConfig {
DisplayDiagnosticConfig {
Expand Down
5 changes: 2 additions & 3 deletions crates/ruff_db/src/diagnostic/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,8 @@ impl<'a> ResolvedDiagnostic<'a> {
})
.collect();

let id = if !config.preview
&& let Some(code) = diag.secondary_code()
{
let use_code = !config.preview || config.prefer_rule_codes;
let id = if use_code && let Some(code) = diag.secondary_code() {
code.to_string()
} else if config.hide_severity {
// When Ruff gets real severities, we should put the colon back in
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_db/src/diagnostic/render/azure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl AzureRenderer<'_> {
)?;
}
}
let code = if self.config.preview {
let code = if self.config.preview && !self.config.prefer_rule_codes {

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.

We may use preview for more checks. But if not, could we set prefer_rule_codes to !linter.preview.is_enabled() || self.config.prefer_rule_codes? It may allow you to reduce the arguments that you need to pass to the rendering code

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.

Hmm, that's interesting. Codex originally tried to fold the preview && !prefer_rule_codes check into the existing preview field, which does work since this is all we're currently using preview for. I kept them separate in case we needed preview again in the future and because it seemed confusing to include that check in a field named preview. Removing the preview field and folding the condition into prefer_rule_codes sounds more appealing.

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.

After thinking about this a bit more, I'm again leaning toward keeping the fields separate. In a very strict sense, we are still using preview for two features: showing the real severity in JSON output and human-readable names. It seems possible that we will expand the preview-gate for severity in the near future and may want to keep that separate from human-readable names.

Happy to follow up if you feel strongly, of course.

diag.id().as_str()
} else {
diag.secondary_code_or_id()
Expand Down
7 changes: 3 additions & 4 deletions crates/ruff_db/src/diagnostic/render/concise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,9 @@ impl<'a> ConciseRenderer<'a> {
write!(f, "{sep} ")?;
}

let use_name = self.config.preview && !self.config.prefer_rule_codes;
if self.config.hide_severity {
if !self.config.preview
&& let Some(code) = diag.secondary_code()
{
if !use_name && let Some(code) = diag.secondary_code() {
write!(
f,
"{code} ",
Expand Down Expand Up @@ -100,7 +99,7 @@ impl<'a> ConciseRenderer<'a> {
Severity::Error => ("error", stylesheet.error),
Severity::Fatal => ("fatal", stylesheet.error),
};
let id = if self.config.preview {
let id = if use_name {
diag.id().as_str()
} else {
diag.secondary_code_or_id()
Expand Down
7 changes: 3 additions & 4 deletions crates/ruff_db/src/diagnostic/render/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ impl<'a> GithubRenderer<'a> {
Severity::Warning => "warning",
Severity::Error | Severity::Fatal => "error",
};
let code = if self.config.preview {
let use_name = self.config.preview && !self.config.prefer_rule_codes;
let code = if use_name {
diagnostic.id().as_str()
} else {
diagnostic.secondary_code_or_id()
Expand Down Expand Up @@ -90,9 +91,7 @@ impl<'a> GithubRenderer<'a> {
write!(f, "::")?;
}

if !self.config.preview
&& let Some(code) = diagnostic.secondary_code()
{
if !use_name && let Some(code) = diagnostic.secondary_code() {
write!(f, "{code}")?;
} else {
write!(f, "{id}:", id = diagnostic.id())?;
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_db/src/diagnostic/render/gitlab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl Serialize for SerializedMessages<'_> {
fingerprints.insert(message_fingerprint);

let description = diagnostic.concise_message();
let check_name = if self.config.preview {
let check_name = if self.config.preview && !self.config.prefer_rule_codes {
diagnostic.id().as_str()
} else {
diagnostic.secondary_code_or_id()
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_db/src/diagnostic/render/junit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ impl<'a> JunitRenderer<'a> {
start_location: location,
} = diagnostic;

let code = if self.config.preview {
let code = if self.config.preview && !self.config.prefer_rule_codes {
diagnostic.id().as_str()
} else {
diagnostic.secondary_code_or_id()
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_db/src/diagnostic/render/pylint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl PylintRenderer<'_> {
})
.unwrap_or_default();

let code = if self.config.preview {
let code = if self.config.preview && !self.config.prefer_rule_codes {
diagnostic.id().as_str()
} else {
diagnostic.secondary_code_or_id()
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff_db/src/diagnostic/render/rdjson.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ fn diagnostic_to_rdjson<'a>(
message: diagnostic.concise_message(),
location,
code: RdjsonCode {
value: if config.preview {
value: if config.preview && !config.prefer_rule_codes {
diagnostic.id().as_str()
} else {
diagnostic.secondary_code_or_id()
Expand Down
Loading
Loading