Fix clippy & use musl target on Rust compiler for static compilation (S…

…chniz#554)
introspection3 · Oct 14, 2021 · 74c9b37 · 74c9b37
1 parent 500c9f5
commit 74c9b37
Show file tree

Hide file tree

Showing 35 changed files with 153 additions and 161 deletions.
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
@@ -17,6 +17,16 @@ jobs:
     - name: cargo fmt
       run: cargo fmt -- --check
 
+  clippy:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: hecrj/setup-rust-action@v1
+      with:
+        rust-version: stable
+    - uses: actions/checkout@v2
+    - name: cargo clippy
+      run: cargo clippy -- -D warnings
+
   unit_tests:
     runs-on: ${{ matrix.os }}
     strategy:
@@ -67,7 +77,7 @@ jobs:
         path: target/release/fnm.exe
 
   build_macos_release:
-    runs-on: macOS-latest
+    runs-on: macos-latest
     name: "Release build for macOS"
     steps:
     - uses: hecrj/setup-rust-action@v1
@@ -91,17 +101,23 @@ jobs:
     name: "Build static Linux binary"
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
-    - name: Build static binary
+    - uses: hecrj/setup-rust-action@v1
+      with:
+        rust-version: stable
+        targets: x86_64-unknown-linux-musl
+    - name: Install musl tools
       run: |
-        sudo chown -R 1000:1000 .
-        docker run --rm -v "$(pwd)":/home/rust/src ekidd/rust-musl-builder:stable \
-          cargo build --target x86_64-unknown-linux-gnu --release
-        sudo chown -R $(whoami):$(whoami) .
+        sudo apt-get update
+        sudo apt-get install -y --no-install-recommends musl-tools
+    - uses: actions/checkout@v2
+    - name: Build release binary
+      run: cargo build --release --target x86_64-unknown-linux-musl
+    - name: Strip binary from debug symbols
+      run: strip target/x86_64-unknown-linux-musl/release/fnm
     - uses: actions/upload-artifact@v2
       with:
         name: fnm-linux
-        path: target/x86_64-unknown-linux-gnu/release/fnm
+        path: target/x86_64-unknown-linux-musl/release/fnm
 
   build_static_arm_binary:
     name: "Build ARM binary"

diff --git a/src/alias.rs b/src/alias.rs
@@ -28,7 +28,7 @@ pub fn create_alias(
 
 pub fn list_aliases(config: &FnmConfig) -> std::io::Result<Vec<StoredAlias>> {
     let vec: Vec<_> = std::fs::read_dir(&config.aliases_dir())?
-        .filter_map(|item| item.ok())
+        .filter_map(Result::ok)
         .filter_map(|x| TryInto::<StoredAlias>::try_into(x.path().as_path()).ok())
         .collect();
     Ok(vec)

diff --git a/src/arch.rs b/src/arch.rs
@@ -18,7 +18,7 @@ pub fn get_safe_arch<'a>(arch: &'a Arch, version: &Version) -> &'a Arch {
 
     return match (platform_name(), platform_arch(), version) {
         ("darwin", "arm64", Version::Semver(v)) if v.major < 16 => &Arch::X64,
-        _ => &arch,
+        _ => arch,
     };
 }
 
@@ -83,7 +83,7 @@ impl ArchError {
 }
 
 impl std::fmt::Display for ArchError {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.details)
     }
 }

diff --git a/src/archive/zip.rs b/src/archive/zip.rs
@@ -33,7 +33,7 @@ impl<R: Read> Extract for Zip<R> {
 
         for i in 0..archive.len() {
             let mut file = archive.by_index(i)?;
-            let outpath = path.join(file.sanitized_name());
+            let outpath = path.join(file.mangled_name());
 
             {
                 let comment = file.comment();

diff --git a/src/choose_version_for_user_input.rs b/src/choose_version_for_user_input.rs
@@ -50,7 +50,7 @@ pub fn choose_version_for_user_input<'a>(
             version: Version::Alias(alias_name),
         })
     } else {
-        let current_version = requested_version.to_version(&all_versions, &config);
+        let current_version = requested_version.to_version(&all_versions, config);
         current_version.map(|version| {
             info!("Using Node {}", version.to_string().cyan());
             let path = config

diff --git a/src/commands/alias.rs b/src/commands/alias.rs
@@ -25,7 +25,7 @@ impl Command for Alias {
                 version: self.to_version,
             })?;
 
-        create_alias(&config, &self.name, applicable_version.version())
+        create_alias(config, &self.name, applicable_version.version())
             .context(CantCreateSymlink)?;
 
         Ok(())

diff --git a/src/commands/current.rs b/src/commands/current.rs
@@ -10,7 +10,7 @@ impl Command for Current {
     type Error = Error;
 
     fn apply(self, config: &FnmConfig) -> Result<(), Self::Error> {
-        let version_string = match current_version(&config)? {
+        let version_string = match current_version(config)? {
             Some(ver) => ver.v_str(),
             None => "none".into(),
         };

diff --git a/src/commands/default.rs b/src/commands/default.rs
@@ -11,14 +11,16 @@ pub struct Default {
 
 impl Command for Default {
     type Error = super::alias::Error;
+
     fn apply(self, config: &FnmConfig) -> Result<(), Self::Error> {
         Alias {
             name: "default".into(),
             to_version: self.version,
         }
         .apply(config)
     }
+
     fn handle_error(err: Self::Error, config: &FnmConfig) {
-        Alias::handle_error(err, config)
+        Alias::handle_error(err, config);
     }
 }
diff --git a/src/commands/env.rs b/src/commands/env.rs
@@ -54,7 +54,7 @@ impl Command for Env {
         }
 
         let shell: Box<dyn Shell> = self.shell.or_else(&infer_shell).context(CantInferShell)?;
-        let multishell_path = make_symlink(&config);
+        let multishell_path = make_symlink(config);
         let binary_path = if cfg!(windows) {
             multishell_path.clone()
         } else {
@@ -82,7 +82,7 @@ impl Command for Env {
             shell.set_env_var("FNM_ARCH", &config.arch.to_string())
         );
         if self.use_on_cd {
-            println!("{}", shell.use_on_cd(&config));
+            println!("{}", shell.use_on_cd(config));
         }
         Ok(())
     }

diff --git a/src/commands/exec.rs b/src/commands/exec.rs
@@ -40,10 +40,10 @@ impl Cmd for Exec {
                 let current_dir = std::env::current_dir().unwrap();
                 UserVersionReader::Path(current_dir)
             })
-            .to_user_version()
+            .into_user_version()
             .context(CantInferVersion)?;
 
-        let applicable_version = choose_version_for_user_input(&version, &config)
+        let applicable_version = choose_version_for_user_input(&version, config)
             .context(ApplicableVersionError)?
             .context(VersionNotFound { version })?;
 

diff --git a/src/commands/install.rs b/src/commands/install.rs
@@ -55,7 +55,7 @@ impl super::command::Command for Install {
 
         let version = match current_version.clone() {
             UserVersion::Full(Version::Semver(actual_version)) => Version::Semver(actual_version),
-            UserVersion::Full(v @ Version::Bypassed) | UserVersion::Full(v @ Version::Alias(_)) => {
+            UserVersion::Full(v @ (Version::Bypassed | Version::Alias(_))) => {
                 ensure!(false, UninstallableVersion { version: v });
                 unreachable!();
             }
@@ -84,7 +84,7 @@ impl super::command::Command for Install {
                     .collect();
 
                 current_version
-                    .to_version(&available_versions, &config)
+                    .to_version(&available_versions, config)
                     .context(CantFindNodeVersion {
                         requested_version: current_version,
                     })?
@@ -117,12 +117,12 @@ impl super::command::Command for Install {
                 alias_name.cyan(),
                 version.v_str().cyan()
             );
-            create_alias(&config, &alias_name, &version).context(IoError)?;
+            create_alias(config, &alias_name, &version).context(IoError)?;
         }
 
         if !config.default_version_dir().exists() {
             debug!("Tagging {} as the default version", version.v_str().cyan());
-            create_alias(&config, "default", &version).context(IoError)?;
+            create_alias(config, "default", &version).context(IoError)?;
         }
 
         Ok(())

diff --git a/src/commands/ls_local.rs b/src/commands/ls_local.rs
@@ -2,7 +2,7 @@ use crate::alias::{list_aliases, StoredAlias};
 use crate::config::FnmConfig;
 use crate::current_version::current_version;
 use crate::version::Version;
-use colored::*;
+use colored::Colorize;
 use snafu::{ResultExt, Snafu};
 use std::collections::HashMap;
 use structopt::StructOpt;
@@ -19,16 +19,16 @@ impl super::command::Command for LsLocal {
             crate::installed_versions::list(base_dir).context(CantListLocallyInstalledVersion)?;
         versions.insert(0, Version::Bypassed);
         versions.sort();
-        let aliases_hash = generate_aliases_hash(&config).context(CantReadAliases)?;
-        let curr_version = current_version(&config).ok().flatten();
+        let aliases_hash = generate_aliases_hash(config).context(CantReadAliases)?;
+        let curr_version = current_version(config).ok().flatten();
 
         for version in versions {
             let version_aliases = match aliases_hash.get(&version.v_str()) {
                 None => "".into(),
                 Some(versions) => {
                     let version_string = versions
                         .iter()
-                        .map(|x| x.name())
+                        .map(StoredAlias::name)
                         .collect::<Vec<_>>()
                         .join(", ");
                     format!(" {}", version_string.dimmed())
@@ -48,7 +48,7 @@ impl super::command::Command for LsLocal {
 }
 
 fn generate_aliases_hash(config: &FnmConfig) -> std::io::Result<HashMap<String, Vec<StoredAlias>>> {
-    let mut aliases = list_aliases(&config)?;
+    let mut aliases = list_aliases(config)?;
     let mut hashmap: HashMap<String, Vec<StoredAlias>> = HashMap::with_capacity(aliases.len());
     for alias in aliases.drain(..) {
         if let Some(value) = hashmap.get_mut(alias.s_ver()) {

diff --git a/src/commands/ls_remote.rs b/src/commands/ls_remote.rs
@@ -10,9 +10,7 @@ impl super::command::Command for LsRemote {
     type Error = Error;
 
     fn apply(self, config: &FnmConfig) -> Result<(), Self::Error> {
-        let mut all_versions =
-            remote_node_index::list(&config.node_dist_mirror).context(HttpError)?;
-        all_versions.sort();
+        let all_versions = remote_node_index::list(&config.node_dist_mirror).context(HttpError)?;
 
         for version in all_versions {
             print!("{}", version.version);

diff --git a/src/commands/uninstall.rs b/src/commands/uninstall.rs
@@ -37,7 +37,7 @@ impl Command for Uninstall {
 
         let available_versions: Vec<&Version> = all_versions
             .iter()
-            .filter(|v| requested_version.matches(v, &config))
+            .filter(|v| requested_version.matches(v, config))
             .collect();
 
         ensure!(
@@ -51,12 +51,12 @@ impl Command for Uninstall {
         );
 
         let version = requested_version
-            .to_version(&all_versions, &config)
+            .to_version(&all_versions, config)
             .context(CantFindVersion)?;
 
-        let matching_aliases = version.find_aliases(&config).context(IoError)?;
+        let matching_aliases = version.find_aliases(config).context(IoError)?;
         let root_path = version
-            .root_path(&config)
+            .root_path(config)
             .with_context(|| RootPathNotFound {
                 version: version.clone(),
             })?;

diff --git a/src/commands/use.rs b/src/commands/use.rs
@@ -24,7 +24,7 @@ impl Command for Use {
 
     fn apply(self, config: &FnmConfig) -> Result<(), Self::Error> {
         let multishell_path = config.multishell_path().context(FnmEnvWasNotSourced)?;
-        warn_if_multishell_path_not_in_path_env_var(&multishell_path, &config);
+        warn_if_multishell_path_not_in_path_env_var(multishell_path, config);
 
         let all_versions =
             installed_versions::list(config.installations_dir()).context(VersionListingError)?;
@@ -34,7 +34,7 @@ impl Command for Use {
                 let current_dir = std::env::current_dir().unwrap();
                 UserVersionReader::Path(current_dir)
             })
-            .to_user_version()
+            .into_user_version()
             .context(CantInferVersion)?;
 
         let version_path = if let UserVersion::Full(Version::Bypassed) = requested_version {
@@ -50,23 +50,20 @@ impl Command for Use {
                 return Ok(());
             }
         } else {
-            let current_version = requested_version.to_version(&all_versions, &config);
-            match current_version {
-                Some(version) => {
-                    outln!(config#Info, "Using Node {}", version.to_string().cyan());
-                    config
-                        .installations_dir()
-                        .join(version.to_string())
-                        .join("installation")
-                }
-                None => {
-                    install_new_version(requested_version, config, self.install_if_missing)?;
-                    return Ok(());
-                }
+            let current_version = requested_version.to_version(&all_versions, config);
+            if let Some(version) = current_version {
+                outln!(config#Info, "Using Node {}", version.to_string().cyan());
+                config
+                    .installations_dir()
+                    .join(version.to_string())
+                    .join("installation")
+            } else {
+                install_new_version(requested_version, config, self.install_if_missing)?;
+                return Ok(());
             }
         };
 
-        replace_symlink(&version_path, &multishell_path).context(SymlinkingCreationIssue)?;
+        replace_symlink(&version_path, multishell_path).context(SymlinkingCreationIssue)?;
 
         Ok(())
     }
@@ -86,7 +83,7 @@ fn install_new_version(
 
     Install {
         version: Some(requested_version.clone()),
-        ..Default::default()
+        ..Install::default()
     }
     .apply(config)
     .context(InstallError)?;
@@ -97,7 +94,7 @@ fn install_new_version(
     }
     .apply(config)?;
 
-    return Ok(());
+    Ok(())
 }
 
 /// Tries to delete `from`, and then tries to symlink `from` to `to` anyway.
@@ -115,11 +112,12 @@ fn replace_symlink(from: &std::path::Path, to: &std::path::Path) -> std::io::Res
 }
 
 fn should_install_interactively(requested_version: &UserVersion) -> bool {
+    use std::io::Write;
+
     if !(atty::is(atty::Stream::Stdout) && atty::is(atty::Stream::Stdin)) {
         return false;
     }
 
-    use std::io::Write;
     let error_message = format!(
         "Can't find an installed Node version matching {}.",
         requested_version.to_string().italic()
@@ -146,7 +144,7 @@ fn warn_if_multishell_path_not_in_path_env_var(
         multishell_path.to_path_buf()
     };
 
-    for path in std::env::split_paths(&std::env::var("PATH").unwrap_or(String::new())) {
+    for path in std::env::split_paths(&std::env::var("PATH").unwrap_or_default()) {
         if bin_path == path {
             return;
         }