feat(harden): activate Clippy 1.94/1.95 ratchets (#191)#227
Conversation
…lippy (#191) Eight planned lints move from [[planned]] to [[active]] in policy/clippy-lints.toml and become live in [workspace.lints.clippy]: same_length_and_capacity (deny) manual_ilog2 (warn) decimal_bitwise_operands (warn) needless_type_cast (warn) manual_checked_ops (warn) manual_take (warn) unnecessary_trailing_comma (warn) disallowed_fields (deny) All eight measured zero workspace fallout at activation (cargo clippy --workspace --all-targets -- -D warnings is clean). duration_suboptimal_units stays in [[planned]] with measured fallout of 204 sites across 20 files (config/runtime/retry/types/lock APIs that accept user-supplied millisecond values). A focused cleanup PR will revisit it; an unconditional warn would create permanent noise. manual_pop_if from the original #191 plan is not a real clippy lint in 1.95. unknown_lints = "deny" (newly added to [workspace.lints.rust] for the same reason it caught the #198 rustc fabrications) rejected it via E0602. The fabrication is recorded in the ledger so future readers can see the omission was intentional, not an oversight. cargo xtask check-lint-policy now reports workspace.lints.clippy=8 active_in_ledger=12 planned_in_ledger=1 with MSRV aligned at 1.95 across Cargo.toml, clippy.toml, and the ledger.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Droid finished @EffortlessSteven's task —— View job Droid Review — Phase 2 Validation Complete ✅PR #227 — feat/clippy-ratchet-activation-191 Validation result
SummaryNo actionable findings emitted. Inspected surfaces:
Checks performed:
Why no comments: Residual risk:
Validation signal:
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d65d9d56d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| manual_checked_ops = "warn" | ||
| manual_take = "warn" | ||
| unnecessary_trailing_comma = "warn" | ||
| disallowed_fields = "deny" |
There was a problem hiding this comment.
Configure protected fields before activating lint
Observed: clippy.toml only sets MSRV/thresholds and has no disallowed-fields entries, and a local Clippy 0.1.95 smoke test with disallowed_fields = "deny" still allowed direct public field access when no field list was configured. Failure mode: this line marks the protected-seam lint as active, but it currently enforces no seams, so new direct accesses across the state/receipt/token boundaries can still pass CI while the ledger says they are banned. Fix direction: either keep this lint planned until the seam list lands, or add the explicit disallowed-fields configuration and coverage in the same activation PR. Validation: CARGO_NET_OFFLINE=true cargo clippy -- -D warnings on a minimal crate with this lint and no config. Confidence: high.
Useful? React with 👍 / 👎.
Summary
[[planned]]to[[active]]inpolicy/clippy-lints.tomland become live in[workspace.lints.clippy]:same_length_and_capacity(deny),manual_ilog2,decimal_bitwise_operands,needless_type_cast,manual_checked_ops,manual_take,unnecessary_trailing_comma,disallowed_fields(deny).cargo clippy --workspace --all-targets -- -D warningsis clean.duration_suboptimal_unitsstays[[planned]]with a measured-fallout note (204 sites across 20 files, mostly config/runtime/retry/types/lock APIs that accept user-supplied millisecond values). A focused cleanup PR will revisit; an unconditional warn would create permanent noise across stable APIs.manual_pop_iffrom the original PR 7: Clippy 1.94/1.95 ratchets #191 plan turned out not to be a real clippy lint in 1.95. Addedunknown_lints = \"deny\"to[workspace.lints.rust](same posture PR 6: Rust 1.95 rustc lint floor #198 took to catch fabricated rustc lints) and it rejected the name via E0602. The fabrication is recorded in the ledger so future readers can see the omission was intentional.Lint-policy state after this PR
```
cargo xtask check-lint-policy
msrv aligned across all three: 1.95
check-lint-policy: workspace.lints.clippy=8 active_in_ledger=12 planned_in_ledger=1
```
(
active_in_ledger=12= 4 pre-existing + 8 newly activated.planned_in_ledger=1=duration_suboptimal_units.)Why activate eight and defer one
The general rollout posture is no surprise enforcement — an active lint shouldn't be the first thing the repo learns about its own fallout. For the eight zero-fallout lints, activation is a free ratchet. For
duration_suboptimal_units, the 204 hits cluster in config/types APIs whereDuration::from_millis(timeout_ms)is the natural ergonomics and a literal-to-larger-unit rewrite would obscure rather than clarify some sites; that warrants a per-site judgment pass, not a blanket warn.Test plan
cargo xtask check-lint-policy— MSRV aligned, workspace.lints.clippy ↔ ledger coverage cleancargo clippy --workspace --all-targets -- -D warnings— clean (exit 0)cargo build --workspace --all-targets— clean (nounknown_lintsregressions from the new rust-side deny)cargo fmt --all -- --check— cleancargo xtask policy-report— all 7 advisory areas cleancargo xtask check-clippy-exceptions— 0 expired, 0 schema errors (16 informational bare-allow sites pre-existing; tracked separately)Closes #191 partially (eight of nine planned 1.94/1.95 ratchets activate here;
duration_suboptimal_unitscarries over to a focused cleanup PR).