From e258ca61b43c0820048fd8b10e814b59a4956d44 Mon Sep 17 00:00:00 2001 From: jif-oai Date: Mon, 22 Sep 2025 19:16:02 +0200 Subject: [PATCH] chore: more clippy rules 2 (#4057) The only file to watch is the cargo.toml All the others come from just fix + a few manual small fix The set of rules have been taken from the list of clippy rules arbitrarily while trying to optimise the learning and style of the code while limiting the loss of productivity --- codex-rs/Cargo.toml | 27 ++++++++++++++++++ codex-rs/core/src/turn_diff_tracker.rs | 10 +++---- codex-rs/tui/src/bottom_pane/chat_composer.rs | 4 +-- .../tui/src/bottom_pane/custom_prompt_view.rs | 4 +-- codex-rs/tui/src/bottom_pane/textarea.rs | 28 +++++++++---------- codex-rs/tui/src/history_cell.rs | 5 ++-- codex-rs/tui/src/markdown_stream.rs | 6 ++-- codex-rs/tui/src/wrapping.rs | 4 +-- 8 files changed, 56 insertions(+), 32 deletions(-) diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 5850d09b..1ec4191e 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -152,8 +152,35 @@ rust = {} [workspace.lints.clippy] expect_used = "deny" +identity_op = "deny" +manual_clamp = "deny" +manual_filter = "deny" +manual_find = "deny" +manual_flatten = "deny" +manual_map = "deny" +manual_memcpy = "deny" +manual_non_exhaustive = "deny" +manual_ok_or = "deny" +manual_range_contains = "deny" +manual_retain = "deny" +manual_strip = "deny" +manual_try_fold = "deny" +manual_unwrap_or = "deny" +needless_borrow = "deny" +needless_borrowed_reference = "deny" +needless_collect = "deny" +needless_late_init = "deny" +needless_option_as_deref = "deny" +needless_question_mark = "deny" +needless_update = "deny" redundant_clone = "deny" +redundant_static_lifetimes = "deny" +trivially_copy_pass_by_ref = "deny" uninlined_format_args = "deny" +unnecessary_filter_map = "deny" +unnecessary_lazy_evaluations = "deny" +unnecessary_sort_by = "deny" +unnecessary_to_owned = "deny" unwrap_used = "deny" # cargo-shear cannot see the platform-specific openssl-sys usage, so we diff --git a/codex-rs/core/src/turn_diff_tracker.rs b/codex-rs/core/src/turn_diff_tracker.rs index 6c12d6cd..06c40deb 100644 --- a/codex-rs/core/src/turn_diff_tracker.rs +++ b/codex-rs/core/src/turn_diff_tracker.rs @@ -65,7 +65,7 @@ impl TurnDiffTracker { let baseline_file_info = if path.exists() { let mode = file_mode_for_path(path); let mode_val = mode.unwrap_or(FileMode::Regular); - let content = blob_bytes(path, &mode_val).unwrap_or_default(); + let content = blob_bytes(path, mode_val).unwrap_or_default(); let oid = if mode == Some(FileMode::Symlink) { format!("{:x}", git_blob_sha1_hex_bytes(&content)) } else { @@ -266,7 +266,7 @@ impl TurnDiffTracker { }; let current_mode = file_mode_for_path(¤t_external_path).unwrap_or(FileMode::Regular); - let right_bytes = blob_bytes(¤t_external_path, ¤t_mode); + let right_bytes = blob_bytes(¤t_external_path, current_mode); // Compute displays with &mut self before borrowing any baseline content. let left_display = self.relative_to_git_root_str(&baseline_external_path); @@ -388,7 +388,7 @@ enum FileMode { } impl FileMode { - fn as_str(&self) -> &'static str { + fn as_str(self) -> &'static str { match self { FileMode::Regular => "100644", #[cfg(unix)] @@ -427,9 +427,9 @@ fn file_mode_for_path(_path: &Path) -> Option { Some(FileMode::Regular) } -fn blob_bytes(path: &Path, mode: &FileMode) -> Option> { +fn blob_bytes(path: &Path, mode: FileMode) -> Option> { if path.exists() { - let contents = if *mode == FileMode::Symlink { + let contents = if mode == FileMode::Symlink { symlink_blob_bytes(path) .ok_or_else(|| anyhow!("failed to read symlink target for {}", path.display())) } else { diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index bf11ff57..d15265c5 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -161,8 +161,8 @@ impl ChatComposer { // Leave 1 for border and 1 for padding textarea_rect.width = textarea_rect.width.saturating_sub(LIVE_PREFIX_COLS); textarea_rect.x = textarea_rect.x.saturating_add(LIVE_PREFIX_COLS); - let state = self.textarea_state.borrow(); - self.textarea.cursor_pos_with_state(textarea_rect, &state) + let state = *self.textarea_state.borrow(); + self.textarea.cursor_pos_with_state(textarea_rect, state) } /// Returns true if the composer currently contains no user input. diff --git a/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs b/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs index 4bad7077..b498d878 100644 --- a/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs +++ b/codex-rs/tui/src/bottom_pane/custom_prompt_view.rs @@ -236,8 +236,8 @@ impl BottomPaneView for CustomPromptView { width: area.width.saturating_sub(2), height: text_area_height, }; - let state = self.textarea_state.borrow(); - self.textarea.cursor_pos_with_state(textarea_rect, &state) + let state = *self.textarea_state.borrow(); + self.textarea.cursor_pos_with_state(textarea_rect, state) } } diff --git a/codex-rs/tui/src/bottom_pane/textarea.rs b/codex-rs/tui/src/bottom_pane/textarea.rs index 9166b8c7..269fa345 100644 --- a/codex-rs/tui/src/bottom_pane/textarea.rs +++ b/codex-rs/tui/src/bottom_pane/textarea.rs @@ -132,11 +132,11 @@ impl TextArea { #[cfg_attr(not(test), allow(dead_code))] pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { - self.cursor_pos_with_state(area, &TextAreaState::default()) + self.cursor_pos_with_state(area, TextAreaState::default()) } /// Compute the on-screen cursor position taking scrolling into account. - pub fn cursor_pos_with_state(&self, area: Rect, state: &TextAreaState) -> Option<(u16, u16)> { + pub fn cursor_pos_with_state(&self, area: Rect, state: TextAreaState) -> Option<(u16, u16)> { let lines = self.wrapped_lines(area.width); let effective_scroll = self.effective_scroll(area.height, &lines, state.scroll); let i = Self::wrapped_line_index_by_start(&lines, self.cursor_pos)?; @@ -1408,7 +1408,7 @@ mod tests { let mut state = TextAreaState::default(); let small_area = Rect::new(0, 0, 6, 1); // First call: cursor not visible -> effective scroll ensures it is - let (_x, y) = t.cursor_pos_with_state(small_area, &state).unwrap(); + let (_x, y) = t.cursor_pos_with_state(small_area, state).unwrap(); assert_eq!(y, 0); // Render with state to update actual scroll value @@ -1429,7 +1429,7 @@ mod tests { // effective scroll is 0 and the cursor position matches cursor_pos. let bad_state = TextAreaState { scroll: 999 }; let (x1, y1) = t.cursor_pos(area).unwrap(); - let (x2, y2) = t.cursor_pos_with_state(area, &bad_state).unwrap(); + let (x2, y2) = t.cursor_pos_with_state(area, bad_state).unwrap(); assert_eq!((x2, y2), (x1, y1)); // Case 2: Cursor below the current window — y should be clamped to the @@ -1442,7 +1442,7 @@ mod tests { t.set_cursor(t.text().len().saturating_sub(2)); let small_area = Rect::new(0, 0, wrap_width, 2); let state = TextAreaState { scroll: 0 }; - let (_x, y) = t.cursor_pos_with_state(small_area, &state).unwrap(); + let (_x, y) = t.cursor_pos_with_state(small_area, state).unwrap(); assert_eq!(y, small_area.y + small_area.height - 1); // Case 3: Cursor above the current window — y should be top row (0) @@ -1456,7 +1456,7 @@ mod tests { let state = TextAreaState { scroll: lines.saturating_mul(2), }; - let (_x, y) = t.cursor_pos_with_state(area, &state).unwrap(); + let (_x, y) = t.cursor_pos_with_state(area, state).unwrap(); assert_eq!(y, area.y); } @@ -1480,7 +1480,7 @@ mod tests { // With state and small height, cursor should be visible at row 0, col 0 let small_area = Rect::new(0, 0, 4, 1); let state = TextAreaState::default(); - let (x, y) = t.cursor_pos_with_state(small_area, &state).unwrap(); + let (x, y) = t.cursor_pos_with_state(small_area, state).unwrap(); assert_eq!((x, y), (0, 0)); // Place cursor in the middle of the second wrapped line ("efgh"), at 'g' @@ -1510,35 +1510,35 @@ mod tests { // Start at beginning t.set_cursor(0); ratatui::widgets::StatefulWidgetRef::render_ref(&(&t), area, &mut buf, &mut state); - let (x, y) = t.cursor_pos_with_state(area, &state).unwrap(); + let (x, y) = t.cursor_pos_with_state(area, state).unwrap(); assert_eq!((x, y), (0, 0)); // Move down to second visual line; should be at bottom row (row 1) within 2-line viewport t.move_cursor_down(); ratatui::widgets::StatefulWidgetRef::render_ref(&(&t), area, &mut buf, &mut state); - let (x, y) = t.cursor_pos_with_state(area, &state).unwrap(); + let (x, y) = t.cursor_pos_with_state(area, state).unwrap(); assert_eq!((x, y), (0, 1)); // Move down to third visual line; viewport scrolls and keeps cursor on bottom row t.move_cursor_down(); ratatui::widgets::StatefulWidgetRef::render_ref(&(&t), area, &mut buf, &mut state); - let (x, y) = t.cursor_pos_with_state(area, &state).unwrap(); + let (x, y) = t.cursor_pos_with_state(area, state).unwrap(); assert_eq!((x, y), (0, 1)); // Move up to second visual line; with current scroll, it appears on top row t.move_cursor_up(); ratatui::widgets::StatefulWidgetRef::render_ref(&(&t), area, &mut buf, &mut state); - let (x, y) = t.cursor_pos_with_state(area, &state).unwrap(); + let (x, y) = t.cursor_pos_with_state(area, state).unwrap(); assert_eq!((x, y), (0, 0)); // Column preservation across moves: set to col 2 on first line, move down t.set_cursor(2); ratatui::widgets::StatefulWidgetRef::render_ref(&(&t), area, &mut buf, &mut state); - let (x0, y0) = t.cursor_pos_with_state(area, &state).unwrap(); + let (x0, y0) = t.cursor_pos_with_state(area, state).unwrap(); assert_eq!((x0, y0), (2, 0)); t.move_cursor_down(); ratatui::widgets::StatefulWidgetRef::render_ref(&(&t), area, &mut buf, &mut state); - let (x1, y1) = t.cursor_pos_with_state(area, &state).unwrap(); + let (x1, y1) = t.cursor_pos_with_state(area, state).unwrap(); assert_eq!((x1, y1), (2, 1)); } @@ -1796,7 +1796,7 @@ mod tests { // cursor_pos_with_state: always within viewport rows let (_x, _y) = ta - .cursor_pos_with_state(area, &state) + .cursor_pos_with_state(area, state) .unwrap_or((area.x, area.y)); // Stateful render should not panic, and updates scroll diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 24e6b284..c060715e 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -390,15 +390,14 @@ impl ExecCell { .iter() .all(|c| matches!(c, ParsedCommand::Read { .. })) { - let names: Vec = call + let names = call .parsed .iter() .map(|c| match c { ParsedCommand::Read { name, .. } => name.clone(), _ => unreachable!(), }) - .unique() - .collect(); + .unique(); vec![( "Read", itertools::Itertools::intersperse( diff --git a/codex-rs/tui/src/markdown_stream.rs b/codex-rs/tui/src/markdown_stream.rs index 12b43082..a9428b89 100644 --- a/codex-rs/tui/src/markdown_stream.rs +++ b/codex-rs/tui/src/markdown_stream.rs @@ -310,10 +310,8 @@ mod tests { let long = "> This is a very long quoted line that should wrap across multiple columns to verify style preservation."; let out = super::simulate_stream_markdown_for_tests(&[long, "\n"], true, &cfg); // Wrap to a narrow width to force multiple output lines. - let wrapped = crate::wrapping::word_wrap_lines( - out.iter().collect::>(), - crate::wrapping::RtOptions::new(24), - ); + let wrapped = + crate::wrapping::word_wrap_lines(out.iter(), crate::wrapping::RtOptions::new(24)); // Filter out purely blank lines let non_blank: Vec<_> = wrapped .into_iter() diff --git a/codex-rs/tui/src/wrapping.rs b/codex-rs/tui/src/wrapping.rs index c97f0e3c..1dc4e2e5 100644 --- a/codex-rs/tui/src/wrapping.rs +++ b/codex-rs/tui/src/wrapping.rs @@ -511,7 +511,7 @@ mod tests { .subsequent_indent(Line::from(" ")); let lines = [Line::from("hello world"), Line::from("foo bar baz")]; - let out = word_wrap_lines_borrowed(lines.iter().collect::>(), opts); + let out = word_wrap_lines_borrowed(lines.iter(), opts); let rendered: Vec = out.iter().map(concat_line).collect(); assert!(rendered.first().unwrap().starts_with("- ")); @@ -523,7 +523,7 @@ mod tests { #[test] fn wrap_lines_borrowed_without_indents_is_concat_of_single_wraps() { let lines = [Line::from("hello"), Line::from("world!")]; - let out = word_wrap_lines_borrowed(lines.iter().collect::>(), 10); + let out = word_wrap_lines_borrowed(lines.iter(), 10); let rendered: Vec = out.iter().map(concat_line).collect(); assert_eq!(rendered, vec!["hello", "world!"]); }