From ced2f306b0598706a73eb677ec0ac93622b6b7eb Mon Sep 17 00:00:00 2001 From: Gal Schlezinger Date: Sun, 26 May 2024 07:43:25 +0300 Subject: [PATCH] fix clippy (#1126) * run cargo clippy --fix * cargo fmt * add rust-toolchain.toml * force rust version * fixup! run cargo clippy --fix fix urls * add clippy and rustfmt * update command docs * export Zip on Windows * remove benchmarking for a while --- .github/workflows/rust.yml | 151 +++++++++++++++++++------------------ docs/commands.md | 52 ++++++------- rust-toolchain.toml | 3 + src/arch.rs | 4 +- src/archive/mod.rs | 4 + src/commands/env.rs | 2 +- src/commands/install.rs | 2 +- src/commands/use.rs | 2 +- src/config.rs | 4 +- src/shell/bash.rs | 6 +- src/shell/fish.rs | 6 +- src/shell/infer/mod.rs | 2 +- src/shell/powershell.rs | 6 +- src/shell/zsh.rs | 6 +- 14 files changed, 130 insertions(+), 120 deletions(-) create mode 100644 rust-toolchain.toml diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 46984f6..9046956 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -10,13 +10,16 @@ concurrency: group: ci-${{ github.head_ref }} cancel-in-progress: true +env: + RUST_VERSION: "1.78" + jobs: fmt: runs-on: ubuntu-latest steps: - uses: hecrj/setup-rust-action@v1 with: - rust-version: stable + rust-version: ${{env.RUST_VERSION}} - uses: Swatinem/rust-cache@v2 - uses: actions/checkout@v3 - name: cargo fmt @@ -27,7 +30,7 @@ jobs: steps: - uses: hecrj/setup-rust-action@v1 with: - rust-version: stable + rust-version: ${{env.RUST_VERSION}} - uses: Swatinem/rust-cache@v2 - uses: actions/checkout@v3 - name: cargo clippy @@ -41,7 +44,7 @@ jobs: steps: - uses: hecrj/setup-rust-action@v1 with: - rust-version: stable + rust-version: ${{env.RUST_VERSION}} - uses: Swatinem/rust-cache@v2 - uses: actions/checkout@v3 - name: Run tests @@ -53,7 +56,7 @@ jobs: steps: - uses: hecrj/setup-rust-action@v1 with: - rust-version: stable + rust-version: ${{env.RUST_VERSION}} - uses: Swatinem/rust-cache@v2 - uses: actions/checkout@v3 - name: Build release binary @@ -71,7 +74,7 @@ jobs: steps: - uses: hecrj/setup-rust-action@v1 with: - rust-version: stable + rust-version: ${{env.RUST_VERSION}} - uses: Swatinem/rust-cache@v2 - uses: actions/checkout@v3 - name: Build release binary @@ -237,7 +240,7 @@ jobs: steps: - uses: hecrj/setup-rust-action@v1 with: - rust-version: stable + rust-version: ${{env.RUST_VERSION}} targets: x86_64-unknown-linux-musl - uses: Swatinem/rust-cache@v2 with: @@ -278,7 +281,7 @@ jobs: uses: docker/setup-qemu-action@v2 - uses: hecrj/setup-rust-action@v1 with: - rust-version: stable + rust-version: ${{env.RUST_VERSION}} - uses: Swatinem/rust-cache@v2 with: key: arm-binary-${{ matrix.arch }} @@ -358,70 +361,70 @@ jobs: run: | pnpm run generate-command-docs --check --binary-path=$(which fnm) - run_e2e_benchmarks: - runs-on: ubuntu-latest - name: bench/linux - needs: [build_static_linux_binary] - permissions: - contents: write - pull-requests: write - steps: - - name: install necessary shells - run: sudo apt-get update && sudo apt-get install -y fish zsh bash hyperfine - - uses: actions/checkout@v3 - - uses: actions/download-artifact@v3 - with: - name: fnm-linux - path: target/release - - name: mark binary as executable - run: chmod +x target/release/fnm - - name: install fnm as binary - run: | - sudo install target/release/fnm /bin - fnm --version - - uses: pnpm/action-setup@v2.2.4 - with: - run_install: false - - uses: actions/setup-node@v3 - with: - node-version: 18.x - cache: "pnpm" - - name: Get pnpm store directory - id: pnpm-cache - run: | - echo "::set-output name=pnpm_cache_dir::$(pnpm store path)" - - uses: actions/cache@v3 - name: Setup pnpm cache - with: - path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }} - key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} - restore-keys: | - ${{ runner.os }}-pnpm-store- - - run: pnpm install - - name: Run benchmarks - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SHOULD_STORE: ${{ toJson(!github.event.pull_request) }} - id: benchmark - run: | - delimiter="$(openssl rand -hex 8)" - echo "markdown<<${delimiter}" >> "${GITHUB_OUTPUT}" - node benchmarks/run.mjs --store=$SHOULD_STORE >> "${GITHUB_OUTPUT}" - echo "${delimiter}" >> "${GITHUB_OUTPUT}" - - - name: Create a PR comment - if: ${{ github.event.pull_request }} - uses: thollander/actions-comment-pull-request@v2 - with: - message: | - ## Linux Benchmarks for ${{ github.event.pull_request.head.sha }} - ${{ steps.benchmark.outputs.markdown }} - comment_tag: "benchy comment" - - - name: Create a commit comment - if: ${{ !github.event.pull_request }} - uses: peter-evans/commit-comment@v2 - with: - body: | - ## Linux Benchmarks - ${{ steps.benchmark.outputs.markdown }} + # TODO: use bnz + # run_e2e_benchmarks: + # runs-on: ubuntu-latest + # name: bench/linux + # needs: [build_static_linux_binary] + # permissions: + # contents: write + # pull-requests: write + # steps: + # - name: install necessary shells + # run: sudo apt-get update && sudo apt-get install -y fish zsh bash hyperfine + # - uses: actions/checkout@v3 + # - uses: actions/download-artifact@v3 + # with: + # name: fnm-linux + # path: target/release + # - name: mark binary as executable + # run: chmod +x target/release/fnm + # - name: install fnm as binary + # run: | + # sudo install target/release/fnm /bin + # fnm --version + # - uses: pnpm/action-setup@v2.2.4 + # with: + # run_install: false + # - uses: actions/setup-node@v3 + # with: + # node-version: 18.x + # cache: "pnpm" + # - name: Get pnpm store directory + # id: pnpm-cache + # run: | + # echo "::set-output name=pnpm_cache_dir::$(pnpm store path)" + # - uses: actions/cache@v3 + # name: Setup pnpm cache + # with: + # path: ${{ steps.pnpm-cache.outputs.pnpm_cache_dir }} + # key: ${{ runner.os }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }} + # restore-keys: | + # ${{ runner.os }}-pnpm-store- + # - run: pnpm install + # - name: Run benchmarks + # env: + # GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # SHOULD_STORE: ${{ toJson(!github.event.pull_request) }} + # id: benchmark + # run: | + # delimiter="$(openssl rand -hex 8)" + # echo "markdown<<${delimiter}" >> "${GITHUB_OUTPUT}" + # node benchmarks/run.mjs --store=$SHOULD_STORE >> "${GITHUB_OUTPUT}" + # echo "${delimiter}" >> "${GITHUB_OUTPUT}" + # - name: Create a PR comment + # if: ${{ github.event.pull_request }} + # uses: thollander/actions-comment-pull-request@v2 + # with: + # message: | + # ## Linux Benchmarks for ${{ github.event.pull_request.head.sha }} + # ${{ steps.benchmark.outputs.markdown }} + # comment_tag: "benchy comment" + # + # - name: Create a commit comment + # if: ${{ !github.event.pull_request }} + # uses: peter-evans/commit-comment@v2 + # with: + # body: | + # ## Linux Benchmarks + # ${{ steps.benchmark.outputs.markdown }} diff --git a/docs/commands.md b/docs/commands.md index d3072aa..631f170 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -22,7 +22,7 @@ Commands: Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -55,7 +55,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -82,7 +82,7 @@ Usage: fnm list-remote [OPTIONS] Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -115,7 +115,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -139,7 +139,7 @@ Usage: fnm list [OPTIONS] Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -172,7 +172,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -203,7 +203,7 @@ Options: Install latest LTS --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -242,7 +242,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -273,7 +273,7 @@ Options: Install the version if it isn't installed yet --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -309,7 +309,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -337,7 +337,7 @@ Usage: fnm env [OPTIONS] Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -381,7 +381,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -405,7 +405,7 @@ Usage: fnm completions [OPTIONS] Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -443,7 +443,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -474,7 +474,7 @@ Arguments: Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -507,7 +507,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -535,7 +535,7 @@ Arguments: Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -568,7 +568,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -598,7 +598,7 @@ Arguments: Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -631,7 +631,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -655,7 +655,7 @@ Usage: fnm current [OPTIONS] Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -688,7 +688,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -721,7 +721,7 @@ Arguments: Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -757,7 +757,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] @@ -787,7 +787,7 @@ Arguments: Options: --node-dist-mirror - https://nodejs.org/dist/ mirror + mirror [env: FNM_NODE_DIST_MIRROR] [default: https://nodejs.org/dist] @@ -820,7 +820,7 @@ Options: - recursive: Use the version of Node defined within the current directory and all parent directories --corepack-enabled - Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see https://nodejs.org/api/corepack.html + Enable corepack support for each new installation. This will make fnm call `corepack enable` on every Node.js installation. For more information about corepack see [env: FNM_COREPACK_ENABLED] diff --git a/rust-toolchain.toml b/rust-toolchain.toml new file mode 100644 index 0000000..d38fc82 --- /dev/null +++ b/rust-toolchain.toml @@ -0,0 +1,3 @@ +[toolchain] +channel = "1.78" +components = ["rustfmt", "clippy"] diff --git a/src/arch.rs b/src/arch.rs index 080552c..b1ed941 100644 --- a/src/arch.rs +++ b/src/arch.rs @@ -17,10 +17,10 @@ pub enum Arch { pub fn get_safe_arch<'a>(arch: &'a Arch, version: &Version) -> &'a Arch { use crate::system_info::{platform_arch, platform_name}; - return match (platform_name(), platform_arch(), version) { + match (platform_name(), platform_arch(), version) { ("darwin", "arm64", Version::Semver(v)) if v.major < 16 => &Arch::X64, _ => arch, - }; + } } #[cfg(windows)] diff --git a/src/archive/mod.rs b/src/archive/mod.rs index e3cf33f..ac44763 100644 --- a/src/archive/mod.rs +++ b/src/archive/mod.rs @@ -3,5 +3,9 @@ pub mod tar_xz; pub mod zip; pub use self::extract::{Error, Extract}; + +#[cfg(unix)] pub use self::tar_xz::TarXz; + +#[cfg(windows)] pub use self::zip::Zip; diff --git a/src/commands/env.rs b/src/commands/env.rs index d8cf2b3..f0eacb7 100644 --- a/src/commands/env.rs +++ b/src/commands/env.rs @@ -44,7 +44,7 @@ fn make_symlink(config: &FnmConfig) -> Result { } match symlink_dir(config.default_version_dir(), &temp_dir) { - Ok(_) => Ok(temp_dir), + Ok(()) => Ok(temp_dir), Err(source) => Err(Error::CantCreateSymlink { source, temp_dir }), } } diff --git a/src/commands/install.rs b/src/commands/install.rs index ed20fe9..4ca15a0 100644 --- a/src/commands/install.rs +++ b/src/commands/install.rs @@ -147,7 +147,7 @@ impl Command for Install { outln!(config, Error, "{} {}", "warning:".bold().yellow(), err); } Err(source) => Err(Error::DownloadError { source })?, - Ok(_) => {} + Ok(()) => {} }; if config.corepack_enabled() { diff --git a/src/commands/use.rs b/src/commands/use.rs index 14bbfb4..3f2751f 100644 --- a/src/commands/use.rs +++ b/src/commands/use.rs @@ -153,7 +153,7 @@ fn install_new_version( fn replace_symlink(from: &std::path::Path, to: &std::path::Path) -> std::io::Result<()> { let symlink_deletion_result = fs::remove_symlink_dir(to); match fs::symlink_dir(from, to) { - ok @ Ok(_) => ok, + ok @ Ok(()) => ok, err @ Err(_) => symlink_deletion_result.and(err), } } diff --git a/src/config.rs b/src/config.rs index 1c65399..2f68a9d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -7,7 +7,7 @@ use url::Url; #[derive(clap::Parser, Debug)] pub struct FnmConfig { - /// https://nodejs.org/dist/ mirror + /// mirror #[clap( long, env = "FNM_NODE_DIST_MIRROR", @@ -67,7 +67,7 @@ pub struct FnmConfig { /// Enable corepack support for each new installation. /// This will make fnm call `corepack enable` on every Node.js installation. - /// For more information about corepack see https://nodejs.org/api/corepack.html + /// For more information about corepack see #[clap( long, env = "FNM_COREPACK_ENABLED", diff --git a/src/shell/bash.rs b/src/shell/bash.rs index cada96d..016809e 100644 --- a/src/shell/bash.rs +++ b/src/shell/bash.rs @@ -28,13 +28,13 @@ impl Shell for Bash { fn use_on_cd(&self, config: &crate::config::FnmConfig) -> anyhow::Result { let autoload_hook = match config.version_file_strategy() { VersionFileStrategy::Local => indoc!( - r#" + r" if [[ -f .node-version || -f .nvmrc ]]; then fnm use --silent-if-unchanged fi - "# + " ), - VersionFileStrategy::Recursive => r#"fnm use --silent-if-unchanged"#, + VersionFileStrategy::Recursive => r"fnm use --silent-if-unchanged", }; Ok(formatdoc!( r#" diff --git a/src/shell/fish.rs b/src/shell/fish.rs index 18f6b32..0b3db7f 100644 --- a/src/shell/fish.rs +++ b/src/shell/fish.rs @@ -28,13 +28,13 @@ impl Shell for Fish { fn use_on_cd(&self, config: &crate::config::FnmConfig) -> anyhow::Result { let autoload_hook = match config.version_file_strategy() { VersionFileStrategy::Local => indoc!( - r#" + r" if test -f .node-version -o -f .nvmrc fnm use --silent-if-unchanged end - "# + " ), - VersionFileStrategy::Recursive => r#"fnm use --silent-if-unchanged"#, + VersionFileStrategy::Recursive => r"fnm use --silent-if-unchanged", }; Ok(formatdoc!( r#" diff --git a/src/shell/infer/mod.rs b/src/shell/infer/mod.rs index e0c22c6..e7b1a1b 100644 --- a/src/shell/infer/mod.rs +++ b/src/shell/infer/mod.rs @@ -7,7 +7,7 @@ pub use self::unix::infer_shell; #[cfg(not(unix))] pub use self::windows::infer_shell; -pub(self) fn shell_from_string(shell: &str) -> Option> { +fn shell_from_string(shell: &str) -> Option> { use super::{Bash, Fish, PowerShell, WindowsCmd, Zsh}; match shell { "sh" | "bash" => return Some(Box::from(Bash)), diff --git a/src/shell/powershell.rs b/src/shell/powershell.rs index be77c9b..99f115f 100644 --- a/src/shell/powershell.rs +++ b/src/shell/powershell.rs @@ -28,11 +28,11 @@ impl Shell for PowerShell { fn use_on_cd(&self, config: &crate::config::FnmConfig) -> anyhow::Result { let autoload_hook = match config.version_file_strategy() { VersionFileStrategy::Local => indoc!( - r#" + r" If ((Test-Path .nvmrc) -Or (Test-Path .node-version)) { & fnm use --silent-if-unchanged } - "# + " ), - VersionFileStrategy::Recursive => r#"fnm use --silent-if-unchanged"#, + VersionFileStrategy::Recursive => r"fnm use --silent-if-unchanged", }; Ok(formatdoc!( r#" diff --git a/src/shell/zsh.rs b/src/shell/zsh.rs index 01e039c..f138f22 100644 --- a/src/shell/zsh.rs +++ b/src/shell/zsh.rs @@ -32,13 +32,13 @@ impl Shell for Zsh { fn use_on_cd(&self, config: &crate::config::FnmConfig) -> anyhow::Result { let autoload_hook = match config.version_file_strategy() { VersionFileStrategy::Local => indoc!( - r#" + r" if [[ -f .node-version || -f .nvmrc ]]; then fnm use --silent-if-unchanged fi - "# + " ), - VersionFileStrategy::Recursive => r#"fnm use --silent-if-unchanged"#, + VersionFileStrategy::Recursive => r"fnm use --silent-if-unchanged", }; Ok(formatdoc!( r#"