From b851d0f17d4f96a2ef232fa1799a2557048b7832 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Wed, 22 Jan 2025 11:54:57 -0800 Subject: [PATCH 1/4] Create clippy-stacks alias to run in CI and to run locally Signed-off-by: Jacinta Ferrant --- .cargo/config.toml | 1 + .github/workflows/clippy.yml | 15 +-------------- CONTRIBUTING.md | 20 +++++++++++++++++++- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index 7f7e28a8b8..feaf5fec86 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -1,6 +1,7 @@ [alias] stacks-node = "run --package stacks-node --" fmt-stacks = "fmt -- --config group_imports=StdExternalCrate,imports_granularity=Module" +clippy-stacks = "clippy -p libstackerdb -p stacks-signer -p pox-locking -p clarity -p libsigner -p stacks-common --no-deps --tests --all-features -- -D warnings" # Uncomment to improve performance slightly, at the cost of portability # * Note that native binaries may not run on CPUs that are different from the build machine diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index 1ba4825527..e9fd90e9a2 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -20,21 +20,8 @@ jobs: name: Clippy Check runs-on: ubuntu-latest steps: - - name: Checkout the latest code - id: git_checkout - uses: actions/checkout@v3 - - name: Define Rust Toolchain - id: define_rust_toolchain - run: echo "RUST_TOOLCHAIN=$(cat ./rust-toolchain)" >> $GITHUB_ENV - - name: Setup Rust Toolchain - id: setup_rust_toolchain - uses: actions-rust-lang/setup-rust-toolchain@v1 - with: - toolchain: ${{ env.RUST_TOOLCHAIN }} - components: clippy - name: Clippy id: clippy uses: actions-rs/clippy-check@v1 with: - token: ${{ secrets.GITHUB_TOKEN }} - args: -p libstackerdb -p stacks-signer -p pox-locking -p clarity -p libsigner -p stacks-common --no-deps --tests --all-features -- -D warnings \ No newline at end of file + alias: "clippy-stacks" \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b8c63abc2c..c919de1a95 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ fix: incorporate unlocks in mempool admitter, #3623 ### Recommended githooks -It is helpful to set up the pre-commit git hook set up, so that Rust formatting issues are caught before +It is helpful to set up the pre-commit git hook set up, so that Rust formatting issues and clippy warnings are caught before you push your code. Follow these instruction to set it up: 1. Rename `.git/hooks/pre-commit.sample` to `.git/hooks/pre-commit` @@ -89,10 +89,16 @@ you push your code. Follow these instruction to set it up: ```sh #!/bin/sh +# Format staged Rust files git diff --name-only --staged | grep '\.rs$' | xargs -P 8 -I {} rustfmt {} --edition 2021 --check --config group_imports=StdExternalCrate,imports_granularity=Module || ( echo 'rustfmt failed: run "cargo fmt-stacks"'; exit 1 ) +# Run cargo clippy-stacks and fail the commit if there are any warnings +if ! cargo clippy-stacks; then + echo 'cargo clippy-stacks failed: fix the warnings and try again.'; + exit 1 +fi ``` 3. Make it executable by running `chmod +x .git/hooks/pre-commit` @@ -387,6 +393,18 @@ You can automatically reformat your commit via: cargo fmt-stacks ``` +## Clippy Warnings + +PRs will be checked against `clippy` and will _fail_ if any clippy warnings are generated. +Unfortunately, not all existing clippy warnings have been addressed throughout stacks-core, so arguments must be passed via the command line. +Therefore, we handle `clippy` configurations using a Cargo alias: `cargo clippy-stacks` + +You can check what warnings need to be addressed locally via: + +```bash +cargo clippy-stacks +``` + ## Comments Comments are very important for the readability and correctness of the codebase. The purpose of comments is: From 0c1059c86e10cb34466515768a77f9f37b312d9e Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Wed, 22 Jan 2025 12:49:42 -0800 Subject: [PATCH 2/4] Do not suggest adding clippy to git hooks Signed-off-by: Jacinta Ferrant --- CONTRIBUTING.md | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c919de1a95..7c79fc286c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -81,7 +81,7 @@ fix: incorporate unlocks in mempool admitter, #3623 ### Recommended githooks -It is helpful to set up the pre-commit git hook set up, so that Rust formatting issues and clippy warnings are caught before +It is helpful to set up the pre-commit git hook set up, so that Rust formatting issues are caught before you push your code. Follow these instruction to set it up: 1. Rename `.git/hooks/pre-commit.sample` to `.git/hooks/pre-commit` @@ -89,16 +89,10 @@ you push your code. Follow these instruction to set it up: ```sh #!/bin/sh -# Format staged Rust files git diff --name-only --staged | grep '\.rs$' | xargs -P 8 -I {} rustfmt {} --edition 2021 --check --config group_imports=StdExternalCrate,imports_granularity=Module || ( echo 'rustfmt failed: run "cargo fmt-stacks"'; exit 1 ) -# Run cargo clippy-stacks and fail the commit if there are any warnings -if ! cargo clippy-stacks; then - echo 'cargo clippy-stacks failed: fix the warnings and try again.'; - exit 1 -fi ``` 3. Make it executable by running `chmod +x .git/hooks/pre-commit` From 94164e916ac6fb5f5d306d8028e8653e10374a92 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Wed, 22 Jan 2025 13:23:26 -0800 Subject: [PATCH 3/4] Run clippy without clippy check Signed-off-by: Jacinta Ferrant --- .github/workflows/clippy.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/clippy.yml b/.github/workflows/clippy.yml index e9fd90e9a2..2279d42c88 100644 --- a/.github/workflows/clippy.yml +++ b/.github/workflows/clippy.yml @@ -20,8 +20,18 @@ jobs: name: Clippy Check runs-on: ubuntu-latest steps: + - name: Checkout the latest code + id: git_checkout + uses: actions/checkout@v3 + - name: Define Rust Toolchain + id: define_rust_toolchain + run: echo "RUST_TOOLCHAIN=$(cat ./rust-toolchain)" >> $GITHUB_ENV + - name: Setup Rust Toolchain + id: setup_rust_toolchain + uses: actions-rust-lang/setup-rust-toolchain@v1 + with: + toolchain: ${{ env.RUST_TOOLCHAIN }} + components: clippy - name: Clippy id: clippy - uses: actions-rs/clippy-check@v1 - with: - alias: "clippy-stacks" \ No newline at end of file + run: cargo clippy-stacks \ No newline at end of file From 589bee10e42042652042827dae25332b53d2567e Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Wed, 22 Jan 2025 13:26:44 -0800 Subject: [PATCH 4/4] Fix existing clippy warnings Signed-off-by: Jacinta Ferrant --- clarity/src/vm/ast/parser/v2/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/clarity/src/vm/ast/parser/v2/mod.rs b/clarity/src/vm/ast/parser/v2/mod.rs index 4c46e76a4d..c8a7252498 100644 --- a/clarity/src/vm/ast/parser/v2/mod.rs +++ b/clarity/src/vm/ast/parser/v2/mod.rs @@ -1124,9 +1124,7 @@ pub fn parse_collect_diagnostics( mod tests { use super::*; use crate::vm::diagnostic::Level; - use crate::vm::types::{ - ASCIIData, CharType, PrincipalData, SequenceData, StandardPrincipalData, UTF8Data, - }; + use crate::vm::types::{ASCIIData, CharType, PrincipalData, SequenceData}; #[test] fn test_parse_int() {