Thoughts on NativePath

[encounter]

First of all, thanks for the library! It’s been very useful.

One problem I’ve run into is the way that NativePath is implemented. It’s very easy to write code that compiles for one platform but fails to compile on another. For example, this code builds on Unix platforms, but not Windows:

std::fs::create(UnixPath::new("path"));

Another example:

fn foo(path: &UnixPath) {}

foo(NativePath::new("path"));

It’s the only time I’ve had to install x86_64-windows-pc-gnu just to make sure the code compiles correctly on Windows. 🙂

Instead of an alias, I think NativePath should be a distinct type that requires an explicit conversion to and from UnixPath and WindowsPath. This guarantees at a type level that proper conversions are made when necessary.

What do you think?

[chipsenkbeil]

Have you tried using TypedPath? You can convert it to and from specific path types, and it implements most methods that the individual types offer.

You can use TypedPath::derive to create it from a string or series of bytes.

[encounter]

I’ve used TypedPath to mark functions that should accept either encoding, but in this case, there’s no way to indicate in a type-safe way that a function expects to accept a path that’s always in the native encoding (for example, because it will deref it to std::path::Path internally) or that a function expects a UnixPath and should not accept a NativePath.

[chipsenkbeil]

@encounter do you have a proposal for the redesign of NativePath and associated types? The original purpose was to allow you to write a program with library usage behaves like using Path, but seamlessly defaulting to UnixPath or WindowsPath underneath. I can see from your comments how this could be confusing or have surprising results, though. 😄

[encounter]

Sure, I was considering prototyping it in a PR if you were interested in the concept. My idea is to have NativePath be an opaque type with conversions into UnixPath and WindowsPath, using the same .with_unix_encoding() and .with_windows_encoding() methods. Only NativePath would have AsRef<std::path::Path>.

Also, with std, an impl TypedPathExt for std::path::Path (open to naming suggestions) and std::path::PathBuf with direct conversions into each type.

I’m wondering if NativePath even makes sense in no_std environments. If not, could we get away with just using std::path::Path in place of NativePath? The TypedPathExt trait could suffice for the easy conversion methods. Maybe this becomes more complicated with the existence of Utf8NativePath, which has no std analogue.

Just to clarify my use case: my application deals with native system paths and also Unix-style paths from configuration files. My desire is to use typed-path to ensure that native paths are converted into Unix-style when writing to configuration, and converted back to native style before being joined with native paths. The other benefit is that I use the Utf8* variants everywhere to avoid OsStr handling in any of the application code.

[chipsenkbeil]

I’m wondering if NativePath even makes sense in no_std environments. If not, could we get away with just using std::path::Path in place of NativePath?

IIRC, there was a previous request to support no_std specifically to use NativePath as a substitute for std::path::Path in some crates and binaries that needed path without std feature. Since this crate re-implements all of the functionality of std::path::Path, it was a seamless drop-in replacement with expected behavior when using NativePath.

My idea is to have NativePath be an opaque type with conversions into UnixPath and WindowsPath, using the same .with_unix_encoding() and .with_windows_encoding() methods. Only NativePath would have AsRefstd::path::Path.

Wondering if we could have NativePath be a newtype struct around UnixPath or WindowsPath, and implement Deref and DerefMut for the inner type. Reasoning being that I still want people to be able to import and seamlessly use an interface that behaves like std::path::Path (and associated). An example (not written to compile):

pub struct NativeType(
    #[cfg(unix)]
    UnixPath,
    #[cfg(windows)]
    WindowsPath,
)

impl Deref for NativeType {
    #[cfg(unix)]
    type Target = UnixPath;

    #[cfg(windows)]
    type Target = WindowsPath;

    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

[encounter]

IIRC, there was a previous request to support no_std specifically to use NativePath as a substitute for std::path::Path in some crates and binaries that needed path without std feature.

I see, keeping NativePath seems fine then.

Wondering if we could have NativePath be a newtype struct around UnixPath or WindowsPath, and implement Deref and DerefMut for the inner type.

I think that means you can still end up doing:

let path: &UnixPath = NativePath::new("path");
// or
fn foo(path: &UnixPath) {}
foo(NativePath::new("path");

But I haven’t tested.

If so, NativePath may have to implement its own implementations of each Path method. Basically a copy of std::path::Path if I understand correctly.

[chipsenkbeil]

If so, NativePath may have to implement its own implementations of each Path method. Basically a copy of std::path::Path if I understand correctly.

Yeah, this seems like the approach that would have to be taken.

[chipsenkbeil]

@encounter I think I've devised a solution that is less taxing than reimplementing the entire path and pathbuf types while still letting you have a distinct type that you can check without needing to use conditional config.

#34 provides PlatformEncoding and Utf8PlatformEncoding as distinct structs that implement encodings in the exact same way as the UnixEncoding/WindowsEncoding at compile time. Since these are structs and not type aliases, you should be able to use these with stronger type safety.

---

Let me know if this solves the problem for you, and if so I'll publish as 0.10.0.

[encounter]

Nice! I just tried it out, and it seems like a good solution.

However, I think only PlatformPath should implement the Path/PathBuf deref. Otherwise you can still accidentally write

fn foo(path: &UnixPath) -> _ {
  std::fs::exists(path)
}

which compiles on Unix, but not Windows.

It's a definite improvement though! Also, the .with_platform_encoding() is a great addition.

[chipsenkbeil]

However, I think only PlatformPath should implement the Path/PathBuf deref.

You mean removing the implementation from NativePath? i.e. removing it from UnixPath or WindowsPath depending on the cfg target family

If so, I'm fine with that as part of the 0.10.0 change.

[encounter]

Yeah, I think it makes sense from a type safety perspective.

If you wanted to bump MSRV to 1.74, I think these may be useful as well:

#[cfg(all(feature = "std", not(target_family = "wasm")))]
impl<'a> From<&'a StdPath> for &'a PlatformPath {
    fn from(std_path: &'a StdPath) -> &'a PlatformPath {
        PlatformPath::new(std_path.as_os_str().as_encoded_bytes())
    }
}

#[cfg(all(feature = "std", not(target_family = "wasm")))]
impl From<StdPathBuf> for PlatformPathBuf {
    fn from(std_path_buf: StdPathBuf) -> PlatformPathBuf {
        PlatformPathBuf::from(std_path_buf.into_os_string().into_encoded_bytes())
    }
}

[chipsenkbeil]

@encounter sure, seems like 1.74 came out nearly a year ago. With 1.58 being near 3 years old, I'm fine with doing a bump in Rust version requirement.

Care to submit a PR or two to make the changes you want? I'll take a look this weekend. 😄

[encounter]

Will do!

[chipsenkbeil]

Will do!

Checking if you're planning to send in a PR 😄

[chipsenkbeil]

Closing out as resolved. Tried to remove the TryAsRef and AsRef implementations that I think were causing this. Shipped in 0.10.0 today.