Skip to content

Tidy cleanup 2 #143960

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 3 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
35 changes: 17 additions & 18 deletions src/tools/tidy/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,11 +331,9 @@ fn collect_lang_features_in(features: &mut Features, base: &Path, file: &str, ba
continue;
}

if in_feature_group {
if let Some(doc_comment) = line.strip_prefix("///") {
doc_comments.push(doc_comment.trim().to_string());
continue;
}
if in_feature_group && let Some(doc_comment) = line.strip_prefix("///") {
doc_comments.push(doc_comment.trim().to_string());
continue;
}

let mut parts = line.split(',');
Expand Down Expand Up @@ -465,19 +463,20 @@ 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(s) = list.get(name) {
if f.tracking_issue != s.tracking_issue && f.level != Status::Accepted {
tidy_error!(
bad,
"{}:{}: feature gate {} has inconsistent `issue`: \"{}\" mismatches the {} `issue` of \"{}\"",
file.display(),
line,
name,
f.tracking_issue_display(),
display,
s.tracking_issue_display(),
);
}
if let Some(s) = list.get(name)
&& f.tracking_issue != s.tracking_issue
&& f.level != Status::Accepted
{
tidy_error!(
bad,
"{}:{}: feature gate {} has inconsistent `issue`: \"{}\" mismatches the {} `issue` of \"{}\"",
file.display(),
line,
name,
f.tracking_issue_display(),
display,
s.tracking_issue_display(),
);
}
};
check_features(&f, lang_features, "corresponding lang feature");
Expand Down
29 changes: 15 additions & 14 deletions src/tools/tidy/src/fluent_period.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ fn check_period(filename: &str, contents: &str, bad: &mut bool) {
continue;
}

if let Some(pat) = &m.value {
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 name = m.id.name;
tidy_error!(bad, "{filename}:{ll}: message `{name}` ends in a period");
}
if let Some(pat) = &m.value
&& 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 name = m.id.name;
tidy_error!(bad, "{filename}:{ll}: message `{name}` ends in a period");
}
}

Expand All @@ -50,12 +50,13 @@ fn check_period(filename: &str, contents: &str, bad: &mut bool) {
continue;
}

if let Some(PatternElement::TextElement { value }) = attr.value.elements.last() {
if value.ends_with(".") && !value.ends_with("...") {
let ll = find_line(contents, value);
let name = attr.id.name;
tidy_error!(bad, "{filename}:{ll}: attr `{name}` ends in a period");
}
if let Some(PatternElement::TextElement { value }) = attr.value.elements.last()
&& value.ends_with(".")
&& !value.ends_with("...")
{
let ll = find_line(contents, value);
let name = attr.id.name;
tidy_error!(bad, "{filename}:{ll}: attr `{name}` ends in a period");
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/fluent_used.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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.
// so we check messages appear only once in all Rust and Fluent files.
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()) {
Expand Down
7 changes: 4 additions & 3 deletions src/tools/tidy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ use termcolor::WriteColor;

macro_rules! static_regex {
($re:literal) => {{
static RE: ::std::sync::OnceLock<::regex::Regex> = ::std::sync::OnceLock::new();
RE.get_or_init(|| ::regex::Regex::new($re).unwrap())
static RE: ::std::sync::LazyLock<::regex::Regex> =
::std::sync::LazyLock::new(|| ::regex::Regex::new($re).unwrap());
&*RE
}};
}

Expand Down Expand Up @@ -134,7 +135,7 @@ pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool {
eprintln!("No base commit, assuming all files are modified");
return true;
};
match crate::git_diff(&base_commit, "--name-status") {
match crate::git_diff(base_commit, "--name-status") {
Some(output) => {
let modified_files = output.lines().filter_map(|ln| {
let (status, name) = ln
Expand Down
31 changes: 15 additions & 16 deletions src/tools/tidy/src/mir_opt_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,22 +55,21 @@ fn check_dash_files(path: &Path, bless: bool, bad: &mut bool) {
.filter(|e| e.file_type().is_file())
{
let path = file.path();
if path.extension() == Some("rs".as_ref()) {
if let Some(name) = path.file_name().and_then(|s| s.to_str()) {
if name.contains('-') {
if !bless {
tidy_error!(
bad,
"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();
new_path.set_file_name(new_name);
let _ = std::fs::rename(path, new_path);
}
}
if path.extension() == Some("rs".as_ref())
&& let Some(name) = path.file_name().and_then(|s| s.to_str())
&& name.contains('-')
{
if !bless {
tidy_error!(
bad,
"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();
new_path.set_file_name(new_name);
let _ = std::fs::rename(path, new_path);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/tools/tidy/src/rustdoc_templates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ pub fn check(librustdoc_path: &Path, bad: &mut bool) {
None
// Then we check if this a comment tag.
} else if *tag != "{#" {
return Some(false);
Some(false)
// And finally we check if the comment is empty (ie, only there to strip
// extra whitespace characters).
} else if let Some(start_pos) = line.rfind(tag) {
Expand Down
46 changes: 23 additions & 23 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,10 +417,10 @@ pub fn check(path: &Path, bad: &mut bool) {
return;
}
// Shell completions are automatically generated
if let Some(p) = file.parent() {
if p.ends_with(Path::new("src/etc/completions")) {
return;
}
if let Some(p) = file.parent()
&& p.ends_with(Path::new("src/etc/completions"))
{
return;
}
let [
mut skip_cr,
Expand Down Expand Up @@ -604,25 +604,25 @@ pub fn check(path: &Path, bad: &mut bool) {
backtick_count += comment_text.chars().filter(|ch| *ch == '`').count();
}
comment_block = Some((start_line, backtick_count));
} else if let Some((start_line, backtick_count)) = comment_block.take() {
if backtick_count % 2 == 1 {
let mut err = |msg: &str| {
tidy_error!(bad, "{}:{start_line}: {msg}", file.display());
};
let block_len = (i + 1) - start_line;
if block_len == 1 {
suppressible_tidy_err!(
err,
skip_odd_backticks,
"comment with odd number of backticks"
);
} else {
suppressible_tidy_err!(
err,
skip_odd_backticks,
"{block_len}-line comment block with odd number of backticks"
);
}
} else if let Some((start_line, backtick_count)) = comment_block.take()
&& backtick_count % 2 == 1
{
let mut err = |msg: &str| {
tidy_error!(bad, "{}:{start_line}: {msg}", file.display());
};
let block_len = (i + 1) - start_line;
if block_len == 1 {
suppressible_tidy_err!(
err,
skip_odd_backticks,
"comment with odd number of backticks"
);
} else {
suppressible_tidy_err!(
err,
skip_odd_backticks,
"{block_len}-line comment block with odd number of backticks"
);
}
}
}
Expand Down
22 changes: 11 additions & 11 deletions src/tools/tidy/src/target_specific_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,17 @@ pub fn check(tests_path: &Path, bad: &mut bool) {
comp_vec.push(component);
}
}
} else if let Some(compile_flags) = directive.strip_prefix(COMPILE_FLAGS_HEADER) {
if let Some((_, v)) = compile_flags.split_once("--target") {
let v = v.trim_start_matches([' ', '=']);
let v = if v == "{{target}}" { Some((v, v)) } else { v.split_once("-") };
if let Some((arch, _)) = v {
let info = header_map.entry(revision).or_insert(RevisionInfo::default());
info.target_arch.replace(arch);
} else {
eprintln!("{file}: seems to have a malformed --target value");
*bad = true;
}
} else if let Some(compile_flags) = directive.strip_prefix(COMPILE_FLAGS_HEADER)
&& let Some((_, v)) = compile_flags.split_once("--target")
{
let v = v.trim_start_matches([' ', '=']);
let v = if v == "{{target}}" { Some((v, v)) } else { v.split_once("-") };
if let Some((arch, _)) = v {
let info = header_map.entry(revision).or_insert(RevisionInfo::default());
info.target_arch.replace(arch);
} else {
eprintln!("{file}: seems to have a malformed --target value");
*bad = true;
}
}
});
Expand Down
43 changes: 21 additions & 22 deletions src/tools/tidy/src/ui_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,31 +161,30 @@ pub fn check(root_path: &Path, bless: bool, bad: &mut bool) {
tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path);
}

if let Ok(metadata) = fs::metadata(file_path) {
if metadata.len() == 0 {
tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path);
}
if let Ok(metadata) = fs::metadata(file_path)
&& metadata.len() == 0
{
tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path);
}
}

if ext == "rs" {
if let Some(test_name) = static_regex!(r"^issues?[-_]?(\d{3,})").captures(testname)
{
// these paths are always relative to the passed `path` and always UTF8
let stripped_path = file_path
.strip_prefix(path)
.unwrap()
.to_str()
.unwrap()
.replace(std::path::MAIN_SEPARATOR_STR, "/");

if !remaining_issue_names.remove(stripped_path.as_str()) {
tidy_error!(
bad,
"file `tests/{stripped_path}` must begin with a descriptive name, consider `{{reason}}-issue-{issue_n}.rs`",
issue_n = &test_name[1],
);
}
if ext == "rs"
&& let Some(test_name) = static_regex!(r"^issues?[-_]?(\d{3,})").captures(testname)
{
// these paths are always relative to the passed `path` and always UTF8
let stripped_path = file_path
.strip_prefix(path)
.unwrap()
.to_str()
.unwrap()
.replace(std::path::MAIN_SEPARATOR_STR, "/");

if !remaining_issue_names.remove(stripped_path.as_str()) {
tidy_error!(
bad,
"file `tests/{stripped_path}` must begin with a descriptive name, consider `{{reason}}-issue-{issue_n}.rs`",
issue_n = &test_name[1],
);
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/tools/tidy/src/x_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ pub fn check(root: &Path, cargo: &Path, bad: &mut bool) {
if let Some(version) = iter.next() {
// Check this is the rust-lang/rust x tool installation since it should be
// installed at a path containing `src/tools/x`.
if let Some(path) = iter.next() {
if path.contains("src/tools/x") {
let version = version.strip_prefix("v").unwrap();
installed = Some(Version::parse(version).unwrap());
break;
}
if let Some(path) = iter.next()
&& path.contains("src/tools/x")
{
let version = version.strip_prefix("v").unwrap();
installed = Some(Version::parse(version).unwrap());
break;
};
}
} else {
Expand Down
Loading