Dev/fix command being over aggressive (#185)

* make filter actually work

* log when we fail to read a file, what file it was

* make a failing test

* make it pass

* print the exception in the test

* switch to pathspec for globbing/matching in fix

* move pathspec to core tooling section

* format / linter issues

* simplify

Author: mshafer-NI

ni_python_styleguide/_acknowledge_existing_errors/__init__.py

@@ -134,7 +134,11 @@ def _handle_emergent_violations(exclude, app_import_names, extend_ignore, file_o
 
 def remove_auto_suppressions_from_file(file: pathlib.Path):
     """Removes auto-suppressions from file."""
-    lines = file.read_text(encoding=_utils.DEFAULT_ENCODING).splitlines()
+    try:
+        lines = file.read_text(encoding=_utils.DEFAULT_ENCODING).splitlines()
+    except Exception:
+        _module_logger.warning("Failed to read %s with encoding %s", file, _utils.DEFAULT_ENCODING)
+        raise
     stripped_lines = [_filter_suppresion_from_line(line) for line in lines]
     file.write_text("\n".join(stripped_lines) + "\n", encoding=_utils.DEFAULT_ENCODING)
 

ni_python_styleguide/_fix.py

@@ -1,15 +1,17 @@
 import logging
 import pathlib
 from collections import defaultdict
-from fnmatch import fnmatch
 from typing import Iterable
 
 import isort
+import pathspec
 
-from ni_python_styleguide import _acknowledge_existing_errors
-from ni_python_styleguide import _config_constants
-from ni_python_styleguide import _format
-from ni_python_styleguide import _utils
+from ni_python_styleguide import (
+    _acknowledge_existing_errors,
+    _config_constants,
+    _format,
+    _utils,
+)
 
 _module_logger = logging.getLogger(__name__)
 
@@ -93,17 +95,24 @@ def fix(
 ):
     """Fix basic linter errors and format."""
     file_or_dir = file_or_dir or ["."]
+    extend_ignore = extend_ignore or ""
+    exclude = exclude or ""
     if aggressive:
+        glob_spec = pathspec.PathSpec.from_lines(
+            "gitwildmatch",
+            ["*.py"]
+            + [f"!{exclude_}" for exclude_ in exclude.split(",") if exclude_]
+            + [f"!{ignore_}" for ignore_ in extend_ignore.split(",") if ignore_],
+        )
         all_files = []
         for file_or_dir_ in file_or_dir:
             file_path = pathlib.Path(file_or_dir_)
             if file_path.is_dir():
-                all_files.extend(file_path.rglob("*.py"))
+                all_files.extend(
+                    [pathlib.Path(o) for o in glob_spec.match_tree(str(file_path), negate=False)]
+                )
             else:
                 all_files.append(file_path)
-        all_files = filter(
-            lambda o: not any([fnmatch(o, exclude_) for exclude_ in exclude.split(",")]), all_files
-        )
         for file in all_files:
             if not file.is_file():  # doesn't really exist...
                 continue

poetry.lock

@@ -14,6 +14,7 @@ include = ["ni_python_styleguide/config.toml"]
 
 [tool.poetry.dependencies]
 python = "^3.7"
+pathspec = ">=0.11.1"
 
 # Tools we aggregate
 flake8 = [

pyproject.toml

@@ -51,9 +51,13 @@ def test_given_bad_input__fix__produces_expected_output_aggressive(
     in_file = test_dir / "input.py"
     test_file = tmp_path / "input.py"
     shutil.copyfile(in_file, test_file)
+    test_loading_file = tmp_path / ".venv/file_with_non_unicode_chars.py"
+    test_loading_file.parent.mkdir(parents=True, exist_ok=True)
+    test_loading_file.write_text("Förward to victory!", encoding="ISO 8859-1")
 
     output = styleguide_command(command="fix", command_args=["--aggressive"])
 
+    assert not output.exception, f"Error in running:\n{output.exception}"
     assert output.exit_code in (True, 0), f"Error in running:\n{output}"
     result = test_file.read_text(encoding="UTF-8")
     snapshot.snapshot_dir = test_dir