Skip to content

Commit c36faff

Browse files
committed
Add CheckId, migrate the alphabetical check to diagnostics
1 parent b0010dd commit c36faff

File tree

4 files changed

+63
-45
lines changed

4 files changed

+63
-45
lines changed

src/tools/tidy/src/alphabetical.rs

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use std::fmt::Display;
2424
use std::iter::Peekable;
2525
use std::path::Path;
2626

27+
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
2728
use crate::walk::{filter_dirs, walk};
2829

2930
#[cfg(test)]
@@ -43,8 +44,7 @@ const END_MARKER: &str = "tidy-alphabetical-end";
4344
fn check_section<'a>(
4445
file: impl Display,
4546
lines: impl Iterator<Item = (usize, &'a str)>,
46-
err: &mut dyn FnMut(&str) -> std::io::Result<()>,
47-
bad: &mut bool,
47+
check: &mut RunningCheck,
4848
) {
4949
let mut prev_line = String::new();
5050
let mut first_indent = None;
@@ -56,12 +56,10 @@ fn check_section<'a>(
5656
}
5757

5858
if line.contains(START_MARKER) {
59-
tidy_error_ext!(
60-
err,
61-
bad,
59+
check.error(format!(
6260
"{file}:{} found `{START_MARKER}` expecting `{END_MARKER}`",
6361
idx + 1
64-
);
62+
));
6563
return;
6664
}
6765

@@ -104,45 +102,44 @@ fn check_section<'a>(
104102
let prev_line_trimmed_lowercase = prev_line.trim_start_matches(' ');
105103

106104
if version_sort(trimmed_line, prev_line_trimmed_lowercase).is_lt() {
107-
tidy_error_ext!(err, bad, "{file}:{}: line not in alphabetical order", idx + 1);
105+
check.error(format!("{file}:{}: line not in alphabetical order", idx + 1));
108106
}
109107

110108
prev_line = line;
111109
}
112110

113-
tidy_error_ext!(err, bad, "{file}: reached end of file expecting `{END_MARKER}`")
111+
check.error(format!("{file}: reached end of file expecting `{END_MARKER}`"));
114112
}
115113

116114
fn check_lines<'a>(
117115
file: &impl Display,
118116
mut lines: impl Iterator<Item = (usize, &'a str)>,
119-
err: &mut dyn FnMut(&str) -> std::io::Result<()>,
120-
bad: &mut bool,
117+
check: &mut RunningCheck,
121118
) {
122119
while let Some((idx, line)) = lines.next() {
123120
if line.contains(END_MARKER) {
124-
tidy_error_ext!(
125-
err,
126-
bad,
121+
check.error(format!(
127122
"{file}:{} found `{END_MARKER}` expecting `{START_MARKER}`",
128123
idx + 1
129-
)
124+
));
130125
}
131126

132127
if line.contains(START_MARKER) {
133-
check_section(file, &mut lines, err, bad);
128+
check_section(file, &mut lines, check);
134129
}
135130
}
136131
}
137132

138-
pub fn check(path: &Path, bad: &mut bool) {
133+
pub fn check(path: &Path, diag_ctx: DiagCtx) {
134+
let mut check = diag_ctx.start_check(CheckId::new("alphabetical").path(path));
135+
139136
let skip =
140137
|path: &_, _is_dir| filter_dirs(path) || path.ends_with("tidy/src/alphabetical/tests.rs");
141138

142139
walk(path, skip, &mut |entry, contents| {
143140
let file = &entry.path().display();
144141
let lines = contents.lines().enumerate();
145-
check_lines(file, lines, &mut crate::tidy_error, bad)
142+
check_lines(file, lines, &mut check)
146143
});
147144
}
148145

src/tools/tidy/src/diagnostics.rs

Lines changed: 42 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::collections::HashSet;
22
use std::fmt::Display;
3+
use std::path::{Path, PathBuf};
34
use std::sync::{Arc, Mutex};
45

56
use crate::tidy_error;
@@ -21,12 +22,12 @@ impl DiagCtx {
2122
})))
2223
}
2324

24-
pub fn start_check<T: Display>(&self, name: T) -> RunningCheck {
25-
let name = name.to_string();
25+
pub fn start_check<Id: Into<CheckId>>(&self, id: Id) -> RunningCheck {
26+
let id = id.into();
2627

2728
let mut ctx = self.0.lock().unwrap();
28-
ctx.start_check(&name);
29-
RunningCheck { name, bad: false, ctx: self.0.clone() }
29+
ctx.start_check(id.clone());
30+
RunningCheck { id, bad: false, ctx: self.0.clone() }
3031
}
3132

3233
pub fn into_conclusion(self) -> bool {
@@ -37,45 +38,68 @@ impl DiagCtx {
3738
}
3839

3940
struct DiagCtxInner {
40-
running_checks: HashSet<String>,
41+
running_checks: HashSet<CheckId>,
4142
finished_checks: HashSet<FinishedCheck>,
4243
verbose: bool,
4344
}
4445

4546
impl DiagCtxInner {
46-
fn start_check(&mut self, name: &str) {
47-
if self.has_check(name) {
48-
panic!("Starting a check named {name} for the second time");
47+
fn start_check(&mut self, id: CheckId) {
48+
if self.has_check_id(&id) {
49+
panic!("Starting a check named `{id:?}` for the second time");
4950
}
50-
self.running_checks.insert(name.to_string());
51+
self.running_checks.insert(id);
5152
}
5253

5354
fn finish_check(&mut self, check: FinishedCheck) {
5455
assert!(
55-
self.running_checks.remove(&check.name),
56-
"Finishing check {} that was not started",
57-
check.name
56+
self.running_checks.remove(&check.id),
57+
"Finishing check `{:?}` that was not started",
58+
check.id
5859
);
5960
self.finished_checks.insert(check);
6061
}
6162

62-
fn has_check(&self, name: &str) -> bool {
63+
fn has_check_id(&self, id: &CheckId) -> bool {
6364
self.running_checks
6465
.iter()
65-
.chain(self.finished_checks.iter().map(|c| &c.name))
66-
.any(|c| c == name)
66+
.chain(self.finished_checks.iter().map(|c| &c.id))
67+
.any(|c| c == id)
68+
}
69+
}
70+
71+
/// Identifies a single step
72+
#[derive(PartialEq, Eq, Hash, Clone, Debug)]
73+
pub struct CheckId {
74+
name: String,
75+
path: Option<PathBuf>,
76+
}
77+
78+
impl CheckId {
79+
pub fn new(name: &'static str) -> Self {
80+
Self { name: name.to_string(), path: None }
81+
}
82+
83+
pub fn path(self, path: &Path) -> Self {
84+
Self { path: Some(path.to_path_buf()), ..self }
85+
}
86+
}
87+
88+
impl From<&'static str> for CheckId {
89+
fn from(name: &'static str) -> Self {
90+
Self::new(name)
6791
}
6892
}
6993

7094
#[derive(PartialEq, Eq, Hash, Debug)]
7195
struct FinishedCheck {
72-
name: String,
96+
id: CheckId,
7397
bad: bool,
7498
}
7599

76100
/// Represents a single tidy check, identified by its `name`, running.
77101
pub struct RunningCheck {
78-
name: String,
102+
id: CheckId,
79103
bad: bool,
80104
ctx: Arc<Mutex<DiagCtxInner>>,
81105
}
@@ -94,9 +118,6 @@ impl RunningCheck {
94118

95119
impl Drop for RunningCheck {
96120
fn drop(&mut self) {
97-
self.ctx
98-
.lock()
99-
.unwrap()
100-
.finish_check(FinishedCheck { name: self.name.clone(), bad: self.bad })
121+
self.ctx.lock().unwrap().finish_check(FinishedCheck { id: self.id.clone(), bad: self.bad })
101122
}
102123
}

src/tools/tidy/src/main.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -147,11 +147,11 @@ fn main() {
147147
// check!(edition, &compiler_path);
148148
// check!(edition, &library_path);
149149
//
150-
// check!(alphabetical, &root_manifest);
151-
// check!(alphabetical, &src_path);
152-
// check!(alphabetical, &tests_path);
153-
// check!(alphabetical, &compiler_path);
154-
// check!(alphabetical, &library_path);
150+
check!(alphabetical, &root_manifest);
151+
check!(alphabetical, &src_path);
152+
check!(alphabetical, &tests_path);
153+
check!(alphabetical, &compiler_path);
154+
check!(alphabetical, &library_path);
155155
//
156156
// check!(x_version, &root_path, &cargo);
157157
//

src/tools/tidy/src/style.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ use std::sync::LazyLock;
2424
use regex::RegexSetBuilder;
2525
use rustc_hash::FxHashMap;
2626

27-
use crate::diagnostics::DiagCtx;
27+
use crate::diagnostics::{CheckId, DiagCtx};
2828
use crate::walk::{filter_dirs, walk};
2929

3030
#[cfg(test)]
@@ -340,7 +340,7 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool {
340340
}
341341

342342
pub fn check(path: &Path, diag_ctx: DiagCtx) {
343-
let mut check = diag_ctx.start_check(format!("style {}", path.display()));
343+
let mut check = diag_ctx.start_check(CheckId::new("style").path(path));
344344

345345
fn skip(path: &Path, is_dir: bool) -> bool {
346346
if path.file_name().is_some_and(|name| name.to_string_lossy().starts_with(".#")) {

0 commit comments

Comments
 (0)