ci: lint test targets with clippy --all-targets - #4330
Merged
Conversation
Both clippy steps ran without `--all-targets`, so clippy only ever saw the lib/bin targets. Lints in `#[cfg(test)]` code merged unnoticed and surfaced only for whoever ran clippy locally. Add `--all-targets` to the workspace step and to the wallet fast path (which lints platform-wallet-ffi, so it had the same blind spot), then clear the backlog that exposes: - platform-wallet: `await_holding_lock` in the asset-lock recovery test, two `useless_vec` in the masternode withdrawal tests. - platform-wallet-ffi: seven `field_reassign_with_default`, two `unnecessary_map_or`, one `unnecessary_get_then_check`. - wasm-drive-verify: `items_after_test_module`. The `await_holding_lock` site already dropped its guard before the awaits. The lint fires anyway because it reasons about the binding's scope rather than its liveness, so an explicit `drop()` does not clear it; scoping the guard to a nested block does, with no behavior change. Everything outside the two workflow lines is test-only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
QuantumExplorer
requested review from
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
August 7, 2026 15:54
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThe pull request expands Rust Clippy coverage to all targets and applies equivalent idiomatic updates to wallet FFI, wallet synchronization, withdrawal, and proof verification tests. ChangesRust lint and test updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Collaborator
|
🕓 Ready for review — next in queue (commit 557bd78) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4330 +/- ##
============================================
- Coverage 87.78% 87.61% -0.18%
============================================
Files 2677 2704 +27
Lines 342371 345211 +2840
============================================
+ Hits 300551 302446 +1895
- Misses 41820 42765 +945
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Clippy never linted test code in CI. Both gates ran without
--all-targets:tests-rs-workspace.yml—cargo clippy --workspace --all-features --locked -- --no-deps -D warningstests-rs-wallet.yml— the same, scoped to the wallet crates + dependentsWithout
--all-targetsclippy only sees the lib/bin targets, so lints inside#[cfg(test)]code merge unnoticed and only surface for whoever runs clippy locally.cargo clippy -p platform-wallet --all-targets --all-features -- -D warningscurrently fails onv4.2-devwith 3 errors, none of which CI can see.What was done?
CI
Added
--all-targetsto both clippy steps. The wallet fast path needed it too — it lintsplatform-wallet-ffi, so it carried the same blind spot.Lints cleared
A full
--workspace --all-targets --all-featuressweep over all 46 members turned up 11 findings in 2 crates beyond the 3 that prompted this. All are in#[cfg(test)]code:rs-platform-wallet—await_holding_lockinrecovery.rs; twouseless_vecinwithdrawal.rs(vec![…]→[…])rs-platform-wallet-ffi— sevenfield_reassign_with_defaultinpersistence.rs(PersistenceCallbacks::default()+ field assignment → struct literal with..Default::default()); twounnecessary_map_or(map_or(false, …)→is_some_and); oneunnecessary_get_then_check(get(&k).is_none()→!contains_key(&k))wasm-drive-verify—items_after_test_module;mod testsmoved belowconvert_proof_result_to_jsNote on the
await_holding_lockfixbuilt_resume_rebroadcasts_original_and_typed_failures_do_not_broadcastalready released its guard with an explicitdrop(broadcast)before the awaits, so the guard was never live across them at runtime. The lint fires regardless because it reasons about the binding's scope, not its liveness — aletbinding's scope runs to the end of its enclosing block whateverdrop()does. Scoping the guard to a nested block ends it for real. No#[allow], no behavior change.Everything outside the two workflow lines is test-only.
How Has This Been Tested?
No new tests — this fixes lints in existing ones. All commands below exited 0:
cargo clippy --workspace --all-targets --all-features --locked -- --no-deps -D warnings(the exact new CI command)--all-targetscargo clippy -p platform-wallet --all-targets --all-features -- -D warningscargo test -p platform-wallet --lib— 574 passedcargo test -p platform-wallet-ffi --lib --all-features— 246 passed (--all-featuresso theshielded-gated test is included)cargo test -p wasm-drive-verify --lib— 15 passedcargo fmt --check --allEach edited test was confirmed present in the runner output rather than filtered out.
Breaking Changes
None.
Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Refactor