summaryrefslogtreecommitdiffstats
path: root/src/tools/tidy
diff options
context:
space:
mode:
Diffstat (limited to 'src/tools/tidy')
-rw-r--r--src/tools/tidy/Cargo.toml2
-rw-r--r--src/tools/tidy/src/alphabetical.rs2
-rw-r--r--src/tools/tidy/src/bins.rs24
-rw-r--r--src/tools/tidy/src/debug_artifacts.rs30
-rw-r--r--src/tools/tidy/src/deps.rs6
-rw-r--r--src/tools/tidy/src/edition.rs37
-rw-r--r--src/tools/tidy/src/error_codes.rs10
-rw-r--r--src/tools/tidy/src/features.rs23
-rw-r--r--src/tools/tidy/src/main.rs23
-rw-r--r--src/tools/tidy/src/mir_opt_tests.rs41
-rw-r--r--src/tools/tidy/src/pal.rs5
-rw-r--r--src/tools/tidy/src/rustdoc_gui_tests.rs5
-rw-r--r--src/tools/tidy/src/style.rs111
-rw-r--r--src/tools/tidy/src/target_specific_tests.rs100
-rw-r--r--src/tools/tidy/src/ui_tests.rs146
-rw-r--r--src/tools/tidy/src/unit_tests.rs21
-rw-r--r--src/tools/tidy/src/walk.rs60
17 files changed, 363 insertions, 283 deletions
diff --git a/src/tools/tidy/Cargo.toml b/src/tools/tidy/Cargo.toml
index cdf1dd366..8c6b1eb22 100644
--- a/src/tools/tidy/Cargo.toml
+++ b/src/tools/tidy/Cargo.toml
@@ -5,7 +5,7 @@ edition = "2021"
autobins = false
[dependencies]
-cargo_metadata = "0.14"
+cargo_metadata = "0.15"
cargo-platform = "0.1.2"
regex = "1"
miropt-test-tools = { path = "../miropt-test-tools" }
diff --git a/src/tools/tidy/src/alphabetical.rs b/src/tools/tidy/src/alphabetical.rs
index f913f6cde..fdc411c89 100644
--- a/src/tools/tidy/src/alphabetical.rs
+++ b/src/tools/tidy/src/alphabetical.rs
@@ -95,7 +95,7 @@ fn check_section<'a>(
}
pub fn check(path: &Path, bad: &mut bool) {
- walk(path, &mut filter_dirs, &mut |entry, contents| {
+ walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
let file = &entry.path().display();
let mut lines = contents.lines().enumerate().peekable();
diff --git a/src/tools/tidy/src/bins.rs b/src/tools/tidy/src/bins.rs
index b898f20a5..197e9a996 100644
--- a/src/tools/tidy/src/bins.rs
+++ b/src/tools/tidy/src/bins.rs
@@ -57,8 +57,8 @@ mod os_impl {
match fs::File::create(&path) {
Ok(file) => {
let exec = is_executable(&path).unwrap_or(false);
- std::mem::drop(file);
- std::fs::remove_file(&path).expect("Deleted temp file");
+ drop(file);
+ fs::remove_file(&path).expect("Deleted temp file");
// If the file is executable, then we assume that this
// filesystem does not track executability, so skip this check.
return if exec { Unsupported } else { Supported };
@@ -101,23 +101,11 @@ mod os_impl {
const ALLOWED: &[&str] = &["configure", "x"];
+ // FIXME: we don't need to look at all binaries, only files that have been modified in this branch
+ // (e.g. using `git ls-files`).
walk_no_read(
- path,
- &mut |path| {
- filter_dirs(path)
- || path.ends_with("src/etc")
- // This is a list of directories that we almost certainly
- // don't need to walk. A future PR will likely want to
- // remove these in favor of crate::walk_no_read using git
- // ls-files to discover the paths we should check, which
- // would naturally ignore all of these directories. It's
- // also likely faster than walking the directory tree
- // directly (since git is just reading from a couple files
- // to produce the results).
- || path.ends_with("target")
- || path.ends_with("build")
- || path.ends_with(".git")
- },
+ &[path],
+ |path, _is_dir| filter_dirs(path) || path.ends_with("src/etc"),
&mut |entry| {
let file = entry.path();
let extension = file.extension();
diff --git a/src/tools/tidy/src/debug_artifacts.rs b/src/tools/tidy/src/debug_artifacts.rs
index 0dd9c1e16..582014d50 100644
--- a/src/tools/tidy/src/debug_artifacts.rs
+++ b/src/tools/tidy/src/debug_artifacts.rs
@@ -1,22 +1,26 @@
//! Tidy check to prevent creation of unnecessary debug artifacts while running tests.
-use crate::walk::{filter_dirs, walk};
+use crate::walk::{filter_dirs, filter_not_rust, walk};
use std::path::Path;
const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in test";
pub fn check(test_dir: &Path, bad: &mut bool) {
- walk(test_dir, &mut filter_dirs, &mut |entry, contents| {
- let filename = entry.path();
- let is_rust = filename.extension().map_or(false, |ext| ext == "rs");
- if !is_rust {
- return;
- }
-
- for (i, line) in contents.lines().enumerate() {
- if line.contains("borrowck_graphviz_postflow") {
- tidy_error!(bad, "{}:{}: {}", filename.display(), i + 1, GRAPHVIZ_POSTFLOW_MSG);
+ walk(
+ test_dir,
+ |path, _is_dir| filter_dirs(path) || filter_not_rust(path),
+ &mut |entry, contents| {
+ for (i, line) in contents.lines().enumerate() {
+ if line.contains("borrowck_graphviz_postflow") {
+ tidy_error!(
+ bad,
+ "{}:{}: {}",
+ entry.path().display(),
+ i + 1,
+ GRAPHVIZ_POSTFLOW_MSG
+ );
+ }
}
- }
- });
+ },
+ );
}
diff --git a/src/tools/tidy/src/deps.rs b/src/tools/tidy/src/deps.rs
index bddfdcfaf..a9eb6c8d0 100644
--- a/src/tools/tidy/src/deps.rs
+++ b/src/tools/tidy/src/deps.rs
@@ -42,13 +42,16 @@ const EXCEPTIONS: &[(&str, &str)] = &[
("bitmaps", "MPL-2.0+"), // cargo via im-rc
("fiat-crypto", "MIT OR Apache-2.0 OR BSD-1-Clause"), // cargo via pasetors
("subtle", "BSD-3-Clause"), // cargo via pasetors
+ ("dunce", "CC0-1.0 OR MIT-0"), // cargo via gix (and dev dependency)
+ ("imara-diff", "Apache-2.0"), // cargo via gix
+ ("sha1_smol", "BSD-3-Clause"), // cargo via gix
+ ("unicode-bom", "Apache-2.0"), // cargo via gix
("instant", "BSD-3-Clause"), // rustc_driver/tracing-subscriber/parking_lot
("snap", "BSD-3-Clause"), // rustc
("fluent-langneg", "Apache-2.0"), // rustc (fluent translations)
("self_cell", "Apache-2.0"), // rustc (fluent translations)
// FIXME: this dependency violates the documentation comment above:
("fortanix-sgx-abi", "MPL-2.0"), // libstd but only for `sgx` target
- ("dunce", "CC0-1.0"), // cargo (dev dependency)
("similar", "Apache-2.0"), // cargo (dev dependency)
("normalize-line-endings", "Apache-2.0"), // cargo (dev dependency)
("dissimilar", "Apache-2.0"), // rustdoc, rustc_lexer (few tests) via expect-test, (dev deps)
@@ -257,6 +260,7 @@ const PERMITTED_RUSTC_DEPENDENCIES: &[&str] = &[
"valuable",
"version_check",
"wasi",
+ "windows",
"winapi",
"winapi-i686-pc-windows-gnu",
"winapi-util",
diff --git a/src/tools/tidy/src/edition.rs b/src/tools/tidy/src/edition.rs
index 8172e3d29..f28f677e0 100644
--- a/src/tools/tidy/src/edition.rs
+++ b/src/tools/tidy/src/edition.rs
@@ -9,27 +9,20 @@ fn is_edition_2021(mut line: &str) -> bool {
}
pub fn check(path: &Path, bad: &mut bool) {
- walk(
- path,
- &mut |path| {
- filter_dirs(path)
- || (path.ends_with("tests") && path.join("COMPILER_TESTS.md").exists())
- },
- &mut |entry, contents| {
- let file = entry.path();
- let filename = file.file_name().unwrap();
- if filename != "Cargo.toml" {
- return;
- }
+ walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
+ let file = entry.path();
+ let filename = file.file_name().unwrap();
+ if filename != "Cargo.toml" {
+ return;
+ }
- let is_2021 = contents.lines().any(is_edition_2021);
- if !is_2021 {
- tidy_error!(
- bad,
- "{} doesn't have `edition = \"2021\"` on a separate line",
- file.display()
- );
- }
- },
- );
+ let is_2021 = contents.lines().any(is_edition_2021);
+ if !is_2021 {
+ tidy_error!(
+ bad,
+ "{} doesn't have `edition = \"2021\"` on a separate line",
+ file.display()
+ );
+ }
+ });
}
diff --git a/src/tools/tidy/src/error_codes.rs b/src/tools/tidy/src/error_codes.rs
index c60caa0d4..417ace58c 100644
--- a/src/tools/tidy/src/error_codes.rs
+++ b/src/tools/tidy/src/error_codes.rs
@@ -46,8 +46,10 @@ pub fn check(root_path: &Path, search_paths: &[&Path], verbose: bool, bad: &mut
// Stage 1: create list
let error_codes = extract_error_codes(root_path, &mut errors);
- println!("Found {} error codes", error_codes.len());
- println!("Highest error code: `{}`", error_codes.iter().max().unwrap());
+ if verbose {
+ println!("Found {} error codes", error_codes.len());
+ println!("Highest error code: `{}`", error_codes.iter().max().unwrap());
+ }
// Stage 2: check list has docs
let no_longer_emitted = check_error_codes_docs(root_path, &error_codes, &mut errors, verbose);
@@ -127,7 +129,7 @@ fn check_error_codes_docs(
let mut no_longer_emitted_codes = Vec::new();
- walk(&docs_path, &mut |_| false, &mut |entry, contents| {
+ walk(&docs_path, |_, _| false, &mut |entry, contents| {
let path = entry.path();
// Error if the file isn't markdown.
@@ -319,7 +321,7 @@ fn check_error_codes_used(
let mut found_codes = Vec::new();
- walk_many(search_paths, &mut filter_dirs, &mut |entry, contents| {
+ walk_many(search_paths, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
let path = entry.path();
// Return early if we aren't looking at a source file.
diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs
index af92e6eb8..2fd4c797b 100644
--- a/src/tools/tidy/src/features.rs
+++ b/src/tools/tidy/src/features.rs
@@ -9,8 +9,9 @@
//! * All unstable lang features have tests to ensure they are actually unstable.
//! * Language features in a group are sorted by feature name.
-use crate::walk::{filter_dirs, walk, walk_many};
+use crate::walk::{filter_dirs, filter_not_rust, walk, walk_many};
use std::collections::hash_map::{Entry, HashMap};
+use std::ffi::OsStr;
use std::fmt;
use std::fs;
use std::num::NonZeroU32;
@@ -101,17 +102,15 @@ pub fn check(
&tests_path.join("rustdoc-ui"),
&tests_path.join("rustdoc"),
],
- &mut filter_dirs,
+ |path, _is_dir| {
+ filter_dirs(path)
+ || filter_not_rust(path)
+ || path.file_name() == Some(OsStr::new("features.rs"))
+ || path.file_name() == Some(OsStr::new("diagnostic_list.rs"))
+ },
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
- if !filename.ends_with(".rs")
- || filename == "features.rs"
- || filename == "diagnostic_list.rs"
- {
- return;
- }
-
let filen_underscore = filename.replace('-', "_").replace(".rs", "");
let filename_is_gate_test = test_filen_gate(&filen_underscore, &mut features);
@@ -219,8 +218,6 @@ pub fn check(
for line in lines {
println!("* {line}");
}
- } else {
- println!("* {} features", features.len());
}
CollectedFeatures { lib: lib_features, lang: features }
@@ -477,11 +474,11 @@ fn get_and_check_lib_features(
fn map_lib_features(
base_src_path: &Path,
- mf: &mut dyn FnMut(Result<(&str, Feature), &str>, &Path, usize),
+ mf: &mut (dyn Send + Sync + FnMut(Result<(&str, Feature), &str>, &Path, usize)),
) {
walk(
base_src_path,
- &mut |path| filter_dirs(path) || path.ends_with("tests"),
+ |path, _is_dir| filter_dirs(path) || path.ends_with("tests"),
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs
index 505f9d724..f59406c40 100644
--- a/src/tools/tidy/src/main.rs
+++ b/src/tools/tidy/src/main.rs
@@ -13,7 +13,7 @@ use std::path::PathBuf;
use std::process;
use std::str::FromStr;
use std::sync::atomic::{AtomicBool, Ordering};
-use std::thread::{scope, ScopedJoinHandle};
+use std::thread::{self, scope, ScopedJoinHandle};
fn main() {
let root_path: PathBuf = env::args_os().nth(1).expect("need path to root of repo").into();
@@ -55,16 +55,28 @@ fn main() {
VecDeque::with_capacity(concurrency.get());
macro_rules! check {
- ($p:ident $(, $args:expr)* ) => {
+ ($p:ident) => {
+ check!(@ $p, name=format!("{}", stringify!($p)));
+ };
+ ($p:ident, $path:expr $(, $args:expr)* ) => {
+ let shortened = $path.strip_prefix(&root_path).unwrap();
+ let name = if shortened == std::path::Path::new("") {
+ format!("{} (.)", stringify!($p))
+ } else {
+ format!("{} ({})", stringify!($p), shortened.display())
+ };
+ check!(@ $p, name=name, $path $(,$args)*);
+ };
+ (@ $p:ident, name=$name:expr $(, $args:expr)* ) => {
drain_handles(&mut handles);
- let handle = s.spawn(|| {
+ let handle = thread::Builder::new().name($name).spawn_scoped(s, || {
let mut flag = false;
$p::check($($args, )* &mut flag);
if (flag) {
bad.store(true, Ordering::Relaxed);
}
- });
+ }).unwrap();
handles.push_back(handle);
}
}
@@ -91,7 +103,6 @@ fn main() {
// Checks that need to be done for both the compiler and std libraries.
check!(unit_tests, &src_path);
- check!(unit_tests, &tests_path);
check!(unit_tests, &compiler_path);
check!(unit_tests, &library_path);
@@ -107,10 +118,8 @@ fn main() {
check!(edition, &src_path);
check!(edition, &compiler_path);
check!(edition, &library_path);
- check!(edition, &tests_path);
check!(alphabetical, &src_path);
- check!(alphabetical, &tests_path);
check!(alphabetical, &compiler_path);
check!(alphabetical, &library_path);
diff --git a/src/tools/tidy/src/mir_opt_tests.rs b/src/tools/tidy/src/mir_opt_tests.rs
index 2a9dcac2e..2f6918510 100644
--- a/src/tools/tidy/src/mir_opt_tests.rs
+++ b/src/tools/tidy/src/mir_opt_tests.rs
@@ -3,19 +3,24 @@
use std::collections::HashSet;
use std::path::{Path, PathBuf};
+use crate::walk::walk_no_read;
+
fn check_unused_files(path: &Path, bless: bool, bad: &mut bool) {
let mut rs_files = Vec::<PathBuf>::new();
let mut output_files = HashSet::<PathBuf>::new();
- let files = walkdir::WalkDir::new(&path.join("mir-opt")).into_iter();
- for file in files.filter_map(Result::ok).filter(|e| e.file_type().is_file()) {
- let filepath = file.path();
- if filepath.extension() == Some("rs".as_ref()) {
- rs_files.push(filepath.to_owned());
- } else {
- output_files.insert(filepath.to_owned());
- }
- }
+ walk_no_read(
+ &[&path.join("mir-opt")],
+ |path, _is_dir| path.file_name() == Some("README.md".as_ref()),
+ &mut |file| {
+ let filepath = file.path();
+ if filepath.extension() == Some("rs".as_ref()) {
+ rs_files.push(filepath.to_owned());
+ } else {
+ output_files.insert(filepath.to_owned());
+ }
+ },
+ );
for file in rs_files {
for bw in [32, 64] {
@@ -26,16 +31,14 @@ fn check_unused_files(path: &Path, bless: bool, bad: &mut bool) {
}
for extra in output_files {
- if extra.file_name() != Some("README.md".as_ref()) {
- if !bless {
- tidy_error!(
- bad,
- "the following output file is not associated with any mir-opt test, you can remove it: {}",
- extra.display()
- );
- } else {
- let _ = std::fs::remove_file(extra);
- }
+ if !bless {
+ tidy_error!(
+ bad,
+ "the following output file is not associated with any mir-opt test, you can remove it: {}",
+ extra.display()
+ );
+ } else {
+ let _ = std::fs::remove_file(extra);
}
}
}
diff --git a/src/tools/tidy/src/pal.rs b/src/tools/tidy/src/pal.rs
index f4592fdcf..6fd41e833 100644
--- a/src/tools/tidy/src/pal.rs
+++ b/src/tools/tidy/src/pal.rs
@@ -31,7 +31,6 @@
//! this in the long term.
use crate::walk::{filter_dirs, walk};
-use std::iter::Iterator;
use std::path::Path;
// Paths that may contain platform-specific code.
@@ -59,7 +58,6 @@ const EXCEPTION_PATHS: &[&str] = &[
"library/std/src/path.rs",
"library/std/src/sys_common", // Should only contain abstractions over platforms
"library/std/src/net/test.rs", // Utility helpers for tests
- "library/std/src/panic.rs", // fuchsia-specific panic backtrace handling
"library/std/src/personality.rs",
"library/std/src/personality/",
];
@@ -68,7 +66,7 @@ pub fn check(path: &Path, bad: &mut bool) {
// Sanity check that the complex parsing here works.
let mut saw_target_arch = false;
let mut saw_cfg_bang = false;
- walk(path, &mut filter_dirs, &mut |entry, contents| {
+ walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
let file = entry.path();
let filestr = file.to_string_lossy().replace("\\", "/");
if !filestr.ends_with(".rs") {
@@ -128,6 +126,7 @@ fn check_cfgs(
|| cfg.contains("target_env")
|| cfg.contains("target_abi")
|| cfg.contains("target_vendor")
+ || cfg.contains("target_family")
|| cfg.contains("unix")
|| cfg.contains("windows");
diff --git a/src/tools/tidy/src/rustdoc_gui_tests.rs b/src/tools/tidy/src/rustdoc_gui_tests.rs
index feb513df3..91776bc98 100644
--- a/src/tools/tidy/src/rustdoc_gui_tests.rs
+++ b/src/tools/tidy/src/rustdoc_gui_tests.rs
@@ -5,10 +5,7 @@ use std::path::Path;
pub fn check(path: &Path, bad: &mut bool) {
crate::walk::walk(
&path.join("rustdoc-gui"),
- &mut |p| {
- // If there is no extension, it's very likely a folder and we want to go into it.
- p.extension().map(|e| e != "goml").unwrap_or(false)
- },
+ |p, is_dir| !is_dir && p.extension().map_or(true, |e| e != "goml"),
&mut |entry, content| {
for line in content.lines() {
if !line.starts_with("// ") {
diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs
index 6a0855405..a2012db90 100644
--- a/src/tools/tidy/src/style.rs
+++ b/src/tools/tidy/src/style.rs
@@ -19,7 +19,7 @@
use crate::walk::{filter_dirs, walk};
use regex::{Regex, RegexSet};
-use std::path::Path;
+use std::{ffi::OsStr, path::Path};
/// Error code markdown is restricted to 80 columns because they can be
/// displayed on the console with --example.
@@ -171,9 +171,9 @@ fn contains_ignore_directive(can_contain: bool, contents: &str, check: &str) ->
}
macro_rules! suppressible_tidy_err {
- ($err:ident, $skip:ident, $msg:expr) => {
+ ($err:ident, $skip:ident, $msg:literal) => {
if let Directive::Deny = $skip {
- $err($msg);
+ $err(&format!($msg));
} else {
$skip = Directive::Ignore(true);
}
@@ -227,22 +227,41 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool {
}
pub fn check(path: &Path, bad: &mut bool) {
- fn skip(path: &Path) -> bool {
- filter_dirs(path) || skip_markdown_path(path)
+ fn skip(path: &Path, is_dir: bool) -> bool {
+ if path.file_name().map_or(false, |name| name.to_string_lossy().starts_with(".#")) {
+ // vim or emacs temporary file
+ return true;
+ }
+
+ if filter_dirs(path) || skip_markdown_path(path) {
+ return true;
+ }
+
+ // Don't check extensions for directories
+ if is_dir {
+ return false;
+ }
+
+ let extensions = ["rs", "py", "js", "sh", "c", "cpp", "h", "md", "css", "ftl", "goml"];
+
+ // NB: don't skip paths without extensions (or else we'll skip all directories and will only check top level files)
+ if path.extension().map_or(true, |ext| !extensions.iter().any(|e| ext == OsStr::new(e))) {
+ return true;
+ }
+
+ // We only check CSS files in rustdoc.
+ path.extension().map_or(false, |e| e == "css") && !is_in(path, "src", "librustdoc")
}
+
let problematic_consts_strings: Vec<String> = (PROBLEMATIC_CONSTS.iter().map(u32::to_string))
.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| {
+
+ walk(path, skip, &mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
- let extensions =
- [".rs", ".py", ".js", ".sh", ".c", ".cpp", ".h", ".md", ".css", ".ftl", ".goml"];
- if extensions.iter().all(|e| !filename.ends_with(e)) || filename.starts_with(".#") {
- return;
- }
let is_style_file = filename.ends_with(".css");
let under_rustfmt = filename.ends_with(".rs") &&
@@ -253,11 +272,6 @@ pub fn check(path: &Path, bad: &mut bool) {
a.ends_with("src/doc/book")
});
- if is_style_file && !is_in(file, "src", "librustdoc") {
- // We only check CSS files in rustdoc.
- return;
- }
-
if contents.is_empty() {
tidy_error!(bad, "{}: empty file", file.display());
}
@@ -300,10 +314,13 @@ pub fn check(path: &Path, bad: &mut bool) {
contains_ignore_directive(can_contain, &contents, "leading-newlines");
let mut skip_copyright = contains_ignore_directive(can_contain, &contents, "copyright");
let mut skip_dbg = contains_ignore_directive(can_contain, &contents, "dbg");
+ let mut skip_odd_backticks =
+ contains_ignore_directive(can_contain, &contents, "odd-backticks");
let mut leading_new_lines = false;
let mut trailing_new_lines = 0;
let mut lines = 0;
let mut last_safety_comment = false;
+ let mut comment_block: Option<(usize, usize)> = None;
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.
@@ -351,7 +368,7 @@ pub fn check(path: &Path, bad: &mut bool) {
suppressible_tidy_err!(
err,
skip_line_length,
- &format!("line longer than {max_columns} chars")
+ "line longer than {max_columns} chars"
);
}
if !is_style_file && line.contains('\t') {
@@ -415,15 +432,55 @@ pub fn check(path: &Path, bad: &mut bool) {
// For now only enforce in compiler
let is_compiler = || file.components().any(|c| c.as_os_str() == "compiler");
- if is_compiler()
- && line.contains("//")
- && line
- .chars()
- .collect::<Vec<_>>()
- .windows(4)
- .any(|cs| matches!(cs, ['.', ' ', ' ', last] if last.is_alphabetic()))
- {
- err(DOUBLE_SPACE_AFTER_DOT)
+
+ if is_compiler() {
+ if line.contains("//")
+ && line
+ .chars()
+ .collect::<Vec<_>>()
+ .windows(4)
+ .any(|cs| matches!(cs, ['.', ' ', ' ', last] if last.is_alphabetic()))
+ {
+ err(DOUBLE_SPACE_AFTER_DOT)
+ }
+
+ if filename.ends_with(".ftl") {
+ let line_backticks = trimmed.chars().filter(|ch| *ch == '`').count();
+ if line_backticks % 2 == 1 {
+ suppressible_tidy_err!(err, skip_odd_backticks, "odd number of backticks");
+ }
+ } else if trimmed.contains("//") {
+ let (start_line, mut backtick_count) = comment_block.unwrap_or((i + 1, 0));
+ let line_backticks = trimmed.chars().filter(|ch| *ch == '`').count();
+ let comment_text = trimmed.split("//").nth(1).unwrap();
+ // This check ensures that we don't lint for code that has `//` in a string literal
+ if line_backticks % 2 == 1 {
+ backtick_count += comment_text.chars().filter(|ch| *ch == '`').count();
+ }
+ comment_block = Some((start_line, backtick_count));
+ } else {
+ if let Some((start_line, backtick_count)) = comment_block.take() {
+ if backtick_count % 2 == 1 {
+ let mut err = |msg: &str| {
+ tidy_error!(bad, "{}:{start_line}: {msg}", file.display());
+ };
+ let block_len = (i + 1) - start_line;
+ if block_len == 1 {
+ suppressible_tidy_err!(
+ err,
+ skip_odd_backticks,
+ "comment with odd number of backticks"
+ );
+ } else {
+ suppressible_tidy_err!(
+ err,
+ skip_odd_backticks,
+ "{block_len}-line comment block with odd number of backticks"
+ );
+ }
+ }
+ }
+ }
}
}
if leading_new_lines {
@@ -441,7 +498,7 @@ pub fn check(path: &Path, bad: &mut bool) {
n => suppressible_tidy_err!(
err,
skip_trailing_newlines,
- &format!("too many trailing newlines ({n})")
+ "too many trailing newlines ({n})"
),
};
if lines > LINES {
diff --git a/src/tools/tidy/src/target_specific_tests.rs b/src/tools/tidy/src/target_specific_tests.rs
index d7a157672..de022be28 100644
--- a/src/tools/tidy/src/target_specific_tests.rs
+++ b/src/tools/tidy/src/target_specific_tests.rs
@@ -4,6 +4,8 @@
use std::collections::BTreeMap;
use std::path::Path;
+use crate::walk::filter_not_rust;
+
const COMMENT: &str = "//";
const LLVM_COMPONENTS_HEADER: &str = "needs-llvm-components:";
const COMPILE_FLAGS_HEADER: &str = "compile-flags:";
@@ -35,61 +37,57 @@ struct RevisionInfo<'a> {
}
pub fn check(path: &Path, bad: &mut bool) {
- crate::walk::walk(
- path,
- &mut |path| path.extension().map(|p| p == "rs") == Some(false),
- &mut |entry, content| {
- let file = entry.path().display();
- let mut header_map = BTreeMap::new();
- iter_header(content, &mut |cfg, directive| {
- if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) {
- let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
- let comp_vec = info.llvm_components.get_or_insert(Vec::new());
- for component in value.split(' ') {
- let component = component.trim();
- if !component.is_empty() {
- comp_vec.push(component);
- }
- }
- } else if directive.starts_with(COMPILE_FLAGS_HEADER) {
- let compile_flags = &directive[COMPILE_FLAGS_HEADER.len()..];
- if let Some((_, v)) = compile_flags.split_once("--target") {
- if let Some((arch, _)) =
- v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-")
- {
- let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
- info.target_arch.replace(arch);
- } else {
- eprintln!("{file}: seems to have a malformed --target value");
- *bad = true;
- }
+ crate::walk::walk(path, |path, _is_dir| filter_not_rust(path), &mut |entry, content| {
+ let file = entry.path().display();
+ let mut header_map = BTreeMap::new();
+ iter_header(content, &mut |cfg, directive| {
+ if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) {
+ let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
+ let comp_vec = info.llvm_components.get_or_insert(Vec::new());
+ for component in value.split(' ') {
+ let component = component.trim();
+ if !component.is_empty() {
+ comp_vec.push(component);
}
}
- });
- for (rev, RevisionInfo { target_arch, llvm_components }) in &header_map {
- let rev = rev.unwrap_or("[unspecified]");
- match (target_arch, llvm_components) {
- (None, None) => {}
- (Some(_), None) => {
- eprintln!(
- "{}: revision {} should specify `{}` as it has `--target` set",
- file, rev, LLVM_COMPONENTS_HEADER
- );
+ } else if directive.starts_with(COMPILE_FLAGS_HEADER) {
+ let compile_flags = &directive[COMPILE_FLAGS_HEADER.len()..];
+ if let Some((_, v)) = compile_flags.split_once("--target") {
+ if let Some((arch, _)) =
+ v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-")
+ {
+ let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
+ info.target_arch.replace(arch);
+ } else {
+ eprintln!("{file}: seems to have a malformed --target value");
*bad = true;
}
- (None, Some(_)) => {
- eprintln!(
- "{}: revision {} should not specify `{}` as it doesn't need `--target`",
- file, rev, LLVM_COMPONENTS_HEADER
- );
- *bad = true;
- }
- (Some(_), Some(_)) => {
- // FIXME: check specified components against the target architectures we
- // gathered.
- }
}
}
- },
- );
+ });
+ for (rev, RevisionInfo { target_arch, llvm_components }) in &header_map {
+ let rev = rev.unwrap_or("[unspecified]");
+ match (target_arch, llvm_components) {
+ (None, None) => {}
+ (Some(_), None) => {
+ eprintln!(
+ "{}: revision {} should specify `{}` as it has `--target` set",
+ file, rev, LLVM_COMPONENTS_HEADER
+ );
+ *bad = true;
+ }
+ (None, Some(_)) => {
+ eprintln!(
+ "{}: revision {} should not specify `{}` as it doesn't need `--target`",
+ file, rev, LLVM_COMPONENTS_HEADER
+ );
+ *bad = true;
+ }
+ (Some(_), Some(_)) => {
+ // FIXME: check specified components against the target architectures we
+ // gathered.
+ }
+ }
+ }
+ });
}
diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs
index 409f75631..29664c854 100644
--- a/src/tools/tidy/src/ui_tests.rs
+++ b/src/tools/tidy/src/ui_tests.rs
@@ -3,87 +3,99 @@
//! - there are no stray `.stderr` files
use ignore::Walk;
-use ignore::WalkBuilder;
+use std::collections::HashMap;
use std::fs;
-use std::path::Path;
+use std::path::{Path, PathBuf};
-const ENTRY_LIMIT: usize = 1000;
// FIXME: The following limits should be reduced eventually.
-const ROOT_ENTRY_LIMIT: usize = 940;
-const ISSUES_ENTRY_LIMIT: usize = 1978;
+const ENTRY_LIMIT: usize = 885;
+const ROOT_ENTRY_LIMIT: usize = 891;
+const ISSUES_ENTRY_LIMIT: usize = 1977;
-fn check_entries(path: &Path, bad: &mut bool) {
- for dir in Walk::new(&path.join("ui")) {
- if let Ok(entry) = dir {
- if entry.file_type().map(|ft| ft.is_dir()).unwrap_or(false) {
- let dir_path = entry.path();
- // Use special values for these dirs.
- let is_root = path.join("ui") == dir_path;
- let is_issues_dir = path.join("ui/issues") == dir_path;
- let limit = if is_root {
- ROOT_ENTRY_LIMIT
- } else if is_issues_dir {
- ISSUES_ENTRY_LIMIT
- } else {
- ENTRY_LIMIT
- };
+fn check_entries(tests_path: &Path, bad: &mut bool) {
+ let mut directories: HashMap<PathBuf, usize> = HashMap::new();
- let count = WalkBuilder::new(&dir_path)
- .max_depth(Some(1))
- .build()
- .into_iter()
- .collect::<Vec<_>>()
- .len()
- - 1; // remove the dir itself
+ for dir in Walk::new(&tests_path.join("ui")) {
+ if let Ok(entry) = dir {
+ let parent = entry.path().parent().unwrap().to_path_buf();
+ *directories.entry(parent).or_default() += 1;
+ }
+ }
- if count > limit {
- tidy_error!(
- bad,
- "following path contains more than {} entries, \
- you should move the test to some relevant subdirectory (current: {}): {}",
- limit,
- count,
- dir_path.display()
- );
- }
- }
+ let (mut max, mut max_root, mut max_issues) = (0usize, 0usize, 0usize);
+ for (dir_path, count) in directories {
+ // Use special values for these dirs.
+ let is_root = tests_path.join("ui") == dir_path;
+ let is_issues_dir = tests_path.join("ui/issues") == dir_path;
+ let (limit, maxcnt) = if is_root {
+ (ROOT_ENTRY_LIMIT, &mut max_root)
+ } else if is_issues_dir {
+ (ISSUES_ENTRY_LIMIT, &mut max_issues)
+ } else {
+ (ENTRY_LIMIT, &mut max)
+ };
+ *maxcnt = (*maxcnt).max(count);
+ if count > limit {
+ tidy_error!(
+ bad,
+ "following path contains more than {} entries, \
+ you should move the test to some relevant subdirectory (current: {}): {}",
+ limit,
+ count,
+ dir_path.display()
+ );
}
}
+ if ENTRY_LIMIT > max {
+ tidy_error!(bad, "`ENTRY_LIMIT` is too high (is {ENTRY_LIMIT}, should be {max})");
+ }
+ if ROOT_ENTRY_LIMIT > max_root {
+ tidy_error!(
+ bad,
+ "`ROOT_ENTRY_LIMIT` is too high (is {ROOT_ENTRY_LIMIT}, should be {max_root})"
+ );
+ }
+ if ISSUES_ENTRY_LIMIT > max_issues {
+ tidy_error!(
+ bad,
+ "`ISSUES_ENTRY_LIMIT` is too high (is {ISSUES_ENTRY_LIMIT}, should be {max_issues})"
+ );
+ }
}
pub fn check(path: &Path, bad: &mut bool) {
check_entries(&path, bad);
- for path in &[&path.join("ui"), &path.join("ui-fulldeps")] {
- crate::walk::walk_no_read(path, &mut |_| false, &mut |entry| {
- let file_path = entry.path();
- if let Some(ext) = file_path.extension() {
- if ext == "stderr" || ext == "stdout" {
- // Test output filenames have one of the formats:
- // ```
- // $testname.stderr
- // $testname.$mode.stderr
- // $testname.$revision.stderr
- // $testname.$revision.$mode.stderr
- // ```
- //
- // For now, just make sure that there is a corresponding
- // `$testname.rs` file.
- //
- // NB: We do not use file_stem() as some file names have multiple `.`s and we
- // must strip all of them.
- let testname =
- file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0;
- if !file_path.with_file_name(testname).with_extension("rs").exists() {
- tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path);
- }
+ let (ui, ui_fulldeps) = (path.join("ui"), path.join("ui-fulldeps"));
+ let paths = [ui.as_path(), ui_fulldeps.as_path()];
+ crate::walk::walk_no_read(&paths, |_, _| false, &mut |entry| {
+ let file_path = entry.path();
+ if let Some(ext) = file_path.extension() {
+ if ext == "stderr" || ext == "stdout" {
+ // Test output filenames have one of the formats:
+ // ```
+ // $testname.stderr
+ // $testname.$mode.stderr
+ // $testname.$revision.stderr
+ // $testname.$revision.$mode.stderr
+ // ```
+ //
+ // For now, just make sure that there is a corresponding
+ // `$testname.rs` file.
+ //
+ // NB: We do not use file_stem() as some file names have multiple `.`s and we
+ // must strip all of them.
+ let testname =
+ file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0;
+ if !file_path.with_file_name(testname).with_extension("rs").exists() {
+ tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path);
+ }
- if let Ok(metadata) = fs::metadata(file_path) {
- if metadata.len() == 0 {
- tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path);
- }
+ if let Ok(metadata) = fs::metadata(file_path) {
+ if metadata.len() == 0 {
+ tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path);
}
}
}
- });
- }
+ }
+ });
}
diff --git a/src/tools/tidy/src/unit_tests.rs b/src/tools/tidy/src/unit_tests.rs
index 27f36c855..0a5dad887 100644
--- a/src/tools/tidy/src/unit_tests.rs
+++ b/src/tools/tidy/src/unit_tests.rs
@@ -11,18 +11,19 @@ use crate::walk::{filter_dirs, walk};
use std::path::Path;
pub fn check(root_path: &Path, bad: &mut bool) {
- let core = &root_path.join("core");
- let core_tests = &core.join("tests");
- let core_benches = &core.join("benches");
- let is_core = |path: &Path| {
- path.starts_with(core) && !(path.starts_with(core_tests) || path.starts_with(core_benches))
+ let core = root_path.join("core");
+ let core_copy = core.clone();
+ let core_tests = core.join("tests");
+ let core_benches = core.join("benches");
+ let is_core = move |path: &Path| {
+ path.starts_with(&core)
+ && !(path.starts_with(&core_tests) || path.starts_with(&core_benches))
};
- let mut skip = |path: &Path| {
+ let skip = move |path: &Path, is_dir| {
let file_name = path.file_name().unwrap_or_default();
- if path.is_dir() {
+ if is_dir {
filter_dirs(path)
- || path.ends_with("tests")
|| path.ends_with("src/doc")
|| (file_name == "tests" || file_name == "benches") && !is_core(path)
} else {
@@ -35,9 +36,9 @@ pub fn check(root_path: &Path, bad: &mut bool) {
}
};
- walk(root_path, &mut skip, &mut |entry, contents| {
+ walk(root_path, skip, &mut |entry, contents| {
let path = entry.path();
- let is_core = path.starts_with(core);
+ let is_core = path.starts_with(&core_copy);
for (i, line) in contents.lines().enumerate() {
let line = line.trim();
let is_test = || line.contains("#[test]") && !line.contains("`#[test]");
diff --git a/src/tools/tidy/src/walk.rs b/src/tools/tidy/src/walk.rs
index 4cfb70fa3..3539943eb 100644
--- a/src/tools/tidy/src/walk.rs
+++ b/src/tools/tidy/src/walk.rs
@@ -1,9 +1,8 @@
-use std::fs::File;
-use std::io::Read;
-use walkdir::{DirEntry, WalkDir};
+use ignore::DirEntry;
-use std::path::Path;
+use std::{ffi::OsStr, fs::File, io::Read, path::Path};
+/// The default directory filter.
pub fn filter_dirs(path: &Path) -> bool {
let skip = [
"tidy-test-file",
@@ -30,40 +29,57 @@ pub fn filter_dirs(path: &Path) -> bool {
// Filter RLS output directories
"target/rls",
"src/bootstrap/target",
+ "vendor",
];
skip.iter().any(|p| path.ends_with(p))
}
-pub fn walk_many(
- paths: &[&Path],
- skip: &mut dyn FnMut(&Path) -> bool,
+/// Filter for only files that end in `.rs`.
+pub fn filter_not_rust(path: &Path) -> bool {
+ path.extension() != Some(OsStr::new("rs")) && !path.is_dir()
+}
+
+pub fn walk(
+ path: &Path,
+ skip: impl Send + Sync + 'static + Fn(&Path, bool) -> bool,
f: &mut dyn FnMut(&DirEntry, &str),
) {
- for path in paths {
- walk(path, skip, f);
- }
+ walk_many(&[path], skip, f);
}
-pub fn walk(path: &Path, skip: &mut dyn FnMut(&Path) -> bool, f: &mut dyn FnMut(&DirEntry, &str)) {
- let mut contents = String::new();
- walk_no_read(path, skip, &mut |entry| {
+pub fn walk_many(
+ paths: &[&Path],
+ skip: impl Send + Sync + 'static + Fn(&Path, bool) -> bool,
+ f: &mut dyn FnMut(&DirEntry, &str),
+) {
+ let mut contents = Vec::new();
+ walk_no_read(paths, skip, &mut |entry| {
contents.clear();
- if t!(File::open(entry.path()), entry.path()).read_to_string(&mut contents).is_err() {
- contents.clear();
- }
- f(&entry, &contents);
+ let mut file = t!(File::open(entry.path()), entry.path());
+ t!(file.read_to_end(&mut contents), entry.path());
+ let contents_str = match std::str::from_utf8(&contents) {
+ Ok(s) => s,
+ Err(_) => return, // skip this file
+ };
+ f(&entry, &contents_str);
});
}
pub(crate) fn walk_no_read(
- path: &Path,
- skip: &mut dyn FnMut(&Path) -> bool,
+ paths: &[&Path],
+ skip: impl Send + Sync + 'static + Fn(&Path, bool) -> bool,
f: &mut dyn FnMut(&DirEntry),
) {
- let walker = WalkDir::new(path).into_iter().filter_entry(|e| !skip(e.path()));
- for entry in walker {
+ let mut walker = ignore::WalkBuilder::new(paths[0]);
+ for path in &paths[1..] {
+ walker.add(path);
+ }
+ let walker = walker.filter_entry(move |e| {
+ !skip(e.path(), e.file_type().map(|ft| ft.is_dir()).unwrap_or(false))
+ });
+ for entry in walker.build() {
if let Ok(entry) = entry {
- if entry.file_type().is_dir() {
+ if entry.file_type().map_or(true, |kind| kind.is_dir() || kind.is_symlink()) {
continue;
}
f(&entry);
generated by cgit v1.2.3 (git 2.39.1) at 2024-10-18 21:09:48 +0000