fix clippy warnings on bootstrap

Signed-off-by: onur-ozkan <[email protected]>

src/bootstrap/src/bin/main.rs

@@ -144,7 +144,7 @@ fn check_version(config: &Config) -> Option<String> {
         // more than once.
         if let Ok(t) = fs::read_to_string(&warned_id_path) {
             let last_warned_id =
-                usize::from_str(&t).expect(&format!("{} is corrupted.", warned_id_path.display()));
+                usize::from_str(&t).unwrap_or_else(|_| panic!("{} is corrupted.", warned_id_path.display()));
 
             // We only use the last_warned_id if it exists in `CONFIG_CHANGE_HISTORY`.
             // Otherwise, we may retrieve all the changes if it's not the highest value.

src/bootstrap/src/core/build_steps/clean.rs

@@ -181,39 +181,33 @@ fn rm_rf(path: &Path) {
         }
         Ok(metadata) => {
             if metadata.file_type().is_file() || metadata.file_type().is_symlink() {
-                do_op(path, "remove file", |p| {
-                    fs::remove_file(p).or_else(|e| {
-                        // Work around the fact that we cannot
-                        // delete an executable while it runs on Windows.
-                        #[cfg(windows)]
+                do_op(path, "remove file", |p| match fs::remove_file(p) {
+                    #[cfg(windows)]
+                    Err(e)
                         if e.kind() == std::io::ErrorKind::PermissionDenied
                             && p.file_name().and_then(std::ffi::OsStr::to_str)
-                                == Some("bootstrap.exe")
-                        {
-                            eprintln!("WARNING: failed to delete '{}'.", p.display());
-                            return Ok(());
-                        }
-                        Err(e)
-                    })
+                                == Some("bootstrap.exe") =>
+                    {
+                        eprintln!("WARNING: failed to delete '{}'.", p.display());
+                        Ok(())
+                    }
+                    r => r,
                 });
+
                 return;
             }
 
             for file in t!(fs::read_dir(path)) {
                 rm_rf(&t!(file).path());
             }
-            do_op(path, "remove dir", |p| {
-                fs::remove_dir(p).or_else(|e| {
-                    // Check for dir not empty on Windows
-                    // FIXME: Once `ErrorKind::DirectoryNotEmpty` is stabilized,
-                    // match on `e.kind()` instead.
-                    #[cfg(windows)]
-                    if e.raw_os_error() == Some(145) {
-                        return Ok(());
-                    }
 
-                    Err(e)
-                })
+            do_op(path, "remove dir", |p| match fs::remove_dir(p) {
+                // Check for dir not empty on Windows
+                // FIXME: Once `ErrorKind::DirectoryNotEmpty` is stabilized,
+                // match on `e.kind()` instead.
+                #[cfg(windows)]
+                Err(e) if e.raw_os_error() == Some(145) => Ok(()),
+                r => r,
             });
         }
     };
@@ -228,14 +222,14 @@ where
         // On windows we can't remove a readonly file, and git will often clone files as readonly.
         // As a result, we have some special logic to remove readonly files on windows.
         // This is also the reason that we can't use things like fs::remove_dir_all().
-        Err(ref e) if cfg!(windows) && e.kind() == ErrorKind::PermissionDenied => {
+        #[cfg(windows)]
+        Err(ref e) if e.kind() == ErrorKind::PermissionDenied => {
             let m = t!(path.symlink_metadata());
             let mut p = m.permissions();
             p.set_readonly(false);
             t!(fs::set_permissions(path, p));
             f(path).unwrap_or_else(|e| {
                 // Delete symlinked directories on Windows
-                #[cfg(windows)]
                 if m.file_type().is_symlink() && path.is_dir() && fs::remove_dir(path).is_ok() {
                     return;
                 }

src/bootstrap/src/core/build_steps/compile.rs

@@ -1146,7 +1146,7 @@ pub fn rustc_cargo_env(
     // busting caches (e.g. like #71152).
     if builder.config.llvm_enabled(target) {
         let building_is_expensive =
-            crate::core::build_steps::llvm::prebuilt_llvm_config(builder, target).is_err();
+            crate::core::build_steps::llvm::prebuilt_llvm_config(builder, target).should_build();
         // `top_stage == stage` might be false for `check --stage 1`, if we are building the stage 1 compiler
         let can_skip_build = builder.kind == Kind::Check && builder.top_stage == stage;
         let should_skip_build = building_is_expensive && can_skip_build;
@@ -1293,7 +1293,7 @@ fn is_codegen_cfg_needed(path: &TaskPath, run: &RunConfig<'_>) -> bool {
                 .path
                 .to_str()
                 .unwrap()
-                .ends_with(&(CODEGEN_BACKEND_PREFIX.to_owned() + &backend))
+                .ends_with(&(CODEGEN_BACKEND_PREFIX.to_owned() + backend))
             {
                 needs_codegen_backend_config = false;
             }
@@ -1853,7 +1853,7 @@ impl Step for Assemble {
                 extra_features: vec![],
             });
             let tool_exe = exe("llvm-bitcode-linker", target_compiler.host);
-            builder.copy_link(&src_path, &libdir_bin.join(&tool_exe));
+            builder.copy_link(&src_path, &libdir_bin.join(tool_exe));
         }
 
         // Ensure that `libLLVM.so` ends up in the newly build compiler directory,

src/bootstrap/src/core/build_steps/dist.rs

@@ -107,7 +107,7 @@ impl Step for JsonDocs {
             builder.top_stage,
             host,
             builder,
-            DocumentationFormat::JSON,
+            DocumentationFormat::Json,
         ));
 
         let dest = "share/doc/rust/json";
@@ -1131,7 +1131,7 @@ impl Step for Rls {
         let rls = builder.ensure(tool::Rls { compiler, target, extra_features: Vec::new() });
 
         let mut tarball = Tarball::new(builder, "rls", &target.triple);
-        tarball.set_overlay(OverlayKind::RLS);
+        tarball.set_overlay(OverlayKind::Rls);
         tarball.is_preview(true);
         tarball.add_file(rls, "bin", 0o755);
         tarball.add_legal_and_readme_to("share/doc/rls");
@@ -2059,7 +2059,7 @@ fn install_llvm_file(
         if install_symlink {
             // For download-ci-llvm, also install the symlink, to match what LLVM does. Using a
             // symlink is fine here, as this is not a rustup component.
-            builder.copy_link(&source, &full_dest);
+            builder.copy_link(source, &full_dest);
         } else {
             // Otherwise, replace the symlink with an equivalent linker script. This is used when
             // projects like miri link against librustc_driver.so. We don't use a symlink, as
@@ -2076,7 +2076,7 @@ fn install_llvm_file(
             }
         }
     } else {
-        builder.install(&source, destination, 0o644);
+        builder.install(source, destination, 0o644);
     }
 }
 
@@ -2121,7 +2121,7 @@ fn maybe_install_llvm(
             builder.install(&llvm_dylib_path, dst_libdir, 0o644);
         }
         !builder.config.dry_run()
-    } else if let Ok(llvm::LlvmResult { llvm_config, .. }) =
+    } else if let llvm::LlvmBuildStatus::AlreadyBuilt(llvm::LlvmResult { llvm_config, .. }) =
         llvm::prebuilt_llvm_config(builder, target)
     {
         let mut cmd = Command::new(llvm_config);
@@ -2202,7 +2202,7 @@ impl Step for LlvmTools {
         builder.ensure(crate::core::build_steps::llvm::Llvm { target });
 
         let mut tarball = Tarball::new(builder, "llvm-tools", &target.triple);
-        tarball.set_overlay(OverlayKind::LLVM);
+        tarball.set_overlay(OverlayKind::Llvm);
         tarball.is_preview(true);
 
         if builder.config.llvm_tools_enabled {
@@ -2305,7 +2305,7 @@ impl Step for RustDev {
         }
 
         let mut tarball = Tarball::new(builder, "rust-dev", &target.triple);
-        tarball.set_overlay(OverlayKind::LLVM);
+        tarball.set_overlay(OverlayKind::Llvm);
         // LLVM requires a shared object symlink to exist on some platforms.
         tarball.permit_symlinks(true);
 

src/bootstrap/src/core/build_steps/doc.rs

@@ -567,9 +567,9 @@ impl Step for Std {
             stage: run.builder.top_stage,
             target: run.target,
             format: if run.builder.config.cmd.json() {
-                DocumentationFormat::JSON
+                DocumentationFormat::Json
             } else {
-                DocumentationFormat::HTML
+                DocumentationFormat::Html
             },
             crates: run.make_run_crates(Alias::Library),
         });
@@ -583,13 +583,13 @@ impl Step for Std {
         let stage = self.stage;
         let target = self.target;
         let out = match self.format {
-            DocumentationFormat::HTML => builder.doc_out(target),
-            DocumentationFormat::JSON => builder.json_doc_out(target),
+            DocumentationFormat::Html => builder.doc_out(target),
+            DocumentationFormat::Json => builder.json_doc_out(target),
         };
 
         t!(fs::create_dir_all(&out));
 
-        if self.format == DocumentationFormat::HTML {
+        if self.format == DocumentationFormat::Html {
             builder.ensure(SharedAssets { target: self.target });
         }
 
@@ -600,10 +600,10 @@ impl Step for Std {
             .into_string()
             .expect("non-utf8 paths are unsupported");
         let mut extra_args = match self.format {
-            DocumentationFormat::HTML => {
+            DocumentationFormat::Html => {
                 vec!["--markdown-css", "rust.css", "--markdown-no-toc", "--index-page", &index_page]
             }
-            DocumentationFormat::JSON => vec!["--output-format", "json"],
+            DocumentationFormat::Json => vec!["--output-format", "json"],
         };
 
         if !builder.config.docs_minification {
@@ -613,7 +613,7 @@ impl Step for Std {
         doc_std(builder, self.format, stage, target, &out, &extra_args, &self.crates);
 
         // Don't open if the format is json
-        if let DocumentationFormat::JSON = self.format {
+        if let DocumentationFormat::Json = self.format {
             return;
         }
 
@@ -646,15 +646,15 @@ const STD_PUBLIC_CRATES: [&str; 5] = ["core", "alloc", "std", "proc_macro", "tes
 
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
 pub enum DocumentationFormat {
-    HTML,
-    JSON,
+    Html,
+    Json,
 }
 
 impl DocumentationFormat {
     fn as_str(&self) -> &str {
         match self {
-            DocumentationFormat::HTML => "HTML",
-            DocumentationFormat::JSON => "JSON",
+            DocumentationFormat::Html => "HTML",
+            DocumentationFormat::Json => "JSON",
         }
     }
 }
@@ -678,7 +678,7 @@ fn doc_std(
 
     let compiler = builder.compiler(stage, builder.config.build);
 
-    let target_doc_dir_name = if format == DocumentationFormat::JSON { "json-doc" } else { "doc" };
+    let target_doc_dir_name = if format == DocumentationFormat::Json { "json-doc" } else { "doc" };
     let target_dir =
         builder.stage_out(compiler, Mode::Std).join(target.triple).join(target_doc_dir_name);
 

src/bootstrap/src/core/build_steps/format.rs

@@ -225,12 +225,10 @@ pub fn format(build: &Builder<'_>, check: bool, paths: &[PathBuf]) {
         Some(first) => {
             let find_shortcut_candidates = |p: &PathBuf| {
                 let mut candidates = Vec::new();
-                for candidate in WalkBuilder::new(src.clone()).max_depth(Some(3)).build() {
-                    if let Ok(entry) = candidate {
-                        if let Some(dir_name) = p.file_name() {
-                            if entry.path().is_dir() && entry.file_name() == dir_name {
-                                candidates.push(entry.into_path());
-                            }
+                for entry in WalkBuilder::new(src.clone()).max_depth(Some(3)).build().map_while(Result::ok) {
+                    if let Some(dir_name) = p.file_name() {
+                        if entry.path().is_dir() && entry.file_name() == dir_name {
+                            candidates.push(entry.into_path());
                         }
                     }
                 }

src/bootstrap/src/core/build_steps/install.rs

@@ -130,7 +130,7 @@ fn prepare_dir(destdir_env: &Option<PathBuf>, mut path: PathBuf) -> String {
     // https://www.gnu.org/prep/standards/html_node/DESTDIR.html
     if let Some(destdir) = destdir_env {
         let without_destdir = path.clone();
-        path = destdir.clone();
+        path.clone_from(destdir);
         // Custom .join() which ignores disk roots.
         for part in without_destdir.components() {
             if let Component::Normal(s) = part {

src/bootstrap/src/core/build_steps/llvm.rs

@@ -42,6 +42,20 @@ pub struct Meta {
     root: String,
 }
 
+pub enum LlvmBuildStatus {
+    AlreadyBuilt(LlvmResult),
+    ShouldBuild(Meta),
+}
+
+impl LlvmBuildStatus {
+    pub fn should_build(&self) -> bool {
+        match self {
+            LlvmBuildStatus::AlreadyBuilt(_) => false,
+            LlvmBuildStatus::ShouldBuild(_) => true
+        }
+    }
+}
+
 // Linker flags to pass to LLVM's CMake invocation.
 #[derive(Debug, Clone, Default)]
 struct LdFlags {
@@ -75,7 +89,7 @@ impl LdFlags {
 pub fn prebuilt_llvm_config(
     builder: &Builder<'_>,
     target: TargetSelection,
-) -> Result<LlvmResult, Meta> {
+) -> LlvmBuildStatus {
     // If we have llvm submodule initialized already, sync it.
     builder.update_existing_submodule(&Path::new("src").join("llvm-project"));
 
@@ -93,7 +107,7 @@ pub fn prebuilt_llvm_config(
             llvm_cmake_dir.push("lib");
             llvm_cmake_dir.push("cmake");
             llvm_cmake_dir.push("llvm");
-            return Ok(LlvmResult { llvm_config, llvm_cmake_dir });
+            return LlvmBuildStatus::AlreadyBuilt(LlvmResult { llvm_config, llvm_cmake_dir });
         }
     }
 
@@ -131,10 +145,10 @@ pub fn prebuilt_llvm_config(
                 stamp.path.display()
             ));
         }
-        return Ok(res);
+        return LlvmBuildStatus::AlreadyBuilt(res);
     }
 
-    Err(Meta { stamp, res, out_dir, root: root.into() })
+    LlvmBuildStatus::ShouldBuild(Meta { stamp, res, out_dir, root: root.into() })
 }
 
 /// This retrieves the LLVM sha we *want* to use, according to git history.
@@ -282,8 +296,8 @@ impl Step for Llvm {
 
         // If LLVM has already been built or been downloaded through download-ci-llvm, we avoid building it again.
         let Meta { stamp, res, out_dir, root } = match prebuilt_llvm_config(builder, target) {
-            Ok(p) => return p,
-            Err(m) => m,
+            LlvmBuildStatus::AlreadyBuilt(p) => return p,
+            LlvmBuildStatus::ShouldBuild(m) => m,
         };
 
         if builder.llvm_link_shared() && target.is_windows() {

src/bootstrap/src/core/build_steps/setup.rs

@@ -93,10 +93,10 @@ impl FromStr for Profile {
                 Ok(Profile::Tools)
             }
             "none" => Ok(Profile::None),
-            "llvm" | "codegen" => Err(format!(
+            "llvm" | "codegen" => Err(
                 "the \"llvm\" and \"codegen\" profiles have been removed,\
-                use \"compiler\" instead which has the same functionality"
-            )),
+                use \"compiler\" instead which has the same functionality".to_string()
+            ),
             _ => Err(format!("unknown profile: '{s}'")),
         }
     }
@@ -474,10 +474,10 @@ impl Step for Hook {
 
 // install a git hook to automatically run tidy, if they want
 fn install_git_hook_maybe(config: &Config) -> io::Result<()> {
-    let git = t!(config.git().args(["rev-parse", "--git-common-dir"]).output().map(|output| {
+    let git = config.git().args(["rev-parse", "--git-common-dir"]).output().map(|output| {
         assert!(output.status.success(), "failed to run `git`");
         PathBuf::from(t!(String::from_utf8(output.stdout)).trim())
-    }));
+    })?;
     let hooks_dir = git.join("hooks");
     let dst = hooks_dir.join("pre-push");
     if dst.exists() {

src/bootstrap/src/core/build_steps/test.rs

@@ -923,7 +923,7 @@ impl Step for RustdocJSStd {
             builder.top_stage,
             self.target,
             builder,
-            DocumentationFormat::HTML,
+            DocumentationFormat::Html,
         ));
         let _guard = builder.msg(
             Kind::Test,
@@ -1389,10 +1389,10 @@ impl Step for RunMakeSupport {
         builder.run(&mut cargo);
 
         let lib_name = "librun_make_support.rlib";
-        let lib = builder.tools_dir(self.compiler).join(&lib_name);
+        let lib = builder.tools_dir(self.compiler).join(lib_name);
 
         let cargo_out =
-            builder.cargo_out(self.compiler, Mode::ToolStd, self.target).join(&lib_name);
+            builder.cargo_out(self.compiler, Mode::ToolStd, self.target).join(lib_name);
         builder.copy_link(&cargo_out, &lib);
         lib
     }
@@ -2483,6 +2483,7 @@ impl Step for CrateLibrustc {
 /// Given a `cargo test` subcommand, add the appropriate flags and run it.
 ///
 /// Returns whether the test succeeded.
+#[allow(clippy::too_many_arguments)] // FIXME: reduce the number of args and remove this.
 fn run_cargo_test<'a>(
     cargo: impl Into<Command>,
     libtest_args: &[&str],