Skip to content

Make tidy warn on unrecognized directives #135421

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 18, 2025
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
4 changes: 2 additions & 2 deletions src/tools/compiletest/src/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,7 +47,7 @@ mod run_make;
mod rustdoc;
mod rustdoc_json;
mod ui;
// tidy-alphabet-end
// tidy-alphabetical-end

#[cfg(test)]
mod tests;
Expand Down
94 changes: 66 additions & 28 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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,
Expand All @@ -231,20 +247,28 @@ 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;
}
// Update `can_contain` when changing this
if contents.contains(&format!("// ignore-tidy-{check}"))
|| contents.contains(&format!("# ignore-tidy-{check}"))
|| contents.contains(&format!("/* ignore-tidy-{check} */"))
|| contents.contains(&format!("<!-- ignore-tidy-{check} -->"))
{
Directive::Ignore(false)
} else {
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}"))
|| contents.contains(&format!("/* ignore-tidy-{check} */"))
|| contents.contains(&format!("<!-- ignore-tidy-{check} -->"))
{
Directive::Ignore(false)
} else {
Directive::Deny
}
})
}

macro_rules! suppressible_tidy_err {
Expand Down Expand Up @@ -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-")
Expand All @@ -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;
Expand Down Expand Up @@ -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") {
Expand Down
Loading