Merge pull request #342 from gdiscry/encodings

Handle files not encoded with UTF-8

Author: akaihola

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

CONTRIBUTORS.rst

@@ -25,6 +25,7 @@
 - Eric Riddoch (@phitoduck)
 - Filippos Giannakos (@philipgian)
 - Fox_white (@foxwhite25)
+- Georges Discry (@gdiscry)
 - Giel van Schijndel (@muggenhor)
 - Hugo Dupras (@jabesq)
 - Iryna (@irynahryshanovich)

README.rst

@@ -977,6 +977,17 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/search?q=foxwhite25" title="Bug reports">🐛</a>
        </td>
+       <td align="center">
+         <a href="https://github.com/gdiscry">
+           <img src="https://avatars.githubusercontent.com/u/476823?v=3" width="100px;" alt="@gdiscry" />
+           <br />
+           <sub>
+             <b>Georges Discry</b>
+           </sub>
+         </a>
+         <br />
+         <a href="https://github.com/akaihola/darker/pulls?q=is%3Apr+author%3Agdiscry" title="Code">💻</a>
+       </td>
        <td align="center">
          <a href="https://github.com/muggenhor">
            <img src="https://avatars.githubusercontent.com/u/484066?v=3" width="100px;" alt="@muggenhor" />
@@ -1011,6 +1022,8 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/issues?q=author%3Airynahryshanovich" title="Bug reports">🐛</a>
        </td>
+     </tr>
+     <tr>
        <td align="center">
          <a href="https://github.com/jasleen19">
            <img src="https://avatars.githubusercontent.com/u/30443449?v=3" width="100px;" alt="@jasleen19" />
@@ -1023,8 +1036,6 @@ Thanks goes to these wonderful people (`emoji key`_):
          <a href="https://github.com/akaihola/darker/issues?q=author%3Ajasleen19" title="Bug reports">🐛</a>
          <a href="https://github.com/akaihola/darker/pulls?q=is%3Apr+reviewed-by%3Ajasleen19" title="Reviewed Pull Requests">👀</a>
        </td>
-     </tr>
-     <tr>
        <td align="center">
          <a href="https://github.com/jedie">
            <img src="https://avatars.githubusercontent.com/u/71315?v=3" width="100px;" alt="@jedie" />
@@ -1080,6 +1091,8 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/issues?q=author%3Amagnunm" title="Bug reports">🐛</a>
        </td>
+     </tr>
+     <tr>
        <td align="center">
          <a href="https://github.com/markddavidoff">
            <img src="https://avatars.githubusercontent.com/u/1360543?v=3" width="100px;" alt="@markddavidoff" />
@@ -1091,8 +1104,6 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/issues?q=author%3Amarkddavidoff" title="Bug reports">🐛</a>
        </td>
-     </tr>
-     <tr>
        <td align="center">
          <a href="https://github.com/matclayton">
            <img src="https://avatars.githubusercontent.com/u/126218?v=3" width="100px;" alt="@matclayton" />
@@ -1153,6 +1164,8 @@ Thanks goes to these wonderful people (`emoji key`_):
          <a href="https://github.com/akaihola/darker/issues?q=author%3Anjhuffman" title="Bug reports">🐛</a>
          <a href="https://github.com/akaihola/darker/commits?author=njhuffman" title="Code">💻</a>
        </td>
+     </tr>
+     <tr>
        <td align="center">
          <a href="https://github.com/CircleOnCircles">
            <img src="https://avatars.githubusercontent.com/u/8089231?v=3" width="100px;" alt="@CircleOnCircles" />
@@ -1164,8 +1177,6 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/issues?q=author%3ACircleOnCircles" title="Bug reports">🐛</a>
        </td>
-     </tr>
-     <tr>
        <td align="center">
          <a href="https://github.com/Pacu2">
            <img src="https://avatars.githubusercontent.com/u/21290461?v=3" width="100px;" alt="@Pacu2" />
@@ -1224,6 +1235,8 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/issues?q=author%3Aroniemartinez" title="Bug reports">🐛</a>
        </td>
+     </tr>
+     <tr>
        <td align="center">
          <a href="https://github.com/rossbar">
            <img src="https://avatars.githubusercontent.com/u/1268991?v=3" width="100px;" alt="@rossbar" />
@@ -1235,8 +1248,6 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/issues?q=author%3Arossbar" title="Bug reports">🐛</a>
        </td>
-     </tr>
-     <tr>
        <td align="center">
          <a href="https://github.com/sherbie">
            <img src="https://avatars.githubusercontent.com/u/15087653?v=3" width="100px;" alt="@sherbie" />
@@ -1292,6 +1303,8 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/issues?q=author%3Aguettli" title="Bug reports">🐛</a>
        </td>
+     </tr>
+     <tr>
        <td align="center">
          <a href="https://github.com/tobiasdiez">
            <img src="https://avatars.githubusercontent.com/u/5037600?v=3" width="100px;" alt="@tobiasdiez" />
@@ -1302,8 +1315,6 @@ Thanks goes to these wonderful people (`emoji key`_):
          </a>
          <br />
        </td>
-     </tr>
-     <tr>
        <td align="center">
          <a href="https://github.com/yoursvivek">
            <img src="https://avatars.githubusercontent.com/u/163296?v=3" width="100px;" alt="@yoursvivek" />
@@ -1362,6 +1373,8 @@ Thanks goes to these wonderful people (`emoji key`_):
          <a href="https://github.com/conda-forge/staged-recipes/search?q=darker&type=issues&author=martinRenou" title="Code">💻</a>
          <a href="https://github.com/akaihola/darker/pulls?q=is%3Apr+reviewed-by%3AmartinRenou" title="Reviewed Pull Requests">👀</a>
        </td>
+     </tr>
+     <tr>
        <td align="center">
          <a href="https://github.com/mayk0gan">
            <img src="https://avatars.githubusercontent.com/u/96263702?v=3" width="100px;" alt="@mayk0gan" />
@@ -1373,8 +1386,6 @@ Thanks goes to these wonderful people (`emoji key`_):
          <br />
          <a href="https://github.com/akaihola/darker/issues?q=author%3Amayk0gan" title="Bug reports">🐛</a>
        </td>
-     </tr>
-     <tr>
        <td align="center">
          <a href="https://github.com/overratedpro">
            <img src="https://avatars.githubusercontent.com/u/1379994?v=3" width="100px;" alt="@overratedpro" />

contributors.yaml

@@ -58,6 +58,8 @@ flying-sheep:
   - {link_type: issues, type: Bug reports}
 foxwhite25:
   - {link_type: search, type: Bug reports}
+gdiscry:
+  - {link_type: pulls-author, type: Code}
 guettli:
   - {link_type: issues, type: Bug reports}
 hauntsaninja:

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
@@ -25,7 +25,7 @@ def create_repository(cls, root: Path) -> "GitRepoFixture":
         env = {"HOME": str(root), "LC_ALL": "C", "PATH": os.environ["PATH"]}
         instance = cls(root, env)
         # pylint: disable=protected-access
-        instance._run("init")
+        instance._run("init", "--initial-branch=master")
         instance._run("config", "user.email", "[email protected]")
         instance._run("config", "user.name", "CI system")
         return instance
@@ -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),