Skip to content

Commit 5eec2b0

Browse files
hyf0claude
andauthored
feat!: explicitly require Cow for absolutize_with base parameter (#37)
* feat!: explicitly require Cow for `absolutize_with` base parameter * fix: address PR review comments and fix Windows CI - Move `path::Path` import into unix-gated test to fix unused import warning on Windows (clippy -D warnings) - Fix comment: `base.normalize()` → `base.absolutize()` since normalize doesn't resolve relative bases against cwd Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a172f38 commit 5eec2b0

File tree

9 files changed

+99
-171
lines changed

9 files changed

+99
-171
lines changed

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ Single-trait library: `SugarPath` trait (`src/sugar_path.rs`) adds path manipula
3333

3434
- **`src/sugar_path.rs`** — Trait definition with doc examples
3535
- **`src/impl_sugar_path.rs`** — All implementations. Two impl blocks: one for `Path`, one for `T: Deref<Target = str>`. Contains `normalize_inner()`, `needs_normalization()`, `relative_str()` and helper functions
36-
- **`src/utils.rs`**`IntoCowPath` trait for flexible base-path input in `absolutize_with`
36+
- **`src/utils.rs`**`get_current_dir()` helper for `absolutize()`
3737

3838
Key patterns:
3939
- `Cow<'_, Path>` return types to avoid allocation when the input is already clean

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,13 @@ assert_eq!(
7373
[`absolutize()`] resolves a relative path against the current working directory. [`absolutize_with()`] lets you supply a custom base.
7474

7575
```rust
76+
use std::borrow::Cow;
7677
use sugar_path::SugarPath;
7778

7879
#[cfg(target_family = "unix")]
7980
{
80-
assert_eq!("./world".absolutize_with("/hello"), "/hello/world".as_path());
81-
assert_eq!("../world".absolutize_with("/hello"), "/world".as_path());
81+
assert_eq!("./world".absolutize_with(Cow::Borrowed("/hello".as_path())), "/hello/world".as_path());
82+
assert_eq!("../world".absolutize_with(Cow::Borrowed("/hello".as_path())), "/world".as_path());
8283
}
8384
```
8485

benches/absolutize.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use std::borrow::Cow;
12
use std::hint::black_box;
23
use std::path::Path;
34

@@ -21,7 +22,7 @@ fn criterion_benchmark(c: &mut Criterion) {
2122
c.bench_function("absolutize_with", |b| {
2223
b.iter(|| {
2324
for absolute_path in ABSOLUTE_PATHS {
24-
black_box(absolute_path.absolutize_with(&cwd));
25+
black_box(absolute_path.absolutize_with(Cow::Borrowed(cwd.as_path())));
2526
}
2627
})
2728
});
@@ -37,7 +38,7 @@ fn criterion_benchmark(c: &mut Criterion) {
3738
c.bench_function("absolutize_with_already_clean_absolute", |b| {
3839
b.iter(|| {
3940
for path in ABSOLUTE_PATHS {
40-
black_box(Path::new(path).absolutize_with(&cwd));
41+
black_box(Path::new(path).absolutize_with(Cow::Borrowed(cwd.as_path())));
4142
}
4243
})
4344
});
@@ -53,7 +54,7 @@ fn criterion_benchmark(c: &mut Criterion) {
5354
c.bench_function("absolutize_with_relative_paths", |b| {
5455
b.iter(|| {
5556
for path in RELATIVE_CLEAN {
56-
black_box(Path::new(path).absolutize_with(&cwd));
57+
black_box(Path::new(path).absolutize_with(Cow::Borrowed(cwd.as_path())));
5758
}
5859
})
5960
});

src/impl_sugar_path.rs

Lines changed: 11 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,7 @@ use std::{
99
use memchr::{memchr, memrchr};
1010
use smallvec::SmallVec;
1111

12-
use crate::{
13-
SugarPath,
14-
utils::{IntoCowPath, get_current_dir},
15-
};
12+
use crate::{SugarPath, utils::get_current_dir};
1613

1714
type StrVec<'a> = SmallVec<[&'a str; 8]>;
1815

@@ -28,23 +25,16 @@ impl SugarPath for Path {
2825
self.absolutize_with(get_current_dir())
2926
}
3027

31-
// Using `Cow` is on purpose.
32-
// - Users could choose to pass a reference or an owned value depending on their use case.
33-
// - If we accept `PathBuf` only, it may cause unnecessary allocations on case that `self` is already absolute.
34-
// - If we accept `&Path` only, it may cause unnecessary cloning that users already have an owned value.
35-
//
36-
// NOTE: we intentionally keep the return lifetime tied to `&self` (not `'a`).
37-
// Unifying them (`&'a self, impl IntoCowPath<'a>) -> Cow<'a, ...>`) would allow
28+
// NOTE: the return lifetime is tied to `&self` (not `'a`).
29+
// Unifying them (`&'a self, Cow<'a, Path>) -> Cow<'a, ...>`) would allow
3830
// borrowing from `base` for noop cases ("", "."), but it constrains callers:
39-
// base's borrowed data must outlive self. That's a semver-breaking trade-off
40-
// for a narrow benefit — callers needing "".absolutize_with(base) can just
41-
// call base.normalize() directly.
42-
fn absolutize_with<'a>(&self, base: impl IntoCowPath<'a>) -> Cow<'_, Path> {
31+
// base's borrowed data must outlive self. Callers needing "".absolutize_with(base)
32+
// can just call base.absolutize() directly.
33+
fn absolutize_with<'a>(&self, base: Cow<'a, Path>) -> Cow<'_, Path> {
4334
if self.is_absolute() {
4435
return self.normalize();
4536
}
4637

47-
let base: Cow<'a, Path> = base.into_cow_path();
4838
let mut base =
4939
if base.is_absolute() { base } else { Cow::Owned(base.absolutize().into_owned()) };
5040

@@ -431,7 +421,7 @@ impl<T: Deref<Target = str>> SugarPath for T {
431421
self.as_path().absolutize()
432422
}
433423

434-
fn absolutize_with<'a>(&self, base: impl IntoCowPath<'a>) -> Cow<'_, Path> {
424+
fn absolutize_with<'a>(&self, base: Cow<'a, Path>) -> Cow<'_, Path> {
435425
self.as_path().absolutize_with(base)
436426
}
437427

@@ -672,7 +662,7 @@ fn replace_main_separator(input: &str) -> Option<String> {
672662

673663
#[cfg(test)]
674664
mod tests {
675-
use std::{borrow::Cow, path::Path, path::PathBuf};
665+
use std::{borrow::Cow, path::PathBuf};
676666

677667
use super::SugarPath;
678668

@@ -701,32 +691,14 @@ mod tests {
701691
#[test]
702692
fn _test_absolutize_with() {
703693
let tmp = "";
704-
705-
let str = "";
706-
tmp.absolutize_with(str);
707-
708-
let string = String::new();
709-
tmp.absolutize_with(string);
710-
711-
let ref_string = &String::new();
712-
tmp.absolutize_with(ref_string);
713-
714-
let path = Path::new("");
715-
tmp.absolutize_with(path);
716-
717-
let path_buf = PathBuf::new();
718-
tmp.absolutize_with(path_buf);
719-
720-
let cow_path = Cow::Borrowed(Path::new(""));
721-
tmp.absolutize_with(cow_path);
722-
723-
let cow_str = Cow::Borrowed("");
724-
tmp.absolutize_with(cow_str);
694+
tmp.absolutize_with(Cow::Borrowed("".as_path()));
695+
tmp.absolutize_with(Cow::Owned(PathBuf::new()));
725696
}
726697

727698
#[cfg(target_family = "unix")]
728699
#[test]
729700
fn normalize() {
701+
use std::path::Path;
730702
assert_eq_str!(Path::new("/foo/../../../bar").normalize(), "/bar");
731703
assert_eq_str!(Path::new("a//b//../b").normalize(), "a/b");
732704
assert_eq_str!(Path::new("/foo/../../../bar").normalize(), "/bar");

src/lib.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,16 +58,17 @@
5858
//! - [SugarPath::absolutize_with] allows you to absolutize the given path with the base path.
5959
//!
6060
//! ```rust
61+
//! use std::borrow::Cow;
6162
//! use sugar_path::SugarPath;
6263
//! #[cfg(target_family = "unix")]
6364
//! {
64-
//! assert_eq!("./world".absolutize_with("/hello"), "/hello/world".as_path());
65-
//! assert_eq!("../world".absolutize_with("/hello"), "/world".as_path());
65+
//! assert_eq!("./world".absolutize_with(Cow::Borrowed("/hello".as_path())), "/hello/world".as_path());
66+
//! assert_eq!("../world".absolutize_with(Cow::Borrowed("/hello".as_path())), "/world".as_path());
6667
//! }
6768
//! #[cfg(target_family = "windows")]
6869
//! {
69-
//! assert_eq!(".\\world".absolutize_with("C:\\hello"), "C:\\hello\\world".as_path());
70-
//! assert_eq!("..\\world".absolutize_with("C:\\hello"), "C:\\world".as_path());
70+
//! assert_eq!(".\\world".absolutize_with(Cow::Borrowed("C:\\hello".as_path())), "C:\\hello\\world".as_path());
71+
//! assert_eq!("..\\world".absolutize_with(Cow::Borrowed("C:\\hello".as_path())), "C:\\world".as_path());
7172
//! }
7273
//! ```
7374
//!

src/sugar_path.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,6 @@ use std::{
33
path::{Path, PathBuf},
44
};
55

6-
use crate::utils::IntoCowPath;
7-
86
pub trait SugarPath {
97
/// Normalizes the given path, resolving `'..'` and `'.'` segments.
108
///
@@ -55,19 +53,20 @@ pub trait SugarPath {
5553
/// ## Examples
5654
///
5755
/// ```rust
56+
/// use std::borrow::Cow;
5857
/// use sugar_path::SugarPath;
5958
/// #[cfg(target_family = "unix")]
6059
/// {
61-
/// assert_eq!("./world".absolutize_with("/hello"), "/hello/world".as_path());
62-
/// assert_eq!("../world".absolutize_with("/hello"), "/world".as_path());
60+
/// assert_eq!("./world".absolutize_with(Cow::Borrowed("/hello".as_path())), "/hello/world".as_path());
61+
/// assert_eq!("../world".absolutize_with(Cow::Borrowed("/hello".as_path())), "/world".as_path());
6362
/// }
6463
/// #[cfg(target_family = "windows")]
6564
/// {
66-
/// assert_eq!(".\\world".absolutize_with("C:\\hello"), "C:\\hello\\world".as_path());
67-
/// assert_eq!("..\\world".absolutize_with("C:\\hello"), "C:\\world".as_path());
65+
/// assert_eq!(".\\world".absolutize_with(Cow::Borrowed("C:\\hello".as_path())), "C:\\hello\\world".as_path());
66+
/// assert_eq!("..\\world".absolutize_with(Cow::Borrowed("C:\\hello".as_path())), "C:\\world".as_path());
6867
/// }
6968
/// ```
70-
fn absolutize_with<'a>(&self, base: impl IntoCowPath<'a>) -> Cow<'_, Path>;
69+
fn absolutize_with<'a>(&self, base: Cow<'a, Path>) -> Cow<'_, Path>;
7170

7271
///
7372
/// ```rust

src/utils.rs

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -14,61 +14,3 @@ pub fn get_current_dir() -> Cow<'static, Path> {
1414
Cow::Owned(std::env::current_dir().unwrap())
1515
}
1616
}
17-
18-
pub trait IntoCowPath<'a> {
19-
fn into_cow_path(self) -> Cow<'a, Path>;
20-
}
21-
22-
impl<'a> IntoCowPath<'a> for &'a Path {
23-
fn into_cow_path(self) -> Cow<'a, Path> {
24-
Cow::Borrowed(self)
25-
}
26-
}
27-
28-
impl<'a> IntoCowPath<'a> for PathBuf {
29-
fn into_cow_path(self) -> Cow<'a, Path> {
30-
Cow::Owned(self)
31-
}
32-
}
33-
34-
impl<'a> IntoCowPath<'a> for &'a PathBuf {
35-
fn into_cow_path(self) -> Cow<'a, Path> {
36-
Cow::Borrowed(self.as_path())
37-
}
38-
}
39-
40-
impl<'a> IntoCowPath<'a> for &'a str {
41-
fn into_cow_path(self) -> Cow<'a, Path> {
42-
Cow::Borrowed(Path::new(self))
43-
}
44-
}
45-
46-
impl<'a> IntoCowPath<'a> for String {
47-
fn into_cow_path(self) -> Cow<'a, Path> {
48-
Cow::Owned(PathBuf::from(self))
49-
}
50-
}
51-
52-
impl<'a> IntoCowPath<'a> for &'a String {
53-
fn into_cow_path(self) -> Cow<'a, Path> {
54-
Cow::Borrowed(Path::new(self))
55-
}
56-
}
57-
58-
impl<'a> IntoCowPath<'a> for Cow<'a, Path> {
59-
fn into_cow_path(self) -> Cow<'a, Path> {
60-
match self {
61-
Cow::Borrowed(path) => Cow::Borrowed(path),
62-
Cow::Owned(path) => Cow::Owned(path),
63-
}
64-
}
65-
}
66-
67-
impl<'a> IntoCowPath<'a> for Cow<'a, str> {
68-
fn into_cow_path(self) -> Cow<'a, Path> {
69-
match self {
70-
Cow::Borrowed(s) => s.into_cow_path(),
71-
Cow::Owned(s) => s.into_cow_path(),
72-
}
73-
}
74-
}

0 commit comments

Comments
 (0)