Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't skip all directories when tidy-checking #109440

Merged
merged 3 commits into from
Mar 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions compiler/rustc_attr/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ impl UnstableReason {
}

/// Collects stability info from `stable`/`unstable`/`rustc_allowed_through_unstable_modules`
/// attributes in `attrs`. Returns `None` if no stability attributes are found.
/// attributes in `attrs`. Returns `None` if no stability attributes are found.
pub fn find_stability(
sess: &Session,
attrs: &[Attribute],
Expand Down Expand Up @@ -281,7 +281,7 @@ pub fn find_stability(
}

/// Collects stability info from `rustc_const_stable`/`rustc_const_unstable`/`rustc_promotable`
/// attributes in `attrs`. Returns `None` if no stability attributes are found.
/// attributes in `attrs`. Returns `None` if no stability attributes are found.
pub fn find_const_stability(
sess: &Session,
attrs: &[Attribute],
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1621,7 +1621,7 @@ impl<'a> Linker for AixLinker<'a> {
let path = tmpdir.join("list.exp");
let res: io::Result<()> = try {
let mut f = BufWriter::new(File::create(&path)?);
// TODO: use llvm-nm to generate export list.
// FIXME: use llvm-nm to generate export list.
for symbol in symbols {
debug!(" _{}", symbol);
writeln!(f, " {}", symbol)?;
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_error_codes/src/error_codes/E0080.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ or causing an integer overflow are two ways to induce this error.
Ensure that the expressions given can be evaluated as the desired integer type.

See the [Discriminants] section of the Reference for more information about
setting custom integer types on enums using the [`repr` attribute][repr-attribute].
setting custom integer types on enums using the
[`repr` attribute][repr-attribute].

[discriminants]: https://doc.rust-lang.org/reference/items/enumerations.html#discriminants
[repr-attribute]: https://doc.rust-lang.org/reference/type-layout.html#representations
6 changes: 3 additions & 3 deletions compiler/rustc_error_codes/src/error_codes/E0794.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,6 @@ In the definition of `bar`, the lifetime parameter `'a` is late-bound, while
where `'a` is universally quantified and `'b` is substituted by a specific
lifetime. It is not allowed to explicitly specify early-bound lifetime
arguments when late-bound lifetime parameters are present (as for `bar_fn2`,
see [issue #42868](https://github.com/rust-lang/rust/issues/42868)), although the
types that are constrained by early-bound parameters can be specified (as for
`bar_fn3`).
see [issue #42868](https://github.com/rust-lang/rust/issues/42868)), although
the types that are constrained by early-bound parameters can be specified (as
for `bar_fn3`).
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/expr/as_rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
BinOp::Shl | BinOp::Shr if self.check_overflow && ty.is_integral() => {
// For an unsigned RHS, the shift is in-range for `rhs < bits`.
// For a signed RHS, `IntToInt` cast to the equivalent unsigned
// type and do that same comparison. Because the type is the
// type and do that same comparison. Because the type is the
// same size, there's no negative shift amount that ends up
// overlapping with valid ones, thus it catches negatives too.
let (lhs_size, _) = ty.int_size_and_signed(self.tcx);
Expand Down
2 changes: 1 addition & 1 deletion src/doc/unstable-book/src/compiler-flags/sanitizer.md
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ See the [Clang ControlFlowIntegrity documentation][clang-cfi] for more details.

## Example

```rust,ignore
```rust,ignore (making doc tests pass cross-platform is hard)
#![feature(naked_functions)]

use std::arch::asm;
Expand Down
2 changes: 1 addition & 1 deletion src/tools/replace-version-placeholder/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ fn main() {
let version_str = version_str.trim();
walk::walk(
&root_path,
|path| {
|path, _is_dir| {
walk::filter_dirs(path)
// We exempt these as they require the placeholder
// for their operation
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/alphabetical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ fn check_section<'a>(
}

pub fn check(path: &Path, bad: &mut bool) {
walk(path, 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();
Expand Down
58 changes: 31 additions & 27 deletions src/tools/tidy/src/bins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,36 +103,40 @@ mod os_impl {

// 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], |path| filter_dirs(path) || path.ends_with("src/etc"), &mut |entry| {
let file = entry.path();
let extension = file.extension();
let scripts = ["py", "sh", "ps1"];
if scripts.into_iter().any(|e| extension == Some(OsStr::new(e))) {
return;
}

if t!(is_executable(&file), file) {
let rel_path = file.strip_prefix(path).unwrap();
let git_friendly_path = rel_path.to_str().unwrap().replace("\\", "/");

if ALLOWED.contains(&git_friendly_path.as_str()) {
walk_no_read(
&[path],
|path, _is_dir| filter_dirs(path) || path.ends_with("src/etc"),
&mut |entry| {
let file = entry.path();
let extension = file.extension();
let scripts = ["py", "sh", "ps1"];
if scripts.into_iter().any(|e| extension == Some(OsStr::new(e))) {
return;
}

let output = Command::new("git")
.arg("ls-files")
.arg(&git_friendly_path)
.current_dir(path)
.stderr(Stdio::null())
.output()
.unwrap_or_else(|e| {
panic!("could not run git ls-files: {e}");
});
let path_bytes = rel_path.as_os_str().as_bytes();
if output.status.success() && output.stdout.starts_with(path_bytes) {
tidy_error!(bad, "binary checked into source: {}", file.display());
if t!(is_executable(&file), file) {
let rel_path = file.strip_prefix(path).unwrap();
let git_friendly_path = rel_path.to_str().unwrap().replace("\\", "/");

if ALLOWED.contains(&git_friendly_path.as_str()) {
return;
}

let output = Command::new("git")
.arg("ls-files")
.arg(&git_friendly_path)
.current_dir(path)
.stderr(Stdio::null())
.output()
.unwrap_or_else(|e| {
panic!("could not run git ls-files: {e}");
});
let path_bytes = rel_path.as_os_str().as_bytes();
if output.status.success() && output.stdout.starts_with(path_bytes) {
tidy_error!(bad, "binary checked into source: {}", file.display());
}
}
}
})
},
)
}
}
22 changes: 16 additions & 6 deletions src/tools/tidy/src/debug_artifacts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,21 @@ 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, |path| 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);
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
);
}
}
}
});
},
);
}
2 changes: 1 addition & 1 deletion src/tools/tidy/src/edition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ fn is_edition_2021(mut line: &str) -> bool {
}

pub fn check(path: &Path, bad: &mut bool) {
walk(path, |path| filter_dirs(path), &mut |entry, contents| {
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" {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/tidy/src/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ fn check_error_codes_docs(

let mut no_longer_emitted_codes = Vec::new();

walk(&docs_path, |_| false, &mut |entry, contents| {
walk(&docs_path, |_, _| false, &mut |entry, contents| {
let path = entry.path();

// Error if the file isn't markdown.
Expand Down Expand Up @@ -321,7 +321,7 @@ fn check_error_codes_used(

let mut found_codes = Vec::new();

walk_many(search_paths, 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.
Expand Down
4 changes: 2 additions & 2 deletions src/tools/tidy/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub fn check(
&tests_path.join("rustdoc-ui"),
&tests_path.join("rustdoc"),
],
|path| {
|path, _is_dir| {
filter_dirs(path)
|| filter_not_rust(path)
|| path.file_name() == Some(OsStr::new("features.rs"))
Expand Down Expand Up @@ -478,7 +478,7 @@ fn map_lib_features(
) {
walk(
base_src_path,
|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();
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/mir_opt_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ fn check_unused_files(path: &Path, bless: bool, bad: &mut bool) {

walk_no_read(
&[&path.join("mir-opt")],
|path| path.file_name() == Some("README.md".as_ref()),
|path, _is_dir| path.file_name() == Some("README.md".as_ref()),
&mut |file| {
let filepath = file.path();
if filepath.extension() == Some("rs".as_ref()) {
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/pal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,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, 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") {
Expand Down
5 changes: 1 addition & 4 deletions src/tools/tidy/src/rustdoc_gui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,7 @@ use std::path::Path;
pub fn check(path: &Path, bad: &mut bool) {
crate::walk::walk(
&path.join("rustdoc-gui"),
|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("// ") {
Expand Down
11 changes: 9 additions & 2 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool {
}

pub fn check(path: &Path, bad: &mut bool) {
fn skip(path: &Path) -> bool {
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;
Expand All @@ -237,8 +237,15 @@ pub fn check(path: &Path, bad: &mut bool) {
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"];
if extensions.iter().all(|e| path.extension() != Some(OsStr::new(e))) {

// 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;
}

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 @@ -37,7 +37,7 @@ struct RevisionInfo<'a> {
}

pub fn check(path: &Path, bad: &mut bool) {
crate::walk::walk(path, filter_not_rust, &mut |entry, content| {
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| {
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 @@ -51,7 +51,7 @@ pub fn check(path: &Path, bad: &mut bool) {
check_entries(&path, bad);
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| {
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" {
Expand Down
4 changes: 2 additions & 2 deletions src/tools/tidy/src/unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ pub fn check(root_path: &Path, bad: &mut bool) {
&& !(path.starts_with(&core_tests) || path.starts_with(&core_benches))
};

let skip = move |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("src/doc")
|| (file_name == "tests" || file_name == "benches") && !is_core(path)
Expand Down
10 changes: 6 additions & 4 deletions src/tools/tidy/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ pub fn filter_not_rust(path: &Path) -> bool {

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

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

pub(crate) fn walk_no_read(
paths: &[&Path],
skip: impl Send + Sync + 'static + Fn(&Path) -> bool,
skip: impl Send + Sync + 'static + Fn(&Path, bool) -> bool,
f: &mut dyn FnMut(&DirEntry),
) {
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()));
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().map_or(true, |kind| kind.is_dir() || kind.is_symlink()) {
Expand Down
1 change: 0 additions & 1 deletion tests/rustdoc/issue-108925.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,3 @@ pub enum MyThing {
#[doc(hidden)]
NotShown,
}

Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const fn to_usize<T: 'static>() -> usize {
match TypeId::of::<T>() {
WHAT_A_TYPE => 0,
_ => 1000,
}
}
}
impl<T: 'static> AssocCt for T {
const ASSOC: usize = to_usize::<T>();
Expand Down