Rollup merge of rust-lang#126088 - onur-ozkan:brooming, r=albertlarsan68

[1/2] clean-up / general improvements

This PR applies various clippy suggestions on the tools. I have only applied the ones that make sense and left out trivial changes (e.g., suggestions like 'remove &' are ignored to keep the original commit history for the lines).

I am planning to do the same for the library and compiler, but those will add too many changes to this PR, so I will handle them in a separate PR later.

Author: matthiaskrgr

Diff for: src/bootstrap/src/core/build_steps/format.rs

@@ -93,7 +93,7 @@ fn get_modified_rs_files(build: &Builder<'_>) -> Result<Option<Vec<String>>, Str
         return Ok(None);
     }
 
-    get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &vec!["rs"])
+    get_git_modified_files(&build.config.git_config(), Some(&build.config.src), &["rs"])
 }
 
 #[derive(serde_derive::Deserialize)]

Diff for: src/tools/build-manifest/src/main.rs

@@ -495,7 +495,7 @@ impl Builder {
                 Some(p) => p,
                 None => return false,
             };
-            pkg.target.get(&c.target).is_some()
+            pkg.target.contains_key(&c.target)
         };
         extensions.retain(&has_component);
         components.retain(&has_component);

Diff for: src/tools/build_helper/src/git.rs

@@ -21,7 +21,7 @@ fn output_result(cmd: &mut Command) -> Result<String, String> {
             String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
         ));
     }
-    Ok(String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))?)
+    String::from_utf8(output.stdout).map_err(|err| format!("{err:?}"))
 }
 
 /// Finds the remote for rust-lang/rust.
@@ -64,18 +64,14 @@ pub fn rev_exists(rev: &str, git_dir: Option<&Path>) -> Result<bool, String> {
     match output.status.code() {
         Some(0) => Ok(true),
         Some(128) => Ok(false),
-        None => {
-            return Err(format!(
-                "git didn't exit properly: {}",
-                String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
-            ));
-        }
-        Some(code) => {
-            return Err(format!(
-                "git command exited with status code: {code}: {}",
-                String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
-            ));
-        }
+        None => Err(format!(
+            "git didn't exit properly: {}",
+            String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
+        )),
+        Some(code) => Err(format!(
+            "git command exited with status code: {code}: {}",
+            String::from_utf8(output.stderr).map_err(|err| format!("{err:?}"))?
+        )),
     }
 }
 
@@ -96,7 +92,7 @@ pub fn updated_master_branch(
         }
     }
 
-    Err(format!("Cannot find any suitable upstream master branch"))
+    Err("Cannot find any suitable upstream master branch".to_owned())
 }
 
 pub fn get_git_merge_base(
@@ -118,7 +114,7 @@ pub fn get_git_merge_base(
 pub fn get_git_modified_files(
     config: &GitConfig<'_>,
     git_dir: Option<&Path>,
-    extensions: &Vec<&str>,
+    extensions: &[&str],
 ) -> Result<Option<Vec<String>>, String> {
     let merge_base = get_git_merge_base(config, git_dir)?;
 

Diff for: src/tools/compiletest/src/common.rs

@@ -582,7 +582,7 @@ impl TargetCfgs {
                         name,
                         Some(
                             value
-                                .strip_suffix("\"")
+                                .strip_suffix('\"')
                                 .expect("key-value pair should be properly quoted"),
                         ),
                     )

Diff for: src/tools/compiletest/src/header.rs

@@ -82,7 +82,7 @@ impl EarlyProps {
             panic!("errors encountered during EarlyProps parsing");
         }
 
-        return props;
+        props
     }
 }
 
@@ -382,7 +382,7 @@ impl TestProps {
                         // Individual flags can be single-quoted to preserve spaces; see
                         // <https://github.com/rust-lang/rust/pull/115948/commits/957c5db6>.
                         flags
-                            .split("'")
+                            .split('\'')
                             .enumerate()
                             .flat_map(|(i, f)| {
                                 if i % 2 == 1 { vec![f] } else { f.split_whitespace().collect() }
@@ -613,7 +613,7 @@ impl TestProps {
 
         for key in &["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] {
             if let Ok(val) = env::var(key) {
-                if self.exec_env.iter().find(|&&(ref x, _)| x == key).is_none() {
+                if !self.exec_env.iter().any(|&(ref x, _)| x == key) {
                     self.exec_env.push(((*key).to_owned(), val))
                 }
             }
@@ -991,7 +991,7 @@ pub(crate) fn check_directive(directive_ln: &str) -> CheckDirectiveResult<'_> {
     let trailing = post.trim().split_once(' ').map(|(pre, _)| pre).unwrap_or(post);
     let trailing_directive = {
         // 1. is the directive name followed by a space? (to exclude `:`)
-        matches!(directive_ln.get(directive_name.len()..), Some(s) if s.starts_with(" "))
+        matches!(directive_ln.get(directive_name.len()..), Some(s) if s.starts_with(' '))
             // 2. is what is after that directive also a directive (ex: "only-x86 only-arm")
             && KNOWN_DIRECTIVE_NAMES.contains(&trailing)
     }
@@ -1363,7 +1363,7 @@ pub fn extract_llvm_version_from_binary(binary_path: &str) -> Option<u32> {
     }
     let version = String::from_utf8(output.stdout).ok()?;
     for line in version.lines() {
-        if let Some(version) = line.split("LLVM version ").skip(1).next() {
+        if let Some(version) = line.split("LLVM version ").nth(1) {
             return extract_llvm_version(version);
         }
     }
@@ -1394,7 +1394,7 @@ where
 
     let min = parse(min)?;
     let max = match max {
-        Some(max) if max.is_empty() => return None,
+        Some("") => return None,
         Some(max) => parse(max)?,
         _ => min,
     };
@@ -1466,12 +1466,12 @@ pub fn make_test_description<R: Read>(
             decision!(ignore_gdb(config, ln));
             decision!(ignore_lldb(config, ln));
 
-            if config.target == "wasm32-unknown-unknown" {
-                if config.parse_name_directive(ln, directives::CHECK_RUN_RESULTS) {
-                    decision!(IgnoreDecision::Ignore {
-                        reason: "ignored on WASM as the run results cannot be checked there".into(),
-                    });
-                }
+            if config.target == "wasm32-unknown-unknown"
+                && config.parse_name_directive(ln, directives::CHECK_RUN_RESULTS)
+            {
+                decision!(IgnoreDecision::Ignore {
+                    reason: "ignored on WASM as the run results cannot be checked there".into(),
+                });
             }
 
             should_fail |= config.parse_name_directive(ln, "should-fail");

Diff for: src/tools/compiletest/src/header/cfg.rs

@@ -58,7 +58,7 @@ pub(super) fn parse_cfg_name_directive<'a>(
 
     // Some of the matchers might be "" depending on what the target information is. To avoid
     // problems we outright reject empty directives.
-    if name == "" {
+    if name.is_empty() {
         return ParsedNameDirective::not_a_directive();
     }
 

Diff for: src/tools/compiletest/src/lib.rs

@@ -1147,7 +1147,7 @@ fn extract_lldb_version(full_version_line: &str) -> Option<(u32, bool)> {
 }
 
 fn not_a_digit(c: char) -> bool {
-    !c.is_digit(10)
+    !c.is_ascii_digit()
 }
 
 fn check_overlapping_tests(found_paths: &HashSet<PathBuf>) {

Diff for: src/tools/compiletest/src/read2.rs

@@ -6,7 +6,6 @@ mod tests;
 
 pub use self::imp::read2;
 use std::io::{self, Write};
-use std::mem::replace;
 use std::process::{Child, Output};
 
 #[derive(Copy, Clone, Debug)]
@@ -101,10 +100,10 @@ impl ProcOutput {
                     return;
                 }
 
-                let mut head = replace(bytes, Vec::new());
+                let mut head = std::mem::take(bytes);
                 // Don't truncate if this as a whole line.
                 // That should make it less likely that we cut a JSON line in half.
-                if head.last() != Some(&('\n' as u8)) {
+                if head.last() != Some(&b'\n') {
                     head.truncate(MAX_OUT_LEN);
                 }
                 let skipped = new_len - head.len();

Diff for: src/tools/compiletest/src/read2/tests.rs

@@ -64,9 +64,9 @@ fn test_abbreviate_filterss_are_detected() {
 #[test]
 fn test_abbreviate_filters_avoid_abbreviations() {
     let mut out = ProcOutput::new();
-    let filters = &[std::iter::repeat('a').take(64).collect::<String>()];
+    let filters = &["a".repeat(64)];
 
-    let mut expected = vec![b'.'; MAX_OUT_LEN - FILTERED_PATHS_PLACEHOLDER_LEN as usize];
+    let mut expected = vec![b'.'; MAX_OUT_LEN - FILTERED_PATHS_PLACEHOLDER_LEN];
     expected.extend_from_slice(filters[0].as_bytes());
 
     out.extend(&expected, filters);
@@ -81,7 +81,7 @@ fn test_abbreviate_filters_avoid_abbreviations() {
 #[test]
 fn test_abbreviate_filters_can_still_cause_abbreviations() {
     let mut out = ProcOutput::new();
-    let filters = &[std::iter::repeat('a').take(64).collect::<String>()];
+    let filters = &["a".repeat(64)];
 
     let mut input = vec![b'.'; MAX_OUT_LEN];
     input.extend_from_slice(filters[0].as_bytes());

Diff for: src/tools/compiletest/src/runtest.rs

@@ -374,11 +374,11 @@ impl<'test> TestCx<'test> {
 
         // if a test does not crash, consider it an error
         if proc_res.status.success() || matches!(proc_res.status.code(), Some(1 | 0)) {
-            self.fatal(&format!(
+            self.fatal(
                 "test no longer crashes/triggers ICE! Please give it a mearningful name, \
             add a doc-comment to the start of the test explaining why it exists and \
-            move it to tests/ui or wherever you see fit."
-            ));
+            move it to tests/ui or wherever you see fit.",
+            );
         }
     }
 
@@ -697,10 +697,10 @@ impl<'test> TestCx<'test> {
             // since it is extensively used in the testsuite.
             check_cfg.push_str("cfg(FALSE");
             for revision in &self.props.revisions {
-                check_cfg.push_str(",");
-                check_cfg.push_str(&normalize_revision(&revision));
+                check_cfg.push(',');
+                check_cfg.push_str(&normalize_revision(revision));
             }
-            check_cfg.push_str(")");
+            check_cfg.push(')');
 
             cmd.args(&["--check-cfg", &check_cfg]);
         }
@@ -818,7 +818,7 @@ impl<'test> TestCx<'test> {
         // Append the other `cdb-command:`s
         for line in &dbg_cmds.commands {
             script_str.push_str(line);
-            script_str.push_str("\n");
+            script_str.push('\n');
         }
 
         script_str.push_str("qq\n"); // Quit the debugger (including remote debugger, if any)
@@ -1200,7 +1200,7 @@ impl<'test> TestCx<'test> {
         // Append the other commands
         for line in &dbg_cmds.commands {
             script_str.push_str(line);
-            script_str.push_str("\n");
+            script_str.push('\n');
         }
 
         // Finally, quit the debugger
@@ -1250,7 +1250,7 @@ impl<'test> TestCx<'test> {
         // Remove options that are either unwanted (-O) or may lead to duplicates due to RUSTFLAGS.
         let options_to_remove = ["-O".to_owned(), "-g".to_owned(), "--debuginfo".to_owned()];
 
-        options.iter().filter(|x| !options_to_remove.contains(x)).map(|x| x.clone()).collect()
+        options.iter().filter(|x| !options_to_remove.contains(x)).cloned().collect()
     }
 
     fn maybe_add_external_args(&self, cmd: &mut Command, args: &Vec<String>) {
@@ -2504,8 +2504,8 @@ impl<'test> TestCx<'test> {
             // This works with both `--emit asm` (as default output name for the assembly)
             // and `ptx-linker` because the latter can write output at requested location.
             let output_path = self.output_base_name().with_extension(extension);
-            let output_file = TargetLocation::ThisFile(output_path.clone());
-            output_file
+
+            TargetLocation::ThisFile(output_path.clone())
         }
     }
 
@@ -2752,7 +2752,7 @@ impl<'test> TestCx<'test> {
             for entry in walkdir::WalkDir::new(dir) {
                 let entry = entry.expect("failed to read file");
                 if entry.file_type().is_file()
-                    && entry.path().extension().and_then(|p| p.to_str()) == Some("html".into())
+                    && entry.path().extension().and_then(|p| p.to_str()) == Some("html")
                 {
                     let status =
                         Command::new("tidy").args(&tidy_args).arg(entry.path()).status().unwrap();
@@ -2783,8 +2783,7 @@ impl<'test> TestCx<'test> {
             &compare_dir,
             self.config.verbose,
             |file_type, extension| {
-                file_type.is_file()
-                    && (extension == Some("html".into()) || extension == Some("js".into()))
+                file_type.is_file() && (extension == Some("html") || extension == Some("js"))
             },
         ) {
             return;
@@ -2830,11 +2829,11 @@ impl<'test> TestCx<'test> {
                 }
                 match String::from_utf8(line.clone()) {
                     Ok(line) => {
-                        if line.starts_with("+") {
+                        if line.starts_with('+') {
                             write!(&mut out, "{}", line.green()).unwrap();
-                        } else if line.starts_with("-") {
+                        } else if line.starts_with('-') {
                             write!(&mut out, "{}", line.red()).unwrap();
-                        } else if line.starts_with("@") {
+                        } else if line.starts_with('@') {
                             write!(&mut out, "{}", line.blue()).unwrap();
                         } else {
                             out.write_all(line.as_bytes()).unwrap();
@@ -2907,7 +2906,7 @@ impl<'test> TestCx<'test> {
                     && line.ends_with(';')
                 {
                     if let Some(ref mut other_files) = other_files {
-                        other_files.push(line.rsplit("mod ").next().unwrap().replace(";", ""));
+                        other_files.push(line.rsplit("mod ").next().unwrap().replace(';', ""));
                     }
                     None
                 } else {
@@ -3139,7 +3138,7 @@ impl<'test> TestCx<'test> {
             let mut string = String::new();
             for cgu in cgus {
                 string.push_str(&cgu[..]);
-                string.push_str(" ");
+                string.push(' ');
             }
 
             string
@@ -3172,10 +3171,7 @@ impl<'test> TestCx<'test> {
         // CGUs joined with "--". This function splits such composite CGU names
         // and handles each component individually.
         fn remove_crate_disambiguators_from_set_of_cgu_names(cgus: &str) -> String {
-            cgus.split("--")
-                .map(|cgu| remove_crate_disambiguator_from_cgu(cgu))
-                .collect::<Vec<_>>()
-                .join("--")
+            cgus.split("--").map(remove_crate_disambiguator_from_cgu).collect::<Vec<_>>().join("--")
         }
     }
 
@@ -3357,7 +3353,7 @@ impl<'test> TestCx<'test> {
             //   endif
         }
 
-        if self.config.target.contains("msvc") && self.config.cc != "" {
+        if self.config.target.contains("msvc") && !self.config.cc.is_empty() {
             // We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe`
             // and that `lib.exe` lives next to it.
             let lib = Path::new(&self.config.cc).parent().unwrap().join("lib.exe");
@@ -3639,7 +3635,7 @@ impl<'test> TestCx<'test> {
             //   endif
         }
 
-        if self.config.target.contains("msvc") && self.config.cc != "" {
+        if self.config.target.contains("msvc") && !self.config.cc.is_empty() {
             // We need to pass a path to `lib.exe`, so assume that `cc` is `cl.exe`
             // and that `lib.exe` lives next to it.
             let lib = Path::new(&self.config.cc).parent().unwrap().join("lib.exe");
@@ -3830,7 +3826,7 @@ impl<'test> TestCx<'test> {
             && !self.props.dont_check_compiler_stderr
         {
             self.fatal_proc_rec(
-                &format!("compiler output got truncated, cannot compare with reference file"),
+                "compiler output got truncated, cannot compare with reference file",
                 &proc_res,
             );
         }
@@ -4011,8 +4007,8 @@ impl<'test> TestCx<'test> {
                     crate_name.to_str().expect("crate name implies file name must be valid UTF-8");
                 // replace `a.foo` -> `a__foo` for crate name purposes.
                 // replace `revision-name-with-dashes` -> `revision_name_with_underscore`
-                let crate_name = crate_name.replace(".", "__");
-                let crate_name = crate_name.replace("-", "_");
+                let crate_name = crate_name.replace('.', "__");
+                let crate_name = crate_name.replace('-', "_");
                 rustc.arg("--crate-name");
                 rustc.arg(crate_name);
             }
@@ -4060,7 +4056,7 @@ impl<'test> TestCx<'test> {
     fn check_mir_dump(&self, test_info: MiroptTest) {
         let test_dir = self.testpaths.file.parent().unwrap();
         let test_crate =
-            self.testpaths.file.file_stem().unwrap().to_str().unwrap().replace("-", "_");
+            self.testpaths.file.file_stem().unwrap().to_str().unwrap().replace('-', "_");
 
         let MiroptTest { run_filecheck, suffix, files, passes: _ } = test_info;
 

Diff for: src/tools/compiletest/src/runtest/debugger.rs

@@ -148,5 +148,5 @@ fn check_single_line(line: &str, check_line: &str) -> bool {
         rest = &rest[pos + current_fragment.len()..];
     }
 
-    if !can_end_anywhere && !rest.is_empty() { false } else { true }
+    can_end_anywhere || rest.is_empty()
 }