chore: make the workspace clippy-clean, and add the job that keeps it so

[itsbalamurali]

Nothing in CI ran clippy, so seventy-four diagnostics had accumulated across dmrtd, mrz-parser, aamva and aadhaar-offline-kyc. This clears them and adds the job, so the next one shows up as a failure rather than as a backlog.

Three that were more than style

A stray (); in BacSession's Completed arm — dead, left over from the guard above it.

Two tests asserting against "IZ3".replace('3', "3"). The no-op was expressing "3 has no look-alike letter, so it passes through". That's worth saying, but not in code that reads as a mistake — it's now the literal with the reason written out.

string_extensions.rs opened with /// where it meant //!, so the module description was attached to the first function rather than to the module.

Seven allowed at the site

Each with its reason in a comment, because a silenced lint with no explanation is indistinguishable from one nobody looked at:

Lint	Why not fixed
large_enum_variant ×2	Exactly one of these exists per PACE session, so the padding is never multiplied. Boxing adds an allocation on the key-agreement path, and changes a public enum's shape.
module_inception ×2	df1::df1 and iso7816::iso7816 hold the application constants distinct from the files beside them. Renaming changes a public path in a published crate.
should_implement_trait ×2	next here is a fallible protocol step driving an APDU exchange, ending in Done rather than None. It could not be Iterator::next.
too_many_arguments	An MRZ carries eleven fields. A builder would exist only to make the number smaller.

The rest

Mechanical, applied as suggested — div_ceil, is_multiple_of, redundant closures, starts_with, index loops replaced with iterators, and nine over-indented doc continuations that rustdoc was reading as nested content.

Verification

cargo clippy --workspace --all-targets -- -D warnings clean, cargo fmt --all --check clean, 907 tests pass.

Worth knowing: the job runs on stable, not a pinned toolchain. That is the deliberate choice — a toolchain bump introducing new lints will fail this job. The alternative, pinning, trades a visible failure for a silent backlog, which is what produced this PR.

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

---

Summary by cubic

Made the workspace clippy-clean across dmrtd, mrz-parser, aamva, and aadhaar-offline-kyc, and added a CI job on stable to keep it that way.

• New Features

  • Added a clippy job to CI on stable that runs cargo clippy --workspace --all-targets -- -D warnings.
  • New lints fail the build; seven intentional cases are #[allow]ed in place with comments (e.g., module_inception, should_implement_trait, large_enum_variant, too_many_arguments) to avoid breaking public APIs or adding allocations.

• Bug Fixes

  • Removed a stray (); in BacSession’s Completed arm.
  • Fixed two tests that used a no-op "IZ3".replace('3', "3"); now assert the literal and document why 3 passes through.
  • Switched string_extensions.rs header to //! so the module docs attach to the module, not the first function.

^{Written for commit 425b3b1. Summary will update on new commits.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request adds a workspace-wide Clippy CI job and applies idiomatic Rust updates across cryptography, protocol serialization, TLV/LDS handling, MRZ/PAN parsing, documentation, lint annotations, and test helpers.

Changes

Workspace Clippy cleanup

Layer / File(s)	Summary
Clippy workflow and targeted lint handling .github/workflows/ci.yml, crates/aadhaar-offline-kyc/src/qr.rs, crates/aamva/src/pdf417.rs, crates/dmrtd/src/{lds/df1/mod.rs,proto/*pace*.rs,proto/bac_session.rs,proto/iso7816/mod.rs}, crates/mrz-parser/src/result.rs	Adds warnings-as-errors Clippy CI and targeted lint annotations or initialization simplifications.
Cryptographic validation and iterator idioms crates/dmrtd/src/crypto/*, crates/dmrtd/src/proto/{dba_key.rs,ssc.rs}, crates/dmrtd/src/utils.rs	Replaces modulo and iterator patterns with standard predicates and helpers while preserving validation behavior.
Protocol state and APDU serialization crates/dmrtd/src/proto/{bac_session.rs,mrtd_sm.rs}, crates/dmrtd/src/proto/iso7816/*	Simplifies session completion, optional-data checks, and APDU byte serialization.
LDS parameter and TLV handling crates/dmrtd/src/lds/{df1/efdg2.rs,substruct/pace_info.rs,tlv.rs}	Simplifies biometric count extraction, PACE matching, and TLV iteration.
Parser, model, and fixture simplification crates/aamva/src/parser.rs, crates/incometax-pan-qr/src/unpacker.rs, crates/mrz-parser/src/*	Simplifies parser checks, defaults, character detection, check-digit comparisons, documentation, and test fixtures.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

• wearekesk/identity-crates#3: Touches related QR, AAMVA, and MRZ parser modules.
• wearekesk/identity-crates#12: Introduces the PACE enum affected by the Clippy suppression.

Poem

A rabbit hops through Clippy’s gate,
Tidying checks that used to wait.
Bytes align and parsers gleam,
APDUs flow like carrot stream.
Rusty lint bells softly ring—
“All warnings cleared!” we sing.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: making the workspace Clippy-clean and adding a CI job to enforce it.
Description check	✅ Passed	The description is directly about the Clippy cleanup and CI job, matching the changeset.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/clippy

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[cubic-dev-ai]

All reported issues were addressed across 32 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[itsbalamurali]

Valid, pre-existing, and not from this PR — raised as #22 rather than folded in here.

This PR's only change to mrtd_sm.rs is map_or(true, …) → is_none_or(…), which is semantically identical. The guard is unchanged from main; the diff just touched the line above it.

I checked how far it actually reaches before filing. The two status arms are mostly covered by the call path: MrtdApi::transceive_cmd rejects anything that is not SUCCESS, so a 6987/6988 passthrough surfaces as an error and its body never reaches a caller.

The third arm is the sharp one. A response with status 9000 and no body satisfies data.is_none(), so it passes through unverified, and transceive_cmd sees SUCCESS and returns it as a legitimate result. For any protected command with an empty response — SELECT, MSE:Set AT — anyone who can modify the channel can strip the secure-messaging wrapper, return a bare 9000, and have the reader treat the command as having succeeded with no MAC checked. The SSC desynchronisation you describe is real too, and makes it self-limiting in practice, but a broken session is not the same as a rejected response.

Not fixed here deliberately. Tightening it changes behaviour on real hardware: if any chip in the field answers a protected exchange with a bare 9000, this will break reads that currently work, and that wants a device test rather than a lint PR. #22 carries the analysis and the suggested direction.