Handle files not encoded with UTF-8

Black can format files encoded with an encoding other than UTF-8, based
on the encoding cookie present in the first two lines. Darker is also
supposed to format those files, but an exception is raised instead.

The exception happens because all the output from git is decoded with
UTF-8, including calls to `git show <revision>:<path>` that returns the
binary content of a file from the history. If a file used an encoding
other than UTF-8, the decoding fails.

`TextDocument.from_bytes()` can now create a `TextDocument` from the raw
binary content of a file, automatically detecting the encoding used.
From now on, this new factory is used when loading a file from disk or
from a revision in the git history.

Author: gdiscry

CHANGES.rst

@@ -12,6 +12,7 @@ Added
 Fixed
 -----
 - Avoid memory leak from using ``@lru_cache`` on a method.
+- Handle files encoded with an encoding other than UTF-8 without an exception.
 
 
 1.4.2_ - 2022-03-12

src/darker/git.py

@@ -9,7 +9,7 @@
 from functools import lru_cache
 from pathlib import Path
 from subprocess import DEVNULL, PIPE, CalledProcessError, check_output, run  # nosec
-from typing import Dict, Iterable, List, Set, Tuple
+from typing import Dict, Iterable, List, Optional, Set, Tuple, Union, overload
 
 from darker.diff import diff_and_get_opcodes, opcodes_to_edit_linenums
 from darker.utils import GIT_DATEFORMAT, TextDocument
@@ -79,8 +79,8 @@ def git_get_content_at_revision(path: Path, revision: str, cwd: Path) -> TextDoc
         return TextDocument.from_file(abspath)
     cmd = ["show", f"{revision}:./{path.as_posix()}"]
     try:
-        return TextDocument.from_lines(
-            _git_check_output_lines(cmd, cwd, exit_on_error=False),
+        return TextDocument.from_bytes(
+            _git_check_output(cmd, cwd, exit_on_error=False),
             mtime=git_get_mtime_at_commit(path, revision, cwd),
         )
     except CalledProcessError as exc_info:
@@ -207,15 +207,45 @@ def _git_check_output_lines(
     cmd: List[str], cwd: Path, exit_on_error: bool = True
 ) -> List[str]:
     """Log command line, run Git, split stdout to lines, exit with 123 on error"""
+    return _git_check_output(
+        cmd,
+        cwd,
+        exit_on_error=exit_on_error,
+        encoding="utf-8",
+    ).splitlines()
+
+
+@overload
+def _git_check_output(
+    cmd: List[str], cwd: Path, *, exit_on_error: bool = ..., encoding: None = ...
+) -> bytes:
+    ...
+
+
+@overload
+def _git_check_output(
+    cmd: List[str], cwd: Path, *, exit_on_error: bool = ..., encoding: str
+) -> str:
+    ...
+
+
+def _git_check_output(
+    cmd: List[str],
+    cwd: Path,
+    *,
+    exit_on_error: bool = True,
+    encoding: Optional[str] = None,
+) -> Union[str, bytes]:
+    """Log command line, run Git, return stdout, exit with 123 on error"""
     logger.debug("[%s]$ git %s", cwd, " ".join(cmd))
     try:
         return check_output(  # nosec
             ["git"] + cmd,
             cwd=str(cwd),
-            encoding="utf-8",
+            encoding=encoding,
             stderr=PIPE,
             env=_make_git_env(),
-        ).splitlines()
+        )
     except CalledProcessError as exc_info:
         if not exit_on_error:
             raise

src/darker/tests/conftest.py

@@ -3,7 +3,7 @@
 import os
 from pathlib import Path
 from subprocess import check_call  # nosec
-from typing import Dict, Optional
+from typing import Dict, Union
 
 import pytest
 from black import find_project_root as black_find_project_root
@@ -39,7 +39,7 @@ def _run_and_get_first_line(self, *args: str) -> str:
         return _git_check_output_lines(list(args), Path(self.root))[0]
 
     def add(
-        self, paths_and_contents: Dict[str, Optional[str]], commit: str = None
+        self, paths_and_contents: Dict[str, Union[str, bytes, None]], commit: str = None
     ) -> Dict[str, Path]:
         """Add/remove/modify files and optionally commit the changes
 
@@ -58,10 +58,12 @@ def add(
             path = absolute_paths[relative_path]
             if content is None:
                 self._run("rm", "--", relative_path)
-            else:
-                path.parent.mkdir(parents=True, exist_ok=True)
-                path.write_bytes(content.encode("utf-8"))
-                self._run("add", "--", relative_path)
+                continue
+            if isinstance(content, str):
+                content = content.encode("utf-8")
+            path.parent.mkdir(parents=True, exist_ok=True)
+            path.write_bytes(content)
+            self._run("add", "--", relative_path)
         if commit:
             self._run("commit", "-m", commit)
         return absolute_paths

src/darker/tests/test_git.py

@@ -139,6 +139,17 @@ def test_revisionrange_parse(revision_range, expect):
     assert result == expect
 
 
+def git_call(cmd, encoding=None):
+    """Returns a mocked call to git"""
+    return call(
+        cmd.split(),
+        cwd=str(Path("/path")),
+        encoding=encoding,
+        stderr=PIPE,
+        env={"LC_ALL": "C", "PATH": os.environ["PATH"]},
+    )
+
+
 @pytest.mark.kwparametrize(
     dict(
         revision=":WORKTREE:",
@@ -147,31 +158,31 @@ def test_revisionrange_parse(revision_range, expect):
     dict(
         revision="HEAD",
         expect_git_calls=[
-            "git show HEAD:./my.txt",
-            "git log -1 --format=%ct HEAD -- my.txt",
+            git_call("git show HEAD:./my.txt"),
+            git_call("git log -1 --format=%ct HEAD -- my.txt", encoding="utf-8"),
         ],
         expect_textdocument_calls=[
-            call.from_lines([b"1627107028"], mtime="2021-07-24 06:10:28.000000 +0000")
+            call.from_bytes(b"1627107028", mtime="2021-07-24 06:10:28.000000 +0000")
         ],
     ),
     dict(
         revision="HEAD^",
         expect_git_calls=[
-            "git show HEAD^:./my.txt",
-            "git log -1 --format=%ct HEAD^ -- my.txt",
+            git_call("git show HEAD^:./my.txt"),
+            git_call("git log -1 --format=%ct HEAD^ -- my.txt", encoding="utf-8"),
         ],
         expect_textdocument_calls=[
-            call.from_lines([b"1627107028"], mtime="2021-07-24 06:10:28.000000 +0000")
+            call.from_bytes(b"1627107028", mtime="2021-07-24 06:10:28.000000 +0000")
         ],
     ),
     dict(
         revision="master",
         expect_git_calls=[
-            "git show master:./my.txt",
-            "git log -1 --format=%ct master -- my.txt",
+            git_call("git show master:./my.txt"),
+            git_call("git log -1 --format=%ct master -- my.txt", encoding="utf-8"),
         ],
         expect_textdocument_calls=[
-            call.from_lines([b"1627107028"], mtime="2021-07-24 06:10:28.000000 +0000")
+            call.from_bytes(b"1627107028", mtime="2021-07-24 06:10:28.000000 +0000")
         ],
     ),
     expect_git_calls=[],
@@ -189,17 +200,7 @@ def test_git_get_content_at_revision_obtain_file_content(
 
         git.git_get_content_at_revision(Path("my.txt"), revision, Path("/path"))
 
-        expected_calls = [
-            call(
-                expected_call.split(),
-                cwd=str(Path("/path")),
-                encoding="utf-8",
-                stderr=PIPE,
-                env={"LC_ALL": "C", "PATH": os.environ["PATH"]},
-            )
-            for expected_call in expect_git_calls
-        ]
-        assert check_output.call_args_list == expected_calls
+        assert check_output.call_args_list == expect_git_calls
         assert text_document_class.method_calls == expect_textdocument_calls
 
 
@@ -403,6 +404,58 @@ def test_git_get_content_at_revision_stderr(git_repo, capfd, caplog):
     assert caplog.text == ""
 
 
+@pytest.fixture(scope="module")
+def encodings_repo(tmp_path_factory):
+    """Create an example Git repository using various encodings for the same file"""
+    tmpdir = tmp_path_factory.mktemp("branched_repo")
+    git_repo = GitRepoFixture.create_repository(tmpdir)
+    # Commit without an encoding cookie, defaults to utf-8
+    git_repo.add({"file.py": "darker = 'plus foncé'\n"}, commit="Default encoding")
+    git_repo.create_tag("default")
+    # Commit without an encoding cookie but with a utf-8 signature
+    content = "darker = 'plus foncé'\n".encode("utf-8-sig")
+    git_repo.add({"file.py": content}, commit="utf-8-sig")
+    git_repo.create_tag("utf-8-sig")
+    # Commit with an iso-8859-1 encoding cookie
+    content = "# coding: iso-8859-1\ndarker = 'plus foncé'\n".encode("iso-8859-1")
+    git_repo.add({"file.py": content}, commit="iso-8859-1")
+    git_repo.create_tag("iso-8859-1")
+    # Commit with a utf-8 encoding cookie
+    content = "# coding: utf-8\npython = 'パイソン'\n".encode("utf-8")
+    git_repo.add({"file.py": content}, commit="utf-8")
+    git_repo.create_tag("utf-8")
+    # Current worktree content (not committed) with a shitfjs encoding cookie
+    content = "# coding: shiftjis\npython = 'パイソン'\n".encode("shiftjis")
+    git_repo.add({"file.py": content})
+    return git_repo
+
+
+@pytest.mark.kwparametrize(
+    dict(commit="default", encoding="utf-8", lines=("darker = 'plus foncé'",)),
+    dict(commit="utf-8-sig", encoding="utf-8-sig", lines=("darker = 'plus foncé'",)),
+    dict(
+        commit="iso-8859-1",
+        encoding="iso-8859-1",
+        lines=("# coding: iso-8859-1", "darker = 'plus foncé'"),
+    ),
+    dict(
+        commit="utf-8", encoding="utf-8", lines=("# coding: utf-8", "python = 'パイソン'")
+    ),
+    dict(
+        commit=":WORKTREE:",
+        encoding="shiftjis",
+        lines=("# coding: shiftjis", "python = 'パイソン'"),
+    ),
+)
+def test_git_get_content_at_revision_encoding(encodings_repo, commit, encoding, lines):
+    """Git file is loaded using its historical encoding"""
+    result = git.git_get_content_at_revision(
+        Path("file.py"), commit, encodings_repo.root
+    )
+    assert result.encoding == encoding
+    assert result.lines == lines
+
+
 @pytest.mark.kwparametrize(
     dict(retval=0, expect=True),
     dict(retval=1, expect=False),

src/darker/tests/test_utils.py

@@ -1,5 +1,7 @@
 """Unit tests for :mod:`darker.utils`"""
 
+# pylint: disable=redefined-outer-name, comparison-with-callable
+
 import logging
 import os
 from pathlib import Path
@@ -17,6 +19,39 @@
 )
 
 
+@pytest.fixture(params=[TextDocument.from_file, TextDocument.from_bytes])
+def textdocument_factory(request):
+    """Fixture for a factory function that creates a ``TextDocument``
+
+    The fixture can be parametrized with `(bytes) -> TextDocument` functions
+    that take the raw bytes of the document.
+
+    By default, it is parametrized with the ``TextDocument.from_file()`` (for
+    which it creates a temporary file) and the ``TextDocument.from_bytes()``
+    classmethods.
+    """
+    if request.param == TextDocument.from_file:
+
+        def factory(content):
+            tmp_path = request.getfixturevalue("tmp_path")
+            path = tmp_path / "test.py"
+            path.write_bytes(content)
+            return TextDocument.from_file(path)
+
+        return factory
+
+    return request.param
+
+
+@pytest.fixture
+def textdocument(request, textdocument_factory):
+    """Fixture for a ``TextDocument``
+
+    The fixture must be parametrized with the raw bytes of the document.
+    """
+    return textdocument_factory(request.param)
+
+
 @pytest.mark.kwparametrize(
     dict(string="", expect="\n"),
     dict(string="\n", expect="\n"),
@@ -225,31 +260,23 @@ def test_textdocument_from_str(
 
 
 @pytest.mark.kwparametrize(
-    dict(content=b'print("touch\xc3\xa9")\n', expect="utf-8"),
-    dict(content=b'\xef\xbb\xbfprint("touch\xc3\xa9")\n', expect="utf-8-sig"),
-    dict(content=b'# coding: iso-8859-1\n"touch\xe9"\n', expect="iso-8859-1"),
+    dict(textdocument=b'print("touch\xc3\xa9")\n', expect="utf-8"),
+    dict(textdocument=b'\xef\xbb\xbfprint("touch\xc3\xa9")\n', expect="utf-8-sig"),
+    dict(textdocument=b'# coding: iso-8859-1\n"touch\xe9"\n', expect="iso-8859-1"),
+    indirect=["textdocument"],
 )
-def test_textdocument_from_file_detect_encoding(tmp_path, content, expect):
-    """TextDocument.from_file() detects the file encoding correctly"""
-    path = tmp_path / "test.py"
-    path.write_bytes(content)
-
-    textdocument = TextDocument.from_file(path)
-
+def test_textdocument_detect_encoding(textdocument, expect):
+    """TextDocument.from_file/bytes() detects the file encoding correctly"""
     assert textdocument.encoding == expect
 
 
 @pytest.mark.kwparametrize(
-    dict(content=b'print("unix")\n', expect="\n"),
-    dict(content=b'print("windows")\r\n', expect="\r\n"),
+    dict(textdocument=b'print("unix")\n', expect="\n"),
+    dict(textdocument=b'print("windows")\r\n', expect="\r\n"),
+    indirect=["textdocument"],
 )
-def test_textdocument_from_file_detect_newline(tmp_path, content, expect):
-    """TextDocument.from_file() detects the newline character sequence correctly"""
-    path = tmp_path / "test.py"
-    path.write_bytes(content)
-
-    textdocument = TextDocument.from_file(path)
-
+def test_textdocument_detect_newline(textdocument, expect):
+    """TextDocument.from_file/bytes() detects the newline sequence correctly"""
     assert textdocument.newline == expect
 
 

src/darker/utils.py

@@ -106,22 +106,30 @@ def from_str(
             newline = override_newline
         return cls(string, None, encoding=encoding, newline=newline, mtime=mtime)
 
+    @classmethod
+    def from_bytes(cls, data: bytes, mtime: str = "") -> "TextDocument":
+        """Create a document object from a binary string
+
+        :param data: The binary content of the new text document
+        :param mtime: The modification time of the original file
+
+        """
+        srcbuf = io.BytesIO(data)
+        encoding, lines = tokenize.detect_encoding(srcbuf.readline)
+        if not lines:
+            return cls(lines=[], encoding=encoding, mtime=mtime)
+        return cls.from_str(data.decode(encoding), encoding=encoding, mtime=mtime)
+
     @classmethod
     def from_file(cls, path: Path) -> "TextDocument":
-        """Create a document object by reading an UTF-8 encoded text file
+        """Create a document object by reading a text file
 
         Also store the last modification time of the file.
 
         """
         mtime = datetime.utcfromtimestamp(path.stat().st_mtime).strftime(GIT_DATEFORMAT)
-        srcbuf = path.open("rb")
-        encoding, lines = tokenize.detect_encoding(srcbuf.readline)
-        if not lines:
-            return cls(lines=[], encoding=encoding)
-        srcbuf.seek(0)
-        return cls.from_str(
-            srcbuf.read().decode(encoding), encoding=encoding, mtime=mtime
-        )
+        with path.open("rb") as srcbuf:
+            return cls.from_bytes(srcbuf.read(), mtime)
 
     @classmethod
     def from_lines(