optimize line-by-line style checks in tidy

This commit is contained in:
The 8472 2022-11-22 00:25:43 +01:00
parent 49c2279ef6
commit f7bfc48793

View File

@ -17,7 +17,7 @@
//! `// ignore-tidy-CHECK-NAME`.
use crate::walk::{filter_dirs, walk};
use regex::Regex;
use regex::{Regex, RegexSet};
use std::path::Path;
/// Error code markdown is restricted to 80 columns because they can be
@ -225,6 +225,7 @@ pub fn check(path: &Path, bad: &mut bool) {
.chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:x}", v)))
.chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:X}", v)))
.collect();
let problematic_regex = RegexSet::new(problematic_consts_strings.as_slice()).unwrap();
walk(path, &mut skip, &mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
@ -281,7 +282,27 @@ pub fn check(path: &Path, bad: &mut bool) {
let mut trailing_new_lines = 0;
let mut lines = 0;
let mut last_safety_comment = false;
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 {
leading_new_lines = true;
}
trailing_new_lines += 1;
continue;
} else {
trailing_new_lines = 0;
}
let trimmed = line.trim();
if !trimmed.starts_with("//") {
lines += 1;
}
let mut err = |msg: &str| {
tidy_error!(bad, "{}:{}: {}", file.display(), i + 1, msg);
};
@ -308,28 +329,29 @@ pub fn check(path: &Path, bad: &mut bool) {
suppressible_tidy_err!(err, skip_cr, "CR character");
}
if filename != "style.rs" {
if line.contains("TODO") {
if trimmed.contains("TODO") {
err("TODO is deprecated; use FIXME")
}
if line.contains("//") && line.contains(" XXX") {
if trimmed.contains("//") && trimmed.contains(" XXX") {
err("XXX is deprecated; use FIXME")
}
for s in problematic_consts_strings.iter() {
if line.contains(s) {
err("Don't use magic numbers that spell things (consider 0x12345678)");
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)");
}
}
}
}
let is_test = || file.components().any(|c| c.as_os_str() == "tests");
// for now we just check libcore
if line.contains("unsafe {") && !line.trim().starts_with("//") && !last_safety_comment {
if file.components().any(|c| c.as_os_str() == "core") && !is_test() {
if trimmed.contains("unsafe {") && !trimmed.starts_with("//") && !last_safety_comment {
if file.components().any(|c| c.as_os_str() == "core") && !is_test {
suppressible_tidy_err!(err, skip_undocumented_unsafe, "undocumented unsafe");
}
}
if line.contains("// SAFETY:") {
if trimmed.contains("// SAFETY:") {
last_safety_comment = true;
} else if line.trim().starts_with("//") || line.trim().is_empty() {
} else if trimmed.starts_with("//") || trimmed.is_empty() {
// keep previous value
} else {
last_safety_comment = false;
@ -337,7 +359,8 @@ pub fn check(path: &Path, bad: &mut bool) {
if (line.starts_with("// Copyright")
|| line.starts_with("# Copyright")
|| line.starts_with("Copyright"))
&& (line.contains("Rust Developers") || line.contains("Rust Project Developers"))
&& (trimmed.contains("Rust Developers")
|| trimmed.contains("Rust Project Developers"))
{
suppressible_tidy_err!(
err,
@ -351,18 +374,6 @@ pub fn check(path: &Path, bad: &mut bool) {
if filename.ends_with(".cpp") && line.contains("llvm_unreachable") {
err(LLVM_UNREACHABLE_INFO);
}
if line.is_empty() {
if i == 0 {
leading_new_lines = true;
}
trailing_new_lines += 1;
} else {
trailing_new_lines = 0;
}
if !line.trim().starts_with("//") {
lines += 1;
}
}
if leading_new_lines {
let mut err = |_| {