diff --git a/ni_python_styleguide/_acknowledge_existing_errors/__init__.py b/ni_python_styleguide/_acknowledge_existing_errors/__init__.py index 0d7857f2..9b47d015 100644 --- a/ni_python_styleguide/_acknowledge_existing_errors/__init__.py +++ b/ni_python_styleguide/_acknowledge_existing_errors/__init__.py @@ -4,8 +4,7 @@ from collections import defaultdict from ni_python_styleguide import _format -from ni_python_styleguide import _lint -from ni_python_styleguide._acknowledge_existing_errors import _lint_errors_parser +from ni_python_styleguide import _utils _module_logger = logging.getLogger(__name__) @@ -13,45 +12,6 @@ "BLK100", } -DEFAULT_ENCODING = "UTF-8" - - -class _InMultiLineStringChecker: - def __init__(self, error_file): - self._error_file = pathlib.Path(error_file) - self._values = [] - self._load_lines() - - @property - def values(self): - return self._values - - def in_multiline_string(self, lineno): - return self._values[lineno - 1] # 0 indexed, but we number files 1 indexed - - @staticmethod - def _count_multiline_string_endings_in_line(line): - return line.count('"""'), line.count("'''") - - def _load_lines(self): - in_file = self._error_file.read_text(encoding=DEFAULT_ENCODING).splitlines() - current_count = [0, 0] - for line in in_file: - type1, type2 = _InMultiLineStringChecker._count_multiline_string_endings_in_line(line) - current_count[0] += type1 - current_count[1] += type2 - - code_part_of_line = line - if "#" in line: - code_part_of_line = line.split("#", maxsplit=1)[0] - - # if occurrences of multiline string markers is odd, this must be in a multiline - # or, if line continuation token is on the ending, assume in a multiline statement - self._values.append( - any([part % 2 == 1 for part in current_count]) - or code_part_of_line.strip().endswith("\\") - ) - def _add_noqa_to_line(lineno, code_lines, error_code, explanation): line = code_lines[lineno] @@ -72,21 +32,6 @@ def _filter_suppresion_from_line(line: str): return line -def _get_lint_errors_to_process(exclude, app_import_names, extend_ignore, file_or_dir): - lint_errors = _lint.get_lint_output( - format=None, - qs_or_vs=None, - exclude=exclude, - app_import_names=app_import_names, - extend_ignore=extend_ignore, - file_or_dir=file_or_dir, - ).splitlines() - parsed_errors = map(_lint_errors_parser.parse, lint_errors) - parsed_errors = filter(None, parsed_errors) - lint_errors_to_process = [error for error in parsed_errors if error.code not in EXCLUDED_ERRORS] - return lint_errors_to_process - - def acknowledge_lint_errors( exclude, app_import_names, extend_ignore, file_or_dir, *_, aggressive=False ): @@ -95,11 +40,12 @@ def acknowledge_lint_errors( Excluded error (reason): BLK100 - run black """ - lint_errors_to_process = _get_lint_errors_to_process( + lint_errors_to_process = _utils.lint.get_errors_to_process( exclude, app_import_names, extend_ignore, [pathlib.Path(file_or_dir_) for file_or_dir_ in file_or_dir or "."], + excluded_errors=EXCLUDED_ERRORS, ) lint_errors_by_file = defaultdict(list) @@ -108,9 +54,10 @@ def acknowledge_lint_errors( failed_files = [] for bad_file, errors_in_file in lint_errors_by_file.items(): - _suppress_errors_in_file(bad_file, errors_in_file, encoding=DEFAULT_ENCODING) + _suppress_errors_in_file(bad_file, errors_in_file, encoding=_utils.DEFAULT_ENCODING) if aggressive: + # some cases are expected to take up to 4 passes, making this 2x rounded per_file_format_iteration_limit = 10 for _ in range(per_file_format_iteration_limit): @@ -122,11 +69,18 @@ def acknowledge_lint_errors( _format.format(bad_file) # re-apply suppressions on correct lines - _remove_auto_suppressions_from_file(bad_file) - current_lint_errors = _get_lint_errors_to_process( - exclude, app_import_names, extend_ignore, [bad_file] + remove_auto_suppressions_from_file(bad_file) + current_lint_errors = _utils.lint.get_errors_to_process( + exclude=exclude, + app_import_names=app_import_names, + extend_ignore=extend_ignore, + file_or_dir=file_or_dir, + excluded_errors=EXCLUDED_ERRORS, + ) + + _suppress_errors_in_file( + bad_file, current_lint_errors, encoding=_utils.DEFAULT_ENCODING ) - _suppress_errors_in_file(bad_file, current_lint_errors, encoding=DEFAULT_ENCODING) changed = _format.format_check(bad_file) if not changed: # are we done? @@ -140,10 +94,11 @@ def acknowledge_lint_errors( raise RuntimeError("Could not handle some files:\n" + "\n\n".join(failed_files) + "\n\n\n") -def _remove_auto_suppressions_from_file(file): - lines = file.read_text(encoding=DEFAULT_ENCODING).splitlines() +def remove_auto_suppressions_from_file(file: pathlib.Path): + """Remove auto-suppressions from file.""" + lines = file.read_text(encoding=_utils.DEFAULT_ENCODING).splitlines() stripped_lines = [_filter_suppresion_from_line(line) for line in lines] - file.write_text("\n".join(stripped_lines) + "\n", encoding=DEFAULT_ENCODING) + file.write_text("\n".join(stripped_lines) + "\n", encoding=_utils.DEFAULT_ENCODING) def _suppress_errors_in_file(bad_file, errors_in_file, encoding): @@ -153,7 +108,7 @@ def _suppress_errors_in_file(bad_file, errors_in_file, encoding): # to make suppressions work for those cases, add an empty line. if len(lines) == 0: lines = ["\n"] - multiline_checker = _InMultiLineStringChecker(error_file=bad_file) + multiline_checker = _utils.string_helpers.InMultiLineStringChecker(error_file=bad_file) # to avoid double marking a line with the same code, keep track of lines and codes handled_lines = defaultdict(list) diff --git a/ni_python_styleguide/_utils/__init__.py b/ni_python_styleguide/_utils/__init__.py new file mode 100644 index 00000000..96c4e689 --- /dev/null +++ b/ni_python_styleguide/_utils/__init__.py @@ -0,0 +1,4 @@ +from . import lint # noqa: F401 +from . import string_helpers # noqa: F401 + +DEFAULT_ENCODING = "UTF-8" diff --git a/ni_python_styleguide/_acknowledge_existing_errors/_lint_errors_parser.py b/ni_python_styleguide/_utils/lint.py similarity index 78% rename from ni_python_styleguide/_acknowledge_existing_errors/_lint_errors_parser.py rename to ni_python_styleguide/_utils/lint.py index b88c9df2..b4229ec6 100644 --- a/ni_python_styleguide/_acknowledge_existing_errors/_lint_errors_parser.py +++ b/ni_python_styleguide/_utils/lint.py @@ -2,6 +2,25 @@ import re from collections import namedtuple +from ni_python_styleguide import _lint + + +def get_errors_to_process(exclude, app_import_names, extend_ignore, file_or_dir, excluded_errors): + """Get lint errors to process.""" + lint_errors = _lint.get_lint_output( + format=None, + qs_or_vs=None, + exclude=exclude, + app_import_names=app_import_names, + extend_ignore=extend_ignore, + file_or_dir=file_or_dir, + ).splitlines() + parsed_errors = map(parse, lint_errors) + parsed_errors = list(filter(None, parsed_errors)) + lint_errors_to_process = [error for error in parsed_errors if error.code not in excluded_errors] + return lint_errors_to_process + + LintError = namedtuple("LintError", ["file", "line", "column", "code", "explanation"]) diff --git a/ni_python_styleguide/_utils/string_helpers.py b/ni_python_styleguide/_utils/string_helpers.py new file mode 100644 index 00000000..339fba9f --- /dev/null +++ b/ni_python_styleguide/_utils/string_helpers.py @@ -0,0 +1,57 @@ +import pathlib +from typing import List, Optional + +import ni_python_styleguide._utils + + +class InMultiLineStringChecker: + """Provide utility methods to decide if line is within a multiline string.""" + + def __init__(self, error_file: Optional[str] = None, *_, lines: Optional[List[str]] = None): + """Cache off whether each line is in a multiline string or not.""" + self._values = [] + if error_file: + self._error_file = pathlib.Path(error_file) + self._load_lines() + else: + self._error_file = None + if not lines: + raise Exception( + "Error, must provide either path to `error_file` or provide `lines`" + ) + self._set_lines(lines) + + @property + def values(self): + """Return the values for the file.""" + return self._values + + def in_multiline_string(self, lineno): + """Check if lineno is in a multiline string.""" + return self._values[lineno - 1] # 0 indexed, but we number files 1 indexed + + @staticmethod + def _count_multiline_string_endings_in_line(line): + return line.count('"""'), line.count("'''") + + def _set_lines(self, lines): + current_count = [0, 0] + for line in lines: + type1, type2 = InMultiLineStringChecker._count_multiline_string_endings_in_line(line) + current_count[0] += type1 + current_count[1] += type2 + + code_part_of_line = line + if "#" in line: + code_part_of_line = line.split("#", maxsplit=1)[0] + + # if occurrences of multiline string markers is odd, this must be in a multiline + # or, if line continuation token is on the ending, assume in a multiline statement + self._values.append( + any([part % 2 == 1 for part in current_count]) + or code_part_of_line.strip().endswith("\\") + ) + + def _load_lines(self): + in_file = self._error_file.read_text(encoding=ni_python_styleguide._utils.DEFAULT_ENCODING) + self._set_lines(in_file.splitlines()) diff --git a/tests/test_cli/test_acknowledge_existing_errors.py b/tests/test_cli/test_acknowledge_existing_errors.py index 21adeddd..ed2a6b0f 100644 --- a/tests/test_cli/test_acknowledge_existing_errors.py +++ b/tests/test_cli/test_acknowledge_existing_errors.py @@ -5,58 +5,10 @@ import pytest -from ni_python_styleguide import _acknowledge_existing_errors - - TEST_CASE_DIR = ( pathlib.Path(__file__).parent.absolute() / "acknowledge_existing_errors_test_cases__snapshots" ) -EXAMPLE_FILE_LINES = [ - "import pytest", - "apple='''", - "test", - "'''", - "x = 5", - 'orange="""', - "lorem ipsum", - "dolor sit amet", - '"""', - "move_files()", -] - - -@pytest.mark.parametrize( - "lineno,expected_in_multiline", - enumerate( - [ - False, - True, - True, - False, - False, - True, - True, - True, - False, - False, - ] - ), -) -def test_can_acurately_detect_if_in_multiline_string(lineno, expected_in_multiline, tmp_path): - """Assert can accurately detect multiline strings.""" - input_lines = EXAMPLE_FILE_LINES - file_lineno = lineno + 1 # we number files 1-n, not 0-n - offending_file = tmp_path / "python_file.py" - offending_file.write_text("\n".join(input_lines)) - checker = _acknowledge_existing_errors._InMultiLineStringChecker(error_file=str(offending_file)) - - result = checker.in_multiline_string( - lineno=file_lineno, - ) - - assert result == expected_in_multiline - @pytest.mark.parametrize("test_dir", [x for x in TEST_CASE_DIR.iterdir() if x.is_dir()]) def test_given_bad_input_produces_expected_output_simple( diff --git a/tests/test_cli/test_lint_errors_parser.py b/tests/test_cli/test_lint_errors_parser.py index dbe3cb33..e7a29e38 100644 --- a/tests/test_cli/test_lint_errors_parser.py +++ b/tests/test_cli/test_lint_errors_parser.py @@ -2,8 +2,7 @@ import pytest -import ni_python_styleguide._acknowledge_existing_errors._lint_errors_parser - +import ni_python_styleguide._utils.lint EXAMPLE_LINT_ERROR_LINES = [ # noqa W505 r".\source\lorem.py:158:101: W505 doc line too long (186 > 100 characters)", @@ -30,6 +29,6 @@ @pytest.mark.parametrize("input_line", EXAMPLE_LINT_ERROR_LINES) def test_lint_errors_parser_handles_example_line_without_error(input_line): """Test parser yields expected metadata.""" - assert ni_python_styleguide._acknowledge_existing_errors._lint_errors_parser.parse( + assert ni_python_styleguide._utils.lint.parse( input_line ), "should parse without error and return object" diff --git a/tests/test_cli/test_utils.py b/tests/test_cli/test_utils.py new file mode 100644 index 00000000..e9ef7b3d --- /dev/null +++ b/tests/test_cli/test_utils.py @@ -0,0 +1,49 @@ +"""Test the _utils submodule.""" +import pytest + +from ni_python_styleguide import _utils + +EXAMPLE_FILE_LINES = [ + "import pytest", + "apple='''", + "test", + "'''", + "x = 5", + 'orange="""', + "lorem ipsum", + "dolor sit amet", + '"""', + "move_files()", +] + + +@pytest.mark.parametrize( + "lineno,expected_in_multiline", + enumerate( + [ + False, + True, + True, + False, + False, + True, + True, + True, + False, + False, + ] + ), +) +def test_can_acurately_detect_if_in_multiline_string(lineno, expected_in_multiline, tmp_path): + """Assert can accurately detect multiline strings.""" + input_lines = EXAMPLE_FILE_LINES + file_lineno = lineno + 1 # we number files 1-n, not 0-n + offending_file = tmp_path / "python_file.py" + offending_file.write_text("\n".join(input_lines)) + checker = _utils.string_helpers.InMultiLineStringChecker(error_file=str(offending_file)) + + result = checker.in_multiline_string( + lineno=file_lineno, + ) + + assert result == expected_in_multiline