Skip to content

Commit

Permalink
Make [PathExt::normalize] more consistent
Browse files Browse the repository at this point in the history
  • Loading branch information
dylni committed Jan 15, 2021
1 parent 59c86a2 commit 0384878
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 22 deletions.
2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ categories = ["command-line-interface", "filesystem", "os"]
license = "MIT OR Apache-2.0"

[package.metadata.docs.rs]
rustc-args = ["--cfg", "normpath_docs_rs"]
rustdoc-args = [
"--cfg", "normpath_docs_rs",
"--extern-html-root-url", "std=https://doc.rust-lang.org",
"-Zunstable-options",
]
Expand Down
8 changes: 8 additions & 0 deletions src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,14 @@ impl BasePath {
self.as_path().normalize()
}

/// Equivalent to [`PathExt::normalize_virtually`].
#[cfg(any(doc, windows))]
#[cfg_attr(normpath_docs_rs, doc(cfg(windows)))]
#[inline]
pub fn normalize_virtually(&self) -> io::Result<BasePathBuf> {
self.as_path().normalize_virtually()
}

fn check_parent(&self) -> Result<(), ParentError> {
self.components()
.next_back()
Expand Down
50 changes: 47 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@
//! [`PathBuf::pop`]: ::std::path::PathBuf::pop
//! [`PathBuf::push`]: ::std::path::PathBuf::push
// Only require a nightly compiler when building documentation for docs.rs.
// This is a private option that should not be used.
// https://github.com/rust-lang/docs.rs/issues/147#issuecomment-389544407
#![cfg_attr(normpath_docs_rs, feature(doc_cfg))]
#![warn(unused_results)]

use std::io;
Expand Down Expand Up @@ -118,12 +122,16 @@ pub trait PathExt: private::Sealed {
///
/// # Errors
///
/// Returns an error if `self` cannot be normalized or contains a null
/// byte. On Unix, only existing paths can be normalized.
/// Returns an error if `self` cannot be normalized or does not exist, even
/// on Windows.
///
/// This method is designed to give mostly consistent errors on different
/// platforms, even when the functions it calls have different behavior. To
/// normalize paths that might not exist, use [`normalize_virtually`].
///
/// # Examples
///
/// ```
/// ```no_run
/// # use std::io;
/// use std::path::Path;
///
Expand All @@ -141,19 +149,55 @@ pub trait PathExt: private::Sealed {
///
/// [`fs::canonicalize`]: ::std::fs::canonicalize
/// [`GetFullPathNameW`]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfullpathnamew
/// [`normalize_virtually`]: Self::normalize_virtually
/// [rust-lang/rust#42869]: https://github.com/rust-lang/rust/issues/42869
/// [rust-lang/rust#49342]: https://github.com/rust-lang/rust/issues/49342
/// [rust-lang/rust#52440]: https://github.com/rust-lang/rust/issues/52440
/// [prefix]: ::std::path::Prefix
/// [verbatim]: ::std::path::Prefix::is_verbatim
fn normalize(&self) -> io::Result<BasePathBuf>;

/// Equivalent to [`normalize`] but does not access the file system.
///
/// # Errors
///
/// Returns an error if `self` cannot be normalized or contains a null
/// byte. Nonexistent paths will not cause an error.
///
/// # Examples
///
/// ```
/// # use std::io;
/// use std::path::Path;
///
/// use normpath::PathExt;
///
/// #[cfg(windows)]
/// assert_eq!(
/// Path::new(r"X:\foo\baz\test.rs"),
/// Path::new("X:/foo/bar/../baz/test.rs").normalize_virtually()?,
/// );
/// #
/// # Ok::<_, io::Error>(())
/// ```
///
/// [`normalize`]: Self::normalize
#[cfg(any(doc, windows))]
#[cfg_attr(normpath_docs_rs, doc(cfg(windows)))]
fn normalize_virtually(&self) -> io::Result<BasePathBuf>;
}

impl PathExt for Path {
#[inline]
fn normalize(&self) -> io::Result<BasePathBuf> {
imp::normalize(self)
}

#[cfg(any(doc, windows))]
#[inline]
fn normalize_virtually(&self) -> io::Result<BasePathBuf> {
imp::normalize_virtually(self)
}
}

mod private {
Expand Down
8 changes: 7 additions & 1 deletion src/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ fn normalize_verbatim(path: &Path) -> BasePathBuf {
BasePathBuf(OsString::from_wide(&path))
}

pub(super) fn normalize(initial_path: &Path) -> io::Result<BasePathBuf> {
pub(super) fn normalize_virtually(
initial_path: &Path,
) -> io::Result<BasePathBuf> {
// [GetFullPathNameW] always converts separators.
let (mut wide_path, path) = convert_separators(initial_path);

Expand Down Expand Up @@ -124,6 +126,10 @@ pub(super) fn normalize(initial_path: &Path) -> io::Result<BasePathBuf> {
}
}

pub(super) fn normalize(path: &Path) -> io::Result<BasePathBuf> {
path.metadata().and_then(|_| normalize_virtually(path))
}

fn get_prefix(base: &BasePath) -> PrefixComponent<'_> {
if let Some(Component::Prefix(prefix)) = base.components().next() {
prefix
Expand Down
20 changes: 18 additions & 2 deletions tests/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::io;
use std::path::Path;

use normpath::BasePath;
use normpath::BasePathBuf;
use normpath::PathExt;

// https://github.com/rust-lang/rust/issues/76483
Expand All @@ -33,6 +34,21 @@ where
assert_eq!(Wrapper(expected), result.as_ref().map(AsRef::as_ref));
}

pub(crate) fn normalize<P>(path: P) -> io::Result<BasePathBuf>
where
P: AsRef<Path>,
{
let path = path.as_ref();
#[cfg(windows)]
{
path.normalize_virtually()
}
#[cfg(not(windows))]
{
path.normalize()
}
}

#[rustversion::attr(since(1.46.0), track_caller)]
pub(crate) fn test(path: &str, joined_path: &str, normalized_path: &str) {
let joined_path = Path::new(joined_path);
Expand All @@ -43,8 +59,8 @@ pub(crate) fn test(path: &str, joined_path: &str, normalized_path: &str) {
.unwrap();
assert_eq!(joined_path, base.join(path));

assert_eq(normalized_path, joined_path.normalize());
assert_eq(normalized_path, normalized_path.normalize());
assert_eq(normalized_path, normalize(joined_path));
assert_eq(normalized_path, normalize(normalized_path));
}

macro_rules! test {
Expand Down
29 changes: 14 additions & 15 deletions tests/integration.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::convert::TryInto;
use std::env;
use std::fs::File;
use std::io;
use std::os::raw::c_int;
use std::path::Path;

use normpath::BasePath;
Expand All @@ -12,24 +12,23 @@ use tempfile::tempdir;
#[macro_use]
mod common;

const ERROR_INVALID_NAME: c_int = {
#[cfg(windows)]
{
123
}
#[cfg(not(windows))]
{
use libc::ENOENT;

ENOENT
}
};
#[cfg(not(windows))]
use libc::ENOENT as ERROR_INVALID_NAME;
#[cfg(windows)]
use winapi::shared::winerror::ERROR_INVALID_NAME;

#[test]
fn test_empty() -> io::Result<()> {
assert_eq!(
Some(ERROR_INVALID_NAME),
Path::new("").normalize().unwrap_err().raw_os_error(),
Some(Ok(ERROR_INVALID_NAME)),
common::normalize("")
.unwrap_err()
.raw_os_error()
.map(TryInto::try_into),
);
assert_eq!(
io::ErrorKind::NotFound,
Path::new("").normalize().unwrap_err().kind(),
);

let base = env::current_dir()?;
Expand Down
5 changes: 4 additions & 1 deletion tests/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ fn test_unc_absolute() {

assert_eq!(
"normpath: partial UNC prefixes are invalid",
Path::new(r"\\server").normalize().unwrap_err().to_string(),
Path::new(r"\\server")
.normalize_virtually()
.unwrap_err()
.to_string(),
);
}

Expand Down

0 comments on commit 0384878

Please sign in to comment.