Repository: seaofvoices/darklua (rust)

Issue:
Avoid `unable to obtain parent directory from ""` error for certain paths
When providing a sourcemap as a path like `sourcemap.json` which is relative to the directory containing the darklua configuration file, this error is thrown:

```
unable to obtain parent directory from ``
```

To fix it, the user has to provide the path as `./sourcemap.json`. Darklua can simplify this and adjust its relative path logic to assume that the parent path of `sourcemap.json` is `.`

Pull request:
Improve relative parent paths handling for sourcemaps
Closes #159 

- [x] add entry to the changelog


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6bf2f98..e59ff31 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* fix relative parent paths when working with sourcemaps ([#164](https://github.com/seaofvoices/darklua/pull/164))
 * add rule to remove assertions (`remove_assertions`) ([#163](https://github.com/seaofvoices/darklua/pull/163))
 * add rule to remove Roblox profiling function calls (`remove_debug_profiling`) ([#162](https://github.com/seaofvoices/darklua/pull/162))
 * add rule to remove interpolated strings (`remove_interpolated_string`) ([#156](https://github.com/seaofvoices/darklua/pull/156))
diff --git a/src/rules/convert_require/roblox_require_mode.rs b/src/rules/convert_require/roblox_require_mode.rs
index 39921ca..fc22943 100644
--- a/src/rules/convert_require/roblox_require_mode.rs
+++ b/src/rules/convert_require/roblox_require_mode.rs
@@ -31,9 +31,7 @@ impl RobloxRequireMode {
             .as_ref()
             .map(|rojo_sourcemap_path| context.project_location().join(rojo_sourcemap_path))
         {
-            let sourcemap_parent_location = rojo_sourcemap_path
-                .parent()
-                .unwrap_or_else(|| Path::new("."));
+            let sourcemap_parent_location = get_relative_parent_path(rojo_sourcemap_path);
             let sourcemap = RojoSourcemap::parse(
                 &context
                     .resources()
@@ -83,7 +81,7 @@ impl RobloxRequireMode {
         {
             if let Some(require_relative_to_sourcemap) = get_relative_path(
                 require_path,
-                sourcemap_path.parent().unwrap_or_else(|| Path::new("")),
+                get_relative_parent_path(sourcemap_path),
                 false,
             )? {
                 log::trace!(
@@ -214,26 +212,33 @@ fn get_relative_path(
     source_path: &Path,
     use_current_dir_prefix: bool,
 ) -> Result<Option<PathBuf>, DarkluaError> {
-    Ok(pathdiff::diff_paths(
-        require_path,
-        source_path.parent().ok_or_else(|| {
-            DarkluaError::custom(format!(
-                "unable to get parent directory from `{}`",
-                source_path.display()
-            ))
-        })?,
+    Ok(
+        pathdiff::diff_paths(require_path, get_relative_parent_path(source_path))
+            .map(|path| {
+                if use_current_dir_prefix && !path.starts_with(".") && !path.starts_with("..") {
+                    Path::new(".").join(path)
+                } else if !use_current_dir_prefix && path.starts_with(".") {
+                    path.strip_prefix(".")
+                        .map(Path::to_path_buf)
+                        .ok()
+                        .unwrap_or(path)
+                } else {
+                    path
+                }
+            })
+            .map(utils::normalize_path_with_current_dir),
     )
-    .map(|path| {
-        if use_current_dir_prefix && !path.starts_with(".") && !path.starts_with("..") {
-            Path::new(".").join(path)
-        } else if !use_current_dir_prefix && path.starts_with(".") {
-            path.strip_prefix(".")
-                .map(Path::to_path_buf)
-                .ok()
-                .unwrap_or(path)
-        } else {
-            path
+}
+
+fn get_relative_parent_path(path: &Path) -> &Path {
+    match path.parent() {
+        Some(parent) => {
+            if parent == Path::new("") {
+                Path::new(".")
+            } else {
+                parent
+            }
         }
-    })
-    .map(utils::normalize_path_with_current_dir))
+        None => Path::new(".."),
+    }
 }


Test patch:
diff --git a/tests/rule_tests/convert_require.rs b/tests/rule_tests/convert_require.rs
index d904a10..a2f9b54 100644
--- a/tests/rule_tests/convert_require.rs
+++ b/tests/rule_tests/convert_require.rs
@@ -206,26 +206,30 @@ fn convert_parent_init_module_from_init_module() {
 mod sourcemap {
     use super::*;
 
-    const CONVERT_PATH_TO_ROJO_SOURCEMAP_CONFIG: &str = r#"{
-        generator: 'retain_lines',
-        rules: [
-            {
-                rule: 'convert_require',
-                current: {
-                    name: 'path',
-                    sources: {
-                        '@pkg': './Packages'
-                    }
-                },
-                target: {
-                    name: 'roblox',
-                    rojo_sourcemap: './sourcemap.json',
-                }
-            }
-        ]
-    }"#;
-
-    fn get_resources_for_sourcemap(datamodel_case: bool) -> Resources {
+    fn get_darklua_config_with_sourcemap(sourcemap_path: &str) -> String {
+        format!(
+            r#"{{
+                generator: 'retain_lines',
+                rules: [
+                    {{
+                        rule: 'convert_require',
+                        current: {{
+                            name: 'path',
+                            sources: {{
+                                '@pkg': './Packages'
+                            }}
+                        }},
+                        target: {{
+                            name: 'roblox',
+                            rojo_sourcemap: '{sourcemap_path}',
+                        }}
+                    }}
+                ]
+            }}"#
+        )
+    }
+
+    fn get_resources_for_sourcemap(datamodel_case: bool, sourcemap_path: &str) -> Resources {
         memory_resources!(
             "src/init.lua" => include_str!("../test_cases/sourcemap/src/init.lua"),
             "src/a.lua" => include_str!("../test_cases/sourcemap/src/a.lua"),
@@ -241,7 +245,7 @@ mod sourcemap {
 
             "main.server.lua" => include_str!("../test_cases/sourcemap/main.server.lua"),
 
-            ".darklua.json" => CONVERT_PATH_TO_ROJO_SOURCEMAP_CONFIG,
+            ".darklua.json" => get_darklua_config_with_sourcemap(sourcemap_path),
             "sourcemap.json" => if datamodel_case {
                 include_str!("../test_cases/sourcemap/place-sourcemap.json")
             } else {
@@ -254,7 +258,7 @@ mod sourcemap {
     fn invalid_sourcemap() {
         let resources = memory_resources!(
             "src/init.lua" => "return nil",
-            ".darklua.json" => CONVERT_PATH_TO_ROJO_SOURCEMAP_CONFIG,
+            ".darklua.json" => get_darklua_config_with_sourcemap("./sourcemap.json"),
             "sourcemap.json" => "",
         );
         utils::snapshot_file_process_file_errors(&resources, "src/init.lua", "invalid_sourcemap")
@@ -263,7 +267,16 @@ mod sourcemap {
     #[test]
     fn convert_sibling_module_from_init_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(false),
+            &get_resources_for_sourcemap(false, "./sourcemap.json"),
+            "src/d/init.lua",
+            "convert_sibling_module_from_init_module",
+        );
+    }
+
+    #[test]
+    fn convert_sibling_module_from_init_module_with_sourcemap_without_current_dir() {
+        snapshot_file_process(
+            &get_resources_for_sourcemap(false, "sourcemap.json"),
             "src/d/init.lua",
             "convert_sibling_module_from_init_module",
         );
@@ -301,7 +314,7 @@ mod sourcemap {
     #[test]
     fn in_datamodel_convert_sibling_module_from_init_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(true),
+            &get_resources_for_sourcemap(true, "./sourcemap.json"),
             "src/d/init.lua",
             "convert_sibling_module_from_init_module",
         );
@@ -310,7 +323,7 @@ mod sourcemap {
     #[test]
     fn convert_module_from_child_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(false),
+            &get_resources_for_sourcemap(false, "./sourcemap.json"),
             "src/d/d2.lua",
             "convert_module_from_child_module",
         );
@@ -319,7 +332,7 @@ mod sourcemap {
     #[test]
     fn in_datamodel_convert_module_from_child_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(true),
+            &get_resources_for_sourcemap(true, "./sourcemap.json"),
             "src/d/d2.lua",
             "convert_module_from_child_module",
         );
@@ -328,7 +341,7 @@ mod sourcemap {
     #[test]
     fn convert_multiple_sibling_modules_from_root_init_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(false),
+            &get_resources_for_sourcemap(false, "./sourcemap.json"),
             "src/init.lua",
             "convert_multiple_sibling_modules_from_root_init_module",
         );
@@ -337,7 +350,7 @@ mod sourcemap {
     #[test]
     fn in_datamodel_convert_multiple_sibling_modules_from_root_init_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(true),
+            &get_resources_for_sourcemap(true, "./sourcemap.json"),
             "src/init.lua",
             "convert_multiple_sibling_modules_from_root_init_module",
         );
@@ -346,7 +359,7 @@ mod sourcemap {
     #[test]
     fn convert_sibling_module_from_sibling_module() {
         expect_file_process(
-            &get_resources_for_sourcemap(false),
+            &get_resources_for_sourcemap(false, "./sourcemap.json"),
             "src/b.lua",
             "local a = require(script.Parent:FindFirstChild('a'))\n\nreturn a\n",
         );
@@ -355,7 +368,7 @@ mod sourcemap {
     #[test]
     fn in_datamodel_convert_sibling_module_from_sibling_module() {
         expect_file_process(
-            &get_resources_for_sourcemap(true),
+            &get_resources_for_sourcemap(true, "./sourcemap.json"),
             "src/b.lua",
             "local a = require(script.Parent:FindFirstChild('a'))\n\nreturn a\n",
         );
@@ -364,7 +377,7 @@ mod sourcemap {
     #[test]
     fn convert_package_module_from_nested_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(false),
+            &get_resources_for_sourcemap(false, "./sourcemap.json"),
             "src/d/d1.lua",
             "convert_package_module_from_nested_module",
         );
@@ -373,7 +386,7 @@ mod sourcemap {
     #[test]
     fn in_datamodel_convert_package_module_from_nested_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(true),
+            &get_resources_for_sourcemap(true, "./sourcemap.json"),
             "src/d/d1.lua",
             "convert_package_module_from_nested_module",
         );
@@ -382,7 +395,7 @@ mod sourcemap {
     #[test]
     fn convert_nested_package_module_from_sibling_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(false),
+            &get_resources_for_sourcemap(false, "./sourcemap.json"),
             "Packages/Package1/init.lua",
             "convert_nested_package_module_from_sibling_module",
         );
@@ -391,7 +404,7 @@ mod sourcemap {
     #[test]
     fn in_datamodel_convert_nested_package_module_from_sibling_module() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(true),
+            &get_resources_for_sourcemap(true, "./sourcemap.json"),
             "Packages/Package1/init.lua",
             "convert_nested_package_module_from_sibling_module",
         );
@@ -401,7 +414,7 @@ mod sourcemap {
     #[test]
     fn in_datamodel_convert_module_require_across_service_instance() {
         snapshot_file_process(
-            &get_resources_for_sourcemap(true),
+            &get_resources_for_sourcemap(true, "./sourcemap.json"),
             "main.server.lua",
             "convert_module_require_across_service_instance",
         );
diff --git a/tests/utils.rs b/tests/utils.rs
index 8484380..968ab74 100644
--- a/tests/utils.rs
+++ b/tests/utils.rs
@@ -87,7 +87,7 @@ macro_rules! memory_resources {
     ($($path:literal => $content:expr),+$(,)?) => ({
         let resources = Resources::from_memory();
         $(
-            resources.write($path, $content).unwrap();
+            resources.write($path, &$content).unwrap();
         )*
         resources
     });