chore(clippy): forbid .expect() in production code#1882
Conversation
Adds `clippy::expect_used = "deny"` to the workspace lint set, mirroring the existing `unwrap_used` policy. Configures `allow-expect-in-tests = true` in clippy.toml so `#[cfg(test)]` modules remain free to use `.expect()`. Pre-existing production `.expect()` sites are quarantined behind a transitional `#![allow(clippy::expect_used)]` at the crate root of the four affected library crates (lading, lading_capture, lading_payload, lading_throttle). The lint fires immediately on new sites in any other location. Cleanup PRs will remove each crate's quarantine atomically with its violations. Benches in lading_payload/benches/ get a permanent crate-root allow because each bench is a separate crate root and clippy has no `allow-expect-in-benches` knob to mirror the in-tests one. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Implementation planWritten before the changes were applied; preserved here as PR context. PlanEnforce no
|
| Crate | Production .expect() calls |
|---|---|
| lading_throttle | 28 |
| lading_capture | 149 |
| lading_payload | 164 |
| lading | 243 |
| lading_capture_schema | 0 |
| lading_signal | 0 |
| lading_fuzz | 0 |
Fixing all 602 sites in a single PR would be a multi-thousand-line
refactor. We will instead deliver the enforcement via a stack of PRs,
of which this branch is the first.
This branch lands only the mechanism at warn level. Subsequent
per-crate cleanup PRs eliminate the sites; a final PR upgrades the lint
to deny, producing deterministic enforcement.
Approach for this branch
Feature branch name: enforce-no-expect-in-production.
Two file edits, no code changes:
1. Cargo.toml — add the lint at warn
Under [workspace.lints.clippy], alongside the existing
unwrap_used = "deny" line, add:
expect_used = "warn"Placement: directly after the unwrap_used line so the two related
gates sit together.
Rationale for warn, not deny: the workspace cannot pass clippy with
602 violations. warn makes every site visible (so reviewers and
cleanup PRs can see them) without breaking CI. The level is upgraded
to deny in the final PR of the stack, after the per-crate cleanups
land.
2. clippy.toml — exempt test code
Add immediately after the existing allow-unwrap-in-tests = true
line:
allow-expect-in-tests = trueThis exempts .expect() calls inside #[cfg(test)] modules and
#[test] functions, mirroring the existing unwrap policy. No other
config change is required — clippy's defaults already exempt
tests/, benches/, and examples/ directories.
Scope
[workspace.lints.clippy] is inherited only by crates that declare
[lints] workspace = true. Verified opt-ins:
lading/Cargo.tomllading_payload/Cargo.tomllading_throttle/Cargo.tomllading_capture/Cargo.tomllading_capture_schema/Cargo.tomllading_signal/Cargo.tomllading_fuzz/Cargo.toml
Integration crates (integration/sheepdog, integration/ducks,
integration/shared) do not opt in, so they remain free to use
.expect(). This matches the user's stated scope choice
("library crates only") and is consistent with the current unwrap_used
behavior.
What this branch deliberately does NOT do
- Does not modify any
.rssource file. - Does not change CI scripts (
ci/clippyis unchanged; it will still
runcargo clippy --all-targets --all-featuresand pass). - Does not introduce a baseline counter or regression guard. New
.expect()calls in production will surface as warnings, not CI
failures, until the final PR in the stack upgrades the level. - Does not touch the
.unwrap()gate — it is already atdeny.
Verification
After the edits, run from the repo root:
cargo clippy --all-targets --all-features— must succeed
(warnings allowed, errors not).cargo clippy --all-targets --all-features 2>&1 | grep -c 'clippy::expect_used'
— count should be in the same order of magnitude as 602 (exact
number may vary slightly depending on how clippy groups
occurrences, but it must be > 0 to confirm the lint is active).cargo clippy --all-targets --all-features --tests 2>&1 | grep -c 'clippy::expect_used'
— count under test-only configuration should be unchanged or lower
relative to step 2, confirmingallow-expect-in-tests = trueis
honored. (Test-only.expect()calls in#[cfg(test)]modules
should not appear.)- Run
bash ci/clippy— confirm the existing CI script still passes. cargo build --all-targets --all-features— sanity-check
compilation is unaffected.cargo test --workspace— sanity-check tests still pass (no
behavior change expected; this guards against accidental config
typos breaking the build).
Commit / PR
- Commit message (Conventional Commits per global CLAUDE.md):
chore(clippy): add expect_used lint at warn level - PR title:
chore(clippy): forbid .expect() in production (mechanism, warn-level) - PR description should document the stacked-PR plan below so reviewers understand why the lint lands at
warn.
Follow-up PRs (not in this branch)
To be opened sequentially after this branch merges:
- lading_throttle cleanup (28 sites) — smallest, good first.
- lading_payload cleanup (164 sites).
- lading_capture cleanup (149 sites).
- lading cleanup (243 sites).
- Lock the gate: change
expect_used = "warn"to
expect_used = "deny"inCargo.toml. After this PR merges,
enforcement is deterministic.
Each cleanup PR should:
- Convert
.expect("msg")to?-propagation where the call site can
returnResult. Reuse existingthiserrorerror enums; extend them
with new variants when needed. - For sites that genuinely cannot recover (e.g., binary-startup
invariants), preferpanic!("...")with explicit context —panic!
itself is not denied byclippy::expect_used. - For test helpers that live under
src/but are only used by tests
(e.g.,lading_capture/src/test/writer.rs), gate the module with
#[cfg(test)]soallow-expect-in-testscovers it. - Add
#[expect(clippy::expect_used, reason = "...")]only as a last
resort and only with a real, specific reason.
Critical files
/home/bits/go/src/github.com/DataDog/lading/Cargo.toml— add lint
at workspace level./home/bits/go/src/github.com/DataDog/lading/clippy.toml— add
allow-expect-in-tests = true.

What does this PR do?
Lands the mechanism that deterministically enforces no
.expect()calls in production code, mirroring the existingclippy::unwrap_usedpolicy.Concretely:
Cargo.toml: addsexpect_used = "deny"to[workspace.lints.clippy], next to the existingunwrap_used = "deny".clippy.toml: addsallow-expect-in-tests = true, alongside the existingallow-unwrap-in-tests..expect()inside#[cfg(test)]modules and#[test]functions remains permitted.#![allow(clippy::expect_used)]is added at the crate root of each of the four library crates that already contain production.expect()sites:lading/src/lib.rs,lading/src/bin/lading.rs,lading/src/bin/captool/main.rs,lading/src/bin/payloadtool.rslading_capture/src/lib.rs,lading_capture/src/bin/fuzz_capture_harness.rslading_payload/src/lib.rslading_throttle/src/lib.rs#![allow(clippy::expect_used)]is added to each of the 14 files inlading_payload/benches/. Each bench is a separate crate root, and clippy has noallow-expect-in-benchesequivalent ofallow-expect-in-tests. The workspace'swarnings = "deny"(under[workspace.lints.rust]) promotes any clippy warning to a hard error, so a per-bench allow is the minimal targeted fix that keeps full clippy coverage on the rest of the bench code.CHANGELOG.md: entry added under## Unreleased.The gate fires immediately on any new
.expect()outside#[cfg(test)]in any non-quarantined location. New.expect()calls added inside the quarantined crates will not fire — that visibility returns crate-by-crate as cleanup PRs land.Motivation
.unwrap()was already gated atdenyworkspace-wide, but.expect()was not — leaving an easy way to introduce production panics. A targeted survey of the workspace turned up 602 production.expect()sites across four library crates:.expect()sitesladinglading_payloadlading_capturelading_throttlelading_capture_schema/lading_signal/lading_fuzzFixing all 602 in a single PR would be a multi-thousand-line refactor. This PR ships only the mechanism plus per-crate quarantines, so the gate is live today for new code and for the three already-clean crates, and the cleanup work can be split into one focused PR per crate.
Stacked PR plan
This PR is the first in a stack. Follow-ups (smallest first):
lading_throttlecleanup (28 sites)lading_payloadcleanup (164 sites)lading_capturecleanup (149 sites)ladingcleanup (243 sites)allow-expect-in-benchesequivalent (optional).Each cleanup PR removes one crate's
#![allow(clippy::expect_used)]atomically with its violations. Strategy per site: convert to?-propagation where the call site can returnResult(extendingthiserrorenums as needed); usepanic!("...")only for genuinely unrecoverable startup invariants; gate test helpers that live undersrc/(e.g.,lading_capture/src/test/writer.rs) with#[cfg(test)]soallow-expect-in-testscovers them.After the four cleanup PRs land, the workspace allow is already at
deny— no further config change needed.Related issues
None.
Additional Notes
.expect()tolading_capture_schema/src/lib.rs(a non-quarantined crate with zero existing sites): clippy errored as expected. A second smoke test confirmed that.expect()inside a#[cfg(test)]block is correctly exempted byallow-expect-in-tests. Both smoke tests were reverted before commit.ci/validatepasses locally (shellcheck, fmt, check, custom lints, clippy, deny, config validation, nextest, loom forlading-signal, kani forlading_throttle, buf).