Skip to content

Commit 436afdf

Browse files
WaffleLapkinWaffleLapkin
committed
Don't skip all directories when tidy-checking
1 parent dd19135 commit 436afdf

15 files changed

+76
-56
lines changed

‎src/tools/replace-version-placeholder/src/main.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ fn main() {
1010
let version_str = version_str.trim();
1111
walk::walk(
1212
&root_path,
13-
|path| {
13+
|path, _is_dir| {
1414
walk::filter_dirs(path)
1515
// We exempt these as they require the placeholder
1616
// for their operation

‎src/tools/tidy/src/alphabetical.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ fn check_section<'a>(
9595
}
9696

9797
pub fn check(path: &Path, bad: &mut bool) {
98-
walk(path, filter_dirs, &mut |entry, contents| {
98+
walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
9999
let file = &entry.path().display();
100100

101101
let mut lines = contents.lines().enumerate().peekable();

‎src/tools/tidy/src/bins.rs

+31-27
Original file line numberDiff line numberDiff line change
@@ -103,36 +103,40 @@ mod os_impl {
103103

104104
// FIXME: we don't need to look at all binaries, only files that have been modified in this branch
105105
// (e.g. using `git ls-files`).
106-
walk_no_read(&[path], |path| filter_dirs(path) || path.ends_with("src/etc"), &mut |entry| {
107-
let file = entry.path();
108-
let extension = file.extension();
109-
let scripts = ["py", "sh", "ps1"];
110-
if scripts.into_iter().any(|e| extension == Some(OsStr::new(e))) {
111-
return;
112-
}
113-
114-
if t!(is_executable(&file), file) {
115-
let rel_path = file.strip_prefix(path).unwrap();
116-
let git_friendly_path = rel_path.to_str().unwrap().replace("\\", "/");
117-
118-
if ALLOWED.contains(&git_friendly_path.as_str()) {
106+
walk_no_read(
107+
&[path],
108+
|path, _is_dir| filter_dirs(path) || path.ends_with("src/etc"),
109+
&mut |entry| {
110+
let file = entry.path();
111+
let extension = file.extension();
112+
let scripts = ["py", "sh", "ps1"];
113+
if scripts.into_iter().any(|e| extension == Some(OsStr::new(e))) {
119114
return;
120115
}
121116

122-
let output = Command::new("git")
123-
.arg("ls-files")
124-
.arg(&git_friendly_path)
125-
.current_dir(path)
126-
.stderr(Stdio::null())
127-
.output()
128-
.unwrap_or_else(|e| {
129-
panic!("could not run git ls-files: {e}");
130-
});
131-
let path_bytes = rel_path.as_os_str().as_bytes();
132-
if output.status.success() && output.stdout.starts_with(path_bytes) {
133-
tidy_error!(bad, "binary checked into source: {}", file.display());
117+
if t!(is_executable(&file), file) {
118+
let rel_path = file.strip_prefix(path).unwrap();
119+
let git_friendly_path = rel_path.to_str().unwrap().replace("\\", "/");
120+
121+
if ALLOWED.contains(&git_friendly_path.as_str()) {
122+
return;
123+
}
124+
125+
let output = Command::new("git")
126+
.arg("ls-files")
127+
.arg(&git_friendly_path)
128+
.current_dir(path)
129+
.stderr(Stdio::null())
130+
.output()
131+
.unwrap_or_else(|e| {
132+
panic!("could not run git ls-files: {e}");
133+
});
134+
let path_bytes = rel_path.as_os_str().as_bytes();
135+
if output.status.success() && output.stdout.starts_with(path_bytes) {
136+
tidy_error!(bad, "binary checked into source: {}", file.display());
137+
}
134138
}
135-
}
136-
})
139+
},
140+
)
137141
}
138142
}

‎src/tools/tidy/src/debug_artifacts.rs

+16-6
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,21 @@ use std::path::Path;
66
const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in test";
77

88
pub fn check(test_dir: &Path, bad: &mut bool) {
9-
walk(test_dir, |path| filter_dirs(path) || filter_not_rust(path), &mut |entry, contents| {
10-
for (i, line) in contents.lines().enumerate() {
11-
if line.contains("borrowck_graphviz_postflow") {
12-
tidy_error!(bad, "{}:{}: {}", entry.path().display(), i + 1, GRAPHVIZ_POSTFLOW_MSG);
9+
walk(
10+
test_dir,
11+
|path, _is_dir| filter_dirs(path) || filter_not_rust(path),
12+
&mut |entry, contents| {
13+
for (i, line) in contents.lines().enumerate() {
14+
if line.contains("borrowck_graphviz_postflow") {
15+
tidy_error!(
16+
bad,
17+
"{}:{}: {}",
18+
entry.path().display(),
19+
i + 1,
20+
GRAPHVIZ_POSTFLOW_MSG
21+
);
22+
}
1323
}
14-
}
15-
});
24+
},
25+
);
1626
}

‎src/tools/tidy/src/edition.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ fn is_edition_2021(mut line: &str) -> bool {
99
}
1010

1111
pub fn check(path: &Path, bad: &mut bool) {
12-
walk(path, |path| filter_dirs(path), &mut |entry, contents| {
12+
walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
1313
let file = entry.path();
1414
let filename = file.file_name().unwrap();
1515
if filename != "Cargo.toml" {

‎src/tools/tidy/src/error_codes.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ fn check_error_codes_docs(
129129

130130
let mut no_longer_emitted_codes = Vec::new();
131131

132-
walk(&docs_path, |_| false, &mut |entry, contents| {
132+
walk(&docs_path, |_, _| false, &mut |entry, contents| {
133133
let path = entry.path();
134134

135135
// Error if the file isn't markdown.
@@ -321,7 +321,7 @@ fn check_error_codes_used(
321321

322322
let mut found_codes = Vec::new();
323323

324-
walk_many(search_paths, filter_dirs, &mut |entry, contents| {
324+
walk_many(search_paths, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
325325
let path = entry.path();
326326

327327
// Return early if we aren't looking at a source file.

‎src/tools/tidy/src/features.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ pub fn check(
102102
&tests_path.join("rustdoc-ui"),
103103
&tests_path.join("rustdoc"),
104104
],
105-
|path| {
105+
|path, _is_dir| {
106106
filter_dirs(path)
107107
|| filter_not_rust(path)
108108
|| path.file_name() == Some(OsStr::new("features.rs"))
@@ -478,7 +478,7 @@ fn map_lib_features(
478478
) {
479479
walk(
480480
base_src_path,
481-
|path| filter_dirs(path) || path.ends_with("tests"),
481+
|path, _is_dir| filter_dirs(path) || path.ends_with("tests"),
482482
&mut |entry, contents| {
483483
let file = entry.path();
484484
let filename = file.file_name().unwrap().to_string_lossy();

‎src/tools/tidy/src/mir_opt_tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ fn check_unused_files(path: &Path, bless: bool, bad: &mut bool) {
1111

1212
walk_no_read(
1313
&[&path.join("mir-opt")],
14-
|path| path.file_name() == Some("README.md".as_ref()),
14+
|path, _is_dir| path.file_name() == Some("README.md".as_ref()),
1515
&mut |file| {
1616
let filepath = file.path();
1717
if filepath.extension() == Some("rs".as_ref()) {

‎src/tools/tidy/src/pal.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ pub fn check(path: &Path, bad: &mut bool) {
6767
// Sanity check that the complex parsing here works.
6868
let mut saw_target_arch = false;
6969
let mut saw_cfg_bang = false;
70-
walk(path, filter_dirs, &mut |entry, contents| {
70+
walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
7171
let file = entry.path();
7272
let filestr = file.to_string_lossy().replace("\\", "/");
7373
if !filestr.ends_with(".rs") {

‎src/tools/tidy/src/rustdoc_gui_tests.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@ use std::path::Path;
55
pub fn check(path: &Path, bad: &mut bool) {
66
crate::walk::walk(
77
&path.join("rustdoc-gui"),
8-
|p| {
9-
// If there is no extension, it's very likely a folder and we want to go into it.
10-
p.extension().map(|e| e != "goml").unwrap_or(false)
11-
},
8+
|p, is_dir| !is_dir && p.extension().map_or(true, |e| e != "goml"),
129
&mut |entry, content| {
1310
for line in content.lines() {
1411
if !line.starts_with("// ") {

‎src/tools/tidy/src/style.rs

+9-2
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool {
227227
}
228228

229229
pub fn check(path: &Path, bad: &mut bool) {
230-
fn skip(path: &Path) -> bool {
230+
fn skip(path: &Path, is_dir: bool) -> bool {
231231
if path.file_name().map_or(false, |name| name.to_string_lossy().starts_with(".#")) {
232232
// vim or emacs temporary file
233233
return true;
@@ -237,8 +237,15 @@ pub fn check(path: &Path, bad: &mut bool) {
237237
return true;
238238
}
239239

240+
// Don't check extensions for directories
241+
if is_dir {
242+
return false;
243+
}
244+
240245
let extensions = ["rs", "py", "js", "sh", "c", "cpp", "h", "md", "css", "ftl", "goml"];
241-
if extensions.iter().all(|e| path.extension() != Some(OsStr::new(e))) {
246+
247+
// NB: don't skip paths without extensions (or else we'll skip all directories and will only check top level files)
248+
if path.extension().map_or(true, |ext| !extensions.iter().any(|e| ext == OsStr::new(e))) {
242249
return true;
243250
}
244251

‎src/tools/tidy/src/target_specific_tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ struct RevisionInfo<'a> {
3737
}
3838

3939
pub fn check(path: &Path, bad: &mut bool) {
40-
crate::walk::walk(path, filter_not_rust, &mut |entry, content| {
40+
crate::walk::walk(path, |path, _is_dir| filter_not_rust(path), &mut |entry, content| {
4141
let file = entry.path().display();
4242
let mut header_map = BTreeMap::new();
4343
iter_header(content, &mut |cfg, directive| {

‎src/tools/tidy/src/ui_tests.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ pub fn check(path: &Path, bad: &mut bool) {
5151
check_entries(&path, bad);
5252
let (ui, ui_fulldeps) = (path.join("ui"), path.join("ui-fulldeps"));
5353
let paths = [ui.as_path(), ui_fulldeps.as_path()];
54-
crate::walk::walk_no_read(&paths, |_| false, &mut |entry| {
54+
crate::walk::walk_no_read(&paths, |_, _| false, &mut |entry| {
5555
let file_path = entry.path();
5656
if let Some(ext) = file_path.extension() {
5757
if ext == "stderr" || ext == "stdout" {

‎src/tools/tidy/src/unit_tests.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ pub fn check(root_path: &Path, bad: &mut bool) {
2020
&& !(path.starts_with(&core_tests) || path.starts_with(&core_benches))
2121
};
2222

23-
let skip = move |path: &Path| {
23+
let skip = move |path: &Path, is_dir| {
2424
let file_name = path.file_name().unwrap_or_default();
25-
if path.is_dir() {
25+
if is_dir {
2626
filter_dirs(path)
2727
|| path.ends_with("src/doc")
2828
|| (file_name == "tests" || file_name == "benches") && !is_core(path)

‎src/tools/tidy/src/walk.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,15 @@ pub fn filter_not_rust(path: &Path) -> bool {
4141

4242
pub fn walk(
4343
path: &Path,
44-
skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool,
44+
skip: impl Clone + Send + Sync + 'static + Fn(&Path, bool) -> bool,
4545
f: &mut dyn FnMut(&DirEntry, &str),
4646
) {
4747
walk_many(&[path], skip, f);
4848
}
4949

5050
pub fn walk_many(
5151
paths: &[&Path],
52-
skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool,
52+
skip: impl Clone + Send + Sync + 'static + Fn(&Path, bool) -> bool,
5353
f: &mut dyn FnMut(&DirEntry, &str),
5454
) {
5555
let mut contents = Vec::new();
@@ -67,14 +67,16 @@ pub fn walk_many(
6767

6868
pub(crate) fn walk_no_read(
6969
paths: &[&Path],
70-
skip: impl Send + Sync + 'static + Fn(&Path) -> bool,
70+
skip: impl Send + Sync + 'static + Fn(&Path, bool) -> bool,
7171
f: &mut dyn FnMut(&DirEntry),
7272
) {
7373
let mut walker = ignore::WalkBuilder::new(paths[0]);
7474
for path in &paths[1..] {
7575
walker.add(path);
7676
}
77-
let walker = walker.filter_entry(move |e| !skip(e.path()));
77+
let walker = walker.filter_entry(move |e| {
78+
!skip(e.path(), e.file_type().map(|ft| ft.is_dir()).unwrap_or(false))
79+
});
7880
for entry in walker.build() {
7981
if let Ok(entry) = entry {
8082
if entry.file_type().map_or(true, |kind| kind.is_dir() || kind.is_symlink()) {

0 commit comments

Comments
 (0)