Skip to content

Commit 67f4901

Browse files
committed
add tidy warning for unrecognized directives
This makes tidy warn on the presence of any directives it does not recognize. There are changes in compiletest because that file used "tidy-alphabet" instead of "tidy-alphabetical".
1 parent 48a426e commit 67f4901

File tree

2 files changed

+68
-30
lines changed

2 files changed

+68
-30
lines changed

src/tools/compiletest/src/runtest.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use crate::{ColorConfig, json, stamp_file_path};
3232
mod debugger;
3333

3434
// Helper modules that implement test running logic for each test suite.
35-
// tidy-alphabet-start
35+
// tidy-alphabetical-start
3636
mod assembly;
3737
mod codegen;
3838
mod codegen_units;
@@ -47,7 +47,7 @@ mod run_make;
4747
mod rustdoc;
4848
mod rustdoc_json;
4949
mod ui;
50-
// tidy-alphabet-end
50+
// tidy-alphabetical-end
5151

5252
#[cfg(test)]
5353
mod tests;

src/tools/tidy/src/style.rs

Lines changed: 66 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,21 @@ const ANNOTATIONS_TO_IGNORE: &[&str] = &[
7272
"//@ normalize-stderr",
7373
];
7474

75+
// If you edit this, also edit where it gets used in `check` (calling `contains_ignore_directives`)
76+
const CONFIGURABLE_CHECKS: [&str; 11] = [
77+
"cr",
78+
"undocumented-unsafe",
79+
"tab",
80+
"linelength",
81+
"filelength",
82+
"end-whitespace",
83+
"trailing-newlines",
84+
"leading-newlines",
85+
"copyright",
86+
"dbg",
87+
"odd-backticks",
88+
];
89+
7590
fn generate_problems<'a>(
7691
consts: &'a [u32],
7792
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
220235
}
221236
}
222237

238+
#[derive(Clone, Copy)]
223239
enum Directive {
224240
/// By default, tidy always warns against style issues.
225241
Deny,
@@ -231,20 +247,28 @@ enum Directive {
231247
Ignore(bool),
232248
}
233249

234-
fn contains_ignore_directive(can_contain: bool, contents: &str, check: &str) -> Directive {
250+
// Use a fixed size array in the return type to catch mistakes with changing `CONFIGURABLE_CHECKS`
251+
// without changing the code in `check` easier.
252+
fn contains_ignore_directives<const N: usize>(
253+
can_contain: bool,
254+
contents: &str,
255+
checks: [&str; N],
256+
) -> [Directive; N] {
235257
if !can_contain {
236-
return Directive::Deny;
237-
}
238-
// Update `can_contain` when changing this
239-
if contents.contains(&format!("// ignore-tidy-{check}"))
240-
|| contents.contains(&format!("# ignore-tidy-{check}"))
241-
|| contents.contains(&format!("/* ignore-tidy-{check} */"))
242-
|| contents.contains(&format!("<!-- ignore-tidy-{check} -->"))
243-
{
244-
Directive::Ignore(false)
245-
} else {
246-
Directive::Deny
258+
return [Directive::Deny; N];
247259
}
260+
checks.map(|check| {
261+
// Update `can_contain` when changing this
262+
if contents.contains(&format!("// ignore-tidy-{check}"))
263+
|| contents.contains(&format!("# ignore-tidy-{check}"))
264+
|| contents.contains(&format!("/* ignore-tidy-{check} */"))
265+
|| contents.contains(&format!("<!-- ignore-tidy-{check} -->"))
266+
{
267+
Directive::Ignore(false)
268+
} else {
269+
Directive::Deny
270+
}
271+
})
248272
}
249273

250274
macro_rules! suppressible_tidy_err {
@@ -370,6 +394,7 @@ pub fn check(path: &Path, bad: &mut bool) {
370394
COLS
371395
};
372396

397+
// When you change this, also change the `directive_line_starts` variable below
373398
let can_contain = contents.contains("// ignore-tidy-")
374399
|| contents.contains("# ignore-tidy-")
375400
|| contents.contains("/* ignore-tidy-")
@@ -385,22 +410,19 @@ pub fn check(path: &Path, bad: &mut bool) {
385410
return;
386411
}
387412
}
388-
let mut skip_cr = contains_ignore_directive(can_contain, &contents, "cr");
389-
let mut skip_undocumented_unsafe =
390-
contains_ignore_directive(can_contain, &contents, "undocumented-unsafe");
391-
let mut skip_tab = contains_ignore_directive(can_contain, &contents, "tab");
392-
let mut skip_line_length = contains_ignore_directive(can_contain, &contents, "linelength");
393-
let mut skip_file_length = contains_ignore_directive(can_contain, &contents, "filelength");
394-
let mut skip_end_whitespace =
395-
contains_ignore_directive(can_contain, &contents, "end-whitespace");
396-
let mut skip_trailing_newlines =
397-
contains_ignore_directive(can_contain, &contents, "trailing-newlines");
398-
let mut skip_leading_newlines =
399-
contains_ignore_directive(can_contain, &contents, "leading-newlines");
400-
let mut skip_copyright = contains_ignore_directive(can_contain, &contents, "copyright");
401-
let mut skip_dbg = contains_ignore_directive(can_contain, &contents, "dbg");
402-
let mut skip_odd_backticks =
403-
contains_ignore_directive(can_contain, &contents, "odd-backticks");
413+
let [
414+
mut skip_cr,
415+
mut skip_undocumented_unsafe,
416+
mut skip_tab,
417+
mut skip_line_length,
418+
mut skip_file_length,
419+
mut skip_end_whitespace,
420+
mut skip_trailing_newlines,
421+
mut skip_leading_newlines,
422+
mut skip_copyright,
423+
mut skip_dbg,
424+
mut skip_odd_backticks,
425+
] = contains_ignore_directives(can_contain, &contents, CONFIGURABLE_CHECKS);
404426
let mut leading_new_lines = false;
405427
let mut trailing_new_lines = 0;
406428
let mut lines = 0;
@@ -474,6 +496,22 @@ pub fn check(path: &Path, bad: &mut bool) {
474496
suppressible_tidy_err!(err, skip_cr, "CR character");
475497
}
476498
if !is_this_file {
499+
let directive_line_starts = ["// ", "# ", "/* ", "<!-- "];
500+
let possible_line_start =
501+
directive_line_starts.into_iter().any(|s| line.starts_with(s));
502+
let contains_potential_directive =
503+
possible_line_start && (line.contains("-tidy") || line.contains("tidy-"));
504+
let has_recognized_ignore_directive =
505+
contains_ignore_directives(can_contain, line, CONFIGURABLE_CHECKS)
506+
.into_iter()
507+
.any(|directive| matches!(directive, Directive::Ignore(_)));
508+
let has_alphabetical_directive = line.contains("tidy-alphabetical-start")
509+
|| line.contains("tidy-alphabetical-end");
510+
let has_recognized_directive =
511+
has_recognized_ignore_directive || has_alphabetical_directive;
512+
if contains_potential_directive && (!has_recognized_directive) {
513+
err("Unrecognized tidy directive")
514+
}
477515
// Allow using TODO in diagnostic suggestions by marking the
478516
// relevant line with `// ignore-tidy-todo`.
479517
if trimmed.contains("TODO") && !trimmed.contains("ignore-tidy-todo") {

0 commit comments

Comments
 (0)