Skip to content

Commit

Permalink
tidy: apply considerable clippy suggestions
Browse files Browse the repository at this point in the history
Signed-off-by: onur-ozkan <work@onurozkan.dev>
  • Loading branch information
onur-ozkan committed Jun 6, 2024
1 parent 749b2f4 commit 158e1bb
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 58 deletions.
2 changes: 1 addition & 1 deletion src/tools/tidy/src/alphabetical.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ fn check_section<'a>(
let trimmed_line = line.trim_start_matches(' ');

if trimmed_line.starts_with("//")
|| (trimmed_line.starts_with("#") && !trimmed_line.starts_with("#!"))
|| (trimmed_line.starts_with('#') && !trimmed_line.starts_with("#!"))
|| trimmed_line.starts_with(is_close_bracket)
{
continue;
Expand Down
6 changes: 3 additions & 3 deletions src/tools/tidy/src/bins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ mod os_impl {
fs::remove_file(&path).expect("Deleted temp file");
// If the file is executable, then we assume that this
// filesystem does not track executability, so skip this check.
return if exec { Unsupported } else { Supported };
if exec { Unsupported } else { Supported }
}
Err(e) => {
// If the directory is read-only or we otherwise don't have rights,
Expand All @@ -76,7 +76,7 @@ mod os_impl {

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

for &source_dir in sources {
Expand All @@ -92,7 +92,7 @@ mod os_impl {
}
}

return true;
true
}

// FIXME: check when rust-installer test sh files will be removed,
Expand Down
8 changes: 3 additions & 5 deletions src/tools/tidy/src/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -692,11 +692,9 @@ fn check_permitted_dependencies(
for dep in deps {
let dep = pkg_from_id(metadata, dep);
// If this path is in-tree, we don't require it to be explicitly permitted.
if dep.source.is_some() {
if !permitted_dependencies.contains(dep.name.as_str()) {
tidy_error!(bad, "Dependency for {descr} not explicitly permitted: {}", dep.id);
has_permitted_dep_error = true;
}
if dep.source.is_some() && !permitted_dependencies.contains(dep.name.as_str()) {
tidy_error!(bad, "Dependency for {descr} not explicitly permitted: {}", dep.id);
has_permitted_dep_error = true;
}
}

Expand Down
8 changes: 3 additions & 5 deletions src/tools/tidy/src/error_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,9 @@ fn check_error_codes_tests(
for line in file.lines() {
let s = line.trim();
// Assuming the line starts with `error[E`, we can substring the error code out.
if s.starts_with("error[E") {
if &s[6..11] == code {
found_code = true;
break;
}
if s.starts_with("error[E") && &s[6..11] == code {
found_code = true;
break;
};
}

Expand Down
9 changes: 5 additions & 4 deletions src/tools/tidy/src/ext_tool_checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ fn check_impl(
let mut py_path = None;

let (cfg_args, file_args): (Vec<_>, Vec<_>) = pos_args
.into_iter()
.iter()
.map(OsStr::new)
.partition(|arg| arg.to_str().is_some_and(|s| s.starts_with("-")));
.partition(|arg| arg.to_str().is_some_and(|s| s.starts_with('-')));

if python_lint || python_fmt {
let venv_path = outdir.join("venv");
Expand Down Expand Up @@ -275,10 +275,11 @@ fn create_venv_at_path(path: &Path) -> Result<(), Error> {
return Ok(());
}
let err = if String::from_utf8_lossy(&out.stderr).contains("No module named virtualenv") {
Error::Generic(format!(
Error::Generic(
"virtualenv not found: you may need to install it \
(`python3 -m pip install venv`)"
))
.to_owned(),
)
} else {
Error::Generic(format!("failed to create venv at '{}' using {sys_py}", path.display()))
};
Expand Down
57 changes: 29 additions & 28 deletions src/tools/tidy/src/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,10 +461,13 @@ pub fn check(path: &Path, bad: &mut bool) {
}
}
// for now we just check libcore
if trimmed.contains("unsafe {") && !trimmed.starts_with("//") && !last_safety_comment {
if file.components().any(|c| c.as_os_str() == "core") && !is_test {
suppressible_tidy_err!(err, skip_undocumented_unsafe, "undocumented unsafe");
}
if trimmed.contains("unsafe {")
&& !trimmed.starts_with("//")
&& !last_safety_comment
&& file.components().any(|c| c.as_os_str() == "core")
&& !is_test
{
suppressible_tidy_err!(err, skip_undocumented_unsafe, "undocumented unsafe");
}
if trimmed.contains("// SAFETY:") {
last_safety_comment = true;
Expand All @@ -485,10 +488,10 @@ pub fn check(path: &Path, bad: &mut bool) {
"copyright notices attributed to the Rust Project Developers are deprecated"
);
}
if !file.components().any(|c| c.as_os_str() == "rustc_baked_icu_data") {
if is_unexplained_ignore(&extension, line) {
err(UNEXPLAINED_IGNORE_DOCTEST_INFO);
}
if !file.components().any(|c| c.as_os_str() == "rustc_baked_icu_data")
&& is_unexplained_ignore(&extension, line)
{
err(UNEXPLAINED_IGNORE_DOCTEST_INFO);
}

if filename.ends_with(".cpp") && line.contains("llvm_unreachable") {
Expand Down Expand Up @@ -523,26 +526,24 @@ 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() {
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"
);
}
}
}
Expand Down
18 changes: 7 additions & 11 deletions src/tools/tidy/src/walk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,11 @@ pub(crate) fn walk_no_read(
let walker = walker.filter_entry(move |e| {
!skip(e.path(), e.file_type().map(|ft| ft.is_dir()).unwrap_or(false))
});
for entry in walker.build() {
if let Ok(entry) = entry {
if entry.file_type().map_or(true, |kind| kind.is_dir() || kind.is_symlink()) {
continue;
}
f(&entry);
for entry in walker.build().flatten() {
if entry.file_type().map_or(true, |kind| kind.is_dir() || kind.is_symlink()) {
continue;
}
f(&entry);
}
}

Expand All @@ -96,11 +94,9 @@ pub(crate) fn walk_dir(
) {
let mut walker = ignore::WalkBuilder::new(path);
let walker = walker.filter_entry(move |e| !skip(e.path()));
for entry in walker.build() {
if let Ok(entry) = entry {
if entry.path().is_dir() {
f(&entry);
}
for entry in walker.build().flatten() {
if entry.path().is_dir() {
f(&entry);
}
}
}
2 changes: 1 addition & 1 deletion src/tools/tidy/src/x_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub fn check(root: &Path, cargo: &Path, bad: &mut bool) {
);
}
} else {
return tidy_error!(bad, "failed to check version of `x`: {}", cargo_list.status);
tidy_error!(bad, "failed to check version of `x`: {}", cargo_list.status)
}
}

Expand Down

0 comments on commit 158e1bb

Please sign in to comment.