Changes from PR feedback:

* Now canonicalizes relative path.
* Additional tests.
* Doc clarifications.

crates/bevy_asset/src/path.rs

@@ -122,47 +122,62 @@ impl<'a> AssetPath<'a> {
     }
 
     /// Resolves a relative asset path. The result will be an `AssetPath` which is resolved relative
-    /// to this path. There are three cases:
+    /// to this path.
     ///
     /// If the relative path begins with `#`, then it is considered an asset label, in which case
     /// the result is the base path with the label portion replaced.
     ///
     /// If the relative path starts with "./" or "../", then the result is combined with the base
-    /// path. The rules are the same as for URIs, which means that the relative path is resolved
-    /// relative to the *directory* of the base path, so a base path of `"x/y/z#foo"` combined with
-    /// a relative path of `"./a#bar"` yields `"x/y/a#bar"`.
+    /// path and canonicalized. The rules are the same as for URIs, which means that the relative
+    /// path is resolved relative to the *directory* of the base path, so a base path of
+    /// `"x/y/z#foo"` combined with a relative path of `"./a#bar"` yields `"x/y/a#bar"`.
     ///
     /// If neither of the above are true, then the relative path is considered a 'full' path,
-    /// and the result is simply a copy of the relative path, converted into an `AssetPath`.
-    /// Note that a 'full' asset path is still relative to the asset root directory, and not
-    /// necessarily an absolute filesystem path.
+    /// and the result is simply a canonicalized version of the relative path string, converted
+    /// into an `AssetPath`. Note that a 'full' asset path is still relative to the asset root
+    /// directory, and not necessarily an absolute filesystem path.
+    ///
+    /// Note that "./" and "../" elements are only canonicalized in the relative path, any such
+    /// elements in the base path are left as-is.
+    ///
+    /// # Panics
+    ///
+    /// This method will panic if the relative path has more 'parent' elements ("../") than
+    /// are available in the base path.
     pub fn resolve(&'a self, relative_path: &'a str) -> AssetPath<'a> {
-        if let Some(without_prefix) = relative_path.strip_prefix('#') {
-            AssetPath::new_ref(&self.path, Some(without_prefix))
-        } else if relative_path.starts_with("./") || relative_path.starts_with("../") {
-            let mut rpath = relative_path;
+        if let Some(label) = relative_path.strip_prefix('#') {
+            // It's a label only
+            AssetPath::new_ref(&self.path, Some(label))
+        } else {
+            let (rpath, rlabel) = match relative_path.split_once('#') {
+                Some((path, label)) => (path, Some(label.to_string())),
+                None => (relative_path, None),
+            };
             let mut fpath = PathBuf::from(self.path());
             if !fpath.pop() {
                 panic!("Can't compute relative path - not enough path elements");
             }
-            loop {
-                if let Some(rest) = rpath.strip_prefix("./") {
-                    rpath = rest;
-                } else if let Some(rest) = rpath.strip_prefix("../") {
-                    rpath = rest;
+
+            let rpath = PathBuf::from(rpath);
+            let mut first = true;
+            for elt in rpath.iter() {
+                if elt == "." {
+                    // Skip
+                } else if elt == ".." {
                     if !fpath.pop() {
                         panic!("Can't compute relative path - not enough path elements");
                     }
                 } else {
-                    break;
+                    if first {
+                        // If the first path element is not '.' or '..' then start fresh.
+                        fpath.clear();
+                    }
+                    fpath.push(elt);
                 }
+                first = false;
             }
-            fpath.push(rpath);
-            // Note: converting from a string causes AssetPath to look for the '#' separator, while
-            // passing fpath directly does not. We want the former.
-            AssetPath::from(String::from(fpath.to_str().unwrap()))
-        } else {
-            AssetPath::from(relative_path)
+
+            AssetPath::new(fpath, rlabel)
         }
     }
 
@@ -236,10 +251,19 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_relative_path() {
+    fn test_resolve_full() {
+        // A "full" path should ignore the base path.
         let base = AssetPath::from("alice/bob#carol");
         assert_eq!(base.resolve("joe/next"), AssetPath::from("joe/next"));
-        assert_eq!(base.resolve("#dave"), AssetPath::from("alice/bob#dave"));
+        assert_eq!(
+            base.resolve("joe/next#dave"),
+            AssetPath::from("joe/next#dave")
+        );
+    }
+
+    #[test]
+    fn test_resolve_partial() {
+        let base = AssetPath::from("alice/bob#carol");
         assert_eq!(
             base.resolve("./martin#dave"),
             AssetPath::from("alice/martin#dave")
@@ -249,4 +273,35 @@ mod tests {
             AssetPath::from("martin#dave")
         );
     }
+
+    #[test]
+    fn test_resolve_canonicalize() {
+        let base = AssetPath::from("alice/bob#carol");
+        assert_eq!(
+            base.resolve("./martin/stephan/..#dave"),
+            AssetPath::from("alice/martin#dave")
+        );
+        assert_eq!(
+            base.resolve("../martin/.#dave"),
+            AssetPath::from("martin#dave")
+        );
+        assert_eq!(
+            base.resolve("martin/stephan/..#dave"),
+            AssetPath::from("martin#dave")
+        );
+    }
+
+    #[test]
+    fn test_resolve_label() {
+        // A relative path with only a label should replace the label portion
+        let base = AssetPath::from("alice/bob#carol");
+        assert_eq!(base.resolve("#dave"), AssetPath::from("alice/bob#dave"));
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_resolve_insufficient_elements() {
+        let base = AssetPath::from("alice/bob#carol");
+        base.resolve("../../joe/next");
+    }
 }