chore: update toolchain to 1.84.1. apply clippy fixes & rustfmt (#1026)

* chore: update to stable toolchain. apply clippy fixes & rustfmt * Bump MSRV * Try MSRV without the patch version * fix: pin toolchain to MSRV * trying again * fix dead code warning --------- Co-authored-by: Dan Sully <[email protected]>
topgrade-rs · Feb 3, 2025 · 224bb96 · 224bb96
1 parent 9a6fe8e
commit 224bb96
Show file tree

Hide file tree

Showing 22 changed files with 105 additions and 129 deletions.
diff --git a/Cargo.toml b/Cargo.toml
@@ -5,7 +5,7 @@ categories = ["os"]
 keywords = ["upgrade", "update"]
 license = "GPL-3.0"
 repository = "https://github.com/topgrade-rs/topgrade"
-rust-version = "1.76.0"
+rust-version = "1.84.1"
 version = "16.0.2"
 authors = ["Roey Darwish Dror <[email protected]>", "Thomas Schönauer <[email protected]>"]
 exclude = ["doc/screenshot.gif", "BREAKINGCHANGES_dev.md"]

diff --git a/rust-toolchain.toml b/rust-toolchain.toml
@@ -1,2 +1,2 @@
 [toolchain]
-channel = "1.76.0"
+channel = "1.84.1"
diff --git a/src/command.rs b/src/command.rs
@@ -45,13 +45,13 @@ impl TryFrom<&Output> for Utf8Output {
     type Error = eyre::Error;
 
     fn try_from(Output { status, stdout, stderr }: &Output) -> Result<Self, Self::Error> {
-        let stdout = String::from_utf8(stdout.to_vec()).map_err(|err| {
+        let stdout = String::from_utf8(stdout.clone()).map_err(|err| {
             eyre!(
                 "Stdout contained invalid UTF-8: {}",
                 String::from_utf8_lossy(err.as_bytes())
             )
         })?;
-        let stderr = String::from_utf8(stderr.to_vec()).map_err(|err| {
+        let stderr = String::from_utf8(stderr.clone()).map_err(|err| {
             eyre!(
                 "Stderr contained invalid UTF-8: {}",
                 String::from_utf8_lossy(err.as_bytes())
@@ -149,6 +149,7 @@ pub trait CommandExt {
     /// Like [`Command::spawn`], but gives a nice error message if the command fails to
     /// execute.
     #[track_caller]
+    #[allow(dead_code)]
     fn spawn_checked(&mut self) -> eyre::Result<Self::Child>;
 }
 

diff --git a/src/config.rs b/src/config.rs
@@ -561,7 +561,7 @@ impl ConfigFile {
         ];
 
         // Search for the main config file
-        for path in possible_config_paths.iter() {
+        for path in &possible_config_paths {
             if path.exists() {
                 debug!("Configuration at {}", path.display());
                 res.0.clone_from(path);
@@ -1475,8 +1475,7 @@ impl Config {
             .misc
             .as_ref()
             .and_then(|misc| misc.ignore_failures.as_ref())
-            .map(|v| v.contains(&step))
-            .unwrap_or(false)
+            .is_some_and(|v| v.contains(&step))
     }
 
     pub fn use_predefined_git_repos(&self) -> bool {
@@ -1694,40 +1693,40 @@ mod test {
 
     #[test]
     fn test_should_execute_remote_different_hostname() {
-        assert!(config().should_execute_remote(Ok("hostname".to_string()), "remote_hostname"))
+        assert!(config().should_execute_remote(Ok("hostname".to_string()), "remote_hostname"));
     }
 
     #[test]
     fn test_should_execute_remote_different_hostname_with_user() {
-        assert!(config().should_execute_remote(Ok("hostname".to_string()), "user@remote_hostname"))
+        assert!(config().should_execute_remote(Ok("hostname".to_string()), "user@remote_hostname"));
     }
 
     #[test]
     fn test_should_execute_remote_unknown_hostname() {
-        assert!(config().should_execute_remote(Err(eyre!("failed to get hostname")), "remote_hostname"))
+        assert!(config().should_execute_remote(Err(eyre!("failed to get hostname")), "remote_hostname"));
     }
 
     #[test]
     fn test_should_not_execute_remote_same_hostname() {
-        assert!(!config().should_execute_remote(Ok("hostname".to_string()), "hostname"))
+        assert!(!config().should_execute_remote(Ok("hostname".to_string()), "hostname"));
     }
 
     #[test]
     fn test_should_not_execute_remote_same_hostname_with_user() {
-        assert!(!config().should_execute_remote(Ok("hostname".to_string()), "user@hostname"))
+        assert!(!config().should_execute_remote(Ok("hostname".to_string()), "user@hostname"));
     }
 
     #[test]
     fn test_should_execute_remote_matching_limit() {
         let mut config = config();
         config.opt = CommandLineArgs::parse_from(["topgrade", "--remote-host-limit", "remote_hostname"]);
-        assert!(config.should_execute_remote(Ok("hostname".to_string()), "user@remote_hostname"))
+        assert!(config.should_execute_remote(Ok("hostname".to_string()), "user@remote_hostname"));
     }
 
     #[test]
     fn test_should_not_execute_remote_not_matching_limit() {
         let mut config = config();
         config.opt = CommandLineArgs::parse_from(["topgrade", "--remote-host-limit", "other_hostname"]);
-        assert!(!config.should_execute_remote(Ok("hostname".to_string()), "user@remote_hostname"))
+        assert!(!config.should_execute_remote(Ok("hostname".to_string()), "user@remote_hostname"));
     }
 }
diff --git a/src/ctrlc/interrupted.rs b/src/ctrlc/interrupted.rs
@@ -11,9 +11,9 @@ pub fn interrupted() -> bool {
 /// Clears the interrupted flag
 pub fn unset_interrupted() {
     debug_assert!(INTERRUPTED.load(Ordering::SeqCst));
-    INTERRUPTED.store(false, Ordering::SeqCst)
+    INTERRUPTED.store(false, Ordering::SeqCst);
 }
 
 pub fn set_interrupted() {
-    INTERRUPTED.store(true, Ordering::SeqCst)
+    INTERRUPTED.store(true, Ordering::SeqCst);
 }
diff --git a/src/ctrlc/unix.rs b/src/ctrlc/unix.rs
@@ -4,7 +4,7 @@ use nix::sys::signal::{sigaction, SaFlags, SigAction, SigHandler, SigSet, Signal
 
 /// Handle SIGINT. Set the interruption flag.
 extern "C" fn handle_sigint(_: i32) {
-    set_interrupted()
+    set_interrupted();
 }
 
 /// Set the necessary signal handlers.

diff --git a/src/executor.rs b/src/executor.rs
@@ -188,7 +188,7 @@ impl Executor {
     pub fn status_checked_with_codes(&mut self, codes: &[i32]) -> Result<()> {
         match self {
             Executor::Wet(c) => c.status_checked_with(|status| {
-                if status.success() || status.code().as_ref().map(|c| codes.contains(c)).unwrap_or(false) {
+                if status.success() || status.code().as_ref().is_some_and(|c| codes.contains(c)) {
                     Ok(())
                 } else {
                     Err(())

diff --git a/src/main.rs b/src/main.rs
@@ -25,7 +25,9 @@ use self::config::{CommandLineArgs, Config, Step};
 use self::error::StepFailed;
 #[cfg(all(windows, feature = "self-update"))]
 use self::error::Upgraded;
+#[allow(clippy::wildcard_imports)]
 use self::steps::{remote::*, *};
+#[allow(clippy::wildcard_imports)]
 use self::terminal::*;
 
 use self::utils::{hostname, install_color_eyre, install_tracing, update_tracing};
@@ -58,6 +60,7 @@ pub(crate) static WINDOWS_DIRS: Lazy<Windows> = Lazy::new(|| Windows::new().expe
 // Init and load the i18n files
 i18n!("locales", fallback = "en");
 
+#[allow(clippy::too_many_lines)]
 fn run() -> Result<()> {
     install_color_eyre()?;
     ctrlc::set_handler();
@@ -494,13 +497,13 @@ fn run() -> Result<()> {
         print_info(t!("\n(R)eboot\n(S)hell\n(Q)uit"));
         loop {
             match get_key() {
-                Ok(Key::Char('s')) | Ok(Key::Char('S')) => {
+                Ok(Key::Char('s' | 'S')) => {
                     run_shell().context("Failed to execute shell")?;
                 }
-                Ok(Key::Char('r')) | Ok(Key::Char('R')) => {
+                Ok(Key::Char('r' | 'R')) => {
                     reboot().context("Failed to reboot")?;
                 }
-                Ok(Key::Char('q')) | Ok(Key::Char('Q')) => (),
+                Ok(Key::Char('q' | 'Q')) => (),
                 _ => {
                     continue;
                 }
@@ -519,7 +522,7 @@ fn run() -> Result<()> {
                 t!("Topgrade finished successfully")
             },
             Some(Duration::from_secs(10)),
-        )
+        );
     }
 
     if failed {

diff --git a/src/self_update.rs b/src/self_update.rs
@@ -9,7 +9,7 @@ use rust_i18n::t;
 use self_update_crate::backends::github::Update;
 use self_update_crate::update::UpdateStatus;
 
-use super::terminal::*;
+use super::terminal::{print_info, print_separator};
 #[cfg(windows)]
 use crate::error::Upgraded;
 

diff --git a/src/steps/containers.rs b/src/steps/containers.rs
@@ -140,7 +140,7 @@ pub fn run_containers(ctx: &ExecutionContext) -> Result<()> {
         list_containers(&crt, ctx.config().containers_ignored_tags()).context("Failed to list Docker containers")?;
     debug!("Containers to inspect: {:?}", containers);
 
-    for container in containers.iter() {
+    for container in &containers {
         debug!("Pulling container '{}'", container);
         let args = vec![
             "pull",

diff --git a/src/steps/generic.rs b/src/steps/generic.rs
@@ -40,8 +40,7 @@ pub fn is_wsl() -> Result<bool> {
 
 pub fn run_cargo_update(ctx: &ExecutionContext) -> Result<()> {
     let cargo_dir = env::var_os("CARGO_HOME")
-        .map(PathBuf::from)
-        .unwrap_or_else(|| HOME_DIR.join(".cargo"))
+        .map_or_else(|| HOME_DIR.join(".cargo"), PathBuf::from)
         .require()?;
     require("cargo").or_else(|_| {
         require_option(
@@ -60,13 +59,11 @@ pub fn run_cargo_update(ctx: &ExecutionContext) -> Result<()> {
     let cargo_update = require("cargo-install-update")
         .ok()
         .or_else(|| cargo_dir.join("bin/cargo-install-update").if_exists());
-    let cargo_update = match cargo_update {
-        Some(e) => e,
-        None => {
-            let message = String::from("cargo-update isn't installed so Topgrade can't upgrade cargo packages.\nInstall cargo-update by running `cargo install cargo-update`");
-            print_warning(&message);
-            return Err(SkipStep(message).into());
-        }
+
+    let Some(cargo_update) = cargo_update else {
+        let message = String::from("cargo-update isn't installed so Topgrade can't upgrade cargo packages.\nInstall cargo-update by running `cargo install cargo-update`");
+        print_warning(&message);
+        return Err(SkipStep(message).into());
     };
 
     ctx.run_type()
@@ -78,14 +75,11 @@ pub fn run_cargo_update(ctx: &ExecutionContext) -> Result<()> {
         let cargo_cache = require("cargo-cache")
             .ok()
             .or_else(|| cargo_dir.join("bin/cargo-cache").if_exists());
-        match cargo_cache {
-            Some(e) => {
-                ctx.run_type().execute(e).args(["-a"]).status_checked()?;
-            }
-            None => {
-                let message = String::from("cargo-cache isn't installed so Topgrade can't cleanup cargo packages.\nInstall cargo-cache by running `cargo install cargo-cache`");
-                print_warning(message);
-            }
+        if let Some(e) = cargo_cache {
+            ctx.run_type().execute(e).args(["-a"]).status_checked()?;
+        } else {
+            let message = String::from("cargo-cache isn't installed so Topgrade can't cleanup cargo packages.\nInstall cargo-cache by running `cargo install cargo-cache`");
+            print_warning(message);
         }
     }
 
@@ -423,7 +417,7 @@ pub fn run_vscodium_extensions_update(ctx: &ExecutionContext) -> Result<()> {
         .lines()
         .next()
     {
-        Some(item) => Version::parse(item).map_err(|err| err.into()),
+        Some(item) => Version::parse(item).map_err(std::convert::Into::into),
         _ => return Err(SkipStep(String::from("Cannot find vscodium version")).into()),
     };
 
@@ -459,7 +453,7 @@ pub fn run_vscode_extensions_update(ctx: &ExecutionContext) -> Result<()> {
         .lines()
         .next()
     {
-        Some(item) => Version::parse(item).map_err(|err| err.into()),
+        Some(item) => Version::parse(item).map_err(std::convert::Into::into),
         _ => return Err(SkipStep(String::from("Cannot find vscode version")).into()),
     };
 
@@ -489,7 +483,7 @@ pub fn run_pipx_update(ctx: &ExecutionContext) -> Result<()> {
         .map(|s| s.stdout.trim().to_owned());
     let version = Version::parse(&version_str?);
     if matches!(version, Ok(version) if version >= Version::new(1, 4, 0)) {
-        command_args.push("--quiet")
+        command_args.push("--quiet");
     }
 
     ctx.run_type().execute(pipx).args(command_args).status_checked()
@@ -555,7 +549,7 @@ pub fn run_pip3_update(ctx: &ExecutionContext) -> Result<()> {
         (Ok(py), _) => py,
         (Err(_), Ok(py3)) => py3,
         (Err(py_err), Err(py3_err)) => {
-            return Err(SkipStep(format!("Skip due to following reasons: {} {}", py_err, py3_err)).into());
+            return Err(SkipStep(format!("Skip due to following reasons: {py_err} {py3_err}")).into());
         }
     };
 
@@ -904,7 +898,7 @@ pub fn run_dotnet_upgrade(ctx: &ExecutionContext) -> Result<()> {
             .execute(&dotnet)
             .args(["tool", "update", package_name, "--global"])
             .status_checked()
-            .with_context(|| format!("Failed to update .NET package {:?}", package_name))?;
+            .with_context(|| format!("Failed to update .NET package {package_name:?}"))?;
     }
 
     Ok(())

diff --git a/src/steps/git.rs b/src/steps/git.rs
@@ -105,7 +105,7 @@ pub fn run_git_pull(ctx: &ExecutionContext) -> Result<()> {
         print_warning(t!(
             "Path {pattern} did not contain any git repositories",
             pattern = pattern
-        ))
+        ));
     });
 
     if repos.is_repos_empty() {
@@ -207,10 +207,13 @@ impl RepoStep {
 
                 return output;
             }
-            Err(e) => match e.kind() {
-                io::ErrorKind::NotFound => debug!("{} does not exist", path.as_ref().display()),
-                _ => error!("Error looking for {}: {e}", path.as_ref().display(),),
-            },
+            Err(e) => {
+                if e.kind() == io::ErrorKind::NotFound {
+                    debug!("{} does not exist", path.as_ref().display());
+                } else {
+                    error!("Error looking for {}: {e}", path.as_ref().display());
+                }
+            }
         }
 
         None
@@ -321,7 +324,7 @@ impl RepoStep {
             .output()
             .await?;
         let result = output_checked_utf8(pull_output)
-            .and_then(|_| output_checked_utf8(submodule_output))
+            .and_then(|()| output_checked_utf8(submodule_output))
             .wrap_err_with(|| format!("Failed to pull {}", repo.as_ref().display()));
 
         if result.is_err() {
@@ -359,7 +362,7 @@ impl RepoStep {
             }
         }
 
-        result.map(|_| ())
+        result
     }
 
     /// Pull the repositories specified in `self.repos`.
@@ -410,7 +413,7 @@ impl RepoStep {
         let basic_rt = runtime::Runtime::new()?;
         let results = basic_rt.block_on(async { stream_of_futures.collect::<Vec<Result<()>>>().await });
 
-        let error = results.into_iter().find(|r| r.is_err());
+        let error = results.into_iter().find(std::result::Result::is_err);
         error.unwrap_or(Ok(()))
     }
 }
diff --git a/src/steps/node.rs b/src/steps/node.rs
@@ -87,7 +87,7 @@ impl NPM {
             .args(["--version"])
             .output_checked_utf8()
             .map(|s| s.stdout.trim().to_owned());
-        Version::parse(&version_str?).map_err(|err| err.into())
+        Version::parse(&version_str?).map_err(std::convert::Into::into)
     }
 
     fn upgrade(&self, ctx: &ExecutionContext, use_sudo: bool) -> Result<()> {
@@ -266,7 +266,7 @@ impl Deno {
             .args(["-V"])
             .output_checked_utf8()
             .map(|s| s.stdout.trim().to_owned().split_off(5)); // remove "deno " prefix
-        Version::parse(&version_str?).map_err(|err| err.into())
+        Version::parse(&version_str?).map_err(std::convert::Into::into)
     }
 }
 
@@ -399,7 +399,7 @@ pub fn run_volta_packages_upgrade(ctx: &ExecutionContext) -> Result<()> {
         return Ok(());
     }
 
-    for package in installed_packages.iter() {
+    for package in &installed_packages {
         ctx.run_type()
             .execute(&volta)
             .args(["install", package])

diff --git a/src/steps/os/macos.rs b/src/steps/os/macos.rs
@@ -203,7 +203,7 @@ pub fn update_xcodes(ctx: &ExecutionContext) -> Result<()> {
                     .execute(&xcodes)
                     .args([
                         "uninstall",
-                        releases_new_installed.iter().next().cloned().unwrap_or_default(),
+                        releases_new_installed.iter().next().copied().unwrap_or_default(),
                     ])
                     .status_checked();
             }
@@ -216,12 +216,7 @@ pub fn update_xcodes(ctx: &ExecutionContext) -> Result<()> {
 pub fn process_xcodes_releases(releases_filtered: Vec<String>, should_ask: bool, ctx: &ExecutionContext) -> Result<()> {
     let xcodes = require("xcodes")?;
 
-    if releases_filtered
-        .last()
-        .map(|s| !s.contains("(Installed)"))
-        .unwrap_or(true)
-        && !releases_filtered.is_empty()
-    {
+    if releases_filtered.last().map_or(true, |s| !s.contains("(Installed)")) && !releases_filtered.is_empty() {
         println!(
             "{} {}",
             t!("New Xcode release detected:"),