Skip to content

Clear the workspace lint debt (A+B) - #145

Merged
ApiliumDevTeam merged 4 commits into
devfrom
chore/clippy-clean
Jul 30, 2026
Merged

Clear the workspace lint debt (A+B)#145
ApiliumDevTeam merged 4 commits into
devfrom
chore/clippy-clean

Conversation

@ApiliumDevTeam

Copy link
Copy Markdown
Contributor

First of a four-PR stack. Merge order: this → #146#147#148.

Three commits, deliberately separate

1. test: — tests that asserted nothing. Nine assertions compared an unsigned counter against zero: peer_count >= 0, uptime_secs >= 0, verification_time_ms >= 0. Every one held for every possible value, including the failure the test exists to catch — test_add_peer passed whether or not the peer was added.

They now assert the claim the test is named for, or an invariant that can actually break (time in a power profile cannot exceed total uptime), or nothing at all where the author's own comment says the value is legitimately zero — with a line saying why, so it does not get "fixed" back.

Also here: aingle_zk's memory benchmark had not compiled since prove_knowledge settled its signature — it still passed the last two arguments swapped. cargo test does not build benches, so nothing said so.

2. style:cargo clippy --fix. Machine-applicable suggestions only, tool-applied, 40 files. Droppable without losing commit 1.

3. style: — the 21 that needed judgement. Ten Config::default() values built by assigning fields afterwards; one loop that existed only to index; one unwrap guarded by a separate is_some; six descending sorts to sort_by_key(Reverse(..)), which removes the class of bug where a and b are transposed. All six keys turned out to be Copy, so none needed the borrow-shaped exception this pass was ready to grant.

What this uncovered about measuring

The count was not 24. With -D warnings, a failing crate stops everything downstream from being linted at all, so aingle_logic was invisible while aingle_graph was red, and aingle_cortex while aingle_logic was. It took four passes to reach a clean run, and each layer held real material — including a flatten() over io::Lines in the audit log that spins forever on a repeated read error.

Do not trust a single clippy measurement on this repository until a full pass comes back empty.

Verification

cargo clippy --workspace --all-targets -- -D warnings and cargo fmt --all --check both exit 0. 2242 tests green. Formatting applied only to files this branch touches.

Nine assertions compared an unsigned counter against zero — `peer_count
>= 0`, `uptime_secs >= 0`, `verification_time_ms >= 0`. Every one of them
held for every possible value, including the failure each test exists to
catch: `test_add_peer` passed whether or not the peer was added.

Replace them with the claim the test is named for where there is one
(peer counts), an invariant that can actually break where there is not
(time in a power profile cannot exceed total uptime; energy is never
negative), and nothing at all where the author's own comment says the
value is legitimately zero — with a line explaining why no bound is
asserted, so it does not get "fixed" back.

Also: `3.14` as an arbitrary test float in eight places, which clippy
reads as an approximation of PI. These tests are about storing a
decimal, not about geometry, so use a number that does not claim to be a
constant.

And `aingle_zk`'s memory benchmark had not compiled since
`prove_knowledge` settled on `(secret, public_point, message)` — it was
still passing the last two swapped. Invisible to `cargo test`, which
does not build benches.
Machine-applicable suggestions only, applied by the tool and left
otherwise untouched: needless borrows, redundant closures, `is_empty`
over length comparisons, `io::Error::other`, derivable impls, unused
imports.

Mechanical and behaviour-preserving by construction, but it is 40 files
of churn in the engine, so it is a separate commit: drop it without
losing the fixes in the commit before, which are not mechanical.

Twenty-four lints remain that clippy declines to auto-fix because they
need judgement — clamp (NaN handling differs), `sort_by_key` where the
key borrows, `Default::default()` field reassignment, and a public
`from_str` that shadows the trait method and cannot be renamed without
breaking callers. Left for a later pass rather than rushed.
Twenty-one of the twenty-four remaining lints, split by the kind of
decision each one represents rather than by the lint that reports it.

Notation (15). Ten `Config::default()` values built by assigning fields
afterwards become struct literals with `..Default::default()`. One loop
that existed only to index an array becomes an iterator. One `unwrap`
guarded by a separate `is_some` becomes `if let`. One doc list item that
rustdoc read as a continuation gets its blank line. And one more
assertion that asserted nothing: `!contexts.is_empty() || matches.len()
>= 0` held for every usize, so the test passed even when the query
returned nothing — the failure it exists to catch.

Descending sorts (6). `sort_by(|a, b| b.k.cmp(&a.k))` becomes
`sort_by_key(|x| Reverse(x.k))`, which is stable in the same way and
removes the class of bug where `a` and `b` are quietly transposed. All
six keys turned out to be `Copy`, so none needed the borrow-shaped
exception this pass was ready to grant.

Formatting is applied to the files this branch touches and to no others,
so the diff stays readable.

Three lints are deliberately NOT here, because they are decisions and not
cleanup, and burying them in a commit called "style" is how a semantic
change ships unreviewed. Both `clamp` sites change NaN behaviour, and the
`from_str` one shapes a public API. Each has its own issue.
@ApiliumDevTeam
ApiliumDevTeam merged commit c5d967e into dev Jul 30, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant