rust-lang / libs-team

The home of the library team
Apache License 2.0
110 stars 18 forks source link

Extend the `Path` API with some useful helper functions. #396

Open ChrisDenton opened 3 weeks ago

ChrisDenton commented 3 weeks ago

Proposal

Problem statement

Path/Pathbuf could use some helper methods for handling paths correctly and consistently across platforms.

Motivating examples or use cases

Currently PathBuf::push replaces the base path if the given path is absolute (or on Windows, has a prefix). This kind of emulates the way the OSes decide how a path passed to fs APIs should be interpreted (e.g. as relative to the cwd or not). While this can be useful, it shouldn't be the only way of joining paths. Given appropriate checks upfront it is technically possible to implement other forms of push on top of this but doing the obvious thing is error prone (oops I forgot to check that time!) and often wrong (i.e. checking for is_absolute).

In general it's useful to be able to push to a path without risking replacement so I think that's motivation enough. However a further issue is that implementing this on top of push is tricky:

// Extend a base path given a relative path, panics if `push` would replace the base path.
// This doesn't work on Windows because a subpath of e.g. `/biscuits` is a relative path but also replaces the base path.
fn extend_path(base: &mut PathBuf, subpath: &Path) {
    if !subpath.is_absolute() {
       base.push(subpath);
    } else {
        panic!("subpath must not be absolute!");
    }
}

For Unix platforms, we take pains to warn about the dangers of naively resolving .. components (i.e. resolving /path/to/../file as /path/file). However, that doesn't mean it's never useful. Sometimes when working within a subdirectory we don't intend to follow .. links. Also people have a habit of using a literal .. when they really did mean pop(). If nothing else, providing a function for this case can be a good hook to add documentation on the issue in a central location.

Finally a simple way to split off the prefix component would be useful. Paths without a root or prefix are much more consistent cross-platform and while this can technically be done using Path::components() it's not necessarily trivial. E.g. components() splits C:\path as [Prefix("C:'"), RootDir, Normal("path")], whereas C:path is split as [Prefix("C:"), Normal("path")].

Solution sketch

We should have a couple of new methods for pushing paths, depending on what the user wants. These would also have equivalent join methods on Path,

impl PathBuf {
    // Attempt to join paths. If `path` is absolute or otherwise can't be joined, return an error.
    fn push_checked(&mut self, path: impl AsRef<Path>) -> Result<PathBuf, PathJoinError>;

    // Push a path, naively resolving `.` and `..`
    // This is risky for Unix paths but sometimes it is what people need (e.g. shells)
    // Also people like to use a literal `..` when they do really mean `pop`.
    // The documentation can contain appropriate warnings and direct people to better functions for particular use cases.
    // The funny name also does make it stick out.
    fn push_lexically(&mut self, path: impl AsRef<Path>) -> Result<PathBuf, PathJoinError>;
}

Paths without a prefix are much easier to work with cross platform so let's make that easy to do:

impl Path {
    // split the prefix from the path, returning `(root, relative path)`.
    // Example: `C:\path\to\file` -> `Some("C:\", "path\to\file")`
    // returns None if there is no root
    fn split_prefix(&self) -> Option<(&Path, &Path)>
}

Alternatives

Not strictly speaking an alternative as they're not exclusive but another possibility would be to have a RelativePath type. This would avoid a lot of the complexity of prefixes and allow for more ergonomic APIs. RelativePath::new and RelativePath::new_unchecked handling absolute paths upfront could be a lot simpler to work with than requiring every function to handle absolute paths. It could also be an opportunity to fix other issues with the Path API without the nightmare of deprecating Path. Would compose well with #259 too. The more I type here the more I'm talking myself into liking the idea so I'll stop now.

Links and related work

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

Second, if there's a concrete solution: