user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Migrate the remaining tidy checks to diagnostics

Browse source
This commit is contained in:
Jakub Beránek 2025-09-15 13:26:29 +02:00
parent c36faff900
commit 352fa3960a
No known key found for this signature in database
GPG key ID: 909CD0D26483516B
34 changed files with 560 additions and 572 deletions
Download patch file Download diff file Expand all files Collapse all files

14
src/tools/tidy/src/bins.rs
View file

@ -12,11 +12,13 @@ pub use os_impl::*;
mod os_impl {
use std::path::Path;
use crate::diagnostics::DiagCtx;
pub fn check_filesystem_support(_sources: &[&Path], _output: &Path) -> bool {
return false;
}
pub fn check(_path: &Path, _bad: &mut bool) {}
pub fn check(_path: &Path, _diag_ctx: DiagCtx) {}
}
#[cfg(unix)]
@ -36,6 +38,8 @@ mod os_impl {
use FilesystemSupport::*;
use crate::diagnostics::DiagCtx;
fn is_executable(path: &Path) -> std::io::Result<bool> {
Ok(path.metadata()?.mode() & 0o111 != 0)
}
@ -106,14 +110,16 @@ mod os_impl {
}
#[cfg(unix)]
pub fn check(path: &Path, bad: &mut bool) {
pub fn check(path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check("bins");
use std::ffi::OsStr;
const ALLOWED: &[&str] = &["configure", "x"];
for p in RI_EXCLUSION_LIST {
if !path.join(Path::new(p)).exists() {
tidy_error!(bad, "rust-installer test bins missed: {p}");
check.error(format!("rust-installer test bins missed: {p}"));
}
}
@ -153,7 +159,7 @@ mod os_impl {
});
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());
check.error(format!("binary checked into source: {}", file.display()));
}
}
},

15
src/tools/tidy/src/debug_artifacts.rs
View file

@ -2,24 +2,25 @@
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx};
use crate::walk::{filter_dirs, filter_not_rust, walk};
const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in test";
pub fn check(test_dir: &Path, bad: &mut bool) {
pub fn check(test_dir: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("debug_artifacts").path(test_dir));
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,
"{}:{}: {}",
check.error(format!(
"{}:{}: {GRAPHVIZ_POSTFLOW_MSG}",
entry.path().display(),
i + 1,
GRAPHVIZ_POSTFLOW_MSG
);
i + 1
));
}
}
},

115
src/tools/tidy/src/deps.rs
View file

@ -9,6 +9,8 @@ use build_helper::ci::CiEnv;
use cargo_metadata::semver::Version;
use cargo_metadata::{Metadata, Package, PackageId};
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
#[path = "../../../bootstrap/src/utils/proc_macro_deps.rs"]
mod proc_macro_deps;
@ -661,10 +663,12 @@ const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[
///
/// `root` is path to the directory with the root `Cargo.toml` (for the workspace). `cargo` is path
/// to the cargo executable.
pub fn check(root: &Path, cargo: &Path, bless: bool, bad: &mut bool) {
pub fn check(root: &Path, cargo: &Path, bless: bool, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("deps").path(root));
let mut checked_runtime_licenses = false;
check_proc_macro_dep_list(root, cargo, bless, bad);
check_proc_macro_dep_list(root, cargo, bless, &mut check);
for &WorkspaceInfo { path, exceptions, crates_and_deps, submodules } in WORKSPACES {
if has_missing_submodule(root, submodules) {
@ -672,7 +676,7 @@ pub fn check(root: &Path, cargo: &Path, bless: bool, bad: &mut bool) {
}
if !root.join(path).join("Cargo.lock").exists() {
tidy_error!(bad, "the `{path}` workspace doesn't have a Cargo.lock");
check.error(format!("the `{path}` workspace doesn't have a Cargo.lock"));
continue;
}
@ -683,16 +687,23 @@ pub fn check(root: &Path, cargo: &Path, bless: bool, bad: &mut bool) {
.other_options(vec!["--locked".to_owned()]);
let metadata = t!(cmd.exec());
check_license_exceptions(&metadata, path, exceptions, bad);
check_license_exceptions(&metadata, path, exceptions, &mut check);
if let Some((crates, permitted_deps, location)) = crates_and_deps {
let descr = crates.get(0).unwrap_or(&path);
check_permitted_dependencies(&metadata, descr, permitted_deps, crates, location, bad);
check_permitted_dependencies(
&metadata,
descr,
permitted_deps,
crates,
location,
&mut check,
);
}
if path == "library" {
check_runtime_license_exceptions(&metadata, bad);
check_runtime_no_duplicate_dependencies(&metadata, bad);
check_runtime_no_proc_macros(&metadata, bad);
check_runtime_license_exceptions(&metadata, &mut check);
check_runtime_no_duplicate_dependencies(&metadata, &mut check);
check_runtime_no_proc_macros(&metadata, &mut check);
checked_runtime_licenses = true;
}
}
@ -703,7 +714,7 @@ pub fn check(root: &Path, cargo: &Path, bless: bool, bad: &mut bool) {
}
/// Ensure the list of proc-macro crate transitive dependencies is up to date
fn check_proc_macro_dep_list(root: &Path, cargo: &Path, bless: bool, bad: &mut bool) {
fn check_proc_macro_dep_list(root: &Path, cargo: &Path, bless: bool, check: &mut RunningCheck) {
let mut cmd = cargo_metadata::MetadataCommand::new();
cmd.cargo_path(cargo)
.manifest_path(root.join("Cargo.toml"))
@ -750,22 +761,22 @@ pub static CRATES: &[&str] = &[
)
.unwrap();
} else {
let old_bad = *bad;
let mut error_found = false;
for missing in proc_macro_deps.difference(&expected) {
tidy_error!(
bad,
error_found = true;
check.error(format!(
"proc-macro crate dependency `{missing}` is not registered in `src/bootstrap/src/utils/proc_macro_deps.rs`",
);
));
}
for extra in expected.difference(&proc_macro_deps) {
tidy_error!(
bad,
error_found = true;
check.error(format!(
"`{extra}` is registered in `src/bootstrap/src/utils/proc_macro_deps.rs`, but is not a proc-macro crate dependency",
);
));
}
if *bad != old_bad {
eprintln!("Run `./x.py test tidy --bless` to regenerate the list");
if error_found {
check.message("Run `./x.py test tidy --bless` to regenerate the list");
}
}
}
@ -787,7 +798,7 @@ pub fn has_missing_submodule(root: &Path, submodules: &[&str]) -> bool {
///
/// Unlike for tools we don't allow exceptions to the `LICENSES` list for the runtime with the sole
/// exception of `fortanix-sgx-abi` which is only used on x86_64-fortanix-unknown-sgx.
fn check_runtime_license_exceptions(metadata: &Metadata, bad: &mut bool) {
fn check_runtime_license_exceptions(metadata: &Metadata, check: &mut RunningCheck) {
for pkg in &metadata.packages {
if pkg.source.is_none() {
// No need to check local packages.
@ -796,7 +807,8 @@ fn check_runtime_license_exceptions(metadata: &Metadata, bad: &mut bool) {
let license = match &pkg.license {
Some(license) => license,
None => {
tidy_error!(bad, "dependency `{}` does not define a license expression", pkg.id);
check
.error(format!("dependency `{}` does not define a license expression", pkg.id));
continue;
}
};
@ -809,7 +821,7 @@ fn check_runtime_license_exceptions(metadata: &Metadata, bad: &mut bool) {
continue;
}
tidy_error!(bad, "invalid license `{}` in `{}`", license, pkg.id);
check.error(format!("invalid license `{}` in `{}`", license, pkg.id));
}
}
}
@ -821,37 +833,32 @@ fn check_license_exceptions(
metadata: &Metadata,
workspace: &str,
exceptions: &[(&str, &str)],
bad: &mut bool,
check: &mut RunningCheck,
) {
// Validate the EXCEPTIONS list hasn't changed.
for (name, license) in exceptions {
// Check that the package actually exists.
if !metadata.packages.iter().any(|p| *p.name == *name) {
tidy_error!(
bad,
"could not find exception package `{}` in workspace `{workspace}`\n\
check.error(format!(
"could not find exception package `{name}` in workspace `{workspace}`\n\
Remove from EXCEPTIONS list if it is no longer used.",
name
);
));
}
// Check that the license hasn't changed.
for pkg in metadata.packages.iter().filter(|p| *p.name == *name) {
match &pkg.license {
None => {
tidy_error!(
bad,
check.error(format!(
"dependency exception `{}` in workspace `{workspace}` does not declare a license expression",
pkg.id
);
));
}
Some(pkg_license) => {
if pkg_license.as_str() != *license {
println!(
"dependency exception `{name}` license in workspace `{workspace}` has changed"
);
println!(" previously `{license}` now `{pkg_license}`");
println!(" update EXCEPTIONS for the new license");
*bad = true;
check.error(format!(r#"dependency exception `{name}` license in workspace `{workspace}` has changed
previously `{license}` now `{pkg_license}`
update EXCEPTIONS for the new license
"#));
}
}
}
@ -872,26 +879,23 @@ fn check_license_exceptions(
let license = match &pkg.license {
Some(license) => license,
None => {
tidy_error!(
bad,
check.error(format!(
"dependency `{}` in workspace `{workspace}` does not define a license expression",
pkg.id
);
));
continue;
}
};
if !LICENSES.contains(&license.as_str()) {
tidy_error!(
bad,
check.error(format!(
"invalid license `{}` for package `{}` in workspace `{workspace}`",
license,
pkg.id
);
license, pkg.id
));
}
}
}
fn check_runtime_no_duplicate_dependencies(metadata: &Metadata, bad: &mut bool) {
fn check_runtime_no_duplicate_dependencies(metadata: &Metadata, check: &mut RunningCheck) {
let mut seen_pkgs = HashSet::new();
for pkg in &metadata.packages {
if pkg.source.is_none() {
@ -902,25 +906,23 @@ fn check_runtime_no_duplicate_dependencies(metadata: &Metadata, bad: &mut bool)
// depends on two version of (one for the `wasm32-wasip1` target and
// another for the `wasm32-wasip2` target).
if pkg.name.to_string() != "wasi" && !seen_pkgs.insert(&*pkg.name) {
tidy_error!(
bad,
check.error(format!(
"duplicate package `{}` is not allowed for the standard library",
pkg.name
);
));
}
}
}
fn check_runtime_no_proc_macros(metadata: &Metadata, bad: &mut bool) {
fn check_runtime_no_proc_macros(metadata: &Metadata, check: &mut RunningCheck) {
for pkg in &metadata.packages {
if pkg.targets.iter().any(|target| target.is_proc_macro()) {
tidy_error!(
bad,
check.error(format!(
"proc macro `{}` is not allowed as standard library dependency.\n\
Using proc macros in the standard library would break cross-compilation \
as proc-macros don't get shipped for the host tuple.",
pkg.name
);
));
}
}
}
@ -935,7 +937,7 @@ fn check_permitted_dependencies(
permitted_dependencies: &[&'static str],
restricted_dependency_crates: &[&'static str],
permitted_location: ListLocation,
bad: &mut bool,
check: &mut RunningCheck,
) {
let mut has_permitted_dep_error = false;
let mut deps = HashSet::new();
@ -957,11 +959,10 @@ fn check_permitted_dependencies(
}
}
if !deps.iter().any(|dep_id| compare(pkg_from_id(metadata, dep_id), permitted)) {
tidy_error!(
bad,
check.error(format!(
"could not find allowed package `{permitted}`\n\
Remove from PERMITTED_DEPENDENCIES list if it is no longer used.",
);
));
has_permitted_dep_error = true;
}
}
@ -988,7 +989,7 @@ fn check_permitted_dependencies(
false
};
if !is_eq {
tidy_error!(bad, "Dependency for {descr} not explicitly permitted: {}", dep.id);
check.error(format!("Dependency for {descr} not explicitly permitted: {}", dep.id));
has_permitted_dep_error = true;
}
}

44
src/tools/tidy/src/diagnostics.rs
View file

@ -3,7 +3,7 @@ use std::fmt::Display;
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
use crate::tidy_error;
use termcolor::WriteColor;
/// Collects diagnostics from all tidy steps, and contains shared information
/// that determines how should message and logs be presented.
@ -111,6 +111,33 @@ impl RunningCheck {
tidy_error(&t.to_string()).expect("failed to output error");
}
/// Immediately output a warning.
pub fn warning<T: Display>(&mut self, t: T) {
eprintln!("WARNING: {t}");
}
/// Output an informational message
pub fn message<T: Display>(&mut self, t: T) {
eprintln!("{t}");
}
/// Output a message only if verbose output is enabled.
pub fn verbose_msg<T: Display>(&mut self, t: T) {
if self.is_verbose_enabled() {
self.message(t);
}
}
/// Has an error already occured for this check?
pub fn is_bad(&self) -> bool {
self.bad
}
/// Is verbose output enabled?
pub fn is_verbose_enabled(&self) -> bool {
self.ctx.lock().unwrap().verbose
}
fn mark_as_bad(&mut self) {
self.bad = true;
}
@ -121,3 +148,18 @@ impl Drop for RunningCheck {
self.ctx.lock().unwrap().finish_check(FinishedCheck { id: self.id.clone(), bad: self.bad })
}
}
fn tidy_error(args: &str) -> std::io::Result<()> {
use std::io::Write;
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream};
let mut stderr = StandardStream::stdout(ColorChoice::Auto);
stderr.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?;
write!(&mut stderr, "tidy error")?;
stderr.set_color(&ColorSpec::new())?;
writeln!(&mut stderr, ": {args}")?;
Ok(())
}

9
src/tools/tidy/src/edition.rs
View file

@ -2,9 +2,11 @@
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx};
use crate::walk::{filter_dirs, walk};
pub fn check(path: &Path, bad: &mut bool) {
pub fn check(path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("edition").path(path));
walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
let file = entry.path();
let filename = file.file_name().unwrap();
@ -23,11 +25,10 @@ pub fn check(path: &Path, bad: &mut bool) {
// Check that all packages use the 2021 edition. Virtual workspaces don't allow setting an
// edition, so these shouldn't be checked.
if is_package && !is_current_edition {
tidy_error!(
bad,
check.error(format!(
"{} doesn't have `edition = \"2021\"` or `edition = \"2024\"` on a separate line",
file.display()
);
));
}
});
}

124
src/tools/tidy/src/error_codes.rs
View file

@ -22,6 +22,7 @@ use std::path::Path;
use regex::Regex;
use crate::diagnostics::{DiagCtx, RunningCheck};
use crate::walk::{filter_dirs, walk, walk_many};
const ERROR_CODES_PATH: &str = "compiler/rustc_error_codes/src/lib.rs";
@ -35,71 +36,50 @@ const IGNORE_DOCTEST_CHECK: &[&str] = &["E0464", "E0570", "E0601", "E0602", "E07
const IGNORE_UI_TEST_CHECK: &[&str] =
&["E0461", "E0465", "E0514", "E0554", "E0640", "E0717", "E0729"];
macro_rules! verbose_print {
($verbose:expr, $($fmt:tt)*) => {
if $verbose {
println!("{}", format_args!($($fmt)*));
}
};
}
pub fn check(
root_path: &Path,
search_paths: &[&Path],
verbose: bool,
ci_info: &crate::CiInfo,
bad: &mut bool,
) {
let mut errors = Vec::new();
pub fn check(root_path: &Path, search_paths: &[&Path], ci_info: &crate::CiInfo, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check("error_codes");
// Check that no error code explanation was removed.
check_removed_error_code_explanation(ci_info, bad);
check_removed_error_code_explanation(ci_info, &mut check);
// Stage 1: create list
let error_codes = extract_error_codes(root_path, &mut errors);
if verbose {
println!("Found {} error codes", error_codes.len());
println!("Highest error code: `{}`", error_codes.iter().max().unwrap());
}
let error_codes = extract_error_codes(root_path, &mut check);
check.verbose_msg(format!("Found {} error codes", error_codes.len()));
check.verbose_msg(format!("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);
let no_longer_emitted = check_error_codes_docs(root_path, &error_codes, &mut check);
// Stage 3: check list has UI tests
check_error_codes_tests(root_path, &error_codes, &mut errors, verbose, &no_longer_emitted);
check_error_codes_tests(root_path, &error_codes, &mut check, &no_longer_emitted);
// Stage 4: check list is emitted by compiler
check_error_codes_used(search_paths, &error_codes, &mut errors, &no_longer_emitted, verbose);
// Print any errors.
for error in errors {
tidy_error!(bad, "{}", error);
}
check_error_codes_used(search_paths, &error_codes, &mut check, &no_longer_emitted);
}
fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, bad: &mut bool) {
fn check_removed_error_code_explanation(ci_info: &crate::CiInfo, check: &mut RunningCheck) {
let Some(base_commit) = &ci_info.base_commit else {
eprintln!("Skipping error code explanation removal check");
check.verbose_msg("Skipping error code explanation removal check");
return;
};
let Some(diff) = crate::git_diff(base_commit, "--name-status") else {
*bad = true;
eprintln!("removed error code explanation tidy check: Failed to run git diff");
check.error(format!("removed error code explanation: Failed to run git diff"));
return;
};
if diff.lines().any(|line| {
line.starts_with('D') && line.contains("compiler/rustc_error_codes/src/error_codes/")
}) {
*bad = true;
eprintln!("tidy check error: Error code explanations should never be removed!");
eprintln!("Take a look at E0001 to see how to handle it.");
check.error(format!(
r#"Error code explanations should never be removed!
Take a look at E0001 to see how to handle it."#
));
return;
}
println!("No error code explanation was removed!");
check.verbose_msg("No error code explanation was removed!");
}
/// Stage 1: Parses a list of error codes from `error_codes.rs`.
fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String> {
fn extract_error_codes(root_path: &Path, check: &mut RunningCheck) -> Vec<String> {
let path = root_path.join(Path::new(ERROR_CODES_PATH));
let file =
fs::read_to_string(&path).unwrap_or_else(|e| panic!("failed to read `{path:?}`: {e}"));
@ -117,7 +97,7 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
// Extract the error code from the line. Emit a fatal error if it is not in the correct
// format.
let Some(split_line) = split_line else {
errors.push(format!(
check.error(format!(
"{path}:{line_index}: Expected a line with the format `Eabcd: abcd, \
but got \"{line}\" without a `:` delimiter",
));
@ -128,8 +108,9 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
// If this is a duplicate of another error code, emit a fatal error.
if error_codes.contains(&err_code) {
errors
.push(format!("{path}:{line_index}: Found duplicate error code: `{err_code}`"));
check.error(format!(
"{path}:{line_index}: Found duplicate error code: `{err_code}`"
));
continue;
}
@ -140,14 +121,14 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
// Ensure that the line references the correct markdown file.
let rest = split_line.1.split_once(',');
let Some(rest) = rest else {
errors.push(format!(
check.error(format!(
"{path}:{line_index}: Expected a line with the format `Eabcd: abcd, \
but got \"{line}\" without a `,` delimiter",
));
continue;
};
if error_num_as_str != rest.0.trim() {
errors.push(format!(
check.error(format!(
"{path}:{line_index}: `{}:` should be followed by `{},` but instead found `{}` in \
`compiler/rustc_error_codes/src/lib.rs`",
err_code,
@ -157,7 +138,7 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
continue;
}
if !rest.1.trim().is_empty() && !rest.1.trim().starts_with("//") {
errors.push(format!("{path}:{line_index}: should only have one error per line"));
check.error(format!("{path}:{line_index}: should only have one error per line"));
continue;
}
@ -172,8 +153,7 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
fn check_error_codes_docs(
root_path: &Path,
error_codes: &[String],
errors: &mut Vec<String>,
verbose: bool,
check: &mut RunningCheck,
) -> Vec<String> {
let docs_path = root_path.join(Path::new(ERROR_DOCS_PATH));
@ -184,7 +164,7 @@ fn check_error_codes_docs(
// Error if the file isn't markdown.
if path.extension() != Some(OsStr::new("md")) {
errors.push(format!(
check.error(format!(
"Found unexpected non-markdown file in error code docs directory: {}",
path.display()
));
@ -196,7 +176,7 @@ fn check_error_codes_docs(
let err_code = filename.unwrap().0; // `unwrap` is ok because we know the filename is in the correct format.
if error_codes.iter().all(|e| e != err_code) {
errors.push(format!(
check.error(format!(
"Found valid file `{}` in error code docs directory without corresponding \
entry in `rustc_error_codes/src/lib.rs`",
path.display()
@ -208,11 +188,10 @@ fn check_error_codes_docs(
check_explanation_has_doctest(contents, err_code);
if emit_ignore_warning {
verbose_print!(
verbose,
check.verbose_msg(format!(
"warning: Error code `{err_code}` uses the ignore header. This should not be used, add the error code to the \
`IGNORE_DOCTEST_CHECK` constant instead."
);
));
}
if no_longer_emitted {
@ -220,11 +199,10 @@ fn check_error_codes_docs(
}
if !found_code_example {
verbose_print!(
verbose,
check.verbose_msg(format!(
"warning: Error code `{err_code}` doesn't have a code example, all error codes are expected to have one \
(even if untested)."
);
));
return;
}
@ -232,12 +210,12 @@ fn check_error_codes_docs(
// Check that the explanation has a doctest, and if it shouldn't, that it doesn't
if !found_proper_doctest && !test_ignored {
errors.push(format!(
check.error(format!(
"`{}` doesn't use its own error code in compile_fail example",
path.display(),
));
} else if found_proper_doctest && test_ignored {
errors.push(format!(
check.error(format!(
"`{}` has a compile_fail doctest with its own error code, it shouldn't \
be listed in `IGNORE_DOCTEST_CHECK`",
path.display(),
@ -289,8 +267,7 @@ fn check_explanation_has_doctest(explanation: &str, err_code: &str) -> (bool, bo
fn check_error_codes_tests(
root_path: &Path,
error_codes: &[String],
errors: &mut Vec<String>,
verbose: bool,
check: &mut RunningCheck,
no_longer_emitted: &[String],
) {
let tests_path = root_path.join(Path::new(ERROR_TESTS_PATH));
@ -299,15 +276,14 @@ fn check_error_codes_tests(
let test_path = tests_path.join(format!("{code}.stderr"));
if !test_path.exists() && !IGNORE_UI_TEST_CHECK.contains(&code.as_str()) {
verbose_print!(
verbose,
check.verbose_msg(format!(
"warning: Error code `{code}` needs to have at least one UI test in the `tests/error-codes/` directory`!"
);
));
continue;
}
if IGNORE_UI_TEST_CHECK.contains(&code.as_str()) {
if test_path.exists() {
errors.push(format!(
check.error(format!(
"Error code `{code}` has a UI test in `tests/ui/error-codes/{code}.rs`, it shouldn't be listed in `EXEMPTED_FROM_TEST`!"
));
}
@ -317,11 +293,10 @@ fn check_error_codes_tests(
let file = match fs::read_to_string(&test_path) {
Ok(file) => file,
Err(err) => {
verbose_print!(
verbose,
check.verbose_msg(format!(
"warning: Failed to read UI test file (`{}`) for `{code}` but the file exists. The test is assumed to work:\n{err}",
test_path.display()
);
));
continue;
}
};
@ -343,10 +318,9 @@ fn check_error_codes_tests(
}
if !found_code {
verbose_print!(
verbose,
check.verbose_msg(format!(
"warning: Error code `{code}` has a UI test file, but doesn't contain its own error code!"
);
));
}
}
}
@ -355,9 +329,8 @@ fn check_error_codes_tests(
fn check_error_codes_used(
search_paths: &[&Path],
error_codes: &[String],
errors: &mut Vec<String>,
check: &mut RunningCheck,
no_longer_emitted: &[String],
verbose: bool,
) {
// Search for error codes in the form `E0123`.
let regex = Regex::new(r#"\bE\d{4}\b"#).unwrap();
@ -384,7 +357,7 @@ fn check_error_codes_used(
if !error_codes.contains(&error_code) {
// This error code isn't properly defined, we must error.
errors.push(format!("Error code `{error_code}` is used in the compiler but not defined and documented in `compiler/rustc_error_codes/src/lib.rs`."));
check.error(format!("Error code `{error_code}` is used in the compiler but not defined and documented in `compiler/rustc_error_codes/src/lib.rs`."));
continue;
}
@ -397,7 +370,7 @@ fn check_error_codes_used(
for code in error_codes {
if !found_codes.contains(code) && !no_longer_emitted.contains(code) {
errors.push(format!(
check.error(format!(
"Error code `{code}` exists, but is not emitted by the compiler!\n\
Please mark the code as no longer emitted by adding the following note to the top of the `EXXXX.md` file:\n\
`#### Note: this error code is no longer emitted by the compiler`\n\
@ -406,10 +379,9 @@ fn check_error_codes_used(
}
if found_codes.contains(code) && no_longer_emitted.contains(code) {
verbose_print!(
verbose,
check.verbose_msg(format!(
"warning: Error code `{code}` is used when it's marked as \"no longer emitted\""
);
));
}
}
}

9
src/tools/tidy/src/extdeps.rs
View file

@ -4,6 +4,7 @@ use std::fs;
use std::path::Path;
use crate::deps::WorkspaceInfo;
use crate::diagnostics::{CheckId, DiagCtx};
/// List of allowed sources for packages.
const ALLOWED_SOURCES: &[&str] = &[
@ -14,7 +15,9 @@ const ALLOWED_SOURCES: &[&str] = &[
/// Checks for external package sources. `root` is the path to the directory that contains the
/// workspace `Cargo.toml`.
pub fn check(root: &Path, bad: &mut bool) {
pub fn check(root: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("extdeps").path(root));
for &WorkspaceInfo { path, submodules, .. } in crate::deps::WORKSPACES {
if crate::deps::has_missing_submodule(root, submodules) {
continue;
@ -25,7 +28,7 @@ pub fn check(root: &Path, bad: &mut bool) {
let lockfile = root.join(path).join("Cargo.lock");
if !lockfile.exists() {
tidy_error!(bad, "the `{path}` workspace doesn't have a Cargo.lock");
check.error(format!("the `{path}` workspace doesn't have a Cargo.lock"));
continue;
}
@ -44,7 +47,7 @@ pub fn check(root: &Path, bad: &mut bool) {
// Ensure source is allowed.
if !ALLOWED_SOURCES.contains(&source) {
tidy_error!(bad, "invalid source: {}", source);
check.error(format!("invalid source: {}", source));
}
}
}

7
src/tools/tidy/src/extra_checks/mod.rs
View file

@ -24,6 +24,7 @@ use std::str::FromStr;
use std::{fmt, fs, io};
use crate::CiInfo;
use crate::diagnostics::DiagCtx;
mod rustdoc_js;
@ -54,8 +55,10 @@ pub fn check(
bless: bool,
extra_checks: Option<&str>,
pos_args: &[String],
bad: &mut bool,
diag_ctx: DiagCtx,
) {
let mut check = diag_ctx.start_check("extra_checks");
if let Err(e) = check_impl(
root_path,
outdir,
@ -68,7 +71,7 @@ pub fn check(
extra_checks,
pos_args,
) {
tidy_error!(bad, "{e}");
check.error(e);
}
}

132
src/tools/tidy/src/features.rs
View file

@ -16,6 +16,7 @@ use std::num::NonZeroU32;
use std::path::{Path, PathBuf};
use std::{fmt, fs};
use crate::diagnostics::{DiagCtx, RunningCheck};
use crate::walk::{filter_dirs, filter_not_rust, walk, walk_many};
#[cfg(test)]
@ -91,13 +92,14 @@ pub fn check(
tests_path: &Path,
compiler_path: &Path,
lib_path: &Path,
bad: &mut bool,
verbose: bool,
diag_ctx: DiagCtx,
) -> CollectedFeatures {
let mut features = collect_lang_features(compiler_path, bad);
let mut check = diag_ctx.start_check("features");
let mut features = collect_lang_features(compiler_path, &mut check);
assert!(!features.is_empty());
let lib_features = get_and_check_lib_features(lib_path, bad, &features);
let lib_features = get_and_check_lib_features(lib_path, &mut check, &features);
assert!(!lib_features.is_empty());
walk_many(
@ -121,7 +123,7 @@ pub fn check(
for (i, line) in contents.lines().enumerate() {
let mut err = |msg: &str| {
tidy_error!(bad, "{}:{}: {}", file.display(), i + 1, msg);
check.error(format!("{}:{}: {}", file.display(), i + 1, msg));
};
let gate_test_str = "gate-test-";
@ -175,7 +177,7 @@ pub fn check(
}
if !gate_untested.is_empty() {
tidy_error!(bad, "Found {} features without a gate test.", gate_untested.len());
check.error(format!("Found {} features without a gate test.", gate_untested.len()));
}
let (version, channel) = get_version_and_channel(src_path);
@ -189,39 +191,32 @@ pub fn check(
let file = feature.file.display();
let line = feature.line;
if since > version && since != Version::CurrentPlaceholder {
tidy_error!(
bad,
check.error(format!(
"{file}:{line}: The stabilization version {since} of {kind} feature `{feature_name}` is newer than the current {version}"
);
));
}
if channel == "nightly" && since == version {
tidy_error!(
bad,
check.error(format!(
"{file}:{line}: The stabilization version {since} of {kind} feature `{feature_name}` is written out but should be {}",
version::VERSION_PLACEHOLDER
);
));
}
if channel != "nightly" && since == Version::CurrentPlaceholder {
tidy_error!(
bad,
check.error(format!(
"{file}:{line}: The placeholder use of {kind} feature `{feature_name}` is not allowed on the {channel} channel",
);
));
}
}
if *bad {
return CollectedFeatures { lib: lib_features, lang: features };
}
if verbose {
if !check.is_bad() && check.is_verbose_enabled() {
let mut lines = Vec::new();
lines.extend(format_features(&features, "lang"));
lines.extend(format_features(&lib_features, "lib"));
lines.sort();
for line in lines {
println!("* {line}");
}
check.verbose_msg(
lines.into_iter().map(|l| format!("* {l}")).collect::<Vec<String>>().join("\n"),
);
}
CollectedFeatures { lib: lib_features, lang: features }
@ -275,15 +270,20 @@ fn test_filen_gate<'f>(filen_underscore: &'f str, features: &mut Features) -> Op
None
}
pub fn collect_lang_features(base_compiler_path: &Path, bad: &mut bool) -> Features {
pub fn collect_lang_features(base_compiler_path: &Path, check: &mut RunningCheck) -> Features {
let mut features = Features::new();
collect_lang_features_in(&mut features, base_compiler_path, "accepted.rs", bad);
collect_lang_features_in(&mut features, base_compiler_path, "removed.rs", bad);
collect_lang_features_in(&mut features, base_compiler_path, "unstable.rs", bad);
collect_lang_features_in(&mut features, base_compiler_path, "accepted.rs", check);
collect_lang_features_in(&mut features, base_compiler_path, "removed.rs", check);
collect_lang_features_in(&mut features, base_compiler_path, "unstable.rs", check);
features
}
fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, bad: &mut bool) {
fn collect_lang_features_in(
features: &mut Features,
base: &Path,
file: &str,
check: &mut RunningCheck,
) {
let path = base.join("rustc_feature").join("src").join(file);
let contents = t!(fs::read_to_string(&path));
@ -315,13 +315,11 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba
if line.starts_with(FEATURE_GROUP_START_PREFIX) {
if in_feature_group {
tidy_error!(
bad,
"{}:{}: \
check.error(format!(
"{}:{line_number}: \
new feature group is started without ending the previous one",
path.display(),
line_number,
);
path.display()
));
}
in_feature_group = true;
@ -353,14 +351,10 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba
let since = match since_str.parse() {
Ok(since) => Some(since),
Err(err) => {
tidy_error!(
bad,
"{}:{}: failed to parse since: {} ({:?})",
path.display(),
line_number,
since_str,
err,
);
check.error(format!(
"{}:{line_number}: failed to parse since: {since_str} ({err:?})",
path.display()
));
None
}
};
@ -371,13 +365,10 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba
let correct_index = match prev_names.binary_search(&name) {
Ok(_) => {
// This only occurs when the feature name has already been declared.
tidy_error!(
bad,
"{}:{}: duplicate feature {}",
path.display(),
line_number,
name,
);
check.error(format!(
"{}:{line_number}: duplicate feature {name}",
path.display()
));
// skip any additional checks for this line
continue;
}
@ -398,14 +389,10 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba
)
};
tidy_error!(
bad,
"{}:{}: feature {} is not sorted by feature name (should be {})",
check.error(format!(
"{}:{line_number}: feature {name} is not sorted by feature name (should be {correct_placement})",
path.display(),
line_number,
name,
correct_placement,
);
));
}
prev_names.push(name);
}
@ -413,13 +400,10 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba
let issue_str = parts.next().unwrap().trim();
let tracking_issue = if issue_str.starts_with("None") {
if level == Status::Unstable && !next_feature_omits_tracking_issue {
tidy_error!(
bad,
"{}:{}: no tracking issue for feature {}",
check.error(format!(
"{}:{line_number}: no tracking issue for feature {name}",
path.display(),
line_number,
name,
);
));
}
None
} else {
@ -428,13 +412,11 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba
};
match features.entry(name.to_owned()) {
Entry::Occupied(e) => {
tidy_error!(
bad,
"{}:{} feature {name} already specified with status '{}'",
check.error(format!(
"{}:{line_number} feature {name} already specified with status '{}'",
path.display(),
line_number,
e.get().level,
);
));
}
Entry::Vacant(e) => {
e.insert(Feature {
@ -458,7 +440,7 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba
fn get_and_check_lib_features(
base_src_path: &Path,
bad: &mut bool,
check: &mut RunningCheck,
lang_features: &Features,
) -> Features {
let mut lib_features = Features::new();
@ -469,16 +451,12 @@ fn get_and_check_lib_features(
&& f.tracking_issue != s.tracking_issue
&& f.level != Status::Accepted
{
tidy_error!(
bad,
"{}:{}: feature gate {} has inconsistent `issue`: \"{}\" mismatches the {} `issue` of \"{}\"",
check.error(format!(
"{}:{line}: feature gate {name} has inconsistent `issue`: \"{}\" mismatches the {display} `issue` of \"{}\"",
file.display(),
line,
name,
f.tracking_issue_display(),
display,
s.tracking_issue_display(),
);
));
}
};
check_features(&f, lang_features, "corresponding lang feature");
@ -486,7 +464,7 @@ fn get_and_check_lib_features(
lib_features.insert(name.to_owned(), f);
}
Err(msg) => {
tidy_error!(bad, "{}:{}: {}", file.display(), line, msg);
check.error(format!("{}:{line}: {msg}", file.display()));
}
});
lib_features

20
src/tools/tidy/src/filenames.rs
View file

@ -10,7 +10,10 @@
use std::path::Path;
use std::process::Command;
pub fn check(root_path: &Path, bad: &mut bool) {
use crate::diagnostics::{CheckId, DiagCtx};
pub fn check(root_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("filenames").path(root_path));
let stat_output = Command::new("git")
.arg("-C")
.arg(root_path)
@ -20,20 +23,17 @@ pub fn check(root_path: &Path, bad: &mut bool) {
.stdout;
for filename in stat_output.split(|&b| b == 0) {
match str::from_utf8(filename) {
Err(_) => tidy_error!(
bad,
Err(_) => check.error(format!(
r#"non-UTF8 file names are not supported: "{}""#,
String::from_utf8_lossy(filename),
),
Ok(name) if name.chars().any(|c| c.is_control()) => tidy_error!(
bad,
)),
Ok(name) if name.chars().any(|c| c.is_control()) => check.error(format!(
r#"control characters are not supported in file names: "{}""#,
String::from_utf8_lossy(filename),
),
Ok(name) if name.contains(':') => tidy_error!(
bad,
)),
Ok(name) if name.contains(':') => check.error(format!(
r#"":" is not supported in file names because of Windows compatibility: "{name}""#,
),
)),
_ => (),
}
}

36
src/tools/tidy/src/fluent_alphabetical.rs
View file

@ -7,6 +7,7 @@ use std::path::Path;
use regex::Regex;
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
use crate::walk::{filter_dirs, walk};
fn message() -> &'static Regex {
@ -20,19 +21,17 @@ fn is_fluent(path: &Path) -> bool {
fn check_alphabetic(
filename: &str,
fluent: &str,
bad: &mut bool,
check: &mut RunningCheck,
all_defined_msgs: &mut HashMap<String, String>,
) {
let mut matches = message().captures_iter(fluent).peekable();
while let Some(m) = matches.next() {
let name = m.get(1).unwrap();
if let Some(defined_filename) = all_defined_msgs.get(name.as_str()) {
tidy_error!(
bad,
"{filename}: message `{}` is already defined in {}",
check.error(format!(
"{filename}: message `{}` is already defined in {defined_filename}",
name.as_str(),
defined_filename,
);
));
}
all_defined_msgs.insert(name.as_str().to_owned(), filename.to_owned());
@ -40,13 +39,12 @@ fn check_alphabetic(
if let Some(next) = matches.peek() {
let next = next.get(1).unwrap();
if name.as_str() > next.as_str() {
tidy_error!(
bad,
check.error(format!(
"{filename}: message `{}` appears before `{}`, but is alphabetically later than it
run `./x.py test tidy --bless` to sort the file correctly",
name.as_str(),
next.as_str()
);
));
}
} else {
break;
@ -57,7 +55,7 @@ run `./x.py test tidy --bless` to sort the file correctly",
fn sort_messages(
filename: &str,
fluent: &str,
bad: &mut bool,
check: &mut RunningCheck,
all_defined_msgs: &mut HashMap<String, String>,
) -> String {
let mut chunks = vec![];
@ -65,12 +63,10 @@ fn sort_messages(
for line in fluent.lines() {
if let Some(name) = message().find(line) {
if let Some(defined_filename) = all_defined_msgs.get(name.as_str()) {
tidy_error!(
bad,
"{filename}: message `{}` is already defined in {}",
check.error(format!(
"{filename}: message `{}` is already defined in {defined_filename}",
name.as_str(),
defined_filename,
);
));
}
all_defined_msgs.insert(name.as_str().to_owned(), filename.to_owned());
@ -88,7 +84,9 @@ fn sort_messages(
out
}
pub fn check(path: &Path, bless: bool, bad: &mut bool) {
pub fn check(path: &Path, bless: bool, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("fluent_alphabetical").path(path));
let mut all_defined_msgs = HashMap::new();
walk(
path,
@ -98,7 +96,7 @@ pub fn check(path: &Path, bless: bool, bad: &mut bool) {
let sorted = sort_messages(
ent.path().to_str().unwrap(),
contents,
bad,
&mut check,
&mut all_defined_msgs,
);
if sorted != contents {
@ -110,12 +108,12 @@ pub fn check(path: &Path, bless: bool, bad: &mut bool) {
check_alphabetic(
ent.path().to_str().unwrap(),
contents,
bad,
&mut check,
&mut all_defined_msgs,
);
}
},
);
crate::fluent_used::check(path, all_defined_msgs, bad);
crate::fluent_used::check(path, all_defined_msgs, diag_ctx);
}

13
src/tools/tidy/src/fluent_lowercase.rs
View file

@ -4,6 +4,7 @@ use std::path::Path;
use fluent_syntax::ast::{Entry, Message, PatternElement};
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
use crate::walk::{filter_dirs, walk};
#[rustfmt::skip]
@ -34,7 +35,7 @@ fn is_allowed_capitalized_word(msg: &str) -> bool {
})
}
fn check_lowercase(filename: &str, contents: &str, bad: &mut bool) {
fn check_lowercase(filename: &str, contents: &str, check: &mut RunningCheck) {
let (Ok(parse) | Err((parse, _))) = fluent_syntax::parser::parse(contents);
for entry in &parse.body {
@ -45,20 +46,20 @@ fn check_lowercase(filename: &str, contents: &str, bad: &mut bool) {
&& value.chars().next().is_some_and(char::is_uppercase)
&& !is_allowed_capitalized_word(value)
{
tidy_error!(
bad,
check.error(format!(
"{filename}: message `{value}` starts with an uppercase letter. Fix it or add it to `ALLOWED_CAPITALIZED_WORDS`"
);
));
}
}
}
pub fn check(path: &Path, bad: &mut bool) {
pub fn check(path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("fluent_lowercase").path(path));
walk(
path,
|path, is_dir| filter_dirs(path) || (!is_dir && filter_fluent(path)),
&mut |ent, contents| {
check_lowercase(ent.path().to_str().unwrap(), contents, bad);
check_lowercase(ent.path().to_str().unwrap(), contents, &mut check);
},
);
}

13
src/tools/tidy/src/fluent_period.rs
View file

@ -4,6 +4,7 @@ use std::path::Path;
use fluent_syntax::ast::{Entry, PatternElement};
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
use crate::walk::{filter_dirs, walk};
fn filter_fluent(path: &Path) -> bool {
@ -20,7 +21,7 @@ const ALLOWLIST: &[&str] = &[
"incremental_corrupt_file",
];
fn check_period(filename: &str, contents: &str, bad: &mut bool) {
fn check_period(filename: &str, contents: &str, check: &mut RunningCheck) {
if filename.contains("codegen") {
// FIXME: Too many codegen messages have periods right now...
return;
@ -40,7 +41,7 @@ fn check_period(filename: &str, contents: &str, bad: &mut bool) {
if value.ends_with(".") && !value.ends_with("...") {
let ll = find_line(contents, value);
let name = m.id.name;
tidy_error!(bad, "{filename}:{ll}: message `{name}` ends in a period");
check.error(format!("{filename}:{ll}: message `{name}` ends in a period"));
}
}
@ -56,7 +57,7 @@ fn check_period(filename: &str, contents: &str, bad: &mut bool) {
{
let ll = find_line(contents, value);
let name = attr.id.name;
tidy_error!(bad, "{filename}:{ll}: attr `{name}` ends in a period");
check.error(format!("{filename}:{ll}: attr `{name}` ends in a period"));
}
}
}
@ -74,12 +75,14 @@ fn find_line(haystack: &str, needle: &str) -> usize {
1
}
pub fn check(path: &Path, bad: &mut bool) {
pub fn check(path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("fluent_period").path(path));
walk(
path,
|path, is_dir| filter_dirs(path) || (!is_dir && filter_fluent(path)),
&mut |ent, contents| {
check_period(ent.path().to_str().unwrap(), contents, bad);
check_period(ent.path().to_str().unwrap(), contents, &mut check);
},
);
}

7
src/tools/tidy/src/fluent_used.rs
View file

@ -3,6 +3,7 @@
use std::collections::HashMap;
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx};
use crate::walk::{filter_dirs, walk};
fn filter_used_messages(
@ -27,13 +28,15 @@ fn filter_used_messages(
}
}
pub fn check(path: &Path, mut all_defined_msgs: HashMap<String, String>, bad: &mut bool) {
pub fn check(path: &Path, mut all_defined_msgs: HashMap<String, String>, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("fluent_used").path(path));
let mut msgs_appear_only_once = HashMap::new();
walk(path, |path, _| filter_dirs(path), &mut |_, contents| {
filter_used_messages(contents, &mut all_defined_msgs, &mut msgs_appear_only_once);
});
for (name, filename) in msgs_appear_only_once {
tidy_error!(bad, "{filename}: message `{}` is not used", name,);
check.error(format!("{filename}: message `{name}` is not used"));
}
}

13
src/tools/tidy/src/gcc_submodule.rs
View file

@ -4,7 +4,11 @@
use std::path::Path;
use std::process::Command;
pub fn check(root_path: &Path, compiler_path: &Path, bad: &mut bool) {
use crate::diagnostics::DiagCtx;
pub fn check(root_path: &Path, compiler_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check("gcc_submodule");
let cg_gcc_version_path = compiler_path.join("rustc_codegen_gcc/libgccjit.version");
let cg_gcc_version = std::fs::read_to_string(&cg_gcc_version_path)
.unwrap_or_else(|_| {
@ -26,7 +30,7 @@ pub fn check(root_path: &Path, compiler_path: &Path, bad: &mut bool) {
// Git is not available or we are in a tarball
if !git_output.status.success() {
eprintln!("Cannot figure out the SHA of the GCC submodule");
check.message("Cannot figure out the SHA of the GCC submodule");
return;
}
@ -43,12 +47,11 @@ pub fn check(root_path: &Path, compiler_path: &Path, bad: &mut bool) {
// The SHA can start with + if the submodule is modified or - if it is not checked out.
let gcc_submodule_sha = git_output.trim_start_matches(['+', '-']);
if gcc_submodule_sha != cg_gcc_version {
*bad = true;
eprintln!(
check.error(format!(
r#"Commit SHA of the src/gcc submodule (`{gcc_submodule_sha}`) does not match the required GCC version of the GCC codegen backend (`{cg_gcc_version}`).
Make sure to set the src/gcc submodule to commit {cg_gcc_version}.
The GCC codegen backend commit is configured at {}."#,
cg_gcc_version_path.display(),
);
));
}
}

9
src/tools/tidy/src/known_bug.rs
View file

@ -2,9 +2,11 @@
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx};
use crate::walk::*;
pub fn check(filepath: &Path, bad: &mut bool) {
pub fn check(filepath: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("known_bug").path(filepath));
walk(filepath, |path, _is_dir| filter_not_rust(path), &mut |entry, contents| {
let file: &Path = entry.path();
@ -19,11 +21,10 @@ pub fn check(filepath: &Path, bad: &mut bool) {
[.., "tests", "crashes", "auxiliary", _aux_file_rs]
) && !contents.lines().any(|line| line.starts_with("//@ known-bug: "))
{
tidy_error!(
bad,
check.error(format!(
"{} crash/ice test does not have a \"//@ known-bug: \" directive",
file.display()
);
));
}
});
}

43
src/tools/tidy/src/lib.rs
View file

@ -11,7 +11,8 @@ use std::{env, io};
use build_helper::ci::CiEnv;
use build_helper::git::{GitConfig, get_closest_upstream_commit};
use build_helper::stage0_parser::{Stage0Config, parse_stage0_file};
use termcolor::WriteColor;
use crate::diagnostics::{DiagCtx, RunningCheck};
macro_rules! static_regex {
($re:literal) => {{
@ -43,28 +44,6 @@ macro_rules! t {
};
}
macro_rules! tidy_error {
($bad:expr, $($fmt:tt)*) => ({
$crate::tidy_error(&format_args!($($fmt)*).to_string()).expect("failed to output error");
*$bad = true;
});
}
fn tidy_error(args: &str) -> std::io::Result<()> {
use std::io::Write;
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream};
let mut stderr = StandardStream::stdout(ColorChoice::Auto);
stderr.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?;
write!(&mut stderr, "tidy error")?;
stderr.set_color(&ColorSpec::new())?;
writeln!(&mut stderr, ": {args}")?;
Ok(())
}
pub struct CiInfo {
pub git_merge_commit_email: String,
pub nightly_branch: String,
@ -73,7 +52,9 @@ pub struct CiInfo {
}
impl CiInfo {
pub fn new(bad: &mut bool) -> Self {
pub fn new(diag_ctx: DiagCtx) -> Self {
let mut check = diag_ctx.start_check("CI history");
let stage0 = parse_stage0_file();
let Stage0Config { nightly_branch, git_merge_commit_email, .. } = stage0.config;
@ -86,11 +67,14 @@ impl CiInfo {
let base_commit = match get_closest_upstream_commit(None, &info.git_config(), info.ci_env) {
Ok(Some(commit)) => Some(commit),
Ok(None) => {
info.error_if_in_ci("no base commit found", bad);
info.error_if_in_ci("no base commit found", &mut check);
None
}
Err(error) => {
info.error_if_in_ci(&format!("failed to retrieve base commit: {error}"), bad);
info.error_if_in_ci(
&format!("failed to retrieve base commit: {error}"),
&mut check,
);
None
}
};
@ -105,12 +89,11 @@ impl CiInfo {
}
}
pub fn error_if_in_ci(&self, msg: &str, bad: &mut bool) {
pub fn error_if_in_ci(&self, msg: &str, check: &mut RunningCheck) {
if self.ci_env.is_running_in_ci() {
*bad = true;
eprintln!("tidy check error: {msg}");
check.error(msg);
} else {
eprintln!("tidy check warning: {msg}. Some checks will be skipped.");
check.warning(format!("{msg}. Some checks will be skipped."));
}
}
}

128
src/tools/tidy/src/main.rs
View file

@ -8,8 +8,6 @@ use std::collections::VecDeque;
use std::num::NonZeroUsize;
use std::path::PathBuf;
use std::str::FromStr;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Mutex};
use std::thread::{self, ScopedJoinHandle, scope};
use std::{env, process};
@ -52,11 +50,8 @@ fn main() {
let extra_checks =
cfg_args.iter().find(|s| s.starts_with("--extra-checks=")).map(String::as_str);
let mut bad = false;
let ci_info = CiInfo::new(&mut bad);
let bad = std::sync::Arc::new(AtomicBool::new(bad));
let mut diag_ctx = DiagCtx::new(verbose);
let diag_ctx = DiagCtx::new(verbose);
let ci_info = CiInfo::new(diag_ctx.clone());
let drain_handles = |handles: &mut VecDeque<ScopedJoinHandle<'_, ()>>| {
// poll all threads for completion before awaiting the oldest one
@ -99,98 +94,85 @@ fn main() {
}
}
// check!(target_specific_tests, &tests_path);
check!(target_specific_tests, &tests_path);
// Checks that are done on the cargo workspace.
// check!(deps, &root_path, &cargo, bless);
// check!(extdeps, &root_path);
check!(deps, &root_path, &cargo, bless);
check!(extdeps, &root_path);
// Checks over tests.
// check!(tests_placement, &root_path);
// check!(tests_revision_unpaired_stdout_stderr, &tests_path);
// check!(debug_artifacts, &tests_path);
// check!(ui_tests, &root_path, bless);
// check!(mir_opt_tests, &tests_path, bless);
// check!(rustdoc_gui_tests, &tests_path);
// check!(rustdoc_css_themes, &librustdoc_path);
// check!(rustdoc_templates, &librustdoc_path);
// check!(rustdoc_json, &src_path, &ci_info);
// check!(known_bug, &crashes_path);
// check!(unknown_revision, &tests_path);
check!(tests_placement, &root_path);
check!(tests_revision_unpaired_stdout_stderr, &tests_path);
check!(debug_artifacts, &tests_path);
check!(ui_tests, &root_path, bless);
check!(mir_opt_tests, &tests_path, bless);
check!(rustdoc_gui_tests, &tests_path);
check!(rustdoc_css_themes, &librustdoc_path);
check!(rustdoc_templates, &librustdoc_path);
check!(rustdoc_json, &src_path, &ci_info);
check!(known_bug, &crashes_path);
check!(unknown_revision, &tests_path);
// Checks that only make sense for the compiler.
// check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], verbose, &ci_info);
// check!(fluent_alphabetical, &compiler_path, bless);
// check!(fluent_period, &compiler_path);
// check!(fluent_lowercase, &compiler_path);
// check!(target_policy, &root_path);
// check!(gcc_submodule, &root_path, &compiler_path);
check!(error_codes, &root_path, &[&compiler_path, &librustdoc_path], &ci_info);
check!(fluent_alphabetical, &compiler_path, bless);
check!(fluent_period, &compiler_path);
check!(fluent_lowercase, &compiler_path);
check!(target_policy, &root_path);
check!(gcc_submodule, &root_path, &compiler_path);
// Checks that only make sense for the std libs.
// check!(pal, &library_path);
check!(pal, &library_path);
// Checks that need to be done for both the compiler and std libraries.
// check!(unit_tests, &src_path, false);
// check!(unit_tests, &compiler_path, false);
// check!(unit_tests, &library_path, true);
//
// if bins::check_filesystem_support(&[&root_path], &output_directory) {
// check!(bins, &root_path);
// }
check!(unit_tests, &src_path, false);
check!(unit_tests, &compiler_path, false);
check!(unit_tests, &library_path, true);
if bins::check_filesystem_support(&[&root_path], &output_directory) {
check!(bins, &root_path);
}
check!(style, &src_path);
check!(style, &tests_path);
check!(style, &compiler_path);
check!(style, &library_path);
// check!(edition, &src_path);
// check!(edition, &compiler_path);
// check!(edition, &library_path);
//
check!(edition, &src_path);
check!(edition, &compiler_path);
check!(edition, &library_path);
check!(alphabetical, &root_manifest);
check!(alphabetical, &src_path);
check!(alphabetical, &tests_path);
check!(alphabetical, &compiler_path);
check!(alphabetical, &library_path);
//
// check!(x_version, &root_path, &cargo);
//
// check!(triagebot, &root_path);
//
// check!(filenames, &root_path);
check!(x_version, &root_path, &cargo);
check!(triagebot, &root_path);
check!(filenames, &root_path);
let collected = {
drain_handles(&mut handles);
let mut flag = false;
let r = features::check(
&src_path,
&tests_path,
&compiler_path,
&library_path,
&mut flag,
verbose,
);
if flag {
bad.store(true, Ordering::Relaxed);
}
r
features::check(&src_path, &tests_path, &compiler_path, &library_path, diag_ctx.clone())
};
// check!(unstable_book, &src_path, collected);
//
// check!(
// extra_checks,
// &root_path,
// &output_directory,
// &ci_info,
// &librustdoc_path,
// &tools_path,
// &npm,
// &cargo,
// bless,
// extra_checks,
// pos_args
// );
check!(unstable_book, &src_path, collected);
check!(
extra_checks,
&root_path,
&output_directory,
&ci_info,
&librustdoc_path,
&tools_path,
&npm,
&cargo,
bless,
extra_checks,
pos_args
);
});
if diag_ctx.into_conclusion() {

23
src/tools/tidy/src/mir_opt_tests.rs
View file

@ -5,9 +5,10 @@ use std::path::{Path, PathBuf};
use miropt_test_tools::PanicStrategy;
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
use crate::walk::walk_no_read;
fn check_unused_files(path: &Path, bless: bool, bad: &mut bool) {
fn check_unused_files(path: &Path, bless: bool, check: &mut RunningCheck) {
let mut rs_files = Vec::<PathBuf>::new();
let mut output_files = HashSet::<PathBuf>::new();
@ -37,18 +38,17 @@ fn check_unused_files(path: &Path, bless: bool, bad: &mut bool) {
for extra in output_files {
if !bless {
tidy_error!(
bad,
check.error(format!(
"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);
}
}
}
fn check_dash_files(path: &Path, bless: bool, bad: &mut bool) {
fn check_dash_files(path: &Path, bless: bool, check: &mut RunningCheck) {
for file in walkdir::WalkDir::new(path.join("mir-opt"))
.into_iter()
.filter_map(Result::ok)
@ -60,11 +60,10 @@ fn check_dash_files(path: &Path, bless: bool, bad: &mut bool) {
&& name.contains('-')
{
if !bless {
tidy_error!(
bad,
check.error(format!(
"mir-opt test files should not have dashes in them: {}",
path.display()
);
));
} else {
let new_name = name.replace('-', "_");
let mut new_path = path.to_owned();
@ -75,7 +74,9 @@ fn check_dash_files(path: &Path, bless: bool, bad: &mut bool) {
}
}
pub fn check(path: &Path, bless: bool, bad: &mut bool) {
check_unused_files(path, bless, bad);
check_dash_files(path, bless, bad);
pub fn check(path: &Path, bless: bool, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("mir_opt_tests").path(path));
check_unused_files(path, bless, &mut check);
check_dash_files(path, bless, &mut check);
}

11
src/tools/tidy/src/pal.rs
View file

@ -32,6 +32,7 @@
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
use crate::walk::{filter_dirs, walk};
// Paths that may contain platform-specific code.
@ -67,7 +68,9 @@ const EXCEPTION_PATHS: &[&str] = &[
"library/std/src/io/error.rs", // Repr unpacked needed for UEFI
];
pub fn check(path: &Path, bad: &mut bool) {
pub fn check(path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("pal").path(path));
// Sanity check that the complex parsing here works.
let mut saw_target_arch = false;
let mut saw_cfg_bang = false;
@ -88,7 +91,7 @@ pub fn check(path: &Path, bad: &mut bool) {
return;
}
check_cfgs(contents, file, bad, &mut saw_target_arch, &mut saw_cfg_bang);
check_cfgs(contents, file, &mut check, &mut saw_target_arch, &mut saw_cfg_bang);
});
assert!(saw_target_arch);
@ -98,7 +101,7 @@ pub fn check(path: &Path, bad: &mut bool) {
fn check_cfgs(
contents: &str,
file: &Path,
bad: &mut bool,
check: &mut RunningCheck,
saw_target_arch: &mut bool,
saw_cfg_bang: &mut bool,
) {
@ -115,7 +118,7 @@ fn check_cfgs(
Ok(_) => unreachable!(),
Err(i) => i + 1,
};
tidy_error!(bad, "{}:{}: platform-specific cfg: {}", file.display(), line, cfg);
check.error(format!("{}:{line}: platform-specific cfg: {cfg}", file.display()));
};
for (idx, cfg) in cfgs {

37
src/tools/tidy/src/rustdoc_css_themes.rs
View file

@ -3,7 +3,11 @@
use std::path::Path;
pub fn check(librustdoc_path: &Path, bad: &mut bool) {
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
pub fn check(librustdoc_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("rustdoc_css_themes").path(librustdoc_path));
let rustdoc_css = "html/static/css/rustdoc.css";
let noscript_css = "html/static/css/noscript.css";
let rustdoc_css_contents = std::fs::read_to_string(librustdoc_path.join(rustdoc_css))
@ -14,13 +18,13 @@ pub fn check(librustdoc_path: &Path, bad: &mut bool) {
"light",
rustdoc_css_contents.lines().enumerate().map(|(i, l)| (i + 1, l.trim())),
noscript_css_contents.lines().enumerate().map(|(i, l)| (i + 1, l.trim())),
bad,
&mut check,
);
compare_themes_from_files(
"dark",
rustdoc_css_contents.lines().enumerate(),
noscript_css_contents.lines().enumerate(),
bad,
&mut check,
);
}
@ -28,7 +32,7 @@ fn compare_themes_from_files<'a>(
name: &str,
mut rustdoc_css_lines: impl Iterator<Item = (usize, &'a str)>,
mut noscript_css_lines: impl Iterator<Item = (usize, &'a str)>,
bad: &mut bool,
check: &mut RunningCheck,
) {
let begin_theme_pat = format!("/* Begin theme: {name}");
let mut found_theme = None;
@ -38,10 +42,9 @@ fn compare_themes_from_files<'a>(
continue;
}
if let Some(found_theme) = found_theme {
tidy_error!(
bad,
check.error(format!(
"rustdoc.css contains two {name} themes on lines {rustdoc_css_line_number} and {found_theme}",
);
));
return;
}
found_theme = Some(rustdoc_css_line_number);
@ -50,14 +53,13 @@ fn compare_themes_from_files<'a>(
continue;
}
if let Some(found_theme_noscript) = found_theme_noscript {
tidy_error!(
bad,
check.error(format!(
"noscript.css contains two {name} themes on lines {noscript_css_line_number} and {found_theme_noscript}",
);
));
return;
}
found_theme_noscript = Some(noscript_css_line_number);
compare_themes(name, &mut rustdoc_css_lines, &mut noscript_css_lines, bad);
compare_themes(name, &mut rustdoc_css_lines, &mut noscript_css_lines, check);
}
}
}
@ -66,7 +68,7 @@ fn compare_themes<'a>(
name: &str,
rustdoc_css_lines: impl Iterator<Item = (usize, &'a str)>,
noscript_css_lines: impl Iterator<Item = (usize, &'a str)>,
bad: &mut bool,
check: &mut RunningCheck,
) {
let end_theme_pat = format!("/* End theme: {name}");
for (
@ -90,12 +92,11 @@ fn compare_themes<'a>(
break;
}
if rustdoc_css_line != noscript_css_line {
tidy_error!(
bad,
"noscript.css:{noscript_css_line_number} and rustdoc.css:{rustdoc_css_line_number} contain copies of {name} theme that are not the same",
);
eprintln!("- {noscript_css_line}");
eprintln!("+ {rustdoc_css_line}");
check.error(format!(
r#"noscript.css:{noscript_css_line_number} and rustdoc.css:{rustdoc_css_line_number} contain copies of {name} theme that are not the same
- {noscript_css_line}
+ {rustdoc_css_line}"#,
));
return;
}
}

11
src/tools/tidy/src/rustdoc_gui_tests.rs
View file

@ -2,18 +2,21 @@
use std::path::Path;
pub fn check(path: &Path, bad: &mut bool) {
use crate::diagnostics::{CheckId, DiagCtx};
pub fn check(path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("rustdoc_gui_tests").path(path));
crate::walk::walk(
&path.join("rustdoc-gui"),
|p, is_dir| !is_dir && p.extension().is_none_or(|e| e != "goml"),
&mut |entry, content| {
for line in content.lines() {
if !line.starts_with("// ") {
tidy_error!(
bad,
check.error(format!(
"{}: rustdoc-gui tests must start with a small description",
entry.path().display(),
);
));
return;
} else if line.starts_with("// ") {
let parts = line[2..].trim();

42
src/tools/tidy/src/rustdoc_json.rs
View file

@ -4,19 +4,22 @@
use std::path::Path;
use std::str::FromStr;
use crate::diagnostics::{CheckId, DiagCtx};
const RUSTDOC_JSON_TYPES: &str = "src/rustdoc-json-types";
pub fn check(src_path: &Path, ci_info: &crate::CiInfo, bad: &mut bool) {
println!("Checking tidy rustdoc_json...");
pub fn check(src_path: &Path, ci_info: &crate::CiInfo, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("rustdoc_json").path(src_path));
let Some(base_commit) = &ci_info.base_commit else {
eprintln!("No base commit, skipping rustdoc_json check");
check.verbose_msg("No base commit, skipping rustdoc_json check");
return;
};
// First we check that `src/rustdoc-json-types` was modified.
if !crate::files_modified(ci_info, |p| p == RUSTDOC_JSON_TYPES) {
// `rustdoc-json-types` was not modified so nothing more to check here.
println!("`rustdoc-json-types` was not modified.");
check.verbose_msg("`rustdoc-json-types` was not modified.");
return;
}
// Then we check that if `FORMAT_VERSION` was updated, the `Latest feature:` was also updated.
@ -45,34 +48,29 @@ pub fn check(src_path: &Path, ci_info: &crate::CiInfo, bad: &mut bool) {
}
}
if format_version_updated != latest_feature_comment_updated {
*bad = true;
if latest_feature_comment_updated {
eprintln!(
"error in `rustdoc_json` tidy check: `Latest feature` comment was updated \
whereas `FORMAT_VERSION` wasn't in `{RUSTDOC_JSON_TYPES}/lib.rs`"
);
let msg = if latest_feature_comment_updated {
format!(
"`Latest feature` comment was updated whereas `FORMAT_VERSION` wasn't in `{RUSTDOC_JSON_TYPES}/lib.rs`"
)
} else {
eprintln!(
"error in `rustdoc_json` tidy check: `Latest feature` comment was not \
updated whereas `FORMAT_VERSION` was in `{RUSTDOC_JSON_TYPES}/lib.rs`"
);
}
format!(
"`Latest feature` comment was not updated whereas `FORMAT_VERSION` was in `{RUSTDOC_JSON_TYPES}/lib.rs`"
)
};
check.error(msg);
}
match (new_version, old_version) {
(Some(new_version), Some(old_version)) if new_version != old_version + 1 => {
*bad = true;
eprintln!(
"error in `rustdoc_json` tidy check: invalid `FORMAT_VERSION` increase in \
`{RUSTDOC_JSON_TYPES}/lib.rs`, should be `{}`, found `{new_version}`",
check.error(format!(
"invalid `FORMAT_VERSION` increase in `{RUSTDOC_JSON_TYPES}/lib.rs`, should be `{}`, found `{new_version}`",
old_version + 1,
);
));
}
_ => {}
}
}
None => {
*bad = true;
eprintln!("error: failed to run `git diff` in rustdoc_json check");
check.error("failed to run `git diff` in rustdoc_json check");
}
}
}

15
src/tools/tidy/src/rustdoc_templates.rs
View file

@ -6,12 +6,15 @@ use std::path::Path;
use ignore::DirEntry;
use crate::diagnostics::{CheckId, DiagCtx};
use crate::walk::walk;
// Array containing `("beginning of tag", "end of tag")`.
const TAGS: &[(&str, &str)] = &[("{#", "#}"), ("{%", "%}"), ("{{", "}}")];
pub fn check(librustdoc_path: &Path, bad: &mut bool) {
pub fn check(librustdoc_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("rustdoc_templates").path(librustdoc_path));
walk(
&librustdoc_path.join("html/templates"),
|path, is_dir| is_dir || path.extension().is_none_or(|ext| ext != OsStr::new("html")),
@ -46,12 +49,11 @@ pub fn check(librustdoc_path: &Path, bad: &mut bool) {
})
{
// It seems like ending this line with a jinja tag is not needed after all.
tidy_error!(
bad,
check.error(format!(
"`{}` at line {}: unneeded `{{# #}}` tag at the end of the line",
path.path().display(),
pos + 1,
);
));
}
continue;
}
@ -67,12 +69,11 @@ pub fn check(librustdoc_path: &Path, bad: &mut bool) {
}) {
None => {
// No it's not, let's error.
tidy_error!(
bad,
check.error(format!(
"`{}` at line {}: missing `{{# #}}` at the end of the line",
path.path().display(),
pos + 1,
);
));
}
Some(end_tag) => {
// We skip the tag.

7
src/tools/tidy/src/target_policy.rs
View file

@ -5,6 +5,7 @@
use std::collections::HashSet;
use std::path::Path;
use crate::diagnostics::DiagCtx;
use crate::walk::{filter_not_rust, walk};
const TARGET_DEFINITIONS_PATH: &str = "compiler/rustc_target/src/spec/targets/";
@ -23,7 +24,9 @@ const EXCEPTIONS: &[&str] = &[
"xtensa_esp32s3_espidf",
];
pub fn check(root_path: &Path, bad: &mut bool) {
pub fn check(root_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check("target_policy");
let mut targets_to_find = HashSet::new();
let definitions_path = root_path.join(TARGET_DEFINITIONS_PATH);
@ -55,7 +58,7 @@ pub fn check(root_path: &Path, bad: &mut bool) {
for target in targets_to_find {
if !EXCEPTIONS.contains(&target.as_str()) {
tidy_error!(bad, "{ASSEMBLY_LLVM_TEST_PATH}: missing assembly test for {target}")
check.error(format!("{ASSEMBLY_LLVM_TEST_PATH}: missing assembly test for {target}"));
}
}
}

23
src/tools/tidy/src/target_specific_tests.rs
View file

@ -4,6 +4,7 @@
use std::collections::BTreeMap;
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx};
use crate::iter_header::{HeaderLine, iter_header};
use crate::walk::filter_not_rust;
@ -16,7 +17,9 @@ struct RevisionInfo<'a> {
llvm_components: Option<Vec<&'a str>>,
}
pub fn check(tests_path: &Path, bad: &mut bool) {
pub fn check(tests_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("target-specific-tests").path(tests_path));
crate::walk::walk(tests_path, |path, _is_dir| filter_not_rust(path), &mut |entry, content| {
if content.contains("// ignore-tidy-target-specific-tests") {
return;
@ -44,8 +47,7 @@ pub fn check(tests_path: &Path, bad: &mut bool) {
} else if let Some((arch, _)) = v.split_once("-") {
info.target_arch.replace(Some(arch));
} else {
eprintln!("{file}: seems to have a malformed --target value");
*bad = true;
check.error(format!("{file}: seems to have a malformed --target value"));
}
}
});
@ -62,25 +64,22 @@ pub fn check(tests_path: &Path, bad: &mut bool) {
(Some(target_arch), None) => {
let llvm_component =
target_arch.map_or_else(|| "<arch>".to_string(), arch_to_llvm_component);
eprintln!(
check.error(format!(
"{file}: revision {rev} should specify `{LLVM_COMPONENTS_HEADER} {llvm_component}` as it has `--target` set"
);
*bad = true;
));
}
(None, Some(_)) => {
eprintln!(
check.error(format!(
"{file}: revision {rev} should not specify `{LLVM_COMPONENTS_HEADER}` as it doesn't need `--target`"
);
*bad = true;
));
}
(Some(target_arch), Some(llvm_components)) => {
if let Some(target_arch) = target_arch {
let llvm_component = arch_to_llvm_component(target_arch);
if !llvm_components.contains(&llvm_component.as_str()) {
eprintln!(
check.error(format!(
"{file}: revision {rev} should specify `{LLVM_COMPONENTS_HEADER} {llvm_component}` as it has `--target` set"
);
*bad = true;
));
}
}
}

17
src/tools/tidy/src/tests_placement.rs
View file

@ -1,15 +1,18 @@
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx};
const FORBIDDEN_PATH: &str = "src/test";
const ALLOWED_PATH: &str = "tests";
pub fn check(root_path: impl AsRef<Path>, bad: &mut bool) {
if root_path.as_ref().join(FORBIDDEN_PATH).exists() {
tidy_error!(
bad,
pub fn check(root_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("tests-placement").path(root_path));
if root_path.join(FORBIDDEN_PATH).exists() {
check.error(format!(
"Tests have been moved, please move them from {} to {}",
root_path.as_ref().join(FORBIDDEN_PATH).display(),
root_path.as_ref().join(ALLOWED_PATH).display()
)
root_path.join(FORBIDDEN_PATH).display(),
root_path.join(ALLOWED_PATH).display()
));
}
}

16
src/tools/tidy/src/tests_revision_unpaired_stdout_stderr.rs
View file

@ -4,6 +4,7 @@ use std::collections::{BTreeMap, BTreeSet};
use std::ffi::OsStr;
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx};
use crate::iter_header::*;
use crate::walk::*;
@ -21,7 +22,10 @@ const IGNORES: &[&str] = &[
const EXTENSIONS: &[&str] = &["stdout", "stderr"];
const SPECIAL_TEST: &str = "tests/ui/command/need-crate-arg-ignore-tidy.x.rs";
pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
pub fn check(tests_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx
.start_check(CheckId::new("tests_revision_unpaired_stdout_stderr").path(tests_path));
// Recurse over subdirectories under `tests/`
walk_dir(tests_path.as_ref(), filter, &mut |entry| {
// We are inspecting a folder. Collect the paths to interesting files `.rs`, `.stderr`,
@ -122,12 +126,11 @@ pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
[] | [_] => return,
[_, _] if !expected_revisions.is_empty() => {
// Found unrevisioned output files for a revisioned test.
tidy_error!(
bad,
check.error(format!(
"found unrevisioned output file `{}` for a revisioned test `{}`",
sibling.display(),
test_path.display(),
);
));
}
[_, _] => return,
[_, found_revision, .., extension] => {
@ -138,13 +141,12 @@ pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
{
// Found some unexpected revision-esque component that is not a known
// compare-mode or expected revision.
tidy_error!(
bad,
check.error(format!(
"found output file `{}` for unexpected revision `{}` of test `{}`",
sibling.display(),
found_revision,
test_path.display()
);
));
}
}
}

38
src/tools/tidy/src/triagebot.rs
View file

@ -4,7 +4,10 @@ use std::path::Path;
use toml::Value;
pub fn check(path: &Path, bad: &mut bool) {
use crate::diagnostics::{CheckId, DiagCtx};
pub fn check(path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("triagebot").path(path));
let triagebot_path = path.join("triagebot.toml");
// This check is mostly to catch broken path filters *within* `triagebot.toml`, and not enforce
@ -30,17 +33,14 @@ pub fn check(path: &Path, bad: &mut bool) {
let full_path = path.join(clean_path);
if !full_path.exists() {
tidy_error!(
bad,
"triagebot.toml [mentions.*] contains path '{}' which doesn't exist",
clean_path
);
check.error(format!(
"triagebot.toml [mentions.*] contains path '{clean_path}' which doesn't exist"
));
}
}
} else {
tidy_error!(
bad,
"triagebot.toml missing [mentions.*] section, this wrong for rust-lang/rust repo."
check.error(
"triagebot.toml missing [mentions.*] section, this wrong for rust-lang/rust repo.",
);
}
@ -55,16 +55,13 @@ pub fn check(path: &Path, bad: &mut bool) {
let full_path = path.join(clean_path);
if !full_path.exists() {
tidy_error!(
bad,
"triagebot.toml [assign.owners] contains path '{}' which doesn't exist",
clean_path
);
check.error(format!(
"triagebot.toml [assign.owners] contains path '{clean_path}' which doesn't exist"
));
}
}
} else {
tidy_error!(
bad,
check.error(
"triagebot.toml missing [assign.owners] section, this wrong for rust-lang/rust repo."
);
}
@ -86,12 +83,9 @@ pub fn check(path: &Path, bad: &mut bool) {
// Handle both file and directory paths
if !full_path.exists() {
tidy_error!(
bad,
"triagebot.toml [autolabel.{}] contains trigger_files path '{}' which doesn't exist",
label,
file_str
);
check.error(format!(
"triagebot.toml [autolabel.{label}] contains trigger_files path '{file_str}' which doesn't exist",
));
}
}
}

53
src/tools/tidy/src/ui_tests.rs
View file

@ -7,13 +7,16 @@ use std::fs;
use std::io::Write;
use std::path::{Path, PathBuf};
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
const ISSUES_TXT_HEADER: &str = r#"============================================================
⚠️⚠️⚠️NOTHING SHOULD EVER BE ADDED TO THIS LIST⚠️⚠️⚠️
============================================================
"#;
pub fn check(root_path: &Path, bless: bool, bad: &mut bool) {
pub fn check(root_path: &Path, bless: bool, diag_ctx: DiagCtx) {
let path = &root_path.join("tests");
let mut check = diag_ctx.start_check(CheckId::new("ui_tests").path(path));
// the list of files in ui tests that are allowed to start with `issue-XXXX`
// BTreeSet because we would like a stable ordering so --bless works
@ -33,16 +36,15 @@ pub fn check(root_path: &Path, bless: bool, bad: &mut bool) {
.collect();
if !is_sorted && !bless {
tidy_error!(
bad,
check.error(
"`src/tools/tidy/src/issues.txt` is not in order, mostly because you modified it manually,
please only update it with command `x test tidy --bless`"
);
}
deny_new_top_level_ui_tests(bad, &path.join("ui"));
deny_new_top_level_ui_tests(&mut check, &path.join("ui"));
let remaining_issue_names = recursively_check_ui_tests(bad, path, &allowed_issue_names);
let remaining_issue_names = recursively_check_ui_tests(&mut check, path, &allowed_issue_names);
// if there are any file names remaining, they were moved on the fs.
// our data must remain up to date, so it must be removed from issues.txt
@ -64,16 +66,15 @@ pub fn check(root_path: &Path, bless: bool, bad: &mut bool) {
for file_name in remaining_issue_names {
let mut p = PathBuf::from(path);
p.push(file_name);
tidy_error!(
bad,
check.error(format!(
"file `{}` no longer exists and should be removed from the exclusions in `src/tools/tidy/src/issues.txt`",
p.display()
);
));
}
}
}
fn deny_new_top_level_ui_tests(bad: &mut bool, tests_path: &Path) {
fn deny_new_top_level_ui_tests(check: &mut RunningCheck, tests_path: &Path) {
// See <https://github.com/rust-lang/compiler-team/issues/902> where we propose banning adding
// new ui tests *directly* under `tests/ui/`. For more context, see:
//
@ -93,16 +94,15 @@ fn deny_new_top_level_ui_tests(bad: &mut bool, tests_path: &Path) {
})
.filter(|e| !e.file_type().is_dir());
for entry in top_level_ui_tests {
tidy_error!(
bad,
check.error(format!(
"ui tests should be added under meaningful subdirectories: `{}`",
entry.path().display()
)
));
}
}
fn recursively_check_ui_tests<'issues>(
bad: &mut bool,
check: &mut RunningCheck,
path: &Path,
allowed_issue_names: &'issues BTreeSet<&'issues str>,
) -> BTreeSet<&'issues str> {
@ -113,19 +113,19 @@ fn recursively_check_ui_tests<'issues>(
crate::walk::walk_no_read(&paths, |_, _| false, &mut |entry| {
let file_path = entry.path();
if let Some(ext) = file_path.extension().and_then(OsStr::to_str) {
check_unexpected_extension(bad, file_path, ext);
check_unexpected_extension(check, file_path, ext);
// 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 ext == "stderr" || ext == "stdout" || ext == "fixed" {
check_stray_output_snapshot(bad, file_path, testname);
check_empty_output_snapshot(bad, file_path);
check_stray_output_snapshot(check, file_path, testname);
check_empty_output_snapshot(check, file_path);
}
deny_new_nondescriptive_test_names(
bad,
check,
path,
&mut remaining_issue_names,
file_path,
@ -137,7 +137,7 @@ fn recursively_check_ui_tests<'issues>(
remaining_issue_names
}
fn check_unexpected_extension(bad: &mut bool, file_path: &Path, ext: &str) {
fn check_unexpected_extension(check: &mut RunningCheck, file_path: &Path, ext: &str) {
const EXPECTED_TEST_FILE_EXTENSIONS: &[&str] = &[
"rs", // test source files
"stderr", // expected stderr file, corresponds to a rs file
@ -178,11 +178,11 @@ fn check_unexpected_extension(bad: &mut bool, file_path: &Path, ext: &str) {
if !(EXPECTED_TEST_FILE_EXTENSIONS.contains(&ext)
|| EXTENSION_EXCEPTION_PATHS.iter().any(|path| file_path.ends_with(path)))
{
tidy_error!(bad, "file {} has unexpected extension {}", file_path.display(), ext);
check.error(format!("file {} has unexpected extension {}", file_path.display(), ext));
}
}
fn check_stray_output_snapshot(bad: &mut bool, file_path: &Path, testname: &str) {
fn check_stray_output_snapshot(check: &mut RunningCheck, file_path: &Path, testname: &str) {
// Test output filenames have one of the formats:
// ```
// $testname.stderr
@ -197,20 +197,20 @@ fn check_stray_output_snapshot(bad: &mut bool, file_path: &Path, testname: &str)
if !file_path.with_file_name(testname).with_extension("rs").exists()
&& !testname.contains("ignore-tidy")
{
tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path);
check.error(format!("Stray file with UI testing output: {:?}", file_path));
}
}
fn check_empty_output_snapshot(bad: &mut bool, file_path: &Path) {
fn check_empty_output_snapshot(check: &mut RunningCheck, file_path: &Path) {
if let Ok(metadata) = fs::metadata(file_path)
&& metadata.len() == 0
{
tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path);
check.error(format!("Empty file with UI testing output: {:?}", file_path));
}
}
fn deny_new_nondescriptive_test_names(
bad: &mut bool,
check: &mut RunningCheck,
path: &Path,
remaining_issue_names: &mut BTreeSet<&str>,
file_path: &Path,
@ -231,11 +231,10 @@ fn deny_new_nondescriptive_test_names(
if !remaining_issue_names.remove(stripped_path.as_str())
&& !stripped_path.starts_with("ui/issues/")
{
tidy_error!(
bad,
check.error(format!(
"file `tests/{stripped_path}` must begin with a descriptive name, consider `{{reason}}-issue-{issue_n}.rs`",
issue_n = &test_name[1],
);
));
}
}
}

14
src/tools/tidy/src/unit_tests.rs
View file

@ -11,9 +11,12 @@
use std::path::Path;
use crate::diagnostics::{CheckId, DiagCtx};
use crate::walk::{filter_dirs, walk};
pub fn check(root_path: &Path, stdlib: bool, bad: &mut bool) {
pub fn check(root_path: &Path, stdlib: bool, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("unit_tests").path(root_path));
let skip = move |path: &Path, is_dir| {
let file_name = path.file_name().unwrap_or_default();
@ -92,14 +95,11 @@ pub fn check(root_path: &Path, stdlib: bool, bad: &mut bool) {
.to_owned()
};
let name = if is_test() { "test" } else { "bench" };
tidy_error!(
bad,
"`{}:{}` contains `#[{}]`; {}",
check.error(format!(
"`{}:{}` contains `#[{name}]`; {explanation}",
path.display(),
i + 1,
name,
explanation,
);
));
return;
}
}

17
src/tools/tidy/src/unknown_revision.rs
View file

@ -12,12 +12,14 @@ use std::sync::OnceLock;
use ignore::DirEntry;
use regex::Regex;
use crate::diagnostics::{CheckId, DiagCtx, RunningCheck};
use crate::iter_header::{HeaderLine, iter_header};
use crate::walk::{filter_dirs, filter_not_rust, walk};
pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
pub fn check(tests_path: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("unknown_revision").path(tests_path));
walk(
tests_path.as_ref(),
tests_path,
|path, is_dir| {
filter_dirs(path) || filter_not_rust(path) || {
// Auxiliary source files for incremental tests can refer to revisions
@ -25,11 +27,11 @@ pub fn check(tests_path: impl AsRef<Path>, bad: &mut bool) {
is_dir && path.file_name().is_some_and(|name| name == "auxiliary")
}
},
&mut |entry, contents| visit_test_file(entry, contents, bad),
&mut |entry, contents| visit_test_file(entry, contents, &mut check),
);
}
fn visit_test_file(entry: &DirEntry, contents: &str, bad: &mut bool) {
fn visit_test_file(entry: &DirEntry, contents: &str, check: &mut RunningCheck) {
let mut revisions = HashSet::new();
let mut unused_revision_names = HashSet::new();
@ -68,10 +70,9 @@ fn visit_test_file(entry: &DirEntry, contents: &str, bad: &mut bool) {
// Fail if any revision names appear in both places, since that's probably a mistake.
for rev in revisions.intersection(&unused_revision_names).copied().collect::<BTreeSet<_>>() {
tidy_error!(
bad,
check.error(format!(
"revision name [{rev}] appears in both `revisions` and `unused-revision-names` in {path}"
);
));
}
// Compute the set of revisions that were mentioned but not declared,
@ -84,7 +85,7 @@ fn visit_test_file(entry: &DirEntry, contents: &str, bad: &mut bool) {
bad_revisions.sort();
for (line_number, rev) in bad_revisions {
tidy_error!(bad, "unknown revision [{rev}] at {path}:{line_number}");
check.error(format!("unknown revision [{rev}] at {path}:{line_number}"));
}
}

40
src/tools/tidy/src/unstable_book.rs
View file

@ -2,6 +2,7 @@ use std::collections::BTreeSet;
use std::fs;
use std::path::{Path, PathBuf};
use crate::diagnostics::{DiagCtx, RunningCheck};
use crate::features::{CollectedFeatures, Features, Status};
pub const PATH_STR: &str = "doc/unstable-book";
@ -75,19 +76,18 @@ fn collect_unstable_book_lib_features_section_file_names(base_src_path: &Path) -
}
/// Would switching underscores for dashes work?
fn maybe_suggest_dashes(names: &BTreeSet<String>, feature_name: &str, bad: &mut bool) {
fn maybe_suggest_dashes(names: &BTreeSet<String>, feature_name: &str, check: &mut RunningCheck) {
let with_dashes = feature_name.replace('_', "-");
if names.contains(&with_dashes) {
tidy_error!(
bad,
"the file `{}.md` contains underscores; use dashes instead: `{}.md`",
feature_name,
with_dashes,
);
check.error(format!(
"the file `{feature_name}.md` contains underscores; use dashes instead: `{with_dashes}.md`",
));
}
}
pub fn check(path: &Path, features: CollectedFeatures, bad: &mut bool) {
pub fn check(path: &Path, features: CollectedFeatures, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check("unstable_book");
let lang_features = features.lang;
let lib_features = features
.lib
@ -108,26 +108,22 @@ pub fn check(path: &Path, features: CollectedFeatures, bad: &mut bool) {
// Check for Unstable Book sections that don't have a corresponding unstable feature
for feature_name in &unstable_book_lib_features_section_file_names - &unstable_lib_feature_names
{
tidy_error!(
bad,
"The Unstable Book has a 'library feature' section '{}' which doesn't \
correspond to an unstable library feature",
feature_name
);
maybe_suggest_dashes(&unstable_lib_feature_names, &feature_name, bad);
check.error(format!(
"The Unstable Book has a 'library feature' section '{feature_name}' which doesn't \
correspond to an unstable library feature"
));
maybe_suggest_dashes(&unstable_lib_feature_names, &feature_name, &mut check);
}
// Check for Unstable Book sections that don't have a corresponding unstable feature.
for feature_name in
&unstable_book_lang_features_section_file_names - &unstable_lang_feature_names
{
tidy_error!(
bad,
"The Unstable Book has a 'language feature' section '{}' which doesn't \
correspond to an unstable language feature",
feature_name
);
maybe_suggest_dashes(&unstable_lang_feature_names, &feature_name, bad);
check.error(format!(
"The Unstable Book has a 'language feature' section '{feature_name}' which doesn't \
correspond to an unstable language feature"
));
maybe_suggest_dashes(&unstable_lang_feature_names, &feature_name, &mut check);
}
// List unstable features that don't have Unstable Book sections.

17
src/tools/tidy/src/x_version.rs
View file

@ -3,12 +3,18 @@ use std::process::{Command, Stdio};
use semver::Version;
pub fn check(root: &Path, cargo: &Path, bad: &mut bool) {
use crate::diagnostics::{CheckId, DiagCtx};
pub fn check(root: &Path, cargo: &Path, diag_ctx: DiagCtx) {
let mut check = diag_ctx.start_check(CheckId::new("x_version").path(root));
let cargo_list = Command::new(cargo).args(["install", "--list"]).stdout(Stdio::piped()).spawn();
let child = match cargo_list {
Ok(child) => child,
Err(e) => return tidy_error!(bad, "failed to run `cargo`: {}", e),
Err(e) => {
check.error(format!("failed to run `cargo`: {e}"));
return;
}
};
let cargo_list = child.wait_with_output().unwrap();
@ -47,13 +53,10 @@ pub fn check(root: &Path, cargo: &Path, bad: &mut bool) {
)
}
} else {
tidy_error!(
bad,
"Unable to parse the latest version of `x` at `src/tools/x/Cargo.toml`"
)
check.error("Unable to parse the latest version of `x` at `src/tools/x/Cargo.toml`")
}
} else {
tidy_error!(bad, "failed to check version of `x`: {}", cargo_list.status)
check.error(format!("failed to check version of `x`: {}", cargo_list.status))
}
}