From 4ea29ec9ded66035ada0030c2913ac15221afc16 Mon Sep 17 00:00:00 2001 From: sylph-pi Date: Sat, 5 Sep 2026 16:59:57 +0200 Subject: [PATCH] fix(formats): clear all 43 clippy lints in sylpheed-formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run 204 gave this repository its first clippy measurement — 48 errors, 43 of them in `sylpheed-formats`. This clears that 43 to zero under the exact invocation CI runs, `cargo clippy -p sylpheed-formats -- -D warnings`. Why this crate first, and why it is safe to touch: Every one of the 43 sites was checked against the line ranges that `auto/frame-blend-draw-path` (495 commits) and `auto/port-p6-audio` (366) actually modify. None of them overlap. Eleven of the fifteen affected files are byte-identical on both branches, including `mesh.rs` and `texture.rs`, which carry 28 of the hits between them. The three sites in `audio.rs`, `ui_layout.rs` and `slb.rs` that live in files those branches do change fall outside every modified hunk. The collision argument that defers #12 does not transfer here; it was tested rather than assumed. It also unblocks a measurement. `-D warnings` turns a lint in this crate into a hard compile error, so its dependents never build — `sylpheed-cli` and `sylpheed-viewer` have never been linted at all, and viewer is the largest crate in the workspace. Both depend only on `sylpheed-formats` (`sylpheed-export` pins it from a git tag instead), so this commit is what makes their real counts knowable. 38 applied by `cargo clippy --fix` — chunks_exact_to_as_chunks, manual_div_ceil / is_multiple_of / range_contains, unnecessary_map_or, needless_borrow, let_and_return, dead_code, unused_mut/variables. Purely local expression rewrites: 38 insertions, 39 deletions. 2 by hand: a doc continuation that markdown was parsing as a list, and `d / frame` behind a `frame > 0` guard becoming `checked_div`. 3 `#[allow(clippy::too_many_arguments)]` with a stated reason. On those three allows: 8 parameters against a threshold of 7, in the mesh anchor path. The real fix is a shared params struct across `anchor_pool_mesh`, `validate_block` and `validate_block_report` — the latter two take the same eight arguments and one delegates to the other — which is a change to the decoder's signatures and belongs to whoever owns that path, not to a CI-lint pass. This is not the shape PROTOCOL.md forbids. `continue-on-error` suppresses everything, present and future, at the job level, and cannot tell "not yet" from "no longer". A site-local `#[allow]` with a reason is a decision recorded where it applies: one lint, one function, and any new violation anywhere else still fails the build. `sylpheed-export`'s remaining 5 are deliberately untouched — three of them sit inside hunks both long-lived branches modify, and that crate blocks nothing. Left for #13. Refs #13 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01McNbzUeq1KRBWs4G6X2YVj --- crates/sylpheed-formats/src/audio.rs | 11 ++--- crates/sylpheed-formats/src/game_data.rs | 2 +- crates/sylpheed-formats/src/hash.rs | 2 +- crates/sylpheed-formats/src/ixud.rs | 2 +- crates/sylpheed-formats/src/mesh.rs | 43 ++++++++++++------- crates/sylpheed-formats/src/movie_manifest.rs | 2 +- crates/sylpheed-formats/src/movie_voice.rs | 2 +- crates/sylpheed-formats/src/ship_capture.rs | 4 +- crates/sylpheed-formats/src/slb.rs | 2 +- crates/sylpheed-formats/src/texture.rs | 22 +++++----- crates/sylpheed-formats/src/ui_layout.rs | 2 +- crates/sylpheed-formats/src/vfs.rs | 2 +- crates/sylpheed-formats/src/xiso.rs | 2 +- 13 files changed, 56 insertions(+), 42 deletions(-) diff --git a/crates/sylpheed-formats/src/audio.rs b/crates/sylpheed-formats/src/audio.rs index 90bd0ee0..74448f2f 100644 --- a/crates/sylpheed-formats/src/audio.rs +++ b/crates/sylpheed-formats/src/audio.rs @@ -226,9 +226,10 @@ fn parse_riff_wave(bytes: &[u8]) -> Option { match codec { AudioCodec::Pcm | AudioCodec::PcmFloat => { if let (Some(d), true) = (data_bytes, channels > 0 && bits > 0) { + // `bits` under 8 makes the frame size zero, so this stays + // fallible: `checked_div` states that once, where it happens. let frame = channels as u64 * (bits as u64 / 8); - if frame > 0 { - let spc = d / frame; + if let Some(spc) = d.checked_div(frame) { info.samples_per_channel = Some(spc); if rate > 0 { info.duration_secs = Some(spc as f32 / rate as f32); @@ -275,11 +276,11 @@ impl GameAudio { let samples: Vec = match (info.codec, bits) { (AudioCodec::Pcm, 8) => data.iter().map(|&b| (b as f32 - 128.0) / 128.0).collect(), (AudioCodec::Pcm, 16) => data - .chunks_exact(2) + .as_chunks::<2>().0.iter() .map(|c| i16::from_le_bytes([c[0], c[1]]) as f32 / 32768.0) .collect(), (AudioCodec::Pcm, 24) => data - .chunks_exact(3) + .as_chunks::<3>().0.iter() .map(|c| { let v = ((c[2] as i32) << 16) | ((c[1] as i32) << 8) | c[0] as i32; let v = (v << 8) >> 8; // sign-extend 24→32 @@ -287,7 +288,7 @@ impl GameAudio { }) .collect(), (AudioCodec::PcmFloat, 32) => data - .chunks_exact(4) + .as_chunks::<4>().0.iter() .map(|c| f32::from_le_bytes([c[0], c[1], c[2], c[3]])) .collect(), _ => return Err(AudioError::UnsupportedPcm { tag: 0, bits }), diff --git a/crates/sylpheed-formats/src/game_data.rs b/crates/sylpheed-formats/src/game_data.rs index 08dcbbab..c49b2f8c 100644 --- a/crates/sylpheed-formats/src/game_data.rs +++ b/crates/sylpheed-formats/src/game_data.rs @@ -1131,7 +1131,7 @@ pub fn load_squadrons(pak: &PakArchive) -> Vec { let Some(f) = records.record(id) else { continue }; let slots: Vec<&str> = f.positional.iter().map(|(_, v)| v.as_str()).collect(); let members = slots - .chunks_exact(4) + .as_chunks::<4>().0.iter() .map(|m| SquadronMember { unit: m[0].to_string(), message_set: text(Some(m[1])), diff --git a/crates/sylpheed-formats/src/hash.rs b/crates/sylpheed-formats/src/hash.rs index 7303f1ed..82ac0843 100644 --- a/crates/sylpheed-formats/src/hash.rs +++ b/crates/sylpheed-formats/src/hash.rs @@ -71,7 +71,7 @@ pub fn name_hash(name: &str) -> u32 { // through untouched, including any high-bit bytes, which are then sign-extended). let mut bytes = name.as_bytes().to_vec(); for byte in &mut bytes { - if (b'A'..=b'Z').contains(byte) { + if byte.is_ascii_uppercase() { *byte += 0x20; } } diff --git a/crates/sylpheed-formats/src/ixud.rs b/crates/sylpheed-formats/src/ixud.rs index 6589705a..fd4db12f 100644 --- a/crates/sylpheed-formats/src/ixud.rs +++ b/crates/sylpheed-formats/src/ixud.rs @@ -119,7 +119,7 @@ pub fn parse(bytes: &[u8]) -> Option { fn utf16be_tokens(bytes: &[u8]) -> Vec { let mut tokens = Vec::new(); let mut cur = String::new(); - for pair in bytes.chunks_exact(2) { + for pair in bytes.as_chunks::<2>().0 { let u = u16::from_be_bytes([pair[0], pair[1]]); match char::from_u32(u as u32) { Some(c) if !c.is_control() => cur.push(c), diff --git a/crates/sylpheed-formats/src/mesh.rs b/crates/sylpheed-formats/src/mesh.rs index 84fa4fbf..a2da4384 100644 --- a/crates/sylpheed-formats/src/mesh.rs +++ b/crates/sylpheed-formats/src/mesh.rs @@ -700,7 +700,7 @@ impl Xbg7Model { let (vc, ic) = r.markers[0]; let sig = (r.decl.stride, vc, ic); let floor = last_by_sig.get(&sig).map_or(0, |o| o + 1); - if m.meshes[0].vbuf_offset.map_or(false, |o| o < floor) { + if m.meshes[0].vbuf_offset.is_some_and(|o| o < floor) { if let Some(alt) = anchor_pool_mesh( bytes, starts, @@ -1339,6 +1339,13 @@ fn vertex_run_starts(bytes: &[u8], data_base: usize, stride: usize) -> Vec bytes.len() - || vc.checked_mul(decls[i].stride).map_or(true, |b| vb + b > bytes.len()) + || vc.checked_mul(decls[i].stride).is_none_or(|b| vb + b > bytes.len()) { break; } @@ -1784,7 +1798,7 @@ fn anchor_grouped_meshes( } let (degen, wind) = index_run_quality(bytes, ib_k, vb_k, ick, &decls[kmax]); let cand = (degen, -wind, pad); - if best.map_or(true, |b| cand < b) { + if best.is_none_or(|b| cand < b) { best = Some(cand); } if pad_first_match() { @@ -1797,7 +1811,7 @@ fn anchor_grouped_meshes( if meshes.len() == n { return meshes; } - if partial.as_ref().map_or(true, |p| meshes.len() > p.len()) { + if partial.as_ref().is_none_or(|p| meshes.len() > p.len()) { partial = Some(meshes); } } @@ -1929,7 +1943,7 @@ fn find_index_marker(desc: &[u8]) -> Option<(usize, usize)> { while rel + 40 <= desc.len() { let a = be32(desc, rel); let c = be32(desc, rel + 4); - if c >= 3 && c % 3 == 0 && c < 400_000 && a == c * 2 { + if c >= 3 && c.is_multiple_of(3) && c < 400_000 && a == c * 2 { return Some((rel, c as usize)); } rel += 4; @@ -1951,7 +1965,7 @@ fn all_index_markers(desc: &[u8]) -> Vec<(usize, usize)> { while rel + 8 <= desc.len() { let a = be32(desc, rel); let c = be32(desc, rel + 4); - if c >= 3 && c % 3 == 0 && c < 400_000 && a == c * 2 && rel >= 32 { + if c >= 3 && c.is_multiple_of(3) && c < 400_000 && a == c * 2 && rel >= 32 { let vc = be32(desc, rel - 32) as usize; if (3..=200_000).contains(&vc) { out.push((vc, c as usize)); @@ -1979,7 +1993,7 @@ fn all_vertex_decls(desc: &[u8]) -> Vec { while rel + 8 <= desc.len() { let a = be32(desc, rel); let c = be32(desc, rel + 4); - if c >= 3 && c % 3 == 0 && c < 400_000 && a == c * 2 && rel >= 32 { + if c >= 3 && c.is_multiple_of(3) && c < 400_000 && a == c * 2 && rel >= 32 { let vc = be32(desc, rel - 32) as usize; if (3..=200_000).contains(&vc) { match parse_decl_at(desc, rel + 8) { @@ -2086,9 +2100,8 @@ fn submesh_records(desc: &[u8]) -> Vec<(usize, usize)> { let t = be32(desc, rel + 12); if (3..=65535).contains(&a) && z == 0 - && c >= 3 - && c <= 200_000 - && c % 3 == 0 + && (3..=200_000).contains(&c) + && c.is_multiple_of(3) && (1..=64).contains(&t) { out.push((a as usize, c as usize)); @@ -2151,7 +2164,7 @@ fn topology_report( let mut agree = 0usize; let mut counted = 0usize; let mut edges: HashMap<(u32, u32), u32> = HashMap::new(); - for t in indices.chunks_exact(3) { + for t in indices.as_chunks::<3>().0 { let (a, b, c) = (t[0] as usize, t[1] as usize, t[2] as usize); if a == b || b == c || a == c { degen += 1; @@ -2330,7 +2343,7 @@ pub fn submesh_albedos(bytes: &[u8], resource_name: &str) -> Vec<(usize, usize, if (3..=65535).contains(&vtx) && z == 0 && idx >= 3 - && idx % 3 == 0 + && idx.is_multiple_of(3) && idx < 400_000 && tail > 0 && tail < 0x10_0000 @@ -2423,7 +2436,7 @@ pub fn material_groups(bytes: &[u8], resource_name: &str, target: usize) -> Vec< if (1..=70000).contains(&vtx) && tail == 4 && cnt >= 3 - && cnt % 3 == 0 + && cnt.is_multiple_of(3) && cnt < 400_000 && (off as usize) < 4_000_000 { diff --git a/crates/sylpheed-formats/src/movie_manifest.rs b/crates/sylpheed-formats/src/movie_manifest.rs index 30c5fb5e..1210c30c 100644 --- a/crates/sylpheed-formats/src/movie_manifest.rs +++ b/crates/sylpheed-formats/src/movie_manifest.rs @@ -2,7 +2,7 @@ //! table (statically reversed). Beyond the movie→voice binding, it defines the //! whole cutscene *structure*: which video (+ subtitle, + on-screen text overlay, //! + voice) plays at each mission phase, and of what kind (story intro, in-mission -//! phase cutscene, resupply, …). +//! phase cutscene, resupply, …). //! //! ## On-disc structure (`dat/tables.pak`, entry hash `0x5b983a08`) //! diff --git a/crates/sylpheed-formats/src/movie_voice.rs b/crates/sylpheed-formats/src/movie_voice.rs index a28572cc..3a042d34 100644 --- a/crates/sylpheed-formats/src/movie_voice.rs +++ b/crates/sylpheed-formats/src/movie_voice.rs @@ -96,7 +96,7 @@ pub fn find_descriptor_before(buf: &[u8], before: usize) -> Option { let mut o = (before - 1).min(cap) & !3; loop { let id = be(o); - if id >= 1 && id < ID_MAX && be(o + 4) == DESC_MARK && be(o + DESC_REPEAT) == id { + if (1..ID_MAX).contains(&id) && be(o + 4) == DESC_MARK && be(o + DESC_REPEAT) == id { return Some(o); } if o < 4 { diff --git a/crates/sylpheed-formats/src/ship_capture.rs b/crates/sylpheed-formats/src/ship_capture.rs index b1d50279..e6833493 100644 --- a/crates/sylpheed-formats/src/ship_capture.rs +++ b/crates/sylpheed-formats/src/ship_capture.rs @@ -287,7 +287,7 @@ pub fn parse_drawlog(text: &str) -> Vec { let mut pos: Vec<[f32; 3]> = Vec::new(); let mut consts: Vec<(usize, [f64; 4])> = Vec::new(); - let mut flush = |base: u32, + let flush = |base: u32, size: u32, stride: u32, pos: &mut Vec<[f32; 3]>, @@ -426,7 +426,7 @@ pub fn correlate( let Some((score, mirrored)) = pos_validate(d, &key.ref_pos) else { continue; // positions disagree — not this part }; - if best.map_or(true, |(_, s, _)| score > s) { + if best.is_none_or(|(_, s, _)| score > s) { best = Some((d, score, mirrored)); } } diff --git a/crates/sylpheed-formats/src/slb.rs b/crates/sylpheed-formats/src/slb.rs index 9b198b22..01355daa 100644 --- a/crates/sylpheed-formats/src/slb.rs +++ b/crates/sylpheed-formats/src/slb.rs @@ -517,7 +517,7 @@ pub fn to_xma_riff_best(slb: &[u8]) -> Option> { while let Some(di) = find(slb, b"data", i) { let declared = le32(slb, di + 4).unwrap_or(0) as usize; let usable = declared.min(slb.len().saturating_sub(di + 8)); - if best.map_or(true, |(_, b)| usable > b) { + if best.is_none_or(|(_, b)| usable > b) { best = Some((di, usable)); } i = di + 4; diff --git a/crates/sylpheed-formats/src/texture.rs b/crates/sylpheed-formats/src/texture.rs index 4d791bb4..b3dcfdf3 100644 --- a/crates/sylpheed-formats/src/texture.rs +++ b/crates/sylpheed-formats/src/texture.rs @@ -587,19 +587,19 @@ pub fn apply_endian_swap(data: &mut [u8], endianness: u8) { match endianness { 1 => { // k8in16 — swap the two bytes of each 16-bit half. - for c in data.chunks_exact_mut(2) { + for c in data.as_chunks_mut::<2>().0 { c.swap(0, 1); } } 2 => { // k8in32 — reverse each 32-bit word. - for c in data.chunks_exact_mut(4) { + for c in data.as_chunks_mut::<4>().0 { c.reverse(); } } 3 => { // k16in32 — swap the two 16-bit halves of each 32-bit word. - for c in data.chunks_exact_mut(4) { + for c in data.as_chunks_mut::<4>().0 { c.swap(0, 2); c.swap(1, 3); } @@ -626,8 +626,8 @@ fn decode_surface( } else { // Linear layout — copy only the mip-0 slice. let block_size = format.block_size() as u32; - let bw = ((width + block_size - 1) / block_size).max(1); - let bh = ((height + block_size - 1) / block_size).max(1); + let bw = width.div_ceil(block_size).max(1); + let bh = height.div_ceil(block_size).max(1); let needed = bw as usize * bh as usize * format.bytes_per_block(); if raw_data.len() < needed { return Err(TextureError::BufferTooSmall { needed, have: raw_data.len() }); @@ -651,7 +651,7 @@ fn decode_surface( match format { X360TextureFormat::Dxt1 => swap_bc_block_dwords(&mut linear_data), X360TextureFormat::Dxt3 | X360TextureFormat::Dxt5 => { - for block in linear_data.chunks_exact_mut(16) { + for block in linear_data.as_chunks_mut::<16>().0 { swap_bc_block_dwords(&mut block[8..16]); } } @@ -666,8 +666,8 @@ fn decode_surface( /// rounded up to the 4 KiB subresource alignment (`kTextureSubresourceAlignmentBytes`). fn tiled_face_stride(width: u32, height: u32, format: X360TextureFormat) -> usize { let bs = format.block_size() as u32; - let bw = ((width + bs - 1) / bs).max(1); - let bh = ((height + bs - 1) / bs).max(1); + let bw = width.div_ceil(bs).max(1); + let bh = height.div_ceil(bs).max(1); let pitch_aligned = align_up(bw, STORAGE_ALIGN_BLOCKS).max(MACRO_TILE_BLOCKS); let height_aligned = align_up(bh, STORAGE_ALIGN_BLOCKS).max(MACRO_TILE_BLOCKS); let surface = pitch_aligned as usize * height_aligned as usize * format.bytes_per_block(); @@ -682,7 +682,7 @@ fn tiled_face_stride(width: u32, height: u32, format: X360TextureFormat) -> usiz /// after the byte-level endian swap. Any trailing bytes that don't fill a full /// 8-byte group are left untouched. pub fn swap_bc_block_dwords(data: &mut [u8]) { - for unit in data.chunks_exact_mut(8) { + for unit in data.as_chunks_mut::<8>().0 { // [d0 d1 d2 d3 | d4 d5 d6 d7] → [d4 d5 d6 d7 | d0 d1 d2 d3] let (lo, hi) = unit.split_at_mut(4); lo.swap_with_slice(hi); @@ -742,8 +742,8 @@ pub fn detile( let bpb_log2 = (bpb as u32).trailing_zeros(); // Visible dimensions in blocks, and the padded storage pitch/height. - let blocks_wide = ((width + block_size - 1) / block_size).max(1); - let blocks_tall = ((height + block_size - 1) / block_size).max(1); + let blocks_wide = width.div_ceil(block_size).max(1); + let blocks_tall = height.div_ceil(block_size).max(1); let pitch_aligned = align_up(blocks_wide, STORAGE_ALIGN_BLOCKS).max(MACRO_TILE_BLOCKS); let height_aligned = align_up(blocks_tall, STORAGE_ALIGN_BLOCKS).max(MACRO_TILE_BLOCKS); diff --git a/crates/sylpheed-formats/src/ui_layout.rs b/crates/sylpheed-formats/src/ui_layout.rs index 1154a092..2a2ac43b 100644 --- a/crates/sylpheed-formats/src/ui_layout.rs +++ b/crates/sylpheed-formats/src/ui_layout.rs @@ -910,7 +910,7 @@ pub fn compose( // A dim backdrop stands in for the PRMD dim-quad + the live 3D scene behind // an in-mission screen. let mut canvas = vec![0u8; (w as usize) * (h as usize) * 4]; - for px in canvas.chunks_exact_mut(4) { + for px in canvas.as_chunks_mut::<4>().0 { px.copy_from_slice(&opts.backdrop); } let mut drawn = Vec::new(); diff --git a/crates/sylpheed-formats/src/vfs.rs b/crates/sylpheed-formats/src/vfs.rs index 28749f20..85312c5d 100644 --- a/crates/sylpheed-formats/src/vfs.rs +++ b/crates/sylpheed-formats/src/vfs.rs @@ -237,7 +237,7 @@ pub fn decode_text(bytes: &[u8]) -> (String, &'static str) { fn decode_utf16(bytes: &[u8], big_endian: bool) -> String { let units: Vec = bytes - .chunks_exact(2) + .as_chunks::<2>().0.iter() .map(|c| { if big_endian { u16::from_be_bytes([c[0], c[1]]) diff --git a/crates/sylpheed-formats/src/xiso.rs b/crates/sylpheed-formats/src/xiso.rs index 64bf0699..ea96b29f 100644 --- a/crates/sylpheed-formats/src/xiso.rs +++ b/crates/sylpheed-formats/src/xiso.rs @@ -53,7 +53,7 @@ impl XisoReader { info!( "Opened XISO: root directory table at sector {}", - { let s = volume.root_table.region.sector; s } + { volume.root_table.region.sector } ); Ok(Self { volume, file: wrapper }) }