refactor(payload): annotate intentional-panic .expect() sites#1891
Draft
goxberry wants to merge 1 commit into
Draft
Conversation
This was referenced May 22, 2026
Contributor
Author
This comment has been minimized.
This comment has been minimized.
blt
approved these changes
May 22, 2026
Eleven functions in `lading_payload` contain `.expect()` calls that are the function's documented contract: violating the precondition is a programming error, and panicking is the intended response. Each such function gets `#[expect(clippy::expect_used, reason = "...")]` so the workspace-level lint is bypassed at the source with an explicit reason. Covered functions and the invariant each preserves: - `block::Cache::read_at` — documents in its doc comment that it panics if reads exceed machine-word bytes; the `usize::try_from(u64)` calls inside are the documented contract. - `RandomStringPool::using_handle` and `StringListPool::using_handle` — the `Handle` enum has two variants, one per pool; a handle of the wrong variant is a programming error (cross-pool misuse). - `Display::fmt` impls for `Event` (in `dogstatsd/event.rs`), `ServiceCheck` (in `dogstatsd/service_check.rs`), and `Count`, `Gauge`, `Timer`, `Dist`, `Set`, `Histogram` (all in `dogstatsd/metric.rs`) — each formats tag keys/values by handle-table lookup against `self.pools`, which issued those handles at construction time; a miss indicates an internal invariant violation. The `.expect()` calls themselves are unchanged. No runtime behavior change. Same stack as #1882–#1890. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
8215c40 to
1d0c347
Compare
dfea61d to
69742f3
Compare
This was referenced May 22, 2026
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
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.

What does this PR do?
Annotates 11 functions in
lading_payloadthat contain intentional-panic.expect()calls with#[expect(clippy::expect_used, reason = "...")]. The.expect()calls themselves are unchanged — they document a contract and panicking is the intended response.Annotated functions and the contract each preserves
block::Cache::read_atusize::try_from(u64)calls are that contract.RandomStringPool::using_handleHandlehas two variants (PosAndLength,Index); onlyPosAndLengthis valid here. A cross-variant handle is a cross-pool misuse — a programming error.StringListPool::using_handleIndexis valid here.Display::fmtforEvent(dogstatsd/event.rs),ServiceCheck(dogstatsd/service_check.rs),Count,Gauge,Timer,Dist,Set,Histogram(all indogstatsd/metric.rs)self.pools.tag_name/self.pools.tag_value. The handles were issued by those exact pools at the metric/event/service-check construction site, so the lookup cannot fail unless an internal invariant has been violated.Why
#[expect]and notunreachable!()orpanic!()For these sites the
.expect()message is the contract, and the panic is the intended behavior. Converting tounreachable!()would lose the documented contract text; converting topanic!("...")would just sidestep the lint while saying the same thing more loudly.#[expect(clippy::expect_used, reason = "...")]keeps the code untouched and records, at the source, why the lint allows it. If a future change makes the lint stop firing inside the function (e.g., all.expect()calls get removed),#[expect]itself warns and the annotation can be deleted.The
reasonstrings are tight per-site descriptions; they're not boilerplate.Motivation
Fifth per-crate cleanup PR in the stack started by #1882. Remaining sub-stack:
block.rs:123— thearbitrary::Arbitraryimpl forBlockcallsNonZeroU32::new(total_bytes).expect(...)wheretotal_bytes = u32::arbitrary(u)?can legitimately be 0. Fix: convert toarbitrary::Error::IncorrectFormatso the fuzzer rejects zero-byte inputs.lading_payloadquarantine.Related issues
Stacked on #1890.
Additional Notes
cargo build --all-targets --all-features✓cargo clippy --all-targets --all-features✓cargo test -p lading-payload --lib✓ (244 passed)