From bea4d1b0b893f222a837dcfa2ed1a100f1ed440a Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 14 Apr 2025 22:46:54 -0400 Subject: [PATCH 1/3] chore: add clippy CI tests run cargo clippy for all targets --- Cargo.toml | 22 ++++++++++++++++++++++ ci/style.sh | 3 +++ ci/verify-build.sh | 7 +++++++ ctest-test/Cargo.toml | 14 ++++++++++++++ ctest/Cargo.toml | 22 ++++++++++++++++++++++ libc-test/Cargo.toml | 23 +++++++++++++++++++++++ 6 files changed, 91 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 9e8048cffce70..b8f4dbe9ba33d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -142,3 +142,25 @@ members = [ "ctest-test", "libc-test", ] + +# +# TODO: These should be renamed as `[workspace.lints.*]` once MSRV is abve 1.64 +# This way all crates can use it with `[lints] workspace=true` section +# + +[lints.rust] +# TODO: make ident usage consistent in each file +unused_qualifications = "allow" + +[lints.clippy] +# TODO: all these are default lints and should probably be fixed +identity_op = "allow" +if_same_then_else = "allow" +missing_safety_doc = "allow" +non_minimal_cfg = "allow" +precedence = "allow" +redundant_field_names = "allow" +redundant_static_lifetimes = "allow" +unnecessary_cast = "allow" +unused_unit = "allow" +zero_ptr = "allow" diff --git a/ci/style.sh b/ci/style.sh index 44e371583e84e..dd4b08eb3673f 100755 --- a/ci/style.sh +++ b/ci/style.sh @@ -4,6 +4,9 @@ set -eux [ -n "${CI:-}" ] && check="--check" +# TODO: for some reason using `--workspace` validates a lot of generated code in ./target/** dir +cargo clippy -p libc@1.0.0-alpha.1 -p ctest --all-targets -- -D warnings + cargo test --manifest-path libc-test/Cargo.toml --test style -- --nocapture command -v rustfmt diff --git a/ci/verify-build.sh b/ci/verify-build.sh index 79299f18a08b2..46c2ea5de86b0 100755 --- a/ci/verify-build.sh +++ b/ci/verify-build.sh @@ -44,9 +44,13 @@ test_target() { # The basic command that is run each time cmd="cargo +$rust build --target $target" + # The basic clippy command + clippy_cmd="cargo +$rust clippy --all-targets --target $target" + if [ "${no_dist}" != "0" ]; then # If we can't download a `core`, we need to build it cmd="$cmd -Zbuild-std=core,alloc" + clippy_cmd="$clippy_cmd -Zbuild-std=core,alloc" # FIXME: With `build-std` feature, `compiler_builtins` emits a lof of lint warnings. RUSTFLAGS="${RUSTFLAGS:-} -Aimproper_ctypes_definitions" @@ -67,6 +71,9 @@ test_target() { done fi + # Run cargo clippy first + $clippy_cmd + # Test with expected combinations of features $cmd $cmd --features extra_traits diff --git a/ctest-test/Cargo.toml b/ctest-test/Cargo.toml index 72f926e832998..d76e2af13135a 100644 --- a/ctest-test/Cargo.toml +++ b/ctest-test/Cargo.toml @@ -28,3 +28,17 @@ test = false [[bin]] name = "t2_cxx" test = false + +# +# TODO: These should be moved to the root Cargo.toml as `[workspace.lints.*]` once MSRV is abve 1.64 +# replace it with `[lints] workspace=true` +# + +[lints.rust] +# TODO: make ident usage consistent in each file +unused_qualifications = "allow" + +[lints.clippy] +# TODO: fix these, and enable pedantic lints with needed exceptions +match_like_matches_macro = "allow" +eq_op = "allow" diff --git a/ctest/Cargo.toml b/ctest/Cargo.toml index 1494954a105b7..d01f2c34164f4 100644 --- a/ctest/Cargo.toml +++ b/ctest/Cargo.toml @@ -13,3 +13,25 @@ garando_syntax = "0.1" cc = "1.0.1" rustc_version = "0.4" indoc = "2.0.6" + +# +# TODO: These should be moved to the root Cargo.toml as `[workspace.lints.*]` once MSRV is abve 1.64 +# replace it with `[lints] workspace=true` +# + +[lints.rust] +# TODO: make ident usage consistent in each file +unused_qualifications = "allow" + +[lints.clippy] +# TODO: fix these, and enable pedantic lints with needed exceptions +doc_lazy_continuation = "allow" +if_same_then_else = "allow" +needless_borrow = "allow" +needless_borrowed_reference = "allow" +needless_borrows_for_generic_args = "allow" +needless_lifetimes = "allow" +only_used_in_recursion = "allow" +option_as_ref_deref = "allow" +type_complexity = "allow" +useless_format = "allow" diff --git a/libc-test/Cargo.toml b/libc-test/Cargo.toml index 6dc3d2d1c14de..d410fd0253335 100644 --- a/libc-test/Cargo.toml +++ b/libc-test/Cargo.toml @@ -102,3 +102,26 @@ harness = true name = "style_tests" path = "test/style_tests.rs" harness = true + +# +# TODO: These should be moved to the root Cargo.toml as `[workspace.lints.*]` once MSRV is abve 1.64 +# replace it with `[lints] workspace=true` +# + +[lints.rust] +# TODO: make ident usage consistent in each file +unused_qualifications = "allow" + +[lints.clippy] +# TODO: fix these, and enable pedantic lints with needed exceptions +needless_return = "allow" +comparison_to_empty = "allow" +unused_io_amount = "allow" +write_with_newline = "allow" +needless_borrows_for_generic_args = "allow" +only_used_in_recursion = "allow" +match_like_matches_macro = "allow" +useless_format = "allow" +wildcard_in_or_patterns = "allow" +nonminimal_bool = "allow" +match_single_binding = "allow" From 5997a8b58b7f348300c839a1bd5b82f7eee459a5 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 14 Apr 2025 22:57:49 -0400 Subject: [PATCH 2/3] simplify clippy ci --- .github/workflows/ci.yaml | 12 ++++++++++++ ci/style.sh | 3 --- ci/verify-build.sh | 7 ------- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 893070a3d82fa..d0b22bc6179f6 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -29,6 +29,18 @@ jobs: - name: Check style run: ./ci/style.sh + clippy: + name: Clippy check + runs-on: ubuntu-24.04 + timeout-minutes: 10 + steps: + - uses: actions/checkout@v4 + - uses: Swatinem/rust-cache@v2 + # Here we use the latest stable Rust toolchain already installed by GitHub + # Ideally we should run it for every target, but we cannot rely on unstable toolchains + # due to Clippy not being consistent between them. + - run: cargo clippy --workspace --exclude libc-test --exclude ctest-test --all-targets -- -D warnings + # This runs `cargo build --target ...` for all T1 and T2 targets` verify_build: name: Verify build diff --git a/ci/style.sh b/ci/style.sh index dd4b08eb3673f..44e371583e84e 100755 --- a/ci/style.sh +++ b/ci/style.sh @@ -4,9 +4,6 @@ set -eux [ -n "${CI:-}" ] && check="--check" -# TODO: for some reason using `--workspace` validates a lot of generated code in ./target/** dir -cargo clippy -p libc@1.0.0-alpha.1 -p ctest --all-targets -- -D warnings - cargo test --manifest-path libc-test/Cargo.toml --test style -- --nocapture command -v rustfmt diff --git a/ci/verify-build.sh b/ci/verify-build.sh index 46c2ea5de86b0..79299f18a08b2 100755 --- a/ci/verify-build.sh +++ b/ci/verify-build.sh @@ -44,13 +44,9 @@ test_target() { # The basic command that is run each time cmd="cargo +$rust build --target $target" - # The basic clippy command - clippy_cmd="cargo +$rust clippy --all-targets --target $target" - if [ "${no_dist}" != "0" ]; then # If we can't download a `core`, we need to build it cmd="$cmd -Zbuild-std=core,alloc" - clippy_cmd="$clippy_cmd -Zbuild-std=core,alloc" # FIXME: With `build-std` feature, `compiler_builtins` emits a lof of lint warnings. RUSTFLAGS="${RUSTFLAGS:-} -Aimproper_ctypes_definitions" @@ -71,9 +67,6 @@ test_target() { done fi - # Run cargo clippy first - $clippy_cmd - # Test with expected combinations of features $cmd $cmd --features extra_traits From e09683ea3523cde23c36ffe7ada2289b76a18618 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Mon, 14 Apr 2025 23:03:23 -0400 Subject: [PATCH 3/3] run clippy on 3 major platforms --- .github/workflows/ci.yaml | 11 ++++++++--- Cargo.toml | 11 +++++------ ctest-test/Cargo.toml | 10 ++++------ ctest/Cargo.toml | 10 ++++------ libc-test/Cargo.toml | 10 ++++------ 5 files changed, 25 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index d0b22bc6179f6..935462b022f3b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -30,11 +30,15 @@ jobs: run: ./ci/style.sh clippy: - name: Clippy check - runs-on: ubuntu-24.04 + name: Clippy on ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-24.04, macos-14, windows-2022] + runs-on: ${{ matrix.os }} timeout-minutes: 10 steps: - uses: actions/checkout@v4 + - run: rustup update stable --no-self-update - uses: Swatinem/rust-cache@v2 # Here we use the latest stable Rust toolchain already installed by GitHub # Ideally we should run it for every target, but we cannot rely on unstable toolchains @@ -299,7 +303,8 @@ jobs: - verify_build - ctest_msrv - docs - # Github branch protection is exceedingly silly and treats "jobs skipped because a dependency + - clippy + # GitHub branch protection is exceedingly silly and treats "jobs skipped because a dependency # failed" as success. So we have to do some contortions to ensure the job fails if any of its # dependencies fails. if: always() # make sure this is never "skipped" diff --git a/Cargo.toml b/Cargo.toml index b8f4dbe9ba33d..1101a26ce8159 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -143,20 +143,19 @@ members = [ "libc-test", ] -# -# TODO: These should be renamed as `[workspace.lints.*]` once MSRV is abve 1.64 +# FIXME(msrv): These should be renamed as `[workspace.lints.*]` once MSRV is above 1.64 # This way all crates can use it with `[lints] workspace=true` section -# [lints.rust] -# TODO: make ident usage consistent in each file +# FIXME(cleanup): make ident usage consistent in each file unused_qualifications = "allow" [lints.clippy] -# TODO: all these are default lints and should probably be fixed +missing_safety_doc = "allow" + +# FIXME(clippy): all these are default lints and should probably be fixed identity_op = "allow" if_same_then_else = "allow" -missing_safety_doc = "allow" non_minimal_cfg = "allow" precedence = "allow" redundant_field_names = "allow" diff --git a/ctest-test/Cargo.toml b/ctest-test/Cargo.toml index d76e2af13135a..f0429bbeef0f8 100644 --- a/ctest-test/Cargo.toml +++ b/ctest-test/Cargo.toml @@ -29,16 +29,14 @@ test = false name = "t2_cxx" test = false -# -# TODO: These should be moved to the root Cargo.toml as `[workspace.lints.*]` once MSRV is abve 1.64 -# replace it with `[lints] workspace=true` -# +# FIXME(msrv): These should be moved to the root Cargo.toml as `[workspace.lints.*]` +# once MSRV is above 1.64 and replaced with `[lints] workspace=true` [lints.rust] -# TODO: make ident usage consistent in each file +# FIXME(cleanup): make ident usage consistent in each file unused_qualifications = "allow" [lints.clippy] -# TODO: fix these, and enable pedantic lints with needed exceptions +# FIXME(clippy): fix these match_like_matches_macro = "allow" eq_op = "allow" diff --git a/ctest/Cargo.toml b/ctest/Cargo.toml index d01f2c34164f4..1da098f7bb4ff 100644 --- a/ctest/Cargo.toml +++ b/ctest/Cargo.toml @@ -14,17 +14,15 @@ cc = "1.0.1" rustc_version = "0.4" indoc = "2.0.6" -# -# TODO: These should be moved to the root Cargo.toml as `[workspace.lints.*]` once MSRV is abve 1.64 -# replace it with `[lints] workspace=true` -# +# FIXME(msrv): These should be moved to the root Cargo.toml as `[workspace.lints.*]` +# once MSRV is above 1.64 and replaced with `[lints] workspace=true` [lints.rust] -# TODO: make ident usage consistent in each file +# FIXME(cleanup): make ident usage consistent in each file unused_qualifications = "allow" [lints.clippy] -# TODO: fix these, and enable pedantic lints with needed exceptions +# FIXME(clippy): fix these doc_lazy_continuation = "allow" if_same_then_else = "allow" needless_borrow = "allow" diff --git a/libc-test/Cargo.toml b/libc-test/Cargo.toml index d410fd0253335..4faccfdf8d209 100644 --- a/libc-test/Cargo.toml +++ b/libc-test/Cargo.toml @@ -103,17 +103,15 @@ name = "style_tests" path = "test/style_tests.rs" harness = true -# -# TODO: These should be moved to the root Cargo.toml as `[workspace.lints.*]` once MSRV is abve 1.64 -# replace it with `[lints] workspace=true` -# +# FIXME(msrv): These should be moved to the root Cargo.toml as `[workspace.lints.*]` +# once MSRV is above 1.64 and replaced with `[lints] workspace=true` [lints.rust] -# TODO: make ident usage consistent in each file +# FIXME(cleanup): make ident usage consistent in each file unused_qualifications = "allow" [lints.clippy] -# TODO: fix these, and enable pedantic lints with needed exceptions +# FIXME(clippy): fix these needless_return = "allow" comparison_to_empty = "allow" unused_io_amount = "allow"