Skip to content

Commit

Permalink
Auto merge of #127428 - donno2048:master, r=albertlarsan68
Browse files Browse the repository at this point in the history
Don't use regexes in tidy checks

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

r? `@albertlarsan68`
  • Loading branch information
bors committed Jul 7, 2024
2 parents 35e6e4c + 8d85043 commit 98dcbae
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 28 deletions.
36 changes: 21 additions & 15 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@
// ignore-tidy-dbg

use crate::walk::{filter_dirs, walk};
use regex::RegexSet;
use rustc_hash::FxHashMap;
use std::{ffi::OsStr, path::Path};
use std::{ffi::OsStr, path::Path, sync::LazyLock};

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -110,15 +109,32 @@ const ROOT_PROBLEMATIC_CONSTS: &[u32] = &[
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(
consts: &[u32],
letter_digit: &FxHashMap<char, char>,
) -> Vec<String> {
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()
}

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.";

/// Parser states for `line_is_url`.
Expand Down Expand Up @@ -315,11 +331,6 @@ pub fn check(path: &Path, bad: &mut bool) {
// We only check CSS files in rustdoc.
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| {
let file = entry.path();
Expand Down Expand Up @@ -389,7 +400,6 @@ pub fn check(path: &Path, bad: &mut bool) {
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
// executing lines times needles separate searches.
let any_problematic_line = problematic_regex.is_match(contents);
for (i, line) in contents.split('\n').enumerate() {
if line.is_empty() {
if i == 0 {
Expand Down Expand Up @@ -459,12 +469,8 @@ pub fn check(path: &Path, bad: &mut bool) {
if trimmed.contains("//") && trimmed.contains(" XXX") {
err("Instead of XXX use FIXME")
}
if any_problematic_line {
for s in problematic_consts_strings.iter() {
if trimmed.contains(s) {
err("Don't use magic numbers that spell things (consider 0x12345678)");
}
}
if contains_problematic_const(trimmed) {
err("Don't use magic numbers that spell things (consider 0x12345678)");
}
}
// for now we just check libcore
Expand Down
19 changes: 6 additions & 13 deletions src/tools/tidy/src/style/tests.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
use super::*;

#[test]
fn test_generate_problematic_strings() {
let problematic_regex = RegexSet::new(
generate_problematic_strings(
ROOT_PROBLEMATIC_CONSTS,
&[('A', '4'), ('B', '8'), ('E', '3'), ('0', 'F')].iter().cloned().collect(), // use "futile" F intentionally
)
.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
fn test_contains_problematic_const() {
assert!(contains_problematic_const("786357")); // check with no "decimal" hex digits - converted to integer
assert!(contains_problematic_const("589701")); // check with "decimal" replacements - converted to integer
assert!(contains_problematic_const("8FF85")); // check for hex display
assert!(contains_problematic_const("8fF85")); // check for case-alternating hex display
assert!(!contains_problematic_const("1193046")); // check for non-matching value
}

0 comments on commit 98dcbae

Please sign in to comment.