fix: normalize aliased path (#78)

* fix: normalize aliased path

closes #73
closes #79

* update

Author: Boshen

.github/workflows/ci.yml

@@ -78,13 +78,13 @@ jobs:
 
       - name: corepack
         run: corepack enable
-  
+
       - name: Setup Node.js
         uses: actions/setup-node@v4
         with:
           node-version: 20
           cache: pnpm
-  
+
       - name: Install dependencies
         run: pnpm install --frozen-lockfile
 
@@ -211,6 +211,8 @@ jobs:
           - os: ubuntu-latest
           - os: macos-14
     runs-on: ${{ matrix.os }}
+    env:
+      RUST_BACKTRACE: 1
     steps:
       - uses: actions/checkout@v4
       - uses: ./.github/actions/pnpm

Cargo.lock

@@ -14,9 +14,7 @@ rust-version = "1.60"
 include      = ["/src", "/examples", "/benches"]
 
 [workspace]
-members = [
-  "napi",
-]
+members = ["napi"]
 
 [lib]
 doctest = false
@@ -89,9 +87,10 @@ json-strip-comments = { version = "1.0.2" }
 codspeed-criterion-compat = { version = "2.3.3", default-features = false, optional = true }
 
 [dev-dependencies]
-vfs       = "0.10.0"                                        # for testing with in memory file system
-rayon     = { version = "1.8.1" }
-criterion = { version = "0.5.1", default-features = false }
+vfs            = "0.10.0"                                        # for testing with in memory file system
+rayon          = { version = "1.8.1" }
+criterion      = { version = "0.5.1", default-features = false }
+normalize-path = { version = "0.2.1" }
 
 [features]
 codspeed = ["codspeed-criterion-compat"]

Cargo.toml

@@ -244,7 +244,7 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
 
         match specifier.as_bytes()[0] {
             // 3. If X begins with './' or '/' or '../'
-            b'/' => self.require_absolute(cached_path, specifier, ctx),
+            b'/' | b'\\' => self.require_absolute(cached_path, specifier, ctx),
             // 3. If X begins with './' or '/' or '../'
             b'.' => self.require_relative(cached_path, specifier, ctx),
             // 4. If X begins with '#'
@@ -278,7 +278,7 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
         specifier: &str,
         ctx: &mut Ctx,
     ) -> Result<CachedPath, ResolveError> {
-        debug_assert!(specifier.starts_with('/'));
+        debug_assert!(specifier.starts_with(|c| c == '/' || c == '\\'));
         if !self.options.prefer_relative && self.options.prefer_absolute {
             if let Ok(path) = self.load_package_self_or_node_modules(cached_path, specifier, ctx) {
                 return Ok(path);
@@ -295,9 +295,11 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
         } else {
             for root in &self.options.roots {
                 let cached_path = self.cache.value(root);
-                if let Ok(path) =
-                    self.require_relative(&cached_path, specifier.trim_start_matches('/'), ctx)
-                {
+                if let Ok(path) = self.require_relative(
+                    &cached_path,
+                    specifier.trim_start_matches(|c| c == '/' || c == '\\'),
+                    ctx,
+                ) {
                     return Ok(path);
                 }
             }
@@ -887,16 +889,25 @@ impl<Fs: FileSystem + Default> ResolverGeneric<Fs> {
             && !request.strip_prefix(alias_value).is_some_and(|prefix| prefix.starts_with('/'))
         {
             let tail = &request[alias_key.len()..];
-            // Must not append anything to alias_value if it is a file.
-            if !tail.is_empty() {
-                let alias_value_cached_path = self.cache.value(Path::new(alias_value));
+
+            let new_specifier = if tail.is_empty() {
+                Cow::Borrowed(alias_value)
+            } else {
+                let alias_value = Path::new(alias_value).normalize();
+                // Must not append anything to alias_value if it is a file.
+                let alias_value_cached_path = self.cache.value(&alias_value);
                 if alias_value_cached_path.is_file(&self.cache.fs, ctx) {
                     return Ok(None);
                 }
-            }
-            let new_specifier = format!("{alias_value}{tail}");
+
+                // Remove the leading slash so the final path is concatenated.
+                let tail = tail.trim_start_matches(|c| c == '/' || c == '\\');
+                let normalized = alias_value.normalize_with(tail);
+                Cow::Owned(normalized.to_string_lossy().to_string())
+            };
+
             ctx.with_fully_specified(false);
-            return match self.require(cached_path, &new_specifier, ctx) {
+            return match self.require(cached_path, new_specifier.as_ref(), ctx) {
                 Err(ResolveError::NotFound(_)) => Ok(None),
                 Ok(path) => return Ok(Some(path)),
                 Err(err) => return Err(err),

src/lib.rs

@@ -27,6 +27,7 @@ pub trait PathUtil {
 }
 
 impl PathUtil for Path {
+    // https://github.com/parcel-bundler/parcel/blob/e0b99c2a42e9109a9ecbd6f537844a1b33e7faf5/packages/utils/node-resolver-rs/src/path.rs#L7
     fn normalize(&self) -> PathBuf {
         let mut components = self.components().peekable();
         let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek() {
@@ -39,7 +40,7 @@ impl PathUtil for Path {
 
         for component in components {
             match component {
-                Component::Prefix(..) => unreachable!(),
+                Component::Prefix(..) => unreachable!("Path {:?}", self),
                 Component::RootDir => {
                     ret.push(component.as_os_str());
                 }
@@ -56,6 +57,7 @@ impl PathUtil for Path {
         ret
     }
 
+    // https://github.com/parcel-bundler/parcel/blob/e0b99c2a42e9109a9ecbd6f537844a1b33e7faf5/packages/utils/node-resolver-rs/src/path.rs#L37
     fn normalize_with<B: AsRef<Self>>(&self, subpath: B) -> PathBuf {
         let subpath = subpath.as_ref();
         let mut components = subpath.components().peekable();
@@ -66,7 +68,9 @@ impl PathUtil for Path {
         let mut ret = self.to_path_buf();
         for component in subpath.components() {
             match component {
-                Component::Prefix(..) | Component::RootDir => unreachable!(),
+                Component::Prefix(..) | Component::RootDir => {
+                    unreachable!("Path {:?} Subpath {:?}", self, subpath)
+                }
                 Component::CurDir => {}
                 Component::ParentDir => {
                     ret.pop();

src/path.rs

@@ -1,6 +1,9 @@
 //! <https://github.com/webpack/enhanced-resolve/blob/main/test/alias.test.js>
 
-use crate::{AliasValue, Resolution, ResolveError, ResolveOptions, Resolver};
+use normalize_path::NormalizePath;
+use std::path::Path;
+
+use crate::{AliasValue, Resolution, ResolveContext, ResolveError, ResolveOptions, Resolver};
 
 #[test]
 #[cfg(not(target_os = "windows"))] // MemoryFS's path separator is always `/` so the test will not pass in windows.
@@ -129,29 +132,33 @@ fn absolute_path() {
     assert_eq!(resolution, Err(ResolveError::Ignored(f.join("foo"))));
 }
 
+fn check_slash(path: &Path) {
+    let s = path.to_string_lossy().to_string();
+    #[cfg(target_os = "windows")]
+    {
+        assert!(!s.contains('/'), "{s}");
+        assert!(s.contains('\\'), "{s}");
+    }
+    #[cfg(not(target_os = "windows"))]
+    {
+        assert!(s.contains('/'), "{s}");
+        assert!(!s.contains('\\'), "{s}");
+    }
+}
+
 #[test]
 fn system_path() {
     let f = super::fixture();
     let resolver = Resolver::new(ResolveOptions {
         alias: vec![(
             "@app".into(),
-            vec![AliasValue::Path(f.join("alias").to_str().unwrap().to_string())],
+            vec![AliasValue::Path(f.join("alias").to_string_lossy().to_string())],
         )],
         ..ResolveOptions::default()
     });
-    let resolution = resolver.resolve(&f, "@app/files/a").map(Resolution::into_path_buf);
-    assert_eq!(resolution, Ok(f.join("alias/files/a.js")));
-    let string = resolution.unwrap().to_string_lossy().to_string();
-    #[cfg(target_os = "windows")]
-    {
-        assert!(!string.contains('/'));
-        assert!(string.contains('\\'));
-    }
-    #[cfg(not(target_os = "windows"))]
-    {
-        assert!(string.contains('/'));
-        assert!(!string.contains('\\'));
-    }
+    let path = resolver.resolve(&f, "@app/files/a").map(Resolution::into_path_buf).unwrap();
+    assert_eq!(path, f.join("alias/files/a.js"));
+    check_slash(&path);
 }
 
 // Not part of enhanced-resolve
@@ -168,3 +175,43 @@ fn infinite_recursion() {
     let resolution = resolver.resolve(f, "./a");
     assert_eq!(resolution, Err(ResolveError::Recursion));
 }
+
+#[test]
+fn alias_is_full_path() {
+    let f = super::fixture();
+    let dir = f.join("foo");
+    let dir_str = dir.to_string_lossy().to_string();
+
+    let resolver = Resolver::new(ResolveOptions {
+        alias: vec![("@".into(), vec![AliasValue::Path(dir_str.clone())])],
+        ..ResolveOptions::default()
+    });
+
+    let mut ctx = ResolveContext::default();
+
+    let specifiers = [
+        "@/index".to_string(),
+        // specifier has multiple `/` for reasons we'll never know
+        "@////index".to_string(),
+        // specifier is a full path
+        dir_str,
+    ];
+
+    for specifier in specifiers {
+        let resolution = resolver.resolve_with_context(&f, &specifier, &mut ctx);
+        assert_eq!(resolution.map(|r| r.full_path()), Ok(dir.join("index.js")));
+    }
+
+    for path in ctx.file_dependencies {
+        assert_eq!(path, path.normalize(), "{path:?}");
+        check_slash(&path);
+    }
+
+    for path in ctx.missing_dependencies {
+        assert_eq!(path, path.normalize(), "{path:?}");
+        check_slash(&path);
+        if let Some(path) = path.parent() {
+            assert!(!path.is_file(), "{path:?} must not be a file");
+        }
+    }
+}

src/tests/alias.rs

@@ -1,5 +1,7 @@
 //! https://github.com/webpack/enhanced-resolve/blob/main/test/missing.test.js
 
+use normalize_path::NormalizePath;
+
 use crate::{AliasValue, ResolveContext, ResolveOptions, Resolver};
 
 #[test]
@@ -52,10 +54,15 @@ fn test() {
         let mut ctx = ResolveContext::default();
         let _ = resolver.resolve_with_context(&f, specifier, &mut ctx);
 
-        for dep in missing_dependencies {
+        for path in ctx.file_dependencies {
+            assert_eq!(path, path.normalize(), "{path:?}");
+        }
+
+        for path in missing_dependencies {
+            assert_eq!(path, path.normalize(), "{path:?}");
             assert!(
-                ctx.missing_dependencies.contains(&dep),
-                "{specifier}: {dep:?} not in {:?}",
+                ctx.missing_dependencies.contains(&path),
+                "{specifier}: {path:?} not in {:?}",
                 &ctx.missing_dependencies
             );
         }
@@ -86,8 +93,13 @@ fn alias_and_extensions() {
     let _ = resolver.resolve_with_context(&f, "@scope-js/package-name/dir/router", &mut ctx);
     let _ = resolver.resolve_with_context(&f, "react-dom/client", &mut ctx);
 
-    for dep in ctx.missing_dependencies {
-        if let Some(path) = dep.parent() {
+    for path in ctx.file_dependencies {
+        assert_eq!(path, path.normalize(), "{path:?}");
+    }
+
+    for path in ctx.missing_dependencies {
+        assert_eq!(path, path.normalize(), "{path:?}");
+        if let Some(path) = path.parent() {
             assert!(!path.is_file(), "{path:?} must not be a file");
         }
     }

src/tests/missing.rs

@@ -29,7 +29,7 @@ pub fn fixture_root() -> PathBuf {
 }
 
 pub fn fixture() -> PathBuf {
-    fixture_root().join("enhanced_resolve/test/fixtures")
+    fixture_root().join("enhanced_resolve").join("test").join("fixtures")
 }
 
 #[test]

src/tests/mod.rs

@@ -5,7 +5,7 @@ use std::path::PathBuf;
 use crate::{AliasValue, ResolveError, ResolveOptions, Resolver};
 
 fn dirname() -> PathBuf {
-    super::fixture_root().join("enhanced_resolve/test")
+    super::fixture_root().join("enhanced_resolve").join("test")
 }
 
 #[test]