Add CheckId, migrate the alphabetical check to diagnostics

src/tools/tidy/src/alphabetical.rs

@@ -24,6 +24,7 @@ use std::fmt::Display;
 use std::iter::Peekable;
 use std::path::Path;
 
+use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
 use crate::walk::{filter_dirs, walk};
 
 #[cfg(test)]
@@ -43,8 +44,7 @@ const END_MARKER: &str = "tidy-alphabetical-end";
 fn check_section<'a>(
     file: impl Display,
     lines: impl Iterator<Item = (usize, &'a str)>,
-    err: &mut dyn FnMut(&str) -> std::io::Result<()>,
-    bad: &mut bool,
+    check: &mut RunningCheck,
 ) {
     let mut prev_line = String::new();
     let mut first_indent = None;
@@ -56,12 +56,10 @@ fn check_section<'a>(
         }
 
         if line.contains(START_MARKER) {
-            tidy_error_ext!(
-                err,
-                bad,
+            check.error(format!(
                 "{file}:{} found `{START_MARKER}` expecting `{END_MARKER}`",
                 idx + 1
-            );
+            ));
             return;
         }
 
@@ -104,45 +102,44 @@ fn check_section<'a>(
         let prev_line_trimmed_lowercase = prev_line.trim_start_matches(' ');
 
         if version_sort(trimmed_line, prev_line_trimmed_lowercase).is_lt() {
-            tidy_error_ext!(err, bad, "{file}:{}: line not in alphabetical order", idx + 1);
+            check.error(format!("{file}:{}: line not in alphabetical order", idx + 1));
         }
 
         prev_line = line;
     }
 
-    tidy_error_ext!(err, bad, "{file}: reached end of file expecting `{END_MARKER}`")
+    check.error(format!("{file}: reached end of file expecting `{END_MARKER}`"));
 }
 
 fn check_lines<'a>(
     file: &impl Display,
     mut lines: impl Iterator<Item = (usize, &'a str)>,
-    err: &mut dyn FnMut(&str) -> std::io::Result<()>,
-    bad: &mut bool,
+    check: &mut RunningCheck,
 ) {
     while let Some((idx, line)) = lines.next() {
         if line.contains(END_MARKER) {
-            tidy_error_ext!(
-                err,
-                bad,
+            check.error(format!(
                 "{file}:{} found `{END_MARKER}` expecting `{START_MARKER}`",
                 idx + 1
-            )
+            ));
         }
 
         if line.contains(START_MARKER) {
-            check_section(file, &mut lines, err, bad);
+            check_section(file, &mut lines, check);
         }
     }
 }
 
-pub fn check(path: &Path, bad: &mut bool) {
+pub fn check(path: &Path, diag_ctx: DiagCtx) {
+    let mut check = diag_ctx.start_check(CheckId::new("alphabetical").path(path));
+
     let skip =
         |path: &_, _is_dir| filter_dirs(path) || path.ends_with("tidy/src/alphabetical/tests.rs");
 
     walk(path, skip, &mut |entry, contents| {
         let file = &entry.path().display();
         let lines = contents.lines().enumerate();
-        check_lines(file, lines, &mut crate::tidy_error, bad)
+        check_lines(file, lines, &mut check)
     });
 }
 

src/tools/tidy/src/diagnostics.rs

@@ -1,5 +1,6 @@
 use std::collections::HashSet;
 use std::fmt::Display;
+use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
 
 use crate::tidy_error;
@@ -21,12 +22,12 @@ impl DiagCtx {
         })))
     }
 
-    pub fn start_check<T: Display>(&self, name: T) -> RunningCheck {
-        let name = name.to_string();
+    pub fn start_check<Id: Into<CheckId>>(&self, id: Id) -> RunningCheck {
+        let id = id.into();
 
         let mut ctx = self.0.lock().unwrap();
-        ctx.start_check(&name);
-        RunningCheck { name, bad: false, ctx: self.0.clone() }
+        ctx.start_check(id.clone());
+        RunningCheck { id, bad: false, ctx: self.0.clone() }
     }
 
     pub fn into_conclusion(self) -> bool {
@@ -37,45 +38,68 @@ impl DiagCtx {
 }
 
 struct DiagCtxInner {
-    running_checks: HashSet<String>,
+    running_checks: HashSet<CheckId>,
     finished_checks: HashSet<FinishedCheck>,
     verbose: bool,
 }
 
 impl DiagCtxInner {
-    fn start_check(&mut self, name: &str) {
-        if self.has_check(name) {
-            panic!("Starting a check named {name} for the second time");
+    fn start_check(&mut self, id: CheckId) {
+        if self.has_check_id(&id) {
+            panic!("Starting a check named `{id:?}` for the second time");
         }
-        self.running_checks.insert(name.to_string());
+        self.running_checks.insert(id);
     }
 
     fn finish_check(&mut self, check: FinishedCheck) {
         assert!(
-            self.running_checks.remove(&check.name),
-            "Finishing check {} that was not started",
-            check.name
+            self.running_checks.remove(&check.id),
+            "Finishing check `{:?}` that was not started",
+            check.id
         );
         self.finished_checks.insert(check);
     }
 
-    fn has_check(&self, name: &str) -> bool {
+    fn has_check_id(&self, id: &CheckId) -> bool {
         self.running_checks
             .iter()
-            .chain(self.finished_checks.iter().map(|c| &c.name))
-            .any(|c| c == name)
+            .chain(self.finished_checks.iter().map(|c| &c.id))
+            .any(|c| c == id)
+    }
+}
+
+/// Identifies a single step
+#[derive(PartialEq, Eq, Hash, Clone, Debug)]
+pub struct CheckId {
+    name: String,
+    path: Option<PathBuf>,
+}
+
+impl CheckId {
+    pub fn new(name: &'static str) -> Self {
+        Self { name: name.to_string(), path: None }
+    }
+
+    pub fn path(self, path: &Path) -> Self {
+        Self { path: Some(path.to_path_buf()), ..self }
+    }
+}
+
+impl From<&'static str> for CheckId {
+    fn from(name: &'static str) -> Self {
+        Self::new(name)
     }
 }
 
 #[derive(PartialEq, Eq, Hash, Debug)]
 struct FinishedCheck {
-    name: String,
+    id: CheckId,
     bad: bool,
 }
 
 /// Represents a single tidy check, identified by its `name`, running.
 pub struct RunningCheck {
-    name: String,
+    id: CheckId,
     bad: bool,
     ctx: Arc<Mutex<DiagCtxInner>>,
 }
@@ -94,9 +118,6 @@ impl RunningCheck {
 
 impl Drop for RunningCheck {
     fn drop(&mut self) {
-        self.ctx
-            .lock()
-            .unwrap()
-            .finish_check(FinishedCheck { name: self.name.clone(), bad: self.bad })
+        self.ctx.lock().unwrap().finish_check(FinishedCheck { id: self.id.clone(), bad: self.bad })
     }
 }

src/tools/tidy/src/main.rs

@@ -147,11 +147,11 @@ fn main() {
         // check!(edition, &compiler_path);
         // check!(edition, &library_path);
         //
-        // check!(alphabetical, &root_manifest);
-        // check!(alphabetical, &src_path);
-        // check!(alphabetical, &tests_path);
-        // check!(alphabetical, &compiler_path);
-        // check!(alphabetical, &library_path);
+        check!(alphabetical, &root_manifest);
+        check!(alphabetical, &src_path);
+        check!(alphabetical, &tests_path);
+        check!(alphabetical, &compiler_path);
+        check!(alphabetical, &library_path);
         //
         // check!(x_version, &root_path, &cargo);
         //

src/tools/tidy/src/style.rs

@@ -24,7 +24,7 @@ use std::sync::LazyLock;
 use regex::RegexSetBuilder;
 use rustc_hash::FxHashMap;
 
-use crate::diagnostics::DiagCtx;
+use crate::diagnostics::{CheckId, DiagCtx};
 use crate::walk::{filter_dirs, walk};
 
 #[cfg(test)]
@@ -340,7 +340,7 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool {
 }
 
 pub fn check(path: &Path, diag_ctx: DiagCtx) {
-    let mut check = diag_ctx.start_check(format!("style {}", path.display()));
+    let mut check = diag_ctx.start_check(CheckId::new("style").path(path));
 
     fn skip(path: &Path, is_dir: bool) -> bool {
         if path.file_name().is_some_and(|name| name.to_string_lossy().starts_with(".#")) {