Skip to content

Commit fb3dd23

Browse files
committed
refactor(opentimstdf): eliminate panic surface in reader + py bindings (WP17)
- reader.rs: convert 2 `tdf_bin.lock().unwrap()` mutex calls to `map_err(|_| Error::CorruptFrame(.., "tdf_bin mutex poisoned"))?` so a poisoned guard surfaces as a structured error. - reader.rs / codec.rs: replace `header[a..b].try_into().unwrap()` on fixed-size header arrays with direct `[u8; 4]` literals; `chunks_exact(4)` unwraps are kept under a localized `#[allow(clippy::unwrap_used)]` with rationale (length guaranteed by the iterator contract). - mzml.rs: the two `self.spectra.as_ref().unwrap()` (populated immediately above) become `.expect("populated above")` with a localized lint allow. - opentimstdf-py: convert all 11 `inner.lock().unwrap()` sites to `.lock().map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?`, surfacing the failure to Python as a RuntimeError. - Gate `clippy::unwrap_used` / `expect_used` as warn in non-test production code at both crate roots. Refs: WP17 panic-surface sweep.
1 parent 9bd0213 commit fb3dd23

5 files changed

Lines changed: 37 additions & 19 deletions

File tree

crates/opentimstdf-py/src/lib.rs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![cfg_attr(not(test), warn(clippy::unwrap_used, clippy::expect_used))]
12
//! Python bindings for OpenTimsTDF.
23
//!
34
//! Exposes `opentimstdf.Reader`, which opens a `.d/` (TDF) bundle once and
@@ -438,14 +439,18 @@ impl Reader {
438439

439440
#[getter]
440441
fn compression_type(&self) -> PyResult<u32> {
441-
Ok(self.inner.lock().unwrap().compression_type())
442+
Ok(self
443+
.inner
444+
.lock()
445+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
446+
.compression_type())
442447
}
443448

444449
fn metadata(&self) -> PyResult<Metadata> {
445450
Ok(self
446451
.inner
447452
.lock()
448-
.unwrap()
453+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
449454
.metadata()
450455
.map_err(to_py_err)?
451456
.into())
@@ -455,7 +460,7 @@ impl Reader {
455460
let inner = self
456461
.inner
457462
.lock()
458-
.unwrap()
463+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
459464
.calibration()
460465
.map_err(to_py_err)?;
461466
Ok(Calibration { inner })
@@ -465,7 +470,7 @@ impl Reader {
465470
Ok(self
466471
.inner
467472
.lock()
468-
.unwrap()
473+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
469474
.frame(id)
470475
.map_err(to_py_err)?
471476
.into())
@@ -475,7 +480,7 @@ impl Reader {
475480
Ok(self
476481
.inner
477482
.lock()
478-
.unwrap()
483+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
479484
.frames()
480485
.map_err(to_py_err)?
481486
.into_iter()
@@ -499,7 +504,7 @@ impl Reader {
499504
Ok(self
500505
.inner
501506
.lock()
502-
.unwrap()
507+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
503508
.decode_peaks(&rs_frame)
504509
.map_err(to_py_err)?
505510
.into_iter()
@@ -511,7 +516,7 @@ impl Reader {
511516
Ok(self
512517
.inner
513518
.lock()
514-
.unwrap()
519+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
515520
.dia_windows_for_frame(frame_id)
516521
.map_err(to_py_err)?
517522
.map(Into::into))
@@ -521,7 +526,7 @@ impl Reader {
521526
Ok(self
522527
.inner
523528
.lock()
524-
.unwrap()
529+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
525530
.pasef_msms_info_for_frame(frame_id)
526531
.map_err(to_py_err)?
527532
.into_iter()
@@ -533,7 +538,7 @@ impl Reader {
533538
Ok(self
534539
.inner
535540
.lock()
536-
.unwrap()
541+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
537542
.prm_msms_info_for_frame(frame_id)
538543
.map_err(to_py_err)?
539544
.into_iter()
@@ -545,7 +550,7 @@ impl Reader {
545550
Ok(self
546551
.inner
547552
.lock()
548-
.unwrap()
553+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
549554
.prm_target(target_id)
550555
.map_err(to_py_err)?
551556
.map(Into::into))
@@ -555,7 +560,7 @@ impl Reader {
555560
Ok(self
556561
.inner
557562
.lock()
558-
.unwrap()
563+
.map_err(|_| PyRuntimeError::new_err("reader lock poisoned"))?
559564
.precursor(precursor_id)
560565
.map_err(to_py_err)?
561566
.map(Into::into))

crates/opentimstdf/src/codec.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ pub fn decode_codec1(
131131
let mut tof: u32 = 0;
132132
let mut prev_was_intensity = true;
133133
for chunk in scratch.chunks_exact(4) {
134+
// chunks_exact(4) guarantees chunk.len() == 4
135+
#[allow(clippy::unwrap_used)]
134136
let v = i32::from_le_bytes(chunk.try_into().unwrap());
135137
if v >= 0 {
136138
if prev_was_intensity {

crates/opentimstdf/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#![cfg_attr(not(test), warn(clippy::unwrap_used, clippy::expect_used))]
12
//! OpenTimsTDF - Rust reader for timsTOF `.d/` (TDF) mass spectrometry bundles.
23
//!
34
//! The format and codecs are documented in `re/SPEC.md` (and mirrored on

crates/opentimstdf/src/mzml.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,8 @@ impl<'a> TdfSource<'a> {
395395
&self.calibration,
396396
)?);
397397
}
398-
Ok(self.spectra.as_ref().unwrap())
398+
#[allow(clippy::expect_used)]
399+
Ok(self.spectra.as_ref().expect("populated above"))
399400
}
400401
}
401402

@@ -419,7 +420,8 @@ impl OwnedTdfSource {
419420
&self.calibration,
420421
)?);
421422
}
422-
Ok(self.spectra.as_ref().unwrap())
423+
#[allow(clippy::expect_used)]
424+
Ok(self.spectra.as_ref().expect("populated above"))
423425
}
424426
}
425427

crates/opentimstdf/src/reader.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -420,13 +420,16 @@ impl Reader {
420420
}
421421

422422
fn decode_peaks_codec2(&self, frame: &Frame) -> Result<Vec<Peak>> {
423-
let mut f = self.tdf_bin.lock().unwrap();
423+
let mut f = self
424+
.tdf_bin
425+
.lock()
426+
.map_err(|_| Error::CorruptFrame(frame.id, "tdf_bin mutex poisoned".into()))?;
424427
f.seek(SeekFrom::Start(frame.tims_id))?;
425428

426429
let mut header = [0u8; 8];
427430
f.read_exact(&mut header)?;
428-
let block_size = u32::from_le_bytes(header[0..4].try_into().unwrap());
429-
let scan_count = u32::from_le_bytes(header[4..8].try_into().unwrap());
431+
let block_size = u32::from_le_bytes([header[0], header[1], header[2], header[3]]);
432+
let scan_count = u32::from_le_bytes([header[4], header[5], header[6], header[7]]);
430433
if scan_count != frame.num_scans {
431434
return Err(Error::CorruptFrame(
432435
frame.id,
@@ -473,13 +476,16 @@ impl Reader {
473476
.parse()
474477
.unwrap_or(0);
475478

476-
let mut f = self.tdf_bin.lock().unwrap();
479+
let mut f = self
480+
.tdf_bin
481+
.lock()
482+
.map_err(|_| Error::CorruptFrame(frame.id, "tdf_bin mutex poisoned".into()))?;
477483
f.seek(SeekFrom::Start(frame.tims_id))?;
478484

479485
let mut header = [0u8; 8];
480486
f.read_exact(&mut header)?;
481-
let bin_size = u32::from_le_bytes(header[0..4].try_into().unwrap());
482-
let scan_count = u32::from_le_bytes(header[4..8].try_into().unwrap());
487+
let bin_size = u32::from_le_bytes([header[0], header[1], header[2], header[3]]);
488+
let scan_count = u32::from_le_bytes([header[4], header[5], header[6], header[7]]);
483489
if scan_count != frame.num_scans {
484490
return Err(Error::CorruptFrame(
485491
frame.id,
@@ -505,6 +511,8 @@ impl Reader {
505511
f.read_exact(&mut raw_offsets)?;
506512
let mut scan_offsets = Vec::with_capacity(scan_count as usize + 1);
507513
for chunk in raw_offsets.chunks_exact(4) {
514+
// chunks_exact(4) guarantees chunk.len() == 4
515+
#[allow(clippy::unwrap_used)]
508516
let o = u32::from_le_bytes(chunk.try_into().unwrap());
509517
scan_offsets.push(o.saturating_sub(compression_offset) as usize);
510518
}

0 commit comments

Comments
 (0)