Skip to content

Commit

Permalink
Rollup merge of rust-lang#102030 - est31:tidy_walk_no_reexport, r=Mar…
Browse files Browse the repository at this point in the history
…k-Simulacrum

Don't crate-locally reexport walk functions in tidy

I've moved the walk functions into their own module in rust-lang#100591 and didn't want to make changing the paths everywhere in tidy part of the PRs diff, so I just reexported the functions locally. This PR removes the crate-local reexport and instead does module level reexports. I'm not sure how much it's worth it and whether the new state is better, idk. Feel free to have any opinion on this.
  • Loading branch information
JohnTitor authored Sep 22, 2022
2 parents b08c8cb + b8d2f5c commit 614e18b
Show file tree
Hide file tree
Showing 12 changed files with 28 additions and 21 deletions.
5 changes: 3 additions & 2 deletions src/tools/tidy/src/bins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ mod os_impl {

#[cfg(unix)]
mod os_impl {
use crate::walk::{filter_dirs, walk_no_read};
use std::fs;
use std::os::unix::prelude::*;
use std::path::Path;
Expand Down Expand Up @@ -100,10 +101,10 @@ mod os_impl {

const ALLOWED: &[&str] = &["configure", "x"];

crate::walk_no_read(
walk_no_read(
path,
&mut |path| {
crate::filter_dirs(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
Expand Down
3 changes: 2 additions & 1 deletion src/tools/tidy/src/debug_artifacts.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
//! Tidy check to prevent creation of unnecessary debug artifacts while running tests.

use crate::walk::{filter_dirs, walk};
use std::path::{Path, PathBuf};

const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in test";

pub fn check(path: &Path, bad: &mut bool) {
let test_dir: PathBuf = path.join("test");

super::walk(&test_dir, &mut super::filter_dirs, &mut |entry, contents| {
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 {
Expand Down
5 changes: 3 additions & 2 deletions src/tools/tidy/src/edition.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Tidy check to ensure that crate `edition` is '2018' or '2021'.

use crate::walk::{filter_dirs, walk};
use std::path::Path;

fn is_edition_2021(mut line: &str) -> bool {
Expand All @@ -8,9 +9,9 @@ fn is_edition_2021(mut line: &str) -> bool {
}

pub fn check(path: &Path, bad: &mut bool) {
super::walk(
walk(
path,
&mut |path| super::filter_dirs(path) || path.ends_with("src/test"),
&mut |path| filter_dirs(path) || path.ends_with("src/test"),
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap();
Expand Down
3 changes: 2 additions & 1 deletion src/tools/tidy/src/error_codes_check.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Checks that all error codes have at least one test to prevent having error
//! codes that are silently not thrown by the compiler anymore.

use crate::walk::{filter_dirs, walk};
use std::collections::{HashMap, HashSet};
use std::ffi::OsStr;
use std::fs::read_to_string;
Expand Down Expand Up @@ -217,7 +218,7 @@ pub fn check(paths: &[&Path], bad: &mut bool) {
println!("Checking which error codes lack tests...");

for path in paths {
super::walk(path, &mut super::filter_dirs, &mut |entry, contents| {
walk(path, &mut filter_dirs, &mut |entry, contents| {
let file_name = entry.file_name();
let entry_path = entry.path();

Expand Down
5 changes: 3 additions & 2 deletions src/tools/tidy/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
//! This ensures that error codes are used at most once and also prints out some
//! statistics about the error codes.

use crate::walk::{filter_dirs, walk};
use std::collections::HashMap;
use std::path::Path;

pub fn check(path: &Path, bad: &mut bool) {
let mut map: HashMap<_, Vec<_>> = HashMap::new();
super::walk(
walk(
path,
&mut |path| super::filter_dirs(path) || path.ends_with("src/test"),
&mut |path| filter_dirs(path) || path.ends_with("src/test"),
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
Expand Down
9 changes: 5 additions & 4 deletions src/tools/tidy/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
//! * 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 std::collections::HashMap;
use std::fmt;
use std::fs;
Expand Down Expand Up @@ -92,14 +93,14 @@ pub fn check(
let lib_features = get_and_check_lib_features(lib_path, bad, &features);
assert!(!lib_features.is_empty());

super::walk_many(
walk_many(
&[
&src_path.join("test/ui"),
&src_path.join("test/ui-fulldeps"),
&src_path.join("test/rustdoc-ui"),
&src_path.join("test/rustdoc"),
],
&mut |path| super::filter_dirs(path),
&mut filter_dirs,
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
Expand Down Expand Up @@ -466,9 +467,9 @@ fn map_lib_features(
base_src_path: &Path,
mf: &mut dyn FnMut(Result<(&str, Feature), &str>, &Path, usize),
) {
super::walk(
walk(
base_src_path,
&mut |path| super::filter_dirs(path) || path.ends_with("src/test"),
&mut |path| filter_dirs(path) || path.ends_with("src/test"),
&mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap().to_string_lossy();
Expand Down
2 changes: 0 additions & 2 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
//! This library contains the tidy lints and exposes it
//! to be used by tools.

use walk::{filter_dirs, walk, walk_many, walk_no_read};

/// A helper macro to `unwrap` a result except also print out details like:
///
/// * The expression that failed
Expand Down
3 changes: 2 additions & 1 deletion src/tools/tidy/src/pal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
//! platform-specific cfgs are allowed. Not sure yet how to deal with
//! this in the long term.

use crate::walk::{filter_dirs, walk};
use std::iter::Iterator;
use std::path::Path;

Expand Down Expand Up @@ -67,7 +68,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;
super::walk(path, &mut super::filter_dirs, &mut |entry, contents| {
walk(path, &mut filter_dirs, &mut |entry, contents| {
let file = entry.path();
let filestr = file.to_string_lossy().replace("\\", "/");
if !filestr.ends_with(".rs") {
Expand Down
5 changes: 3 additions & 2 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//! A number of these checks can be opted-out of with various directives of the form:
//! `// ignore-tidy-CHECK-NAME`.

use crate::walk::{filter_dirs, walk};
use regex::Regex;
use std::path::Path;

Expand Down Expand Up @@ -218,13 +219,13 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool {

pub fn check(path: &Path, bad: &mut bool) {
fn skip(path: &Path) -> bool {
super::filter_dirs(path) || skip_markdown_path(path)
filter_dirs(path) || skip_markdown_path(path)
}
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();
super::walk(path, &mut skip, &mut |entry, contents| {
walk(path, &mut 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"];
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/target_specific_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ struct RevisionInfo<'a> {

pub fn check(path: &Path, bad: &mut bool) {
let tests = path.join("test");
super::walk(
crate::walk::walk(
&tests,
&mut |path| path.extension().map(|p| p == "rs") == Some(false),
&mut |entry, content| {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ fn check_entries(path: &Path, bad: &mut bool) {
pub fn check(path: &Path, bad: &mut bool) {
check_entries(&path, bad);
for path in &[&path.join("test/ui"), &path.join("test/ui-fulldeps")] {
super::walk_no_read(path, &mut |_| false, &mut |entry| {
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" {
Expand Down
5 changes: 3 additions & 2 deletions src/tools/tidy/src/unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//! named `tests.rs` or `benches.rs`, or directories named `tests` or `benches` unconfigured
//! during normal build.

use crate::walk::{filter_dirs, walk};
use std::path::Path;

pub fn check(root_path: &Path, bad: &mut bool) {
Expand All @@ -20,7 +21,7 @@ pub fn check(root_path: &Path, bad: &mut bool) {
let mut skip = |path: &Path| {
let file_name = path.file_name().unwrap_or_default();
if path.is_dir() {
super::filter_dirs(path)
filter_dirs(path)
|| path.ends_with("src/test")
|| path.ends_with("src/doc")
|| (file_name == "tests" || file_name == "benches") && !is_core(path)
Expand All @@ -34,7 +35,7 @@ pub fn check(root_path: &Path, bad: &mut bool) {
}
};

super::walk(root_path, &mut skip, &mut |entry, contents| {
walk(root_path, &mut skip, &mut |entry, contents| {
let path = entry.path();
let is_core = path.starts_with(core);
for (i, line) in contents.lines().enumerate() {
Expand Down

0 comments on commit 614e18b

Please sign in to comment.