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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/ruff/src/commands/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ mod test {
}

// Configure
let snapshot = format!("{}_{}", rule_code.noqa_code(), path);
let snapshot = format!("{}_{}", rule_code.name(), path);
// invalid pyproject.toml is not active by default
let settings = Settings {
linter: LinterSettings::for_rules(vec![rule_code, Rule::InvalidPyprojectToml]),
Expand Down
3 changes: 2 additions & 1 deletion crates/ruff/src/commands/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1096,6 +1096,7 @@ pub(super) fn warn_incompatible_formatter_settings(resolver: &Resolver) {
// The formatter always removes blank lines before the docstring.
Rule::IncorrectBlankLineBeforeClass,
] {
debug_assert!(rule.noqa_code().is_some());
if setting.linter.rules.enabled(rule) {
incompatible_rules.insert(rule);
}
Expand All @@ -1105,7 +1106,7 @@ pub(super) fn warn_incompatible_formatter_settings(resolver: &Resolver) {
if !incompatible_rules.is_empty() {
let mut rule_names: Vec<_> = incompatible_rules
.into_iter()
.map(|rule| format!("`{}`", rule.noqa_code()))
.map(|rule| format!("`{}`", rule.noqa_code().unwrap()))

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.

This will be easy to miss when stabilizing rule names. Can we show name (code) instead?

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.

Sure, then I can drop the debug_assert too.

.collect();
rule_names.sort();
if let [rule] = rule_names.as_slice() {
Expand Down
35 changes: 21 additions & 14 deletions crates/ruff/src/commands/rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use crate::args::HelpFormat;
#[derive(Serialize)]
struct Explanation<'a> {
name: &'a str,
code: String,
linter: &'a str,
code: Option<String>,
linter: Option<&'a str>,
summary: &'a str,
message_formats: &'a [&'a str],
fix: String,
Expand All @@ -31,13 +31,16 @@ struct Explanation<'a> {

impl<'a> Explanation<'a> {
fn from_rule(rule: &'a Rule) -> Self {
let code = rule.noqa_code().to_string();
let (linter, _) = Linter::parse_code(&code).unwrap();
let code = rule.noqa_code().map(|code| code.to_string());
let linter = code
.as_deref()
.and_then(Linter::parse_code)
.map(|(linter, _)| linter.name());
let fix = rule.fixable().to_string();
Self {
name: rule.name().as_str(),
code,
linter: linter.name(),
linter,
summary: rule.message_formats()[0],
message_formats: rule.message_formats(),
fix,
Expand All @@ -56,18 +59,22 @@ impl<'a> Explanation<'a> {

fn format_rule_text(rule: Rule) -> String {
let mut output = String::new();
let _ = write!(&mut output, "# {} ({})", rule.name(), rule.noqa_code());
let _ = write!(&mut output, "# {}", rule.name_and_code());
output.push('\n');
output.push('\n');

let (linter, _) = Linter::parse_code(&rule.noqa_code().to_string()).unwrap();
let _ = write!(
&mut output,
"Derived from the **{}** linter.",
linter.name()
);
output.push('\n');
output.push('\n');
if let Some(linter) = rule
.noqa_code()
.and_then(|code| Linter::parse_code(&code.to_string()).map(|(linter, _)| linter))
{
let _ = write!(
&mut output,
"Derived from the **{}** linter.",
linter.name()
);
output.push('\n');
output.push('\n');
}

let fix_availability = rule.fixable();
if matches!(
Expand Down
Comment thread
ntBre marked this conversation as resolved.
File renamed without changes.
4 changes: 2 additions & 2 deletions crates/ruff/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ impl AddAssign for FixMap {
continue;
}
let fixed_in_file = self.0.entry(filename).or_default();
for (rule, name, count) in fixed.iter() {
for (id, code, count) in fixed.iter() {

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.

What we do in this commit seems the right approach to me. We use the name as identifier, and only carry code along because we need it to preserve existing display behavior.

if count > 0 {
*fixed_in_file.entry(rule).or_default(name) += count;
*fixed_in_file.entry(id).or_default(code) += count;
}
}
}
Expand Down
33 changes: 16 additions & 17 deletions crates/ruff/src/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl ExpandedStatistics<'_> {
}
}

/// Accumulator type for grouping diagnostics by code.
/// Accumulator type for grouping diagnostics by code or rule name.
Comment thread
ntBre marked this conversation as resolved.
Outdated
/// Format: (`code`, `representative_diagnostic`, `total_count`, `fixable_count`)
type DiagnosticGroup<'a> = (Option<&'a SecondaryCode>, &'a Diagnostic, usize, usize);

Expand Down Expand Up @@ -280,15 +280,15 @@ impl Printer {
let statistics: Vec<ExpandedStatistics> = diagnostics
.inner
.iter()
.sorted_by_key(|diagnostic| diagnostic.secondary_code())
.sorted_by_key(|diagnostic| diagnostic.name())
.fold(vec![], |mut acc: Vec<DiagnosticGroup>, diagnostic| {
let is_fixable = diagnostic
.fix()
.is_some_and(|fix| fix.applies(required_applicability));
let code = diagnostic.secondary_code();

if let Some((prev_code, _prev_message, count, fixable_count)) = acc.last_mut() {
if *prev_code == code {
if let Some((_prev_code, prev_message, count, fixable_count)) = acc.last_mut() {
if prev_message.name() == diagnostic.name() {
*count += 1;
if is_fixable {
*fixable_count += 1;
Expand Down Expand Up @@ -500,19 +500,18 @@ fn print_fix_summary(
relativize_path(filename).bold(),
":".cyan()
)?;
for (code, name, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) {
if is_human_readable_names_enabled(preview) && !prefer_rule_codes {
writeln!(
writer,
" {count:>num_digits$} × {name} ({code})",
name = name.to_string().red().bold(),
)?;
} else {
writeln!(
writer,
" {count:>num_digits$} × {code} ({name})",
code = code.to_string().red().bold(),
)?;
for (name, code, count) in table.iter().sorted_by_key(|(.., count)| Reverse(*count)) {
write!(writer, " {count:>num_digits$} × ")?;
match code {
Some(code) if is_human_readable_names_enabled(preview) && !prefer_rule_codes => {
writeln!(writer, "{name} ({code})", name = name.as_str().red().bold())?;
}
Some(code) => {
writeln!(writer, "{code} ({name})", code = code.as_str().red().bold())?;
}
None => {
writeln!(writer, "{name}", name = name.as_str().red().bold())?;
}
}
Comment thread
MichaReiser marked this conversation as resolved.
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/ruff_dev/src/generate_default_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub(crate) fn generate() -> String {
output.push_str(" select = [\n");
for (_, rules) in &linters {
for rule in rules {
let _ = writeln!(output, " \"{}\",", rule.noqa_code());
let _ = writeln!(output, " \"{}\",", rule.noqa_code().unwrap());
Comment thread
MichaReiser marked this conversation as resolved.
}
}
output.push_str(" ]\n");
Expand All @@ -56,7 +56,7 @@ pub(crate) fn generate() -> String {

for rule in rules {
let name = rule.name();
let code = rule.noqa_code();
let code = rule.noqa_code().unwrap();
let _ = writeln!(output, "- [`{name}`](rules/{name}.md) (`{code}`)");
}
output.push('\n');
Expand Down
32 changes: 19 additions & 13 deletions crates/ruff_dev/src/generate_docs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub(crate) fn main(args: &Args) -> Result<()> {
if let Some(explanation) = rule.explanation() {
let mut output = String::new();

let _ = writeln!(&mut output, "# {} ({})", rule.name(), rule.noqa_code());
let _ = writeln!(&mut output, "# {}", rule.name_and_code());

let status_text = match rule.status() {
RuleStatus::Stable { since } => {
Expand All @@ -55,26 +55,36 @@ pub(crate) fn main(args: &Args) -> Result<()> {
}
};

let issue_search = format!(
"(%27{encoded_name}%27{code})",
encoded_name =
url::form_urlencoded::byte_serialize(rule.name().as_str().as_bytes())
.collect::<String>(),
code = rule
.noqa_code()
.map(|code| format!("%20OR%20{code}"))
.unwrap_or_default(),
);

let _ = writeln!(
&mut output,
r#"<small>
{status_text} ·
<a href="https://github.com/astral-sh/ruff/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20(%27{encoded_name}%27%20OR%20{rule_code})" target="_blank">Related issues</a> ·
<a href="https://github.com/astral-sh/ruff/issues?q=sort%3Aupdated-desc%20is%3Aissue%20is%3Aopen%20{issue_search}" target="_blank">Related issues</a> ·
<a href="https://github.com/astral-sh/ruff/blob/main/{file}#L{line}" target="_blank">View source</a>
</small>

"#,
encoded_name =
url::form_urlencoded::byte_serialize(rule.name().as_str().as_bytes())
.collect::<String>(),
rule_code = rule.noqa_code(),
file =
url::form_urlencoded::byte_serialize(rule.file().replace('\\', "/").as_bytes())
.collect::<String>(),
line = rule.line(),
);
let (linter, _) = Linter::parse_code(&rule.noqa_code().to_string()).unwrap();
if linter.url().is_some() {
if let Some(linter) = rule
.noqa_code()
.and_then(|code| Linter::parse_code(&code.to_string()).map(|(linter, _)| linter))
.filter(|linter| linter.url().is_some())
{
let common_prefix: String = match linter.common_prefix() {
"" => linter
.upstream_categories()
Expand Down Expand Up @@ -134,11 +144,7 @@ pub(crate) fn main(args: &Args) -> Result<()> {
output.push('\n');
}

process_documentation(
explanation.trim(),
&mut output,
&rule.noqa_code().to_string(),
);
process_documentation(explanation.trim(), &mut output, rule.name().as_str());

let filename = PathBuf::from(ROOT_DIR)
.join("docs")
Expand Down
42 changes: 34 additions & 8 deletions crates/ruff_dev/src/generate_rules_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,18 @@ const SYMBOLS_CONTAINER: &str = "style='display: flex; gap: 0.5rem; justify-cont
fn generate_table(
table_out: &mut String,
rules: impl IntoIterator<Item = Rule>,
linter: &Linter,
linter: Option<&Linter>,
default_rules: &RuleTable,
) {
table_out.push_str("| Code { scope='col' } | Name { scope='col' } | Message { scope='col' } | Status/Fix/Default { scope='col' .sr-only } |");
if linter.is_some() {
table_out.push_str("| Code { scope='col' } ");
}
table_out.push_str("| Name { scope='col' } | Message { scope='col' } | Status/Fix/Default { scope='col' .sr-only } |");
table_out.push('\n');
table_out.push_str("| ---- | ---- | ------- | -: |");
if linter.is_some() {
table_out.push_str("| ---- ");
}
table_out.push_str("| ---- | ------- | -: |");
table_out.push('\n');
for rule in rules {
let status_token = match rule.status() {
Expand Down Expand Up @@ -102,12 +108,19 @@ fn generate_table(
se = "</span>";
}

if let Some(linter) = linter {
let _ = write!(
table_out,
"| {ss}{prefix}{code}{se} {{ #{prefix}{code} }} ",
prefix = linter.common_prefix(),
code = linter.code_for_rule(rule).unwrap(),
);
}

#[expect(clippy::or_fun_call)]
let _ = write!(
table_out,
"| {ss}{prefix}{code}{se} {{ #{prefix}{code} }} | {ss}{explanation}{se} | {ss}{message}{se} | <div {SYMBOLS_CONTAINER}>{status_token}{fix_token}{default_token}</div>|",
prefix = linter.common_prefix(),
code = linter.code_for_rule(rule).unwrap(),
"| {ss}{explanation}{se} | {ss}{message}{se} | <div {SYMBOLS_CONTAINER}>{status_token}{fix_token}{default_token}</div>|",
explanation = rule
.explanation()
.is_some()
Expand Down Expand Up @@ -245,12 +258,25 @@ pub(crate) fn generate() -> String {
}
table_out.push('\n');
table_out.push('\n');
generate_table(&mut table_out, rules.clone(), &linter, &default_rules);
generate_table(&mut table_out, rules.clone(), Some(&linter), &default_rules);
}
} else {
generate_table(&mut table_out, linter.all_rules(), &linter, &default_rules);
generate_table(
&mut table_out,
linter.all_rules(),
Some(&linter),
&default_rules,
);
}
}

let mut codeless_rules = Rule::iter()
.filter(|rule| rule.noqa_code().is_none())
.peekable();
if codeless_rules.peek().is_some() {
table_out.push_str("### Rules without codes\n\n");
generate_table(&mut table_out, codeless_rules, None, &default_rules);
}

table_out
}
1 change: 0 additions & 1 deletion crates/ruff_linter/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ compact_str = { workspace = true }
fern = { workspace = true }
glob = { workspace = true }
globset = { workspace = true }
hashbrown = { workspace = true }
imperative = { workspace = true }
is-macro = { workspace = true }
itertools = { workspace = true }
Expand Down
Loading