diff --git a/CHANGELOG.md b/CHANGELOG.md index e0a51fe00..e8075d268 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +- Retired all production `.expect()` sites in `lading_capture` and dropped + both crate-root quarantines (`lading_capture/src/lib.rs` and + `lading_capture/src/bin/fuzz_capture_harness.rs`). Treatment mix: + 7 fn-level `#[expect(clippy::expect_used, reason = "...")]` for + contract-style invariants (test/fuzz helper in `test/writer.rs`, + state-machine `format` field in `manager/state_machine.rs`, + `CaptureManager::start` consuming `self.shutdown`), and + 4 site-level `.unwrap_or_else(... unreachable!("..."))` for + structural infallibles (`SystemTime` post-UNIX_EPOCH in + `RealClock::now_ms`, prost serialization of in-memory `Dogsketch` + values in `accumulator.rs` ×2, `OpIterator::next()` on an infinite + iterator in `bin/fuzz_capture_harness.rs`). No runtime behavior + change. - Retired the remaining 17 production `.expect()` sites in `lading_payload` and dropped the crate-root `#![allow(clippy::expect_used)]` quarantine. 13 sites became `.unwrap_or_else(... unreachable!("..."))` (structural diff --git a/lading_capture/src/accumulator.rs b/lading_capture/src/accumulator.rs index 34664b67e..287fc6cc4 100644 --- a/lading_capture/src/accumulator.rs +++ b/lading_capture/src/accumulator.rs @@ -227,9 +227,11 @@ impl Iterator for DrainIter { if sketch.count() > 0 { let mut dogsketch = Dogsketch::new(); sketch.merge_to_dogsketch(&mut dogsketch); - let sketch_bytes = dogsketch - .write_to_bytes() - .expect("protobuf serialization failed"); + let sketch_bytes = dogsketch.write_to_bytes().unwrap_or_else(|_| { + unreachable!( + "prost::Message::write_to_bytes on an in-memory Dogsketch cannot fail" + ) + }); metrics.push(( key.clone(), MetricValue::Histogram(sketch_bytes), @@ -522,9 +524,11 @@ impl Accumulator { if sketch.count() > 0 { let mut dogsketch = Dogsketch::new(); sketch.merge_to_dogsketch(&mut dogsketch); - let sketch_bytes = dogsketch - .write_to_bytes() - .expect("protobuf serialization failed"); + let sketch_bytes = dogsketch.write_to_bytes().unwrap_or_else(|_| { + unreachable!( + "prost::Message::write_to_bytes on an in-memory Dogsketch cannot fail" + ) + }); metrics.push(( key.clone(), MetricValue::Histogram(sketch_bytes), diff --git a/lading_capture/src/bin/fuzz_capture_harness.rs b/lading_capture/src/bin/fuzz_capture_harness.rs index 1331da56d..37b0bdb93 100644 --- a/lading_capture/src/bin/fuzz_capture_harness.rs +++ b/lading_capture/src/bin/fuzz_capture_harness.rs @@ -7,9 +7,6 @@ #![expect(clippy::print_stderr)] #![allow(clippy::cast_precision_loss)] -// Quarantine: workspace denies `clippy::expect_used`, but this binary still has -// production `.expect()` sites awaiting cleanup. Remove once cleaned up. -#![allow(clippy::expect_used)] use anyhow::{Context, Result}; use arbitrary::Arbitrary; @@ -389,7 +386,9 @@ async fn run_capture_manager(config: &FuzzInput) -> Result { // Rip through operations, tracking total time advanced while elapsed_ms < runtime_ms { - let op = op_iter.next().expect("infinite iterator"); + let op = op_iter + .next() + .unwrap_or_else(|| unreachable!("OpIterator::next() always returns Some")); if let Op::AdvanceTime { millis } = op { elapsed_ms += millis; } diff --git a/lading_capture/src/lib.rs b/lading_capture/src/lib.rs index c0a1b4c5a..a3ad883e1 100644 --- a/lading_capture/src/lib.rs +++ b/lading_capture/src/lib.rs @@ -1,9 +1,5 @@ //! Crate regarding Lading's 'capture' files -// Quarantine: workspace denies `clippy::expect_used`, but this crate still has -// production `.expect()` sites awaiting cleanup. Remove once cleaned up. -#![allow(clippy::expect_used)] - use std::time::Instant; use manager::HISTORICAL_SENDER; diff --git a/lading_capture/src/manager.rs b/lading_capture/src/manager.rs index ece657684..bea50da57 100644 --- a/lading_capture/src/manager.rs +++ b/lading_capture/src/manager.rs @@ -233,7 +233,11 @@ impl Clock for RealClock { let elapsed = now.duration_since(self.start_instant); (self.start_system_time + elapsed) .duration_since(UNIX_EPOCH) - .expect("system time is before UNIX_EPOCH") + .unwrap_or_else(|_| { + unreachable!( + "RealClock::start_system_time is captured at program start in modern epochs, well after UNIX_EPOCH" + ) + }) .as_millis() } @@ -353,6 +357,10 @@ impl CaptureManager { /// /// Will return an error if there is already a global recorder set. #[expect(clippy::cast_possible_truncation)] + #[expect( + clippy::expect_used, + reason = "self.shutdown is populated at CaptureManager construction and consumed exactly once here in start()" + )] pub async fn start(mut self) -> Result<(), Error> { // Initialize historical sender to allow generators to send metrics with // Instant timestamps. Manager converts these to ticks using clock.start() diff --git a/lading_capture/src/manager/state_machine.rs b/lading_capture/src/manager/state_machine.rs index 5f9f1dc24..f7cb3ddcf 100644 --- a/lading_capture/src/manager/state_machine.rs +++ b/lading_capture/src/manager/state_machine.rs @@ -265,6 +265,10 @@ impl StateMachine { } /// Record all current metrics from the registry and flush mature data + #[expect( + clippy::expect_used, + reason = "self.format is Some throughout the operating state of the state machine; a None here indicates a serious upstream logic bug" + )] fn record_captures(&mut self, now: Instant) -> Result<(), Error> { let tick = self.accumulator.current_tick; @@ -323,6 +327,10 @@ impl StateMachine { } /// Drain all accumulated metrics and write them to the capture file + #[expect( + clippy::expect_used, + reason = "self.format is Some throughout the operating state of the state machine; a None here indicates a serious upstream logic bug" + )] fn drain_and_write(&mut self) -> Result<(), Error> { // Replace the accumulator with a new one and consume the old one for draining // This is only called during shutdown, so we don't need the accumulator anymore @@ -346,6 +354,10 @@ impl StateMachine { } #[allow(clippy::too_many_lines)] + #[expect( + clippy::expect_used, + reason = "self.format is Some throughout the operating state of the state machine; a None here indicates a serious upstream logic bug" + )] fn write_metric_line( &mut self, key: &Key, diff --git a/lading_capture/src/test/writer.rs b/lading_capture/src/test/writer.rs index 0170adb4b..65bc5d867 100644 --- a/lading_capture/src/test/writer.rs +++ b/lading_capture/src/test/writer.rs @@ -30,6 +30,10 @@ impl InMemoryWriter { /// /// Panics if the mutex is poisoned #[must_use] + #[expect( + clippy::expect_used, + reason = "mutex poisoning in this test/fuzz helper is the documented contract above" + )] pub fn get_bytes(&self) -> Vec { self.buffer.lock().expect("mutex poisoned").clone() } @@ -40,6 +44,10 @@ impl InMemoryWriter { /// /// Panics if the buffer contains invalid UTF-8 #[must_use] + #[expect( + clippy::expect_used, + reason = "invalid UTF-8 in this test/fuzz helper is the documented contract above" + )] pub fn get_string(&self) -> String { String::from_utf8(self.get_bytes()).expect("buffer contains invalid UTF-8") } @@ -53,6 +61,10 @@ impl InMemoryWriter { /// # Panics /// /// Panics if the mutex is poisoned + #[expect( + clippy::expect_used, + reason = "mutex poisoning in this test/fuzz helper is the documented contract above" + )] pub fn parse_lines(&self) -> Result, serde_json::Error> { let buffer = self.buffer.lock().expect("mutex poisoned"); let content_str = String::from_utf8_lossy(&buffer); @@ -71,6 +83,10 @@ impl Default for InMemoryWriter { } impl Write for InMemoryWriter { + #[expect( + clippy::expect_used, + reason = "mutex poisoning in this test/fuzz helper is treated as a fatal error per the type's documented contract" + )] fn write(&mut self, buf: &[u8]) -> std::io::Result { self.buffer .lock()