From 9f745e531b4387a09cc27a10b506d58e18397eee Mon Sep 17 00:00:00 2001 From: mshafer-NI Date: Wed, 12 Jun 2024 08:59:30 -0500 Subject: [PATCH] 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 --- .../_acknowledge_existing_errors/__init__.py | 6 ++++- ni_python_styleguide/_fix.py | 27 ++++++++++++------- poetry.lock | 2 +- pyproject.toml | 1 + tests/test_cli/test_fix.py | 4 +++ 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/ni_python_styleguide/_acknowledge_existing_errors/__init__.py b/ni_python_styleguide/_acknowledge_existing_errors/__init__.py index 2a693dc..fa60e58 100644 --- a/ni_python_styleguide/_acknowledge_existing_errors/__init__.py +++ b/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) diff --git a/ni_python_styleguide/_fix.py b/ni_python_styleguide/_fix.py index 178c519..b1f3858 100644 --- a/ni_python_styleguide/_fix.py +++ b/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 diff --git a/poetry.lock b/poetry.lock index f73217a..f2b1908 100644 --- a/poetry.lock +++ b/poetry.lock @@ -406,7 +406,7 @@ testing = ["big-O", "flake8 (<5)", "jaraco.functools", "jaraco.itertools", "more [metadata] lock-version = "1.1" python-versions = "^3.7" -content-hash = "8c3f0d8fe73c715b00b20afd4b8334b143754e3899fd2fa54125708d79190a5a" +content-hash = "fcfd1ca17126ff6de77487c045b7a1d61119c3e525c452df9ad8916945cc51c5" [metadata.files] black = [ diff --git a/pyproject.toml b/pyproject.toml index d88e988..4e439e6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,6 +14,7 @@ include = ["ni_python_styleguide/config.toml"] [tool.poetry.dependencies] python = "^3.7" +pathspec = ">=0.11.1" # Tools we aggregate flake8 = [ diff --git a/tests/test_cli/test_fix.py b/tests/test_cli/test_fix.py index 9b14b74..92deaec 100644 --- a/tests/test_cli/test_fix.py +++ b/tests/test_cli/test_fix.py @@ -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