chore: Adopt cargo-fixit, align clippy lints with crypto, clear cargo-shear warnings - #3479
Open
huitseeker wants to merge 14 commits into
Open
chore: Adopt cargo-fixit, align clippy lints with crypto, clear cargo-shear warnings#3479huitseeker wants to merge 14 commits into
huitseeker wants to merge 14 commits into
Conversation
`make xclippy-fix` now runs `cargo +stable fixit --clippy` (a faster drop-in for `cargo clippy --fix`) instead of the `xclippy-fix` cargo alias. The `xclippy` alias in .cargo/config.toml remains the single source of truth for the lint set. The Makefile extracts those flags into CLIPPY_LINT_FLAGS and passes them to fixit via RUSTFLAGS, since fixit cannot take lint flags as trailing arguments. The check path reads the same alias directly, so the two paths cannot drift apart. The now-unused `xclippy-fix` cargo alias is removed. When cargo-fixit is not installed the target prints the install command and skips rather than falling back to clippy. CI is untouched (it never ran the fix step). CONTRIBUTING.md uses `cargo fixit`.
Replace the manual `Copy`/`Clone` impls on `Span<T>` with a derive, which matches the existing `T: Copy`/`T: Clone` bounds exactly. Enable `clippy::expl_impl_clone_on_copy` in the project clippy aliases.
The `build_aux_trace` stub in the ACE codegen test AIR is never called; use `unreachable!` instead of `unimplemented!`. (`todo!` would trip the already-enabled `clippy::todo`.) Enable `clippy::unimplemented` in the project clippy aliases.
Hoist the mutable iterator `.next()` calls out of `debug_assert!` in the ACE and hasher chiplet traces. Calling `.next()` inside `debug_assert!` means the iterator advances in debug builds but not in release builds; evaluating the call unconditionally keeps behaviour consistent. Enable `clippy::debug_assert_with_mut_call` in the project clippy aliases.
`reexport_target_docs` returns `Option<Option<String>>` to encode three states the caller relies on: target not found (skip the re-export), found without docs (emit the procedure with no docs), and found with docs. Conflating the outer two would emit spurious doc entries for unresolved re-exports, so the nesting is intentional; document it with a scoped `#[allow]`. Enable `clippy::option_option` in the project clippy aliases.
`NodeFlags` packs four independent booleans into the hasher state's second word for END operations; there is no natural smaller grouping and `fn_params_excessive_bools` has no configurable threshold, so scope an `#[allow]` on the constructor with a justification, matching the flag-bundle relaxation already in `.clippy.toml`. Enable `clippy::fn_params_excessive_bools` in the project clippy aliases.
Replace the `x <= (T::MAX as U)` checked-cast guards with the idiomatic `T::try_from(x).is_ok()` form across the seven remaining sites (proof order, parser value shrinking, MAST serialization test, parallel trace test, precompiles bench inputs). Enable `clippy::checked_conversions` in the project clippy aliases.
`air/src/constraints/decoder/tests.rs` is a zero-byte file with no corresponding `mod tests` declaration in `decoder/mod.rs`, so it is never compiled. cargo-shear flagged it as unlinked. Delete it.
`core/tests/ui/mast_forest_immutable/index_node_mut.rs` is a trybuild `compile_fail` fixture loaded by path at runtime in `public_api.rs`, so cargo-shear cannot statically link it. Add a scoped `ignored-paths` entry for `tests/ui/**` rather than deleting the fixture, which would remove a real compile_fail test.
thiserror is a derive-only crate, and no code in miden-assembly-syntax-cst uses `#[derive(thiserror::Error)]` or imports it. Remove the dependency and the stale `"thiserror/std"` entry from the `std` feature.
…re-lib miden-ace-codegen was listed both as an optional `[dependencies]` entry (enabled by the `constraints-tools` feature) and as a `[dev-dependencies]` entry. No non-test code in miden-core-lib imports it; the two test files that use it get the crate via the dev-dependency. Remove the redundant optional declaration and the `dep:miden-ace-codegen` reference from the `constraints-tools` feature.
cargo-shear flagged `ignored = ["serde_json"]` as redundant because serde_json is used in the `#[cfg(test)]` deserialization tests in `section.rs`. The dependency is already correctly declared in `[dev-dependencies]`, so the ignore is stale. Remove it.
Delete crates/lib/core/tests/pcs/ (839 lines across four files), a winter-fri based FRI PCS verifier test module that has not compiled or run since the Plonky3 migration. cargo-shear flags it as an unlinked file because no `mod pcs;` declaration exists in tests/main.rs. How this directory became orphaned (commit-by-commit): 1. #2365 (a13eea1, 2025-11-15) "chore: rename STARK MASM mod" The pcs/ test directory was created alongside a STARK MASM module rename. 2. #2427 (fd2e580, 2025-12-05) "chore: rename miden-libcore to miden-core-lib" The crate rename added `mod pcs;` to tests/main.rs, wiring the directory into the compiled test suite. 3. #2472 (0083f51) "refactor: migrate underlying proving system to Plonky3" The Plonky3 migration replaced winter-fri with p3-fri. The legacy FRI tests in pcs/ and stark/ broke against the new APIs, so both were gated behind a new empty `legacy-stark-tests` feature: #[cfg(feature = "legacy-stark-tests")] mod pcs; 4. #2616 (972e98e, 2026-01-30) "chore: disable legacy tests" The `legacy-stark-tests` feature was removed entirely and both module declarations were commented out: // These tests are disabled until the recursive verifier is // updated to work with Plonky3 proofs // mod pcs; // mod stark; 5. #2869 (e5af1b7, 2026-03-27) "Update the recursive verifier to work with p3-miden" This commit created the current orphaned state. It migrated the stark/ test module to p3-fri and re-enabled it with `mod stark;`, but removed the `// mod pcs;` line entirely instead of uncommenting it. The pcs/ tests use winter-fri APIs directly and were not migrated. The stark/ test module now covers the p3-fri based PCS path. The pcs/ tests are dead and deleting them clears the last cargo-shear warning.
huitseeker
marked this pull request as ready for review
August 5, 2026 22:27
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
1 participant
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.
cargo-fixit
make xclippy-fixrunscargo fixit --clippyinstead ofcargo clippy --fix(TL;DR: a much fastercargo fix)The
xclippycargo alias in.cargo/config.tomlstays the single source for the lint set. fixit cannot take lint flags as trailing arguments, so the Makefile reads that list intoCLIPPY_LINT_FLAGSand passes it throughRUSTFLAGS. The check path reads the same alias, so check and fix cannot drift apart. The oldxclippy-fixalias is removed. CONTRIBUTING.md usescargo fixit.If fixit is missing, the target prints the install command and skips. CI is unchanged.
Clippy lints
Six lints that the 0xmiden/crypto repo already enforced are now on, one per commit:
checked_conversions,debug_assert_with_mut_call,expl_impl_clone_on_copy,unimplemented: code fixed.fn_params_excessive_boolsonNodeFlags,option_optioninbuild.rs: kept as-is with a scoped#[allow]and a reason.cargo-shear
make sheargoes from 6 warnings to 0.air/src/constraints/decoder/tests.rs, an empty unlinked file.crates/lib/core/tests/pcs/, 839 lines of winter-fri tests orphaned since the Plonky3 move. The full history is in the commit.core/Cargo.tomladdsignored-pathsfortests/ui/**, the trybuild fixtures loaded by path at runtime.assembly-syntax-cst: dropped unusedthiserror.core-lib: dropped redundant optionalmiden-ace-codegen, already adev-dependency.
mast-package: dropped staleignored = ["serde_json"].