bjoernager/rust
bjoernager/rust
1
Fork 0
Code Activity

Rollup merge of #135421 - cod10129:warn-tidy-ignore, r=onur-ozkan

Browse source 
Make tidy warn on unrecognized directives

This PR makes it so tidy warns on unrecognized directives, as recommended on [the discussion of #130984](https://github.com/rust-lang/rust/issues/130984#issuecomment-2589284620). This is edited from the previous version of this PR, which only warned on "tidy-ignore" and no other tidy directive typos.

Fixes #130984.

``@rustbot`` label A-tidy C-enhancement
This commit is contained in:
Matthias Krüger 2025-01-18 09:11:04 +01:00 • committed by GitHub
commit 8fec08b988
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 68 additions and 30 deletions
Download patch file Download diff file Expand all files Collapse all files

4
src/tools/compiletest/src/runtest.rs
View file

@ -32,7 +32,7 @@ use crate::{ColorConfig, json, stamp_file_path};
mod debugger;
// Helper modules that implement test running logic for each test suite.
// tidy-alphabet-start
// tidy-alphabetical-start
mod assembly;
mod codegen;
mod codegen_units;
@ -47,7 +47,7 @@ mod run_make;
mod rustdoc;
mod rustdoc_json;
mod ui;
// tidy-alphabet-end
// tidy-alphabetical-end
#[cfg(test)]
mod tests;

74
src/tools/tidy/src/style.rs
View file

@ -72,6 +72,21 @@ const ANNOTATIONS_TO_IGNORE: &[&str] = &[
"//@ normalize-stderr",
];
// If you edit this, also edit where it gets used in `check` (calling `contains_ignore_directives`)
const CONFIGURABLE_CHECKS: [&str; 11] = [
"cr",
"undocumented-unsafe",
"tab",
"linelength",
"filelength",
"end-whitespace",
"trailing-newlines",
"leading-newlines",
"copyright",
"dbg",
"odd-backticks",
];
fn generate_problems<'a>(
consts: &'a [u32],
letter_digit: &'a FxHashMap<char, char>,
@ -220,6 +235,7 @@ fn long_line_is_ok(extension: &str, is_error_code: bool, max_columns: usize, lin
}
}
#[derive(Clone, Copy)]
enum Directive {
/// By default, tidy always warns against style issues.
Deny,
@ -231,10 +247,17 @@ enum Directive {
Ignore(bool),
}
fn contains_ignore_directive(can_contain: bool, contents: &str, check: &str) -> Directive {
// Use a fixed size array in the return type to catch mistakes with changing `CONFIGURABLE_CHECKS`
// without changing the code in `check` easier.
fn contains_ignore_directives<const N: usize>(
can_contain: bool,
contents: &str,
checks: [&str; N],
) -> [Directive; N] {
if !can_contain {
return Directive::Deny;
return [Directive::Deny; N];
}
checks.map(|check| {
// Update `can_contain` when changing this
if contents.contains(&format!("// ignore-tidy-{check}"))
|| contents.contains(&format!("# ignore-tidy-{check}"))
@ -245,6 +268,7 @@ fn contains_ignore_directive(can_contain: bool, contents: &str, check: &str) ->
} else {
Directive::Deny
}
})
}
macro_rules! suppressible_tidy_err {
@ -370,6 +394,7 @@ pub fn check(path: &Path, bad: &mut bool) {
COLS
};
// When you change this, also change the `directive_line_starts` variable below
let can_contain = contents.contains("// ignore-tidy-")
|| contents.contains("# ignore-tidy-")
|| contents.contains("/* ignore-tidy-")
@ -385,22 +410,19 @@ pub fn check(path: &Path, bad: &mut bool) {
return;
}
}
let mut skip_cr = contains_ignore_directive(can_contain, &contents, "cr");
let mut skip_undocumented_unsafe =
contains_ignore_directive(can_contain, &contents, "undocumented-unsafe");
let mut skip_tab = contains_ignore_directive(can_contain, &contents, "tab");
let mut skip_line_length = contains_ignore_directive(can_contain, &contents, "linelength");
let mut skip_file_length = contains_ignore_directive(can_contain, &contents, "filelength");
let mut skip_end_whitespace =
contains_ignore_directive(can_contain, &contents, "end-whitespace");
let mut skip_trailing_newlines =
contains_ignore_directive(can_contain, &contents, "trailing-newlines");
let mut skip_leading_newlines =
contains_ignore_directive(can_contain, &contents, "leading-newlines");
let mut skip_copyright = contains_ignore_directive(can_contain, &contents, "copyright");
let mut skip_dbg = contains_ignore_directive(can_contain, &contents, "dbg");
let mut skip_odd_backticks =
contains_ignore_directive(can_contain, &contents, "odd-backticks");
let [
mut skip_cr,
mut skip_undocumented_unsafe,
mut skip_tab,
mut skip_line_length,
mut skip_file_length,
mut skip_end_whitespace,
mut skip_trailing_newlines,
mut skip_leading_newlines,
mut skip_copyright,
mut skip_dbg,
mut skip_odd_backticks,
] = contains_ignore_directives(can_contain, &contents, CONFIGURABLE_CHECKS);
let mut leading_new_lines = false;
let mut trailing_new_lines = 0;
let mut lines = 0;
@ -474,6 +496,22 @@ pub fn check(path: &Path, bad: &mut bool) {
suppressible_tidy_err!(err, skip_cr, "CR character");
}
if !is_this_file {
let directive_line_starts = ["// ", "# ", "/* ", "<!-- "];
let possible_line_start =
directive_line_starts.into_iter().any(|s| line.starts_with(s));
let contains_potential_directive =
possible_line_start && (line.contains("-tidy") || line.contains("tidy-"));
let has_recognized_ignore_directive =
contains_ignore_directives(can_contain, line, CONFIGURABLE_CHECKS)
.into_iter()
.any(|directive| matches!(directive, Directive::Ignore(_)));
let has_alphabetical_directive = line.contains("tidy-alphabetical-start")
|| line.contains("tidy-alphabetical-end");
let has_recognized_directive =
has_recognized_ignore_directive || has_alphabetical_directive;
if contains_potential_directive && (!has_recognized_directive) {
err("Unrecognized tidy directive")
}
// Allow using TODO in diagnostic suggestions by marking the
// relevant line with `// ignore-tidy-todo`.
if trimmed.contains("TODO") && !trimmed.contains("ignore-tidy-todo") {