Fix formatting files with unicode (#242)

* setup a failing test

* get fixture to actually work with help from copilot

* make failing tests

* make it pass

* simplify fixture setup

* format doesn't fix method names, so make output be able to pass

* clean up changes some

* format

* undo adding pytest-mock, not needed yet

* Apply suggestions from code review

Author: mshafer-NI

ni_python_styleguide/_fix.py

@@ -1,5 +1,6 @@
 import logging
 import pathlib
+import shutil
 import typing
 from collections import defaultdict
 from typing import Iterable
@@ -21,7 +22,7 @@
 
 
 def _sort_imports(file: pathlib.Path, app_import_names):
-    raw = file.read_text()
+    raw = file.read_text(encoding=_utils.DEFAULT_ENCODING)
     isort_config = isort.Config(
         settings_file=str(_config_constants.ISORT_CONFIG_FILE),
         known_first_party=filter(None, app_import_names.split(",")),
@@ -30,7 +31,7 @@ def _sort_imports(file: pathlib.Path, app_import_names):
         raw,
         config=isort_config,
     )
-    file.write_text(output)
+    file.write_text(output, encoding=_utils.DEFAULT_ENCODING)
 
 
 def _format_imports(file: pathlib.Path, app_import_names: Iterable[str]) -> None:
@@ -117,13 +118,13 @@ def fix(
                     )
             else:
                 with temp_file.multi_access_tempfile(suffix="__" + bad_file.name) as working_file:
-                    working_file.write_text(bad_file.read_text())
+                    shutil.copyfile(bad_file, working_file)
                     _format.format(working_file, "-q")
                     _format_imports(file=working_file, app_import_names=app_import_names)
 
                     diff_lines = better_diff.unified_plus.format_diff(
-                        bad_file.read_text(),
-                        working_file.read_text(),
+                        bad_file.read_text(encoding=_utils.DEFAULT_ENCODING),
+                        working_file.read_text(encoding=_utils.DEFAULT_ENCODING),
                         fromfile=f"{_posix_relative_if_under(bad_file, pathlib.Path.cwd())}",
                         tofile=f"{_posix_relative_if_under(bad_file, pathlib.Path.cwd())}_formatted",
                     )

ni_python_styleguide/_utils/code_analysis.py

@@ -11,7 +11,9 @@ def find_import_region(file: pathlib.Path) -> Tuple[int, int]:
     file: pathlib.Path path to file to evaluate
     Return: Tuple[int, int] the start and ending lines (0 based) of the module level imports
     """
-    file_contents = file.read_text()
+    file_contents = file.read_text(
+        encoding="utf-8"
+    )  # can't use DEFAULT_ENCODING here due to possible circular imports
     tree = ast.parse(file_contents)
     end = start = 0
     for node in tree.body:  # only walk top level items

pyproject.toml

@@ -57,7 +57,6 @@ pytest = ">=6.0.1"
 pytest_click = ">=1.0.2"
 pytest-snapshot = ">=0.6.3"
 
-
 [tool.poetry.scripts]
 ni-python-styleguide = 'ni_python_styleguide._cli:main'
 nps = 'ni_python_styleguide._cli:main'

tests/conftest.py

@@ -1,6 +1,7 @@
 """Useful plugins/fixtures which can (and should) be used in any test."""
 
 import os
+import pathlib
 
 import click.testing
 import pytest
@@ -59,3 +60,26 @@ def chdir():
     cwd = os.getcwd()
     yield os.chdir
     os.chdir(cwd)
+
+
+@pytest.fixture(autouse=True)
+def force_ascii_encoding(monkeypatch):
+    """Force ASCII encoding as default for all file operations to catch missing encoding args."""
+    # Patch pathlib.Path.read_text and write_text to use ASCII when encoding not specified
+    original_read_text = pathlib.Path.read_text
+    original_write_text = pathlib.Path.write_text
+
+    def ascii_read_text(self, encoding=None, errors=None):
+        if encoding is None:
+            encoding = "ascii"
+        return original_read_text(self, encoding=encoding, errors=errors)
+
+    def ascii_write_text(self, data, encoding=None, errors=None, **kwargs):
+        if encoding is None:
+            encoding = "ascii"
+        return original_write_text(self, data, encoding=encoding, errors=errors, **kwargs)
+
+    monkeypatch.setattr(pathlib.Path, "read_text", ascii_read_text)
+    monkeypatch.setattr(pathlib.Path, "write_text", ascii_write_text)
+
+    yield None

tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/unicode_in_files/input.py

@@ -17,3 +17,13 @@ def problem_chars(self):
 def method_withBadName_andParams(my_normal_param, myBadlyNamedParam, my_other_Bad_param):
     """Provide example where black will want to split out result."""
     return 5 + 7
+
+data =   ("device_name, supported_encodings",
+    [
+        ("Gerät", ["1252", "iso-8859-1", "utf-8"]),
+        ("l' appareil", ["1252", "iso-8859-1", "utf-8"]),
+        ("デバイス", ["932", "shift-jis", "utf-8"]),
+        ("장치", ["utf-8", "euc-kr"]),
+        ("设备", ["utf-8", "gbk"]),
+    ],
+)

tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/unicode_in_files/output.py.txt

@@ -17,3 +17,13 @@ class Foo:
 def method_withBadName_andParams(my_normal_param, myBadlyNamedParam, my_other_Bad_param):  # noqa: N803, N802 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa), function name 'method_withBadName_andParams' should be lowercase (auto-generated noqa)
     """Provide example where black will want to split out result."""
     return 5 + 7
+
+data =   ("device_name, supported_encodings",
+    [
+        ("Gerät", ["1252", "iso-8859-1", "utf-8"]),
+        ("l' appareil", ["1252", "iso-8859-1", "utf-8"]),
+        ("デバイス", ["932", "shift-jis", "utf-8"]),
+        ("장치", ["utf-8", "euc-kr"]),
+        ("设备", ["utf-8", "gbk"]),
+    ],
+)

tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/unicode_in_files/output__aggressive.py

@@ -21,3 +21,15 @@ def method_withBadName_andParams(  # noqa: N802 - function name 'method_withBadN
 ):
     """Provide example where black will want to split out result."""
     return 5 + 7
+
+
+data = (
+    "device_name, supported_encodings",
+    [
+        ("Gerät", ["1252", "iso-8859-1", "utf-8"]),
+        ("l' appareil", ["1252", "iso-8859-1", "utf-8"]),
+        ("デバイス", ["932", "shift-jis", "utf-8"]),
+        ("장치", ["utf-8", "euc-kr"]),
+        ("设备", ["utf-8", "gbk"]),
+    ],
+)

tests/test_cli/fix_test_cases__snapshots/unicode_example/input.py

@@ -0,0 +1,29 @@
+"""Unicode in file should not cause error (e.g., ©)."""
+
+
+class Foo:
+    """Example class with unicode consts."""
+
+    def __init__(self) -> None:
+        """Instantiate Foo class."""
+        self._problem_chars = "π”"
+
+    @property
+    def problem_chars(self):
+        """Return stored string with a unicode char."""
+        return self._problem_chars
+
+
+def method_withBadName_andParams(my_normal_param, myBadlyNamedParam, my_other_Bad_param):
+    """Provide example where black will want to split out result."""
+    return 5 + 7
+
+data =   ("device_name, supported_encodings",
+    [
+        ("Gerät", ["1252", "iso-8859-1", "utf-8"]),
+        ("l' appareil", ["1252", "iso-8859-1", "utf-8"]),
+        ("デバイス", ["932", "shift-jis", "utf-8"]),
+        ("장치", ["utf-8", "euc-kr"]),
+        ("设备", ["utf-8", "gbk"]),
+    ],
+)

tests/test_cli/fix_test_cases__snapshots/unicode_example/output.py.txt

@@ -0,0 +1,31 @@
+"""Unicode in file should not cause error (e.g., ©)."""
+
+
+class Foo:
+    """Example class with unicode consts."""
+
+    def __init__(self) -> None:
+        """Instantiate Foo class."""
+        self._problem_chars = "π”"
+
+    @property
+    def problem_chars(self):
+        """Return stored string with a unicode char."""
+        return self._problem_chars
+
+
+def method_withBadName_andParams(my_normal_param, myBadlyNamedParam, my_other_Bad_param):
+    """Provide example where black will want to split out result."""
+    return 5 + 7
+
+
+data = (
+    "device_name, supported_encodings",
+    [
+        ("Gerät", ["1252", "iso-8859-1", "utf-8"]),
+        ("l' appareil", ["1252", "iso-8859-1", "utf-8"]),
+        ("デバイス", ["932", "shift-jis", "utf-8"]),
+        ("장치", ["utf-8", "euc-kr"]),
+        ("设备", ["utf-8", "gbk"]),
+    ],
+)

tests/test_cli/fix_test_cases__snapshots/unicode_example/output__aggressive.py

@@ -0,0 +1,35 @@
+"""Unicode in file should not cause error (e.g., ©)."""
+
+
+class Foo:
+    """Example class with unicode consts."""
+
+    def __init__(self) -> None:
+        """Instantiate Foo class."""
+        self._problem_chars = "π”"
+
+    @property
+    def problem_chars(self):
+        """Return stored string with a unicode char."""
+        return self._problem_chars
+
+
+def method_withBadName_andParams(  # noqa: N802 - function name 'method_withBadName_andParams' should be lowercase (auto-generated noqa)
+    my_normal_param,
+    myBadlyNamedParam,  # noqa: N803 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
+    my_other_Bad_param,  # noqa: N803 - argument name 'my_other_Bad_param' should be lowercase (auto-generated noqa)
+):
+    """Provide example where black will want to split out result."""
+    return 5 + 7
+
+
+data = (
+    "device_name, supported_encodings",
+    [
+        ("Gerät", ["1252", "iso-8859-1", "utf-8"]),
+        ("l' appareil", ["1252", "iso-8859-1", "utf-8"]),
+        ("デバイス", ["932", "shift-jis", "utf-8"]),
+        ("장치", ["utf-8", "euc-kr"]),
+        ("设备", ["utf-8", "gbk"]),
+    ],
+)