Auto merge of #127428 - donno2048:master, r=albertlarsan68

Don't use regexes in tidy checks

No need for them, and it makes the tests and the checking simpler

r? `@albertlarsan68`
This commit is contained in:
bors 2024-07-07 08:09:23 +00:00
commit 98dcbae5c9
2 changed files with 27 additions and 28 deletions

View File

@ -18,9 +18,8 @@
// ignore-tidy-dbg // ignore-tidy-dbg
use crate::walk::{filter_dirs, walk}; use crate::walk::{filter_dirs, walk};
use regex::RegexSet;
use rustc_hash::FxHashMap; use rustc_hash::FxHashMap;
use std::{ffi::OsStr, path::Path}; use std::{ffi::OsStr, path::Path, sync::LazyLock};
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
@ -110,15 +109,32 @@ const ROOT_PROBLEMATIC_CONSTS: &[u32] = &[
173390526, 721077, 173390526, 721077,
]; ];
#[cfg(not(test))]
const LETTER_DIGIT: &[(char, char)] = &[('A', '4'), ('B', '8'), ('E', '3')];
#[cfg(test)]
const LETTER_DIGIT: &[(char, char)] = &[('A', '4'), ('B', '8'), ('E', '3'), ('0', 'F')]; // use "futile" F intentionally
fn generate_problematic_strings( fn generate_problematic_strings(
consts: &[u32], consts: &[u32],
letter_digit: &FxHashMap<char, char>, letter_digit: &FxHashMap<char, char>,
) -> Vec<String> { ) -> Vec<String> {
generate_problems(consts, letter_digit) generate_problems(consts, letter_digit)
.flat_map(|v| vec![v.to_string(), format!("{:x}", v), format!("{:X}", v)]) .flat_map(|v| vec![v.to_string(), format!("{:X}", v)])
.collect() .collect()
} }
static PROBLEMATIC_CONSTS_STRINGS: LazyLock<Vec<String>> = LazyLock::new(|| {
generate_problematic_strings(
ROOT_PROBLEMATIC_CONSTS,
&FxHashMap::from_iter(LETTER_DIGIT.iter().copied()),
)
});
fn contains_problematic_const(trimmed: &str) -> bool {
PROBLEMATIC_CONSTS_STRINGS.iter().any(|s| trimmed.to_uppercase().contains(s))
}
const INTERNAL_COMPILER_DOCS_LINE: &str = "#### This error code is internal to the compiler and will not be emitted with normal Rust code."; const INTERNAL_COMPILER_DOCS_LINE: &str = "#### This error code is internal to the compiler and will not be emitted with normal Rust code.";
/// Parser states for `line_is_url`. /// Parser states for `line_is_url`.
@ -315,11 +331,6 @@ pub fn check(path: &Path, bad: &mut bool) {
// We only check CSS files in rustdoc. // We only check CSS files in rustdoc.
path.extension().map_or(false, |e| e == "css") && !is_in(path, "src", "librustdoc") path.extension().map_or(false, |e| e == "css") && !is_in(path, "src", "librustdoc")
} }
let problematic_consts_strings = generate_problematic_strings(
ROOT_PROBLEMATIC_CONSTS,
&[('A', '4'), ('B', '8'), ('E', '3')].iter().cloned().collect(),
);
let problematic_regex = RegexSet::new(problematic_consts_strings.as_slice()).unwrap();
walk(path, skip, &mut |entry, contents| { walk(path, skip, &mut |entry, contents| {
let file = entry.path(); let file = entry.path();
@ -389,7 +400,6 @@ pub fn check(path: &Path, bad: &mut bool) {
let is_test = file.components().any(|c| c.as_os_str() == "tests"); let is_test = file.components().any(|c| c.as_os_str() == "tests");
// scanning the whole file for multiple needles at once is more efficient than // scanning the whole file for multiple needles at once is more efficient than
// executing lines times needles separate searches. // executing lines times needles separate searches.
let any_problematic_line = problematic_regex.is_match(contents);
for (i, line) in contents.split('\n').enumerate() { for (i, line) in contents.split('\n').enumerate() {
if line.is_empty() { if line.is_empty() {
if i == 0 { if i == 0 {
@ -459,12 +469,8 @@ pub fn check(path: &Path, bad: &mut bool) {
if trimmed.contains("//") && trimmed.contains(" XXX") { if trimmed.contains("//") && trimmed.contains(" XXX") {
err("Instead of XXX use FIXME") err("Instead of XXX use FIXME")
} }
if any_problematic_line { if contains_problematic_const(trimmed) {
for s in problematic_consts_strings.iter() { err("Don't use magic numbers that spell things (consider 0x12345678)");
if trimmed.contains(s) {
err("Don't use magic numbers that spell things (consider 0x12345678)");
}
}
} }
} }
// for now we just check libcore // for now we just check libcore

View File

@ -1,17 +1,10 @@
use super::*; use super::*;
#[test] #[test]
fn test_generate_problematic_strings() { fn test_contains_problematic_const() {
let problematic_regex = RegexSet::new( assert!(contains_problematic_const("786357")); // check with no "decimal" hex digits - converted to integer
generate_problematic_strings( assert!(contains_problematic_const("589701")); // check with "decimal" replacements - converted to integer
ROOT_PROBLEMATIC_CONSTS, assert!(contains_problematic_const("8FF85")); // check for hex display
&[('A', '4'), ('B', '8'), ('E', '3'), ('0', 'F')].iter().cloned().collect(), // use "futile" F intentionally assert!(contains_problematic_const("8fF85")); // check for case-alternating hex display
) assert!(!contains_problematic_const("1193046")); // check for non-matching value
.as_slice(),
)
.unwrap();
assert!(problematic_regex.is_match("786357")); // check with no "decimal" hex digits - converted to integer
assert!(problematic_regex.is_match("589701")); // check with "decimal" replacements - converted to integer
assert!(problematic_regex.is_match("8FF85")); // check for hex display
assert!(!problematic_regex.is_match("1193046")); // check for non-matching value
} }