From d65eb5842e074423cfbb228e355b35430b0cf606 Mon Sep 17 00:00:00 2001 From: Albert Hui Date: Sun, 2 Aug 2026 02:54:14 +0800 Subject: [PATCH] fix(fish): saturate dir_length(0); adopt canonical lints GREEN for the preceding RED commit. abbreviate_segment now takes `len.saturating_sub(1)` trailing chars, so dir_length(0) keeps just the leading character instead of underflowing usize. That mirrors the dot branch, which already yields "." at len == 0. Three unwraps removed structurally rather than guarded: strategy/fish.rs chars.next().unwrap() -> let-else returning "" path_info.rs parts.last().unwrap() -> split_last(), which also retires the separate is_empty() guard and the `parts.len() - 1` index fs_aware.rs chars.next().unwrap() -> map_or_else, folding the is_empty() special case into the same expression This crate had no lints block. It now carries the canonical recipe (CLAUDE.core.md "Rust Lint Posture") with unwrap_used/expect_used denied. What the new lints surfaced, and how each was closed - no lint was added to an allow list: clippy::unwrap_used 3 fixed (above) clippy::redundant_closure 5 fixed clippy::return_self_not_must_use 7 fixed - #[must_use] on the builder clippy::doc_markdown 2 fixed clippy::single_char_pattern 2 fixed clippy::map_unwrap_or 1 fixed (is_ok_and) clippy::format_push_string 1 fixed (write! + fmt::Write) Unit tests inside src carry the sanctioned `#![cfg_attr(test, allow(clippy::unwrap_used, clippy::expect_used))]`. Note: the `[lints]` table is honored by Cargo 1.74+. rust-version stays at 1.70 - older cargo ignores the table with a warning rather than failing, and CI runs a modern toolchain, so enforcement is real where it is checked. Raising the declared MSRV is left as a separate decision. Gate: cargo build --all-targets --all-features, cargo test --all-features (88 tests), cargo clippy --all-targets --all-features -- -D warnings, cargo fmt --check - all clean. Co-Authored-By: Claude Opus 5 (1M context) --- Cargo.toml | 19 +++++++++++++++++++ src/fs_aware.rs | 19 +++++++++++-------- src/lib.rs | 24 +++++++++++++++++++++--- src/main.rs | 5 ++++- src/path_info.rs | 12 ++++++------ src/strategy/fish.rs | 20 +++++++++++++++++--- src/strategy/hybrid.rs | 8 ++++---- src/strategy/unique.rs | 5 ++++- 8 files changed, 86 insertions(+), 26 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d6457fd..c49c779 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,6 +23,25 @@ fs = [] name = "shrinkpath" required-features = ["cli"] +[lints.rust] +unsafe_code = "forbid" + +[lints.clippy] +all = { level = "warn", priority = -1 } +pedantic = { level = "warn", priority = -1 } +correctness = "deny" +suspicious = "deny" +unwrap_used = "deny" +expect_used = "deny" +module_name_repetitions = { level = "allow", priority = 1 } +must_use_candidate = { level = "allow", priority = 1 } +missing_errors_doc = { level = "allow", priority = 1 } +missing_panics_doc = { level = "allow", priority = 1 } +cast_possible_truncation = { level = "allow", priority = 1 } +cast_possible_wrap = { level = "allow", priority = 1 } +cast_sign_loss = { level = "allow", priority = 1 } +cast_precision_loss = { level = "allow", priority = 1 } + [profile.release] strip = true lto = true diff --git a/src/fs_aware.rs b/src/fs_aware.rs index e9de0f0..c226d61 100644 --- a/src/fs_aware.rs +++ b/src/fs_aware.rs @@ -16,7 +16,10 @@ pub fn find_git_root(path: &str) -> Option { let mut current = start; loop { if current.join(".git").exists() { - return current.file_name()?.to_str().map(|s| s.to_string()); + return current + .file_name()? + .to_str() + .map(std::string::ToString::to_string); } current = current.parent()?; } @@ -28,8 +31,8 @@ pub fn find_git_root(path: &str) -> Option { pub fn disambiguate_segment(parent_path: &Path, segment: &str) -> String { let siblings: Vec = match std::fs::read_dir(parent_path) { Ok(entries) => entries - .filter_map(|e| e.ok()) - .filter(|e| e.file_type().map(|t| t.is_dir()).unwrap_or(false)) + .filter_map(std::result::Result::ok) + .filter(|e| e.file_type().is_ok_and(|t| t.is_dir())) .filter_map(|e| e.file_name().into_string().ok()) .filter(|name| name != segment) .collect(), @@ -37,11 +40,11 @@ pub fn disambiguate_segment(parent_path: &Path, segment: &str) -> String { }; if siblings.is_empty() { - // No siblings — 1 char is enough - if segment.is_empty() { - return String::new(); - } - return segment.chars().next().unwrap().to_string(); + // No siblings — 1 char is enough; an empty segment has no first char. + return segment + .chars() + .next() + .map_or_else(String::new, |c| c.to_string()); } for len in 1..=segment.len() { diff --git a/src/lib.rs b/src/lib.rs index 1dfbbe8..c13f373 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,3 +1,4 @@ +#![cfg_attr(test, allow(clippy::unwrap_used, clippy::expect_used))] //! # shrinkpath //! //! Smart, cross-platform path shortening for Rust. @@ -69,7 +70,7 @@ pub struct ShrinkOptions { } impl ShrinkOptions { - /// Create options with sensible defaults: Hybrid strategy, max_len as specified. + /// Create options with sensible defaults: Hybrid strategy, `max_len` as specified. pub fn new(max_len: usize) -> Self { ShrinkOptions { max_len, @@ -84,42 +85,49 @@ impl ShrinkOptions { } /// Set the shortening strategy. + #[must_use] pub fn strategy(mut self, s: Strategy) -> Self { self.strategy = s; self } /// Force a specific path style. + #[must_use] pub fn path_style(mut self, s: PathStyle) -> Self { self.path_style = Some(s); self } /// Set a custom ellipsis string. + #[must_use] pub fn ellipsis(mut self, e: impl Into) -> Self { self.ellipsis = e.into(); self } /// Set the number of characters to keep per abbreviated directory segment. + #[must_use] pub fn dir_length(mut self, n: usize) -> Self { self.dir_length = n; self } /// Set the number of trailing directory segments to keep unabbreviated. + #[must_use] pub fn full_length_dirs(mut self, n: usize) -> Self { self.full_length_dirs = n; self } /// Add a mapped location: if the path starts with `from`, replace it with `to`. + #[must_use] pub fn map_location(mut self, from: impl Into, to: impl Into) -> Self { self.mapped_locations.push((from.into(), to.into())); self } /// Add an anchor segment name that should never be abbreviated. + #[must_use] pub fn anchor(mut self, name: impl Into) -> Self { self.anchors.push(name.into()); self @@ -239,7 +247,7 @@ pub fn shrink_detailed(path: &str, opts: &ShrinkOptions) -> ShrinkResult { } } -/// Build per-segment metadata by comparing original and shortened PathInfo. +/// Build per-segment metadata by comparing original and shortened `PathInfo`. fn build_segment_metadata( original: &path_info::PathInfo, shortened: &path_info::PathInfo, @@ -494,6 +502,16 @@ mod tests { assert_eq!(result, "/ho/jo/pr/ru/my/sr/lib.rs"); } + #[test] + fn dir_length_zero_does_not_panic() { + // `dir_length(0)` is accepted by the builder, so it must behave, not panic. + let opts = ShrinkOptions::new(50) + .strategy(Strategy::Fish) + .dir_length(0); + let result = shrink("/home/john/projects/rust/myapp/src/lib.rs", &opts); + assert_eq!(result, "/h/j/p/r/m/s/lib.rs"); + } + #[test] fn full_length_dirs_one() { let opts = ShrinkOptions::new(50) @@ -552,7 +570,7 @@ mod tests { fn mapped_location_windows() { let opts = ShrinkOptions::new(50).map_location("C:\\Users\\Admin", "~"); let result = shrink("C:\\Users\\Admin\\Documents\\file.txt", &opts); - assert!(result.starts_with("~"), "got: {result}"); + assert!(result.starts_with('~'), "got: {result}"); assert!(result.ends_with("file.txt")); } diff --git a/src/main.rs b/src/main.rs index 0a284c6..bfc4e5d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,6 @@ use clap::Parser; use shrinkpath::{shrink, shrink_detailed, PathStyle, ShrinkOptions, Strategy}; +use std::fmt::Write as _; use std::io::{self, BufRead, Write}; #[derive(Parser)] @@ -86,7 +87,9 @@ fn json_escape(s: &str) -> String { '\r' => out.push_str("\\r"), '\t' => out.push_str("\\t"), c if c < '\x20' => { - out.push_str(&format!("\\u{:04x}", c as u32)); + // Writing to a String is infallible; the Result exists only to satisfy + // the fmt::Write signature. + let _ = write!(out, "\\u{:04x}", c as u32); } c => out.push(c), } diff --git a/src/path_info.rs b/src/path_info.rs index 6cf4da7..789d161 100644 --- a/src/path_info.rs +++ b/src/path_info.rs @@ -50,18 +50,18 @@ impl PathInfo { // Split remaining path into parts let parts: Vec<&str> = remainder.split(sep).filter(|s| !s.is_empty()).collect(); - if parts.is_empty() { + // Last part is the filename (sacred); everything before it is directories. + // `split_last` yields both in one step, so the empty case needs no separate + // guard and neither the index nor `parts.len() - 1` can go out of range. + let Some((filename, dir_parts)) = parts.split_last() else { return PathInfo { prefix, segments: Vec::new(), filename: String::new(), style, }; - } - - // Last part is the filename (sacred) - let filename = parts.last().unwrap().to_string(); - let dir_parts = &parts[..parts.len() - 1]; + }; + let filename = (*filename).to_string(); // Classify segments let segments = classify_segments(dir_parts, &prefix, style); diff --git a/src/strategy/fish.rs b/src/strategy/fish.rs index 9b4a635..53366fd 100644 --- a/src/strategy/fish.rs +++ b/src/strategy/fish.rs @@ -13,7 +13,9 @@ pub fn abbreviate_segment(text: &str, len: usize, anchors: &[String]) -> String return text.to_string(); } let mut chars = text.chars(); - let first = chars.next().unwrap(); + let Some(first) = chars.next() else { + return String::new(); + }; if first == '.' { let after_dot: String = chars.take(len).collect(); if after_dot.is_empty() { @@ -21,10 +23,12 @@ pub fn abbreviate_segment(text: &str, len: usize, anchors: &[String]) -> String } return format!(".{after_dot}"); } - // Take `len` chars total (first + len-1 more) + // Take `len` chars total (first + len-1 more). `dir_length` is caller-supplied, + // so len == 0 must saturate rather than underflow; the leading char still + // survives, mirroring the dot branch's "." at len == 0. let mut result = String::with_capacity(len); result.push(first); - for c in chars.take(len - 1) { + for c in chars.take(len.saturating_sub(1)) { result.push(c); } result @@ -97,6 +101,16 @@ mod tests { assert_eq!(abbreviate_segment("", 1, &[]), ""); } + #[test] + fn abbreviate_zero_length_keeps_first_char() { + // `dir_length` is a public builder option, so len == 0 reaches here from + // safe user code. `chars.take(len - 1)` underflows usize for len == 0. + // Expected: the leading character survives, mirroring the dot branch, + // which already yields "." for a dotfile at len == 0. + assert_eq!(abbreviate_segment("projects", 0, &[]), "p"); + assert_eq!(abbreviate_segment(".config", 0, &[]), "."); + } + #[test] fn fish_unix() { let info = PathInfo::parse("/home/john/projects/rust/myapp/src/lib.rs", None); diff --git a/src/strategy/hybrid.rs b/src/strategy/hybrid.rs index 4ac35ee..ee5a4f2 100644 --- a/src/strategy/hybrid.rs +++ b/src/strategy/hybrid.rs @@ -22,7 +22,7 @@ pub fn shrink_hybrid( // Full reassembly check let texts: Vec = info.segments.iter().map(|s| s.text.clone()).collect(); - let text_refs: Vec<&str> = texts.iter().map(|s| s.as_str()).collect(); + let text_refs: Vec<&str> = texts.iter().map(std::string::String::as_str).collect(); let full = info.reassemble(&text_refs); if full.len() <= max_len { return full; @@ -43,7 +43,7 @@ pub fn shrink_hybrid( working[i] = abbreviate_segment(&info.segments[i].text, 1, anchors); } } - let refs: Vec<&str> = working.iter().map(|s| s.as_str()).collect(); + let refs: Vec<&str> = working.iter().map(std::string::String::as_str).collect(); let result = info.reassemble(&refs); if result.len() <= max_len { return result; @@ -55,7 +55,7 @@ pub fn shrink_hybrid( working[i] = abbreviate_segment(&info.segments[i].text, 1, anchors); } } - let refs: Vec<&str> = working.iter().map(|s| s.as_str()).collect(); + let refs: Vec<&str> = working.iter().map(std::string::String::as_str).collect(); let result = info.reassemble(&refs); if result.len() <= max_len { return result; @@ -193,7 +193,7 @@ mod tests { assert!(result.ends_with("file.txt")); // Should try to preserve Admin assert!( - result.contains("Admin") || result.contains("A"), + result.contains("Admin") || result.contains('A'), "should preserve identity somehow: {result}", ); } diff --git a/src/strategy/unique.rs b/src/strategy/unique.rs index 4f93984..2441c2e 100644 --- a/src/strategy/unique.rs +++ b/src/strategy/unique.rs @@ -92,7 +92,10 @@ pub fn shrink_unique(info: &PathInfo, anchors: &[String]) -> String { }) .collect(); - let refs: Vec<&str> = abbreviated.iter().map(|s| s.as_str()).collect(); + let refs: Vec<&str> = abbreviated + .iter() + .map(std::string::String::as_str) + .collect(); info.reassemble(&refs) }