lints: adopt canonical lints; fix usize underflow panic on dir_length(0) - #3
Merged
Conversation
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) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
shrinkpathcarried no[lints]block at all. This adds the canonical recipe and fixes everything it lit up.The bug the missing lint was hiding
ShrinkOptions::dir_length(n)is a public builder setter that accepts anyusize.abbreviate_segmentthen takeslencharacters by pushing the first and takinglen - 1more — sodir_length(0)underflows:Reachable from ordinary safe caller code — no unsafe, no malformed input, just a value the builder advertises as legal. In a release build overflow checks are off, so
0 - 1becomesusize::MAXandtake()yields the whole segment: no panic, but the segment comes back completely unabbreviated, which is the opposite of whatdir_length(0)asks for.Fixed with
len.saturating_sub(1), so the leading character survives — mirroring the dot branch, which already yields"."atlen == 0.Commits
test(fish): prove dir_length(0) panics with usize underflow(two tests: one on the internal helper, one through the publicshrink()/ShrinkOptionsAPI)fix(fish): saturate dir_length(0); adopt canonical lintsUnwraps removed structurally
strategy/fish.rschars.next().unwrap()let-else returning""path_info.rsparts.last().unwrap()+parts[..len-1]split_last()— also retires the separateis_empty()guard and thelen - 1indexfs_aware.rschars.next().unwrap()map_or_else, folding theis_empty()special case into the same expressionWhat lit up, and how each was closed
clippy::unwrap_usedclippy::return_self_not_must_use#[must_use]on the builder methodsclippy::redundant_closureclippy::doc_markdownclippy::single_char_patternclippy::map_unwrap_oris_ok_and)clippy::format_push_stringwrite!+fmt::Write)No lint was added to an allow list to make this pass. The unit tests inside
srccarry the sanctioned#![cfg_attr(test, allow(clippy::unwrap_used, clippy::expect_used))].One thing to decide
The
[lints]table is honored by Cargo 1.74+; this crate declaresrust-version = "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. I leftrust-versionalone rather than raise a published library's MSRV as a side effect of a lints PR — flagging it as a separate call.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.🤖 Generated with Claude Code