Skip to content

Commit

Permalink
Unrolled build for rust-lang#135421
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#135421 - cod10129:warn-tidy-ignore, r=onur-ozkan

Make tidy warn on unrecognized directives

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

Fixes rust-lang#130984.

``@rustbot`` label A-tidy C-enhancement
  • Loading branch information
rust-timer authored Jan 18, 2025
2 parents bd62a45 + 67f4901 commit 7420233
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 30 deletions.
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

0 comments on commit 7420233

Please sign in to comment.