Skip to content

Tidy cleanup #143724

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
2 changes: 1 addition & 1 deletion src/tools/tidy/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[package]
name = "tidy"
version = "0.1.0"
edition = "2021"
edition = "2024"
autobins = false

[dependencies]
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 @@ -103,7 +103,7 @@ fn check_section<'a>(

let prev_line_trimmed_lowercase = prev_line.trim_start_matches(' ');

if version_sort(&trimmed_line, &prev_line_trimmed_lowercase).is_lt() {
if version_sort(trimmed_line, prev_line_trimmed_lowercase).is_lt() {
tidy_error_ext!(err, bad, "{file}:{}: line not in alphabetical order", idx + 1);
}

Expand Down
11 changes: 3 additions & 8 deletions src/tools/tidy/src/bins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,20 +75,15 @@ mod os_impl {
return ReadOnlyFs;
}

panic!("unable to create temporary file `{:?}`: {:?}", path, e);
panic!("unable to create temporary file `{path:?}`: {e:?}");
}
}
}

for &source_dir in sources {
match check_dir(source_dir) {
Unsupported => return false,
ReadOnlyFs => {
return match check_dir(output) {
Supported => true,
_ => false,
};
}
ReadOnlyFs => return matches!(check_dir(output), Supported),
_ => {}
}
}
Expand Down Expand Up @@ -139,7 +134,7 @@ mod os_impl {
return;
}

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

Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ fn check_proc_macro_dep_list(root: &Path, cargo: &Path, bless: bool, bad: &mut b
let is_proc_macro_pkg = |pkg: &Package| pkg.targets.iter().any(|target| target.is_proc_macro());

let mut proc_macro_deps = HashSet::new();
for pkg in metadata.packages.iter().filter(|pkg| is_proc_macro_pkg(*pkg)) {
for pkg in metadata.packages.iter().filter(|pkg| is_proc_macro_pkg(pkg)) {
deps_of(&metadata, &pkg.id, &mut proc_macro_deps);
}
// Remove the proc-macro crates themselves
Expand Down
20 changes: 8 additions & 12 deletions src/tools/tidy/src/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,7 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
let Some(split_line) = split_line else {
errors.push(format!(
"{path}:{line_index}: Expected a line with the format `Eabcd: abcd, \
but got \"{}\" without a `:` delimiter",
line,
but got \"{line}\" without a `:` delimiter",
));
continue;
};
Expand All @@ -129,10 +128,8 @@ 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
));
errors
.push(format!("{path}:{line_index}: Found duplicate error code: `{err_code}`"));
continue;
}

Expand All @@ -145,8 +142,7 @@ fn extract_error_codes(root_path: &Path, errors: &mut Vec<String>) -> Vec<String
let Some(rest) = rest else {
errors.push(format!(
"{path}:{line_index}: Expected a line with the format `Eabcd: abcd, \
but got \"{}\" without a `,` delimiter",
line,
but got \"{line}\" without a `,` delimiter",
));
continue;
};
Expand Down Expand Up @@ -209,7 +205,7 @@ fn check_error_codes_docs(
}

let (found_code_example, found_proper_doctest, emit_ignore_warning, no_longer_emitted) =
check_explanation_has_doctest(&contents, &err_code);
check_explanation_has_doctest(contents, err_code);

if emit_ignore_warning {
verbose_print!(
Expand All @@ -232,7 +228,7 @@ fn check_error_codes_docs(
return;
}

let test_ignored = IGNORE_DOCTEST_CHECK.contains(&&err_code);
let test_ignored = IGNORE_DOCTEST_CHECK.contains(&err_code);

// Check that the explanation has a doctest, and if it shouldn't, that it doesn't
if !found_proper_doctest && !test_ignored {
Expand Down Expand Up @@ -300,7 +296,7 @@ fn check_error_codes_tests(
let tests_path = root_path.join(Path::new(ERROR_TESTS_PATH));

for code in error_codes {
let test_path = tests_path.join(format!("{}.stderr", code));
let test_path = tests_path.join(format!("{code}.stderr"));

if !test_path.exists() && !IGNORE_UI_TEST_CHECK.contains(&code.as_str()) {
verbose_print!(
Expand Down Expand Up @@ -388,7 +384,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 `{}` is used in the compiler but not defined and documented in `compiler/rustc_error_codes/src/lib.rs`.", error_code));
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`."));
continue;
}

Expand Down
16 changes: 8 additions & 8 deletions src/tools/tidy/src/ext_tool_checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ fn check_impl(
extra_checks: Option<&str>,
pos_args: &[String],
) -> Result<(), Error> {
let show_diff = std::env::var("TIDY_PRINT_DIFF")
.map_or(false, |v| v.eq_ignore_ascii_case("true") || v == "1");
let show_diff =
std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("true") || v == "1");

// Split comma-separated args up
let lint_args = match extra_checks {
Expand Down Expand Up @@ -141,7 +141,7 @@ fn check_impl(
);
}
// Rethrow error
let _ = res?;
res?;
}

if python_fmt {
Expand Down Expand Up @@ -173,7 +173,7 @@ fn check_impl(
}

// Rethrow error
let _ = res?;
res?;
}

if cpp_fmt {
Expand Down Expand Up @@ -244,7 +244,7 @@ fn check_impl(
}
}
// Rethrow error
let _ = res?;
res?;
}

if shell_lint {
Expand Down Expand Up @@ -286,8 +286,8 @@ fn run_ruff(
file_args: &[&OsStr],
ruff_args: &[&OsStr],
) -> Result<(), Error> {
let mut cfg_args_ruff = cfg_args.into_iter().copied().collect::<Vec<_>>();
let mut file_args_ruff = file_args.into_iter().copied().collect::<Vec<_>>();
let mut cfg_args_ruff = cfg_args.to_vec();
let mut file_args_ruff = file_args.to_vec();

let mut cfg_path = root_path.to_owned();
cfg_path.extend(RUFF_CONFIG_PATH);
Expand All @@ -305,7 +305,7 @@ fn run_ruff(
file_args_ruff.push(root_path.as_os_str());
}

let mut args: Vec<&OsStr> = ruff_args.into_iter().copied().collect();
let mut args: Vec<&OsStr> = ruff_args.to_vec();
args.extend(merge_args(&cfg_args_ruff, &file_args_ruff));
py_runner(py_path, true, None, "ruff", &args)
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/extdeps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub fn check(root: &Path, bad: &mut bool) {
let source = line.split_once('=').unwrap().1.trim();

// Ensure source is allowed.
if !ALLOWED_SOURCES.contains(&&*source) {
if !ALLOWED_SOURCES.contains(&source) {
tidy_error!(bad, "invalid source: {}", source);
}
}
Expand Down
21 changes: 9 additions & 12 deletions src/tools/tidy/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ pub fn check(
let gate_test_str = "gate-test-";

let feature_name = match line.find(gate_test_str) {
// NB: the `splitn` always succeeds, even if the delimiter is not present.
Some(i) => line[i + gate_test_str.len()..].splitn(2, ' ').next().unwrap(),
// `split` always contains at least 1 element, even if the delimiter is not present.
Some(i) => line[i + gate_test_str.len()..].split(' ').next().unwrap(),
None => continue,
};
match features.get_mut(feature_name) {
Expand All @@ -135,16 +135,14 @@ pub fn check(
err(&format!(
"The file is already marked as gate test \
through its name, no need for a \
'gate-test-{}' comment",
feature_name
'gate-test-{feature_name}' comment"
));
}
f.has_gate_test = true;
}
None => {
err(&format!(
"gate-test test found referencing a nonexistent feature '{}'",
feature_name
"gate-test test found referencing a nonexistent feature '{feature_name}'"
));
}
}
Expand All @@ -170,8 +168,7 @@ pub fn check(
);
println!(
"Hint: If you already have such a test and don't want to rename it,\
\n you can also add a // gate-test-{} line to the test file.",
name
\n you can also add a // gate-test-{name} line to the test file."
);
}

Expand Down Expand Up @@ -231,7 +228,7 @@ pub fn check(
fn get_version_and_channel(src_path: &Path) -> (Version, String) {
let version_str = t!(std::fs::read_to_string(src_path.join("version")));
let version_str = version_str.trim();
let version = t!(std::str::FromStr::from_str(&version_str).map_err(|e| format!("{e:?}")));
let version = t!(std::str::FromStr::from_str(version_str).map_err(|e| format!("{e:?}")));
let channel_str = t!(std::fs::read_to_string(src_path.join("ci").join("channel")));
(version, channel_str.trim().to_owned())
}
Expand Down Expand Up @@ -468,7 +465,7 @@ fn get_and_check_lib_features(
map_lib_features(base_src_path, &mut |res, file, line| match res {
Ok((name, f)) => {
let mut check_features = |f: &Feature, list: &Features, display: &str| {
if let Some(ref s) = list.get(name) {
if let Some(s) = list.get(name) {
if f.tracking_issue != s.tracking_issue && f.level != Status::Accepted {
tidy_error!(
bad,
Expand All @@ -483,7 +480,7 @@ fn get_and_check_lib_features(
}
}
};
check_features(&f, &lang_features, "corresponding lang feature");
check_features(&f, lang_features, "corresponding lang feature");
check_features(&f, &lib_features, "previous");
lib_features.insert(name.to_owned(), f);
}
Expand Down Expand Up @@ -543,7 +540,7 @@ fn map_lib_features(
continue;
}

if let Some((ref name, ref mut f)) = becoming_feature {
if let Some((name, ref mut f)) = becoming_feature {
if f.tracking_issue.is_none() {
f.tracking_issue = find_attr_val(line, "issue").and_then(handle_issue_none);
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/features/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl fmt::Display for Version {
Version::Explicit { parts } => {
f.pad(&format!("{}.{}.{}", parts[0], parts[1], parts[2]))
}
Version::CurrentPlaceholder => f.pad(&format!("CURRENT")),
Version::CurrentPlaceholder => f.pad("CURRENT"),
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/tidy/src/features/version/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ fn test_to_string() {

assert_eq!(v_1_0_0.to_string(), "1.0.0");
assert_eq!(v_1_32_1.to_string(), "1.32.1");
assert_eq!(format!("{:<8}", v_1_32_1), "1.32.1 ");
assert_eq!(format!("{:>8}", v_1_32_1), " 1.32.1");
assert_eq!(format!("{v_1_32_1:<8}"), "1.32.1 ");
assert_eq!(format!("{v_1_32_1:>8}"), " 1.32.1");
}
8 changes: 4 additions & 4 deletions src/tools/tidy/src/fluent_alphabetical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ fn message() -> &'static Regex {
static_regex!(r#"(?m)^([a-zA-Z0-9_]+)\s*=\s*"#)
}

fn filter_fluent(path: &Path) -> bool {
if let Some(ext) = path.extension() { ext.to_str() != Some("ftl") } else { true }
fn is_fluent(path: &Path) -> bool {
path.extension().is_some_and(|ext| ext == "flt")
}

fn check_alphabetic(
Expand Down Expand Up @@ -92,7 +92,7 @@ pub fn check(path: &Path, bless: bool, bad: &mut bool) {
let mut all_defined_msgs = HashMap::new();
walk(
path,
|path, is_dir| filter_dirs(path) || (!is_dir && filter_fluent(path)),
|path, is_dir| filter_dirs(path) || (!is_dir && !is_fluent(path)),
&mut |ent, contents| {
if bless {
let sorted = sort_messages(
Expand All @@ -104,7 +104,7 @@ pub fn check(path: &Path, bless: bool, bad: &mut bool) {
if sorted != contents {
let mut f =
OpenOptions::new().write(true).truncate(true).open(ent.path()).unwrap();
f.write(sorted.as_bytes()).unwrap();
f.write_all(sorted.as_bytes()).unwrap();
}
} else {
check_alphabetic(
Expand Down
4 changes: 2 additions & 2 deletions src/tools/tidy/src/fluent_period.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn check_period(filename: &str, contents: &str, bad: &mut bool) {
if let Some(PatternElement::TextElement { value }) = pat.elements.last() {
// We don't care about ellipses.
if value.ends_with(".") && !value.ends_with("...") {
let ll = find_line(contents, *value);
let ll = find_line(contents, value);
let name = m.id.name;
tidy_error!(bad, "{filename}:{ll}: message `{name}` ends in a period");
}
Expand All @@ -52,7 +52,7 @@ fn check_period(filename: &str, contents: &str, bad: &mut bool) {

if let Some(PatternElement::TextElement { value }) = attr.value.elements.last() {
if value.ends_with(".") && !value.ends_with("...") {
let ll = find_line(contents, *value);
let ll = find_line(contents, value);
let name = attr.id.name;
tidy_error!(bad, "{filename}:{ll}: attr `{name}` ends in a period");
}
Expand Down
4 changes: 2 additions & 2 deletions src/tools/tidy/src/fluent_used.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ fn filter_used_messages(
// we don't just check messages never appear in Rust files,
// because messages can be used as parts of other fluent messages in Fluent files,
// so we do checking messages appear only once in all Rust and Fluent files.
let mut matches = static_regex!(r"\w+").find_iter(contents);
while let Some(name) = matches.next() {
let matches = static_regex!(r"\w+").find_iter(contents);
for name in matches {
if let Some((name, filename)) = msgs_not_appeared_yet.remove_entry(name.as_str()) {
// if one msg appears for the first time,
// remove it from `msgs_not_appeared_yet` and insert it into `msgs_appeared_only_once`.
Expand Down
7 changes: 4 additions & 3 deletions src/tools/tidy/src/gcc_submodule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use std::process::Command;
pub fn check(root_path: &Path, compiler_path: &Path, bad: &mut bool) {
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)
.expect(&format!("Cannot read GCC version from {}", cg_gcc_version_path.display()))
.unwrap_or_else(|_| {
panic!("Cannot read GCC version from {}", cg_gcc_version_path.display())
})
.trim()
.to_string();

Expand All @@ -27,14 +29,13 @@ pub fn check(root_path: &Path, compiler_path: &Path, bad: &mut bool) {
// e607be166673a8de9fc07f6f02c60426e556c5f2 src/gcc (master-e607be166673a8de9fc07f6f02c60426e556c5f2.e607be)
// +e607be166673a8de9fc07f6f02c60426e556c5f2 src/gcc (master-e607be166673a8de9fc07f6f02c60426e556c5f2.e607be)
let git_output = String::from_utf8_lossy(&git_output.stdout)
.trim()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.split_whitespace()
.next()
.unwrap_or_default()
.to_string();

// 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(&['+', '-']);
let gcc_submodule_sha = git_output.trim_start_matches(['+', '-']);
if gcc_submodule_sha != cg_gcc_version {
*bad = true;
eprintln!(
Expand Down
9 changes: 5 additions & 4 deletions src/tools/tidy/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,12 @@ use std::{env, process};
use tidy::*;

fn main() {
// Running Cargo will read the libstd Cargo.toml
// Enable nightly, because Cargo will read the libstd Cargo.toml
// which uses the unstable `public-dependency` feature.
//
// `setenv` might not be thread safe, so run it before using multiple threads.
env::set_var("RUSTC_BOOTSTRAP", "1");
// SAFETY: no other threads have been spawned
unsafe {
env::set_var("RUSTC_BOOTSTRAP", "1");
}

let root_path: PathBuf = env::args_os().nth(1).expect("need path to root of repo").into();
let cargo: PathBuf = env::args_os().nth(2).expect("need path to cargo").into();
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 @@ -49,7 +49,7 @@ fn check_unused_files(path: &Path, bless: bool, bad: &mut bool) {
}

fn check_dash_files(path: &Path, bless: bool, bad: &mut bool) {
for file in walkdir::WalkDir::new(&path.join("mir-opt"))
for file in walkdir::WalkDir::new(path.join("mir-opt"))
.into_iter()
.filter_map(Result::ok)
.filter(|e| e.file_type().is_file())
Expand Down
Loading
Loading