Merge pull request #231 from seddonym/path-components-simplify

Path components simplify

Author: seddonym

rust/src/filesystem.rs

@@ -2,6 +2,7 @@ use pyo3::exceptions::{PyFileNotFoundError, PyUnicodeDecodeError};
 use pyo3::prelude::*;
 use regex::Regex;
 use std::collections::HashMap;
+use std::ffi::OsStr;
 use std::fs;
 use std::path::{Path, PathBuf};
 use unindent::unindent;
@@ -199,33 +200,21 @@ impl FileSystem for FakeBasicFileSystem {
     }
 
     fn split(&self, file_name: &str) -> (String, String) {
-        let components: Vec<&str> = file_name.split('/').collect();
-
-        if components.is_empty() {
-            return ("".to_string(), "".to_string());
-        }
-
-        let tail = components.last().unwrap_or(&""); // Last component, or empty if components is empty (shouldn't happen from split)
-
-        let head_components = &components[..components.len() - 1]; // All components except the last
-
-        let head = if head_components.is_empty() {
-            // Case for single component paths like "filename.txt" or empty string ""
-            "".to_string()
-        } else if file_name.starts_with('/')
-            && head_components.len() == 1
-            && head_components[0].is_empty()
-        {
-            // Special handling for paths starting with '/', e.g., "/" or "/filename.txt"
-            // If components were ["", ""], head_components is [""] -> should be "/"
-            // If components were ["", "file.txt"], head_components is [""] -> should be "/"
-            "/".to_string()
+        let path;
+        let head;
+        let tail;
+        if let Some(file_name_without_trailing_slash) = file_name.strip_suffix("/") {
+            head = Path::new(file_name_without_trailing_slash);
+            tail = OsStr::new("");
         } else {
-            // Default joining for multiple components
-            head_components.join("/")
-        };
-
-        (head, tail.to_string())
+            path = Path::new(file_name);
+            head = path.parent().unwrap_or(Path::new(""));
+            tail = path.file_name().unwrap_or(OsStr::new(""));
+        }
+        (
+            head.to_str().unwrap().to_string(),
+            tail.to_str().unwrap().to_string(),
+        )
     }
 
     /// Checks if a file or directory exists within the file system.

rust/src/lib.rs

@@ -1,14 +1,15 @@
 pub mod errors;
 pub mod exceptions;
+mod filesystem;
 pub mod graph;
 pub mod import_parsing;
-pub mod module_expressions;
-mod filesystem;
 mod import_scanning;
+pub mod module_expressions;
 mod module_finding;
 
 use crate::errors::{GrimpError, GrimpResult};
 use crate::exceptions::{InvalidModuleExpression, ModuleNotPresent, NoSuchContainer, ParseError};
+use crate::filesystem::{PyFakeBasicFileSystem, PyRealBasicFileSystem};
 use crate::graph::higher_order_queries::Level;
 use crate::graph::{Graph, Module, ModuleIterator, ModuleTokenIterator};
 use crate::import_scanning::ImportScanner;
@@ -22,7 +23,6 @@ use pyo3::types::{IntoPyDict, PyDict, PyFrozenSet, PyList, PySet, PyString, PyTu
 use rayon::prelude::*;
 use rustc_hash::FxHashSet;
 use std::collections::HashSet;
-use crate::filesystem::{PyRealBasicFileSystem,PyFakeBasicFileSystem};
 
 #[pymodule]
 fn _rustgrimp(py: Python<'_>, m: &Bound<'_, PyModule>) -> PyResult<()> {

tests/adaptors/filesystem.py

@@ -1,6 +1,5 @@
 from typing import Any, Dict, Generator, List, Optional, Tuple
 
-
 import yaml
 
 from grimp.application.ports.filesystem import AbstractFileSystem, BasicFileSystem
@@ -99,8 +98,13 @@ def join(self, *components: str) -> str:
         return self.sep.join(c.rstrip(self.sep) for c in components)
 
     def split(self, file_name: str) -> Tuple[str, str]:
-        components = file_name.split("/")
-        return ("/".join(components[:-1]), components[-1])
+        components = file_name.split(self.sep)
+        if len(components) == 2:
+            # Handle case where file is child of the root, i.e. /some-file.txt.
+            # In this case, to conform with the interface we ensure the leading slash
+            # is included in the returned head.
+            components.insert(0, "")
+        return (self.sep.join(components[:-1]), components[-1])
 
     def _parse_contents(self, raw_contents: Optional[str]):
         """

tests/unit/adaptors/test_filesystem.py

@@ -18,9 +18,20 @@ def test_join(self, path):
         file_system = self.file_system_cls()
         assert "/path/to/mypackage/file.py" == file_system.join(path, "mypackage", "file.py")
 
-    def test_split(self):
+    @pytest.mark.parametrize(
+        "path, expected",
+        [
+            ("", ("", "")),
+            ("/path", ("/", "path")),
+            ("some-file", ("", "some-file")),
+            ("/path/to/mypackage/", ("/path/to/mypackage", "")),
+            ("/path/to/mypackage/some-file", ("/path/to/mypackage", "some-file")),
+            ("/path/to/mypackage/some-file.py", ("/path/to/mypackage", "some-file.py")),
+        ],
+    )
+    def test_split(self, path, expected):
         file_system = self.file_system_cls()
-        assert ("/path/to/mypackage", "file.py") == file_system.split("/path/to/mypackage/file.py")
+        assert file_system.split(path) == expected
 
     @pytest.mark.parametrize(
         "file_name, expected",