From 27373b06d0d7627dc0071cd3bb294c9a9dfb9987 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Fri, 22 May 2026 12:25:34 +0200 Subject: [PATCH] Style: forbid `.zip` and `.chain` (#8188) The `zip(a, b)` variant produces clearer code imho. Downside: added dependency on `itertools` --- Cargo.lock | 2 ++ Cargo.toml | 1 + crates/ecolor/src/color32.rs | 2 +- crates/ecolor/src/rgba.rs | 2 +- crates/eframe/src/web/input.rs | 11 +++++---- crates/eframe/src/web/web_painter_wgpu.rs | 2 +- crates/egui-wgpu/src/winit.rs | 2 +- crates/egui/Cargo.toml | 1 + crates/egui/src/containers/popup.rs | 16 ++++++------- crates/egui/src/context.rs | 6 +---- crates/egui/src/interaction.rs | 20 +++++++--------- crates/egui/src/memory/mod.rs | 11 +++++---- crates/egui_extras/Cargo.toml | 1 + crates/egui_extras/src/table.rs | 7 ++---- crates/egui_kittest/src/wgpu.rs | 2 +- crates/epaint/src/text/font.rs | 7 +----- crates/epaint/src/text/text_layout.rs | 28 +++++++++-------------- scripts/lint.py | 16 +++++++++++++ 18 files changed, 68 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4650bc10c..e57b05c89 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1257,6 +1257,7 @@ dependencies = [ "document-features", "emath", "epaint", + "itertools 0.14.0", "log", "nohash-hasher", "profiling", @@ -1363,6 +1364,7 @@ dependencies = [ "ehttp", "enum-map", "image", + "itertools 0.14.0", "jiff", "log", "mime_guess2", diff --git a/Cargo.toml b/Cargo.toml index 7c8473675..3710f8cb3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -96,6 +96,7 @@ glutin-winit = { version = "0.5.0", default-features = false } harfrust = "0.7.0" home = "0.5.9" image = { version = "0.25.6", default-features = false } +itertools = "0.14.0" jiff = { version = "0.2.23", default-features = false } js-sys = "0.3.77" kittest = { version = "0.4.0" } diff --git a/crates/ecolor/src/color32.rs b/crates/ecolor/src/color32.rs index b9de323e0..f444b860d 100644 --- a/crates/ecolor/src/color32.rs +++ b/crates/ecolor/src/color32.rs @@ -489,7 +489,7 @@ mod test { } else { // There will be small rounding errors whenever the alpha is not 0 or 255, // because we multiply and then unmultiply the alpha. - for (&a, &b) in in_rgba.iter().zip(out_rgba.iter()) { + for (&a, &b) in std::iter::zip(&in_rgba, &out_rgba) { assert!(a.abs_diff(b) <= 3, "{in_rgba:?} != {out_rgba:?}"); } } diff --git a/crates/ecolor/src/rgba.rs b/crates/ecolor/src/rgba.rs index 3e40af2b0..98c3ce408 100644 --- a/crates/ecolor/src/rgba.rs +++ b/crates/ecolor/src/rgba.rs @@ -336,7 +336,7 @@ mod test { } else { // There will be small rounding errors whenever the alpha is not 0 or 255, // because we multiply and then unmultiply the alpha. - for (&a, &b) in in_rgba.iter().zip(out_rgba.iter()) { + for (&a, &b) in std::iter::zip(&in_rgba, &out_rgba) { assert!(a.abs_diff(b) <= 3, "{in_rgba:?} != {out_rgba:?}"); } } diff --git a/crates/eframe/src/web/input.rs b/crates/eframe/src/web/input.rs index c27897090..62723e8fa 100644 --- a/crates/eframe/src/web/input.rs +++ b/crates/eframe/src/web/input.rs @@ -31,11 +31,12 @@ pub fn primary_touch_pos( runner: &mut AppRunner, event: &web_sys::TouchEvent, ) -> Option<(egui::Pos2, web_sys::Touch)> { - let all_touches: Vec<_> = (0..event.touches().length()) - .filter_map(|i| event.touches().get(i)) - // On touchend we don't get anything in `touches`, but we still get `changed_touches`, so include those: - .chain((0..event.changed_touches().length()).filter_map(|i| event.changed_touches().get(i))) - .collect(); + // On touchend we don't get anything in `touches`, but we still get `changed_touches`, so include those: + let all_touches: Vec<_> = std::iter::chain( + (0..event.touches().length()).filter_map(|i| event.touches().get(i)), + (0..event.changed_touches().length()).filter_map(|i| event.changed_touches().get(i)), + ) + .collect(); if let Some(primary_touch) = runner.input.primary_touch { // Is the primary touch is gone? diff --git a/crates/eframe/src/web/web_painter_wgpu.rs b/crates/eframe/src/web/web_painter_wgpu.rs index a7c6bded2..b16f98a9b 100644 --- a/crates/eframe/src/web/web_painter_wgpu.rs +++ b/crates/eframe/src/web/web_painter_wgpu.rs @@ -365,7 +365,7 @@ impl WebPainter for WebPainterWgpu { // Submit the commands: both the main buffer and user-defined ones. render_state .queue - .submit(user_cmd_bufs.into_iter().chain([encoder.finish()])); + .submit(std::iter::chain(user_cmd_bufs, [encoder.finish()])); if let Some((frame, capture_buffer)) = frame_and_capture_buffer { if let Some(capture_buffer) = capture_buffer diff --git a/crates/egui-wgpu/src/winit.rs b/crates/egui-wgpu/src/winit.rs index 96ab6e29c..df9245ec9 100644 --- a/crates/egui-wgpu/src/winit.rs +++ b/crates/egui-wgpu/src/winit.rs @@ -715,7 +715,7 @@ impl Painter { let start = web_time::Instant::now(); render_state .queue - .submit(user_cmd_bufs.into_iter().chain([encoded])); + .submit(std::iter::chain(user_cmd_bufs, [encoded])); vsync_sec += start.elapsed().as_secs_f32(); }; diff --git a/crates/egui/Cargo.toml b/crates/egui/Cargo.toml index 3a22bf529..fadc27c6c 100644 --- a/crates/egui/Cargo.toml +++ b/crates/egui/Cargo.toml @@ -74,6 +74,7 @@ epaint = { workspace = true, default-features = false } accesskit.workspace = true ahash.workspace = true bitflags.workspace = true +itertools.workspace = true log.workspace = true nohash-hasher.workspace = true profiling.workspace = true diff --git a/crates/egui/src/containers/popup.rs b/crates/egui/src/containers/popup.rs index cf78a6650..2a9335ede 100644 --- a/crates/egui/src/containers/popup.rs +++ b/crates/egui/src/containers/popup.rs @@ -472,17 +472,15 @@ impl<'a> Popup<'a> { RectAlign::find_best_align( #[expect(clippy::iter_on_empty_collections)] #[expect(clippy::or_fun_call)] - once(self.rect_align).chain( + std::iter::chain( + once(self.rect_align), self.alternative_aligns // Need the empty slice so the iters have the same type so we can unwrap_or - .map(|a| a.iter().copied().chain([].iter().copied())) - .unwrap_or( - self.rect_align - .symmetries() - .iter() - .copied() - .chain(RectAlign::MENU_ALIGNS.iter().copied()), - ), + .map(|a| std::iter::chain(a.iter().copied(), [].iter().copied())) + .unwrap_or(std::iter::chain( + self.rect_align.symmetries().iter().copied(), + RectAlign::MENU_ALIGNS.iter().copied(), + )), ), self.ctx.content_rect(), anchor_rect, diff --git a/crates/egui/src/context.rs b/crates/egui/src/context.rs index d348517ef..dee71020f 100644 --- a/crates/egui/src/context.rs +++ b/crates/egui/src/context.rs @@ -641,11 +641,7 @@ impl ContextImpl { } fn all_viewport_ids(&self) -> ViewportIdSet { - self.viewports - .keys() - .copied() - .chain([ViewportId::ROOT]) - .collect() + std::iter::chain(self.viewports.keys().copied(), [ViewportId::ROOT]).collect() } /// The current active viewport diff --git a/crates/egui/src/interaction.rs b/crates/egui/src/interaction.rs index bfac38da7..6e01ec8fb 100644 --- a/crates/egui/src/interaction.rs +++ b/crates/egui/src/interaction.rs @@ -232,20 +232,14 @@ pub(crate) fn interact( // ); // } - let contains_pointer: IdSet = hits - .contains_pointer - .iter() - .chain(&hits.click) - .chain(&hits.drag) - .map(|w| w.id) - .collect(); + let contains_pointer: IdSet = + itertools::chain!(&hits.contains_pointer, &hits.click, &hits.drag) + .map(|w| w.id) + .collect(); let hovered = if clicked.is_some() || dragged.is_some() || long_touched.is_some() { // If currently clicking or dragging, only that and nothing else is hovered. - clicked - .iter() - .chain(&dragged) - .chain(&long_touched) + itertools::chain!(&clicked, &dragged, &long_touched) .copied() .collect() } else { @@ -268,7 +262,9 @@ pub(crate) fn interact( let drag_order = hits.drag.and_then(|w| order(w.id)).unwrap_or(0); let top_interactive_order = click_order.max(drag_order); - let mut hovered: IdSet = hits.click.iter().chain(&hits.drag).map(|w| w.id).collect(); + let mut hovered: IdSet = std::iter::chain(&hits.click, &hits.drag) + .map(|w| w.id) + .collect(); for w in &hits.contains_pointer { let is_interactive = w.sense.senses_click() || w.sense.senses_drag(); diff --git a/crates/egui/src/memory/mod.rs b/crates/egui/src/memory/mod.rs index 674f18f56..de37138d5 100644 --- a/crates/egui/src/memory/mod.rs +++ b/crates/egui/src/memory/mod.rs @@ -1232,11 +1232,12 @@ impl Areas { } pub fn visible_layer_ids(&self) -> ahash::HashSet { - self.visible_areas_last_frame - .iter() - .copied() - .chain(self.visible_areas_current_frame.iter().copied()) - .collect() + std::iter::chain( + &self.visible_areas_last_frame, + &self.visible_areas_current_frame, + ) + .copied() + .collect() } pub(crate) fn visible_windows(&self) -> impl Iterator { diff --git a/crates/egui_extras/Cargo.toml b/crates/egui_extras/Cargo.toml index 944576f08..315ddad0f 100644 --- a/crates/egui_extras/Cargo.toml +++ b/crates/egui_extras/Cargo.toml @@ -74,6 +74,7 @@ egui = { workspace = true, default-features = false } ahash.workspace = true enum-map.workspace = true +itertools.workspace = true log.workspace = true profiling.workspace = true diff --git a/crates/egui_extras/src/table.rs b/crates/egui_extras/src/table.rs index aef17ce51..4832a313f 100644 --- a/crates/egui_extras/src/table.rs +++ b/crates/egui_extras/src/table.rs @@ -619,11 +619,8 @@ impl TableState { // to take up the remainder of the current available width. // Also handles changing item spacing. let mut sizing = crate::sizing::Sizing::default(); - for ((prev_width, max_used), column) in state - .column_widths - .iter() - .zip(&state.max_used_widths) - .zip(columns) + for (prev_width, max_used, column) in + itertools::izip!(&state.column_widths, &state.max_used_widths, columns) { use crate::Size; diff --git a/crates/egui_kittest/src/wgpu.rs b/crates/egui_kittest/src/wgpu.rs index 45152e81e..22972eabc 100644 --- a/crates/egui_kittest/src/wgpu.rs +++ b/crates/egui_kittest/src/wgpu.rs @@ -222,7 +222,7 @@ impl crate::TestRenderer for WgpuTestRenderer { self.render_state .queue - .submit(user_buffers.into_iter().chain(once(encoder.finish()))); + .submit(std::iter::chain(user_buffers, once(encoder.finish()))); self.render_state .device diff --git a/crates/epaint/src/text/font.rs b/crates/epaint/src/text/font.rs index f64557ec7..59ed25fec 100644 --- a/crates/epaint/src/text/font.rs +++ b/crates/epaint/src/text/font.rs @@ -508,12 +508,7 @@ impl FontFace { let axes = font_data.skrifa.axes(); // Override the default coordinates with ones specified via FontTweak, then the ones specified directly via the // argument (probably from TextFormat). - let settings = self - .tweak - .coords - .as_ref() - .iter() - .chain(coords.as_ref().iter()); + let settings = std::iter::chain(self.tweak.coords.as_ref(), coords.as_ref()); let location = axes.location(settings); StyledMetrics { diff --git a/crates/epaint/src/text/text_layout.rs b/crates/epaint/src/text/text_layout.rs index 35859b095..dfa913ea7 100644 --- a/crates/epaint/src/text/text_layout.rs +++ b/crates/epaint/src/text/text_layout.rs @@ -1,7 +1,7 @@ #![expect(clippy::unwrap_used)] // TODO(emilk): remove unwraps -use std::ops::Range; use std::sync::Arc; +use std::{iter, ops::Range}; use emath::{Align, GuiRounding as _, NumExt as _, Pos2, Rect, Vec2, pos2, vec2}; @@ -244,11 +244,7 @@ fn layout_shaped_run( let mut cluster_start_byte: usize = 0; let mut cluster_glyph_count: usize = 0; - for (info, pos) in glyph_buffer - .glyph_infos() - .iter() - .zip(glyph_buffer.glyph_positions()) - { + for (info, pos) in iter::zip(glyph_buffer.glyph_infos(), glyph_buffer.glyph_positions()) { let glyph_id = skrifa::GlyphId::new(info.glyph_id); let cluster = info.cluster; let mut advance_width_px = pos.x_advance as f32 * px_scale; @@ -525,7 +521,7 @@ fn layout_section( /// Avoids `Box` and `Vec<&str>` allocation. enum SplitOrWhole<'a> { Split(std::str::Split<'a, char>), - Whole(std::iter::Once<&'a str>), + Whole(iter::Once<&'a str>), } impl<'a> SplitOrWhole<'a> { @@ -533,7 +529,7 @@ impl<'a> SplitOrWhole<'a> { if split { Self::Split(text.split('\n')) } else { - Self::Whole(std::iter::once(text)) + Self::Whole(iter::once(text)) } } } @@ -1406,11 +1402,7 @@ fn shape_text( let tweak = font_face.tweak(); // Build shaper with variable font instance if variation coordinates are set. - let variations: Vec = tweak - .coords - .as_ref() - .iter() - .chain(coords.as_ref().iter()) + let variations: Vec = iter::chain(tweak.coords.as_ref(), coords.as_ref()) .map(|&(tag, value)| harfrust::Variation { tag, value }) .collect(); @@ -1439,6 +1431,7 @@ fn shape_text( #[cfg(test)] mod tests { + use std::iter; use super::{super::*, *}; use crate::text::cursor::CCursor; @@ -1575,10 +1568,11 @@ mod tests { &mut fonts, pixels_per_point, Arc::new(LayoutJob::single_section( - (0..elided_galley.rows[0].char_count_excluding_newline()) - .map(|_| ch) - .chain(std::iter::once('…')) - .collect::(), + iter::chain( + (0..elided_galley.rows[0].char_count_excluding_newline()).map(|_| ch), + iter::once('…'), + ) + .collect::(), TextFormat::default(), )), ); diff --git a/scripts/lint.py b/scripts/lint.py index 4939d735f..0c1319e8a 100755 --- a/scripts/lint.py +++ b/scripts/lint.py @@ -72,6 +72,16 @@ def lint_lines(filepath, lines_in): f"{filepath}:{line_nr}: write 'TODO(username):' instead" ) + if re.search(r"\.zip\(", line): + errors.append( + f"{filepath}:{line_nr}: use `std::iter::zip` or `itertools::izip!` instead of `.zip(`" + ) + + if re.search(r"\.chain\(", line): + errors.append( + f"{filepath}:{line_nr}: use `std::iter::chain` or `itertools::chain!` instead of `.chain(`" + ) + if ( "(target_os" in line and filepath.startswith("./crates/egui/") @@ -105,6 +115,10 @@ def test_lint(): self } """, + "for (a, b) in std::iter::zip(xs, ys) {}", + "for (a, b, c) in itertools::izip!(xs, ys, zs) {}", + "for x in std::iter::chain(xs, ys) {}", + "for x in itertools::chain!(xs, ys, zs) {}", ] should_fail = [ @@ -121,6 +135,8 @@ def test_lint(): self } """, + "for (a, b) in xs.iter().zip(ys) {}", + "for x in xs.iter().chain(ys) {}", ] for code in should_pass: