fix: further improve file path handling (#479)

- On Windows, the reflection of file URLs to paths requires that we know about the URL hostname too as they could be UNC paths.
- Add some tests that verify that we are round-tripping from Path to URL and back again correctly.
- Simplify scheme testing for relative URLs, we only need to know about supported schemes.
- ci: collect coverage only on Linux

Author: mjpieters

.github/workflows/python.yaml

@@ -48,6 +48,9 @@ jobs:
       # It fails with "Error: Lcov file not found."
       # Solution here: https://github.com/coverallsapp/github-action/issues/30#issuecomment-612523058
       - name: Coveralls
+        # TODO: ci: run code coverage on all platforms to fix missing coverage
+        #   Example: https://github.com/andreoliwa/nitpick/pull/479#issuecomment-1082074275
+        # This condition is needed otherwise it fails with "Error: Container action is only supported on Linux"
         if: matrix.os == 'ubuntu-latest'
         uses: AndreMiras/coveralls-python-action@develop
         with:

src/nitpick/generic.py

@@ -7,10 +7,10 @@
 from __future__ import annotations
 
 import sys
-from pathlib import Path
+from pathlib import Path, PosixPath, WindowsPath
 from typing import Iterable
 
-import furl
+from furl import furl
 
 from nitpick.constants import DOT, PROJECT_NAME
 from nitpick.typedefs import PathOrStr
@@ -89,23 +89,26 @@ def filter_names(iterable: Iterable, *partial_names: str) -> list[str]:
     return rv
 
 
-if sys.platform == "win32":
+def _url_to_windows_path(url: furl) -> Path:
+    """Convert the segments of a file URL to a path."""
+    # ref: https://docs.microsoft.com/en-us/dotnet/standard/io/file-path-formats
+    # UNC file path, \\server\share\path..., with server stored as url.host
+    # DOS Path, drive_letter:\path...
+    share_or_drive, *segments = url.path.segments
+    share_or_drive = rf"\\{url.host}\{share_or_drive}" if url.host else rf"{share_or_drive}\\"
+    return WindowsPath(share_or_drive, *segments)
 
-    def furl_path_to_python_path(path: furl.Path) -> Path:
-        """Convert a file URL to a path."""
-        drive, *segments = path.segments
-        return Path(f"{drive}/", *segments)
 
-    def get_scheme(url: str) -> str | None:
-        """Get the scheme of a URL, or None if there is no scheme."""
-        scheme = furl.get_scheme(url)
-        # Windows drive letters are not a valid scheme
-        return scheme if scheme and len(scheme) > 1 else None
+def _url_to_posix_path(url: furl) -> Path:
+    """Convert the segments of a file URL to a path."""
+    # ref: https://unix.stackexchange.com/a/1919
+    # POSIX paths can start with //part/..., which some implementations
+    # (such as Cygwin) can interpret differently. furl(Path("//part/...").as_uri())
+    # retains the double slash (but doesn't treat it as a host), and in that case
+    # `first` will always be an empty string and `segments` will *not* be empty.
+    first, *segments = url.path.segments
+    slash_or_double_slash = "//" if not first and segments else "/"
+    return PosixPath(slash_or_double_slash, first, *segments)
 
-else:
 
-    def furl_path_to_python_path(path: furl.Path) -> Path:
-        """Convert a file URL to a path."""
-        return Path("/", *path.segments)
-
-    get_scheme = furl.get_scheme
+url_to_python_path = _url_to_windows_path if sys.platform == "win32" else _url_to_posix_path

src/nitpick/style/core.py

@@ -27,7 +27,7 @@
     PYPROJECT_TOML,
 )
 from nitpick.exceptions import QuitComplainingError, pretty_exception
-from nitpick.generic import furl_path_to_python_path
+from nitpick.generic import url_to_python_path
 from nitpick.plugins.base import NitpickPlugin
 from nitpick.plugins.info import FileInfo
 from nitpick.project import Project, glob_files
@@ -131,7 +131,7 @@ def _include_style(self, style_url: furl) -> Iterator[Fuss]:
         # and the URL otherwise.
         display_name = style_url.url
         if style_url.scheme == "file":
-            path = furl_path_to_python_path(style_url.path)
+            path = url_to_python_path(style_url)
             with suppress(ValueError):
                 path = path.relative_to(self.project.root)
             display_name = str(path)

src/nitpick/style/fetchers/__init__.py

@@ -11,7 +11,6 @@
 from strenum import LowercaseStrEnum
 
 from nitpick.enums import CachingEnum
-from nitpick.generic import get_scheme
 from nitpick.style import parse_cache_option
 
 if TYPE_CHECKING:
@@ -40,6 +39,7 @@ class StyleFetcherManager:
 
     session: CachedSession = field(init=False)
     fetchers: dict[str, StyleFetcher] = field(init=False)
+    schemes: tuple[str] = field(init=False)
 
     def __post_init__(self):
         """Initialize dependant properties."""
@@ -52,7 +52,12 @@ def __post_init__(self):
         self.session = CachedSession(
             str(self.cache_dir / "styles"), expire_after=expire_after, cache_control=cache_control
         )
-        self.fetchers = _get_fetchers(self.session)
+        self.fetchers = fetchers = _get_fetchers(self.session)
+
+        # used to test if a string URL is relative or not. These strings
+        # *include the colon*.
+        protocols = {prot for fetcher in fetchers.values() for prot in fetcher.protocols}
+        self.schemes = tuple(f"{prot}:" for prot in protocols)
 
     def normalize_url(self, url: str | furl, base: furl) -> furl:
         """Normalize a style URL.
@@ -61,12 +66,8 @@ def normalize_url(self, url: str | furl, base: furl) -> furl:
         to produce a canonical version of the URL.
 
         """
-        # special case: Windows paths can start with a drive letter, which looks
-        # like a URL scheme.
-        if isinstance(url, str):
-            scheme = get_scheme(url)
-            if not scheme or len(scheme) == 1:
-                url = self._fetcher_for(base).preprocess_relative_url(url)
+        if isinstance(url, str) and not url.startswith(self.schemes):
+            url = self._fetcher_for(base).preprocess_relative_url(url)
         absolute = base.copy().join(url)
         return self._fetcher_for(absolute).normalize(absolute)
 

src/nitpick/style/fetchers/base.py

@@ -4,7 +4,7 @@
 from dataclasses import dataclass
 from typing import ClassVar
 
-from furl import Path, furl
+from furl import furl
 from requests_cache import CachedSession
 
 from nitpick.constants import TOML_EXTENSION
@@ -35,11 +35,12 @@ def preprocess_relative_url(self, url: str) -> str:  # pylint: disable=no-self-u
         """
         return url
 
-    def _normalize_path(self, path: Path) -> Path:  # pylint: disable=no-self-use
+    def _normalize_url_path(self, url: furl) -> furl:  # pylint: disable=no-self-use
         """Normalize the path component of a URL."""
-        if path.segments[-1].endswith(TOML_EXTENSION):
-            return path
-        return Path([*path.segments[:-1], f"{path.segments[-1]}.toml"])
+        if not url.path.segments[-1].endswith(TOML_EXTENSION):
+            url = url.copy()
+            url.path.segments[-1] = f"{url.path.segments[-1]}{TOML_EXTENSION}"
+        return url
 
     def _normalize_scheme(self, scheme: str) -> str:  # pylint: disable=no-self-use
         """Normalize the scheme component of a URL."""
@@ -54,8 +55,10 @@ def normalize(self, url: furl) -> furl:
         - Individual fetchers can further normalize the path and scheme.
 
         """
-        scheme, path = self._normalize_scheme(url.scheme), self._normalize_path(url.path.normalize())
-        return url.copy().set(scheme=scheme, path=path)
+        new_scheme = self._normalize_scheme(url.scheme)
+        if new_scheme != url.scheme:
+            url = url.copy().set(scheme=new_scheme)
+        return self._normalize_url_path(url)
 
     def fetch(self, url: furl) -> str:
         """Fetch a style from a specific fetcher."""

src/nitpick/style/fetchers/file.py

@@ -4,9 +4,9 @@
 from dataclasses import dataclass
 from pathlib import Path
 
-import furl
+from furl import furl
 
-from nitpick.generic import furl_path_to_python_path
+from nitpick.generic import url_to_python_path
 from nitpick.style.fetchers import Scheme
 from nitpick.style.fetchers.base import StyleFetcher
 
@@ -32,10 +32,10 @@ def preprocess_relative_url(self, url: str) -> str:
         # cleanly against a file:// base. Relative paths should use POSIX conventions.
         return path.as_uri() if path.is_absolute() else path.as_posix()
 
-    def _normalize_path(self, path: furl.Path) -> furl.Path:
-        local_path = furl_path_to_python_path(super()._normalize_path(path))
-        return furl.furl(local_path.resolve().as_uri()).path
+    def _normalize_url_path(self, url: furl) -> furl:
+        local_path = url_to_python_path(super()._normalize_url_path(url))
+        return furl(local_path.resolve().as_uri())
 
-    def fetch(self, url: furl.furl) -> str:
+    def fetch(self, url: furl) -> str:
         """Fetch a style from a local file."""
-        return furl_path_to_python_path(url.path).read_text(encoding="UTF-8")
+        return url_to_python_path(url).read_text(encoding="UTF-8")

tests/test_generic.py

@@ -1,13 +1,15 @@
 """Generic functions tests."""
 import os
 import sys
-from pathlib import Path
+from pathlib import Path, PosixPath, WindowsPath
 from unittest import mock
 
+import pytest
+from furl import furl
 from testfixtures import compare
 
 from nitpick.constants import EDITOR_CONFIG, TOX_INI
-from nitpick.generic import relative_to_current_dir
+from nitpick.generic import _url_to_posix_path, _url_to_windows_path, relative_to_current_dir
 
 
 @mock.patch.object(Path, "cwd")
@@ -57,3 +59,35 @@ def test_relative_to_current_dir(home, cwd):
 
     for path, expected in examples.items():
         compare(actual=relative_to_current_dir(path), expected=expected, prefix=f"Path: {path}")
+
+
+@pytest.mark.skipif(os.name != "nt", reason="Windows-only test")
+@pytest.mark.parametrize(
+    "test_path",
+    (
+        "C:\\",
+        r"C:\path\file.toml",
+        r"//server/share/path/file.toml",
+    ),
+)
+def test_url_to_windows_path(test_path):
+    """Verify that Path to URL to Path conversion preserves the path."""
+    path = WindowsPath(test_path)
+    url = furl(path.as_uri())
+    assert _url_to_windows_path(url) == path
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX-only test")
+@pytest.mark.parametrize(
+    "test_path",
+    (
+        "/",
+        "/path/to/file.toml",
+        "//double/slash/path.toml",
+    ),
+)
+def test_url_to_posix_path(test_path):
+    """Verify that Path to URL to Path conversion preserves the path."""
+    path = PosixPath(test_path)
+    url = furl(path.as_uri())
+    assert _url_to_posix_path(url) == path