feat: adopt workspace-level clippy lint denials#245
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
|
There was a problem hiding this comment.
Pull request overview
This PR adopts strict workspace-level Rust/Clippy lint settings (deny unwrap/expect/panic; forbid unsafe) and updates crates to inherit them, while fixing production violations in visualsign-solana and temporarily exempting other crates/tests/build scripts.
Changes:
- Added workspace lint policy in
src/Cargo.tomland enabled workspace lint inheritance across crates via[lints] workspace = true. - Updated
visualsign-solanaproduction code to propagate errors instead of panicking/unwrapping; added targeted test-only lint allowances. - Added temporary crate-level
#![allow(...)]exemptions (TODO #231), plus documentation updates and policy documentation inCLAUDE.md.
Reviewed changes
Copilot reviewed 61 out of 63 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Cargo.toml | Defines workspace-level lint denials/forbid for Clippy/Rust. |
| src/visualsign/Cargo.toml | Enables workspace lint inheritance for visualsign. |
| src/visualsign/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/parser/grpc-server/Cargo.toml | Enables workspace lint inheritance for grpc-server. |
| src/parser/grpc-server/src/main.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/parser/gateway/Cargo.toml | Enables workspace lint inheritance for gateway. |
| src/parser/gateway/src/main.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/parser/cli/Cargo.toml | Enables workspace lint inheritance for parser CLI. |
| src/parser/cli/src/main.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/parser/cli/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/parser/cli/tests/cli_test.rs | Exempts integration-style tests from unwrap/expect/panic denials. |
| src/parser/app/Cargo.toml | Enables workspace lint inheritance for parser app. |
| src/parser/app/src/main.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/parser/app/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/metrics/Cargo.toml | Enables workspace lint inheritance for metrics crate. |
| src/metrics/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/metrics/examples/demo.rs | Exempts example from unwrap/expect/panic denials. |
| src/integration/Cargo.toml | Enables workspace lint inheritance for integration crate. |
| src/integration/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/integration/tests/parser.rs | Exempts integration test from unwrap/expect/panic denials. |
| src/integration/tests/signature_metadata_test.rs | Exempts integration test from unwrap/expect/panic denials. |
| src/integration/tests/signature_metadata_e2e.rs | Exempts integration test from unwrap/expect/panic denials. |
| src/host_primitives/Cargo.toml | Enables workspace lint inheritance for host primitives. |
| src/host_primitives/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/health_check/Cargo.toml | Enables workspace lint inheritance for health_check. |
| src/health_check/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/generated/Cargo.toml | Enables workspace lint inheritance for generated types crate. |
| src/generated/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/examples/Cargo.toml | Enables workspace lint inheritance for examples crate. |
| src/examples/library_integration_test.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/codegen/Cargo.toml | Enables workspace lint inheritance for codegen. |
| src/codegen/src/main.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/chain_parsers/visualsign-unspecified/Cargo.toml | Enables workspace lint inheritance for unspecified parser. |
| src/chain_parsers/visualsign-unspecified/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/chain_parsers/visualsign-tron/Cargo.toml | Enables workspace lint inheritance for Tron parser. |
| src/chain_parsers/visualsign-tron/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/chain_parsers/visualsign-sui/Cargo.toml | Enables workspace lint inheritance for Sui parser. |
| src/chain_parsers/visualsign-sui/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/chain_parsers/visualsign-sui/build.rs | Allows unwrap in build script under new workspace linting. |
| src/chain_parsers/visualsign-solana/Cargo.toml | Enables workspace lint inheritance for Solana parser. |
| src/chain_parsers/visualsign-solana/build.rs | Allows unwrap in build script under new workspace linting. |
| src/chain_parsers/visualsign-solana/src/lib.rs | Exempts crate-internal tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/src/utils/mod.rs | Adds test-only allowances and notes; introduces a test helper allowance. |
| src/chain_parsers/visualsign-solana/src/core/instructions.rs | Replaces panic path with error propagation. |
| src/chain_parsers/visualsign-solana/src/presets/compute_budget/mod.rs | Converts unwrap-based field building to Result + ?. |
| src/chain_parsers/visualsign-solana/src/presets/associated_token_account/mod.rs | Replaces unwraps with ? in expanded field creation. |
| src/chain_parsers/visualsign-solana/src/core/accounts/decode.rs | Exempts module tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/src/core/visualsign.rs | Exempts module tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/src/idl/mod.rs | Exempts module tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/src/presets/token_2022/mod.rs | Exempts module tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/src/presets/swig_wallet/mod.rs | Exempts module tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/src/presets/jupiter_swap/mod.rs | Exempts module tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/tests/common/mod.rs | Exempts test helpers from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/tests/semantic_pipeline.rs | Exempts tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/tests/real_idl_validation.rs | Exempts tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/tests/pipeline_integration.rs | Exempts tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-solana/tests/fuzz_idl_parsing.rs | Exempts tests from unwrap/expect/panic denials. |
| src/chain_parsers/visualsign-ethereum/Cargo.toml | Enables workspace lint inheritance for Ethereum parser. |
| src/chain_parsers/visualsign-ethereum/src/lib.rs | Adds temporary crate-level lint exemptions (TODO #231). |
| src/chain_parsers/visualsign-ethereum/tests/lib_test.rs | Exempts tests from unwrap/expect/panic denials. |
| docs/chains/solana/idl-parsing.mdx | Updates examples to avoid unwrap/expect (now uses ?/errors). |
| docs/adding-new-chain.mdx | Updates examples to avoid unwrap in test snippets. |
| CLAUDE.md | Documents the workspace lint policy and intended exceptions. |
Comments suppressed due to low confidence (5)
src/parser/cli/src/lib.rs:9
- The crate-level
#![allow(clippy::unwrap_used)]is overridden later by#![deny(clippy::all, clippy::unwrap_used)], sounwrap_usedis still denied for this crate (including#[cfg(test)]modules). This defeats the intended TODO(#231) exemption and will makecargo clippy --all-targetsfail on existing test unwraps. Move theallow(clippy::unwrap_used)after thedeny(...), or removeclippy::unwrap_usedfrom the deny list while the exemption is in place (or use a targetedcfg_attr(test, allow(...))).
src/parser/app/src/lib.rs:9 #![allow(clippy::unwrap_used)]at the top is negated by the later#![deny(clippy::all, clippy::unwrap_used)], so the TODO(#231) exemption for unwraps is currently ineffective. If this crate is meant to be temporarily exempt, either move theallowbelow thedeny, or dropclippy::unwrap_usedfrom the deny list until violations are fixed.
src/host_primitives/src/lib.rs:9- The newly added
#![allow(clippy::unwrap_used)]is overridden by the existing#![deny(clippy::all, clippy::unwrap_used)], so unwraps are still denied despite the TODO(#231) exemption comment. Consider moving the allow below the deny, or removingclippy::unwrap_usedfrom the deny list while the exemption is intended to apply.
src/health_check/src/lib.rs:12 #![allow(clippy::unwrap_used)]is overridden by#![deny(clippy::all, clippy::unwrap_used)]later in the crate attributes, so the TODO(#231) unwrap exemption is currently a no-op. If the intent is to temporarily exempt this crate, reorder the attributes (allow after deny) or removeclippy::unwrap_usedfrom the deny list until the violations are addressed.
src/chain_parsers/visualsign-solana/src/utils/mod.rs:15- This helper is marked "Only used in tests" but is compiled into non-test builds and is
pub, and it uses.unwrap()with a lint allow. To keep the workspace-levelunwrap_useddenial meaningful for production code, consider moving this into a#[cfg(test)]module (or gating the function with#[cfg(test)]) and/or making it non-public so it can’t be called from production code.
/// Helper function to create a complete Solana transaction from a message with empty signatures.
/// Only used in tests.
#[allow(clippy::unwrap_used)]
pub fn create_transaction_with_empty_signatures(message_base64: &str) -> String {
// Decode the message
let message_bytes = base64::engine::general_purpose::STANDARD
.decode(message_base64)
.unwrap();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bd5e40e to
9885bea
Compare
Add unwrap_used, expect_used, panic deny and unsafe_code forbid to workspace lints. Every crate inherits via [lints] workspace = true. Fixed: visualsign-solana and visualsign core production code — replaced panic! with error propagation, .unwrap() with ?, exempt test modules and build scripts. Crate-level exemptions remain only where violations exist: visualsign-ethereum, visualsign-sui, generated, metrics, parser_gateway, parser_cli, parser_app, parser_grpc_server, integration, examples. Clean crates with no exemptions needed: visualsign-tron, visualsign-unspecified, health_check, host_primitives, codegen. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fuzz.yml and proptest.yml are duplicates of fuzz-solana.yml and proptest-solana.yml respectively. Keep only the -solana variants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- CLAUDE.md: add Workspace Lint Policy section documenting deny rules and exception patterns - adding-new-chain.mdx: replace .unwrap() with .expect() in test examples for better error messages - idl-parsing.mdx: wrap production code example in function returning Result, replace .expect()/.unwrap() with ? operator Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
9885bea to
d651672
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 61 out of 63 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
src/parser/cli/src/lib.rs:9
#![allow(clippy::unwrap_used)]at the top of the crate is overridden by#![deny(..., clippy::unwrap_used)]a few lines later, so unwrap exemptions aren’t actually enabled (despite the TODO comment). If the intent is to temporarily exempt this crate fromunwrap_used, removeclippy::unwrap_usedfrom thedeny(...)list (or move theallowbelow thedeny). If the intent is to keep denying unwraps, drop theallow(clippy::unwrap_used)line to avoid confusion.
src/parser/app/src/lib.rs:9- Same issue as in
parser/cli:#![allow(clippy::unwrap_used)]is negated by the subsequent#![deny(..., clippy::unwrap_used)], so the stated temporary exemptions won’t apply to unwraps in this crate. Either removeclippy::unwrap_usedfrom thedeny(...)list (or place theallowafter it), or drop theallow(clippy::unwrap_used)attribute if unwraps are intentionally still denied.
src/chain_parsers/visualsign-solana/src/utils/mod.rs:14 - This helper is
puband re-exported from the crate’s publicutils, but it usesdecode(...).unwrap()(and is now exempted fromunwrap_used). Since the workspace policy is specifically trying to eliminate panic-on-bad-input paths in production APIs, consider making this helper fallible (returnResult<String, VisualSignError>or similar) or gating it behind a dedicatedtest-utilsfeature / making itpub(crate)so it can’t be called from non-test code inadvertently.
/// Helper function to create a complete Solana transaction from a message with empty signatures.
/// Only used in tests.
#[allow(clippy::unwrap_used)]
pub fn create_transaction_with_empty_signatures(message_base64: &str) -> String {
// Decode the message
let message_bytes = base64::engine::general_purpose::STANDARD
.decode(message_base64)
.unwrap();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- parser_cli, parser_app: remove clippy::unwrap_used from #![deny] line so the TODO(#231) #![allow] exemption is not overridden - utils/mod.rs: document that create_transaction_with_empty_signatures is used by integration tests (cannot be #[cfg(test)] gated) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verify that the error handling introduced in the clippy lint cleanup works correctly: empty account keys returns DecodeError, OOB program_id_index silently skips instructions, all-OOB produces empty fields without panicking. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Workspace lints (#231, #245) deny `unwrap_used`, `expect_used`, and `panic` — these landed on main after this branch was created. Rather than dodging them with `#[allow(...)]` on the test module (which other presets did and is the lazy fix), convert each test to return `Result<(), Box<dyn std::error::Error>>` and use `?` / `ok_or` to surface failures. Tests now fail with a meaningful error message via the test runner's normal Result reporting instead of a panic, which is also better debugging UX. cargo clippy --workspace --all-targets -- -D warnings: clean. cargo test -p visualsign-solana: 5/5 squads tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Closes #231. Adds strict workspace-level clippy lints and fixes all violations in
visualsign-solanaandvisualsigncore.unwrap_used = "deny",expect_used = "deny",panic = "deny"in[workspace.lints.clippy]unsafe_code = "forbid"in[workspace.lints.rust][lints] workspace = trueWhat was fixed
visualsign-solana(10 production violations):panic!withVisualSignError::DecodeErrorpropagation ininstructions.rs.unwrap()with?inassociated_token_accountandcompute_budgetpresetscreate_compute_budget_expanded_fieldsto returnResultvisualsigncore (1 production violation):#[allow(clippy::expect_used)]onRegex::new().expect()inLazyLockstatic (infallible at runtime)Clean crates (no exemptions needed):
visualsign-tron,visualsign-unspecified,health_check,host_primitives,codegenWhat retains temporary exemptions
Crate-level
#![allow(...)]withTODO(#231):visualsign-ethereumvisualsign-suigeneratedparser_cliparser_appexpect()callsparser_gatewayexpect()parser_grpc_serverexpect()metricsexpect()and server bindintegrationunwrap()examplesunwrap()Test files (
integration/tests/*,cli/tests/*,metrics/examples/*) are exempted with#![allow(...)]as intended — test code does not need strict error handling.Other changes
fuzz.ymlandproptest.ymlwere duplicates offuzz-solana.ymlandproptest-solana.yml)CLAUDE.mdwith Workspace Lint Policy sectionadding-new-chain.mdxandidl-parsing.mdxcode examples to use?instead of.unwrap()Test plan
make lintpasses (clippy clean)make testpasses (all tests green)🤖 Generated with Claude Code