Rollup merge of #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 #130984](#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 #130984.

``@rustbot`` label A-tidy C-enhancement

src/tools/compiletest/src/runtest.rs

@@ -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;

src/tools/tidy/src/style.rs

@@ -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,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 {
@@ -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") {