Auto acknowledge existing linter errors (#37)

* store current code before dog-food testing it

* got single line suppresions working

* remove the lint error altogether

* debug and fix lint errors

* call linter directly

this removes the concern of outdated lint errors or an uhandled format

* add tests

* fix spelling issues

* Update ni_python_styleguide/acknowledge_existing_errors/__init__.py

Co-authored-by: Ryan Zoeller <[email protected]>

* fix lint errors

not addressing test_files as they are going to need additional changes

* missed a rename

* actually get the value of the iostream

* add tests that it correctly suppresses

* add black and pytest-snapshot to dev dependencies

* change to lower case noqa

* make warning more explicit

* move multiple erros on single line to single suppression statement

* fix lint errors

* get lint to pass snapshot test files

* fix multi-code handling line endings

* add method and param name tests

* narrow exclusions to what makes sense for us

* remove pep8-naming from dev branch

* re-arrange tests

* setup generic styleguide_command fixture

* use shutil to copy test input file to tmp_dir

* update formatting

* update doc-string

* fix tests

* switch back to using text transfer??

* miniscule change to re-prompt pr test run

* remove unused shutil

* make tests very verbose

* try asserting that output is an object

* get tests to run on both windows and linux

* debug why snapshots are different on windows vs linux

* handle posix style paths

* remove ignore so that doctests are actually run

* remove unused logging

* reset to match main

* re-run lock after rebasing

* update tests now that pep8-naming is turned on

* shorten auto-noqa comment

* setup multiline checking to only read file once

* change to map and filter for removing unhandled codes

* use specific import

* move lint_errors_parser

* fix parser reference

* utilize the default dict

* just reference the linter

* change to work on whole file at a time

* cache old line ending

* rename for brevity

* classes should be CamelCase

* last cleanups

* fix formatting and ignore stored outputs

Co-authored-by: Ryan Zoeller <[email protected]>

ni_python_styleguide/__init__.py

@@ -0,0 +1 @@
+"""NI's internal and external style rules enforcement tool for Python."""

ni_python_styleguide/_acknowledge_existing_errors/__init__.py

@@ -0,0 +1,108 @@
+from collections import defaultdict
+import logging
+import re
+import pathlib
+
+from ni_python_styleguide._acknowledge_existing_errors import _lint_errors_parser
+
+EXCLUDED_ERRORS = {
+    "BLK100",
+}
+
+
+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().splitlines()
+        current_count = 0
+        for line in in_file:
+            line_value = (
+                current_count
+                + _InMultiLineStringChecker._count_multiline_string_endings_in_line(line)
+            )
+            # if occurances of multiline string markers is odd, this must be in a multiline
+            self._values.append(line_value % 2 == 1)
+            current_count = line_value
+
+
+def _add_noqa_to_line(lineno, code_lines, error_code, explanation):
+    line = code_lines[lineno]
+    old_line_ending = "\n" if line.endswith("\n") else ""
+    line = line.rstrip("\n")
+
+    existing_suppression = re.search(r"noqa (?P<existing_suppresions>[\w\d]+\: [\w\W]+?) -", line)
+    if existing_suppression:
+        before = existing_suppression.groupdict()["existing_suppresions"]
+        if error_code not in before:
+            line = line.replace(before, before + f", {error_code}: {explanation}")
+    else:
+        line += f"  # noqa {error_code}: {explanation} (auto-generated noqa)"
+
+    code_lines[lineno] = line + old_line_ending
+
+
+def acknowledge_lint_errors(lint_errors):
+    """Add a "noqa" comment for each of existing errors (unless excluded).
+
+    Excluded error (reason):
+    BLK100 - run black
+    """
+    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 not in EXCLUDED_ERRORS]
+
+    lint_errors_by_file = defaultdict(list)
+    for error in lint_errors_to_process:
+        lint_errors_by_file[error.file].append(error)
+
+    for bad_file, errors_in_file in lint_errors_by_file.items():
+        path = pathlib.Path(bad_file)
+        lines = path.read_text().splitlines(keepends=True)
+        multiline_checker = _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)
+        for error in errors_in_file:
+            skip = 0
+
+            while multiline_checker.in_multiline_string(
+                lineno=error.line + skip
+            ) and error.line + skip < len(lines):
+                # find when the multiline ends
+                skip += 1
+
+            cached_key = f"{error.file}:{error.line + skip}"
+            if error.code in handled_lines[cached_key]:
+                logging.warning(
+                    "Multiple occurances of error %s code were logged for %s:%s, only suprressing first",
+                    error.code,
+                    error.file,
+                    error.line + skip,
+                )
+                continue
+
+            handled_lines[cached_key].append(error.code)
+
+            _add_noqa_to_line(
+                lineno=error.line - 1 + skip,
+                code_lines=lines,
+                error_code=error.code,
+                explanation=error.explanation,
+            )
+
+        path.write_text("".join(lines))

ni_python_styleguide/_acknowledge_existing_errors/_lint_errors_parser.py

@@ -0,0 +1,58 @@
+import re
+import logging
+from collections import namedtuple
+
+LintError = namedtuple("LintError", ["file", "line", "column", "code", "explanation"])
+
+
+def parse(line):
+    r"""
+    Parse line into :class:`LintError`.
+
+    >>> parse(r'source\arfile.py:55:16: BLK100 Black would make changes.')
+    LintError(file='source\\arfile.py', line=55, column=16, code='BLK100', explanation='Black would make changes.')
+
+    >>> parse(r"source\rpmfile\__init__.py:13:1: F401 'functools.wraps' imported but unused")
+    LintError(file='source\\rpmfile\\__init__.py', line=13, column=1, code='F401', explanation="'functools.wraps' imported but unused")
+
+    >>> parse(r"expected_output.py:77:6: N802 function name 'method_withBadName_with_bad_params_on_multiple_lines_1' should be lowercase")
+    LintError(file='expected_output.py', line=77, column=6, code='N802', explanation="function name 'method_withBadName_with_bad_params_on_multiple_lines_1' should be lowercase")
+
+    >>> parse(r"./tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/doc_line_tests/expected_output.py:1:1: D100 Missing docstring in public module")
+    LintError(file='./tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/doc_line_tests/expected_output.py', line=1, column=1, code='D100', explanation='Missing docstring in public module')
+    """  # NOQA W505: doc line too long (115 > 100 characters)
+    p = Parser()
+    return p.parse(line)
+
+
+class Parser:
+    """Lint errors parser."""
+
+    __MATCHER = re.compile(
+        r"^(?P<file>[\w\\/\.]+):(?P<line>\d+):(?P<column>\d+): (?P<code>\w+) (?P<explanation>.+)"
+    )
+
+    @staticmethod
+    def _to_lint_error(file: str, line: str, column: str, code: str, explanation: str, **kwargs):
+        return LintError(
+            file=file,
+            line=int(line),
+            column=int(column),
+            code=code,
+            explanation=explanation,
+            **kwargs
+        )
+
+    def parse(self, line):
+        """Parse `line` and return a :class:`LintError`.
+
+        :param line: the line to parse
+        :return: lint error as metada object
+        :rtype: LintError
+        """
+        data = Parser.__MATCHER.search(line)
+        logging.debug("parsing line: %s, yielded %s", line, data)
+        if not data:
+            return None
+        result = Parser._to_lint_error(**data.groupdict())
+        return result

ni_python_styleguide/_cli.py

@@ -1,10 +1,12 @@
-import pathlib
-
 import click
-
+import contextlib
 import flake8.main.application
-
+import logging
+import pathlib
 import toml
+from io import StringIO
+
+from ni_python_styleguide import _acknowledge_existing_errors
 
 
 def _qs_or_vs(verbosity):
@@ -94,18 +96,7 @@ def main(ctx, verbose, quiet, config, exclude, extend_exclude):
     ctx.obj["EXCLUDE"] = ",".join(filter(bool, [exclude.strip(","), extend_exclude.strip(",")]))
 
 
-@main.command()
-# @TODO: When we're ready to encourage editor integration, add --diff flag
-@click.option("--format", type=str, help="Format errors according to the chosen formatter.")
-@click.option(
-    "--extend-ignore",
-    type=str,
-    help="Comma-separated list of errors and warnings to ignore (or skip)",
-)
-@click.argument("file_or_dir", nargs=-1)
-@click.pass_obj
-def lint(obj, format, extend_ignore, file_or_dir):
-    """Lint the file(s)/directory(s) given."""  # noqa: D4
+def _lint(obj, format, extend_ignore, file_or_dir):
     app = flake8.main.application.Application()
     args = [
         _qs_or_vs(obj["VERBOSITY"]),
@@ -121,3 +112,43 @@ def lint(obj, format, extend_ignore, file_or_dir):
     ]
     app.run(list(filter(bool, args)))
     app.exit()
+
+
+@main.command()
+# @TODO: When we're ready to encourage editor integration, add --diff flag
+@click.option("--format", type=str, help="Format errors according to the chosen formatter.")
+@click.option(
+    "--extend-ignore",
+    type=str,
+    help="Comma-separated list of errors and warnings to ignore (or skip)",
+)
+@click.argument("file_or_dir", nargs=-1)
+@click.pass_obj
+def lint(obj, format, extend_ignore, file_or_dir):
+    """Lint the file(s)/directory(s) given."""  # noqa: D4
+    _lint(obj=obj, format=format, extend_ignore=extend_ignore, file_or_dir=file_or_dir)
+
+
+@main.command()
+@click.option(
+    "--extend-ignore",
+    type=str,
+    help="Comma-separated list of errors and warnings to ignore (or skip)",
+)
+@click.argument("file_or_dir", nargs=-1)
+@click.pass_obj
+def acknowledge_existing_violations(obj, extend_ignore, file_or_dir):
+    """Lint existing error and suppress.
+
+    Use this command to acknowledge violations in existing code to allow for enforcing new code.
+    """
+    logging.info("linting code")
+    capture = StringIO()
+    with contextlib.redirect_stdout(capture):
+        try:
+            _lint(obj=obj, format=None, extend_ignore=extend_ignore, file_or_dir=file_or_dir)
+        except SystemExit:
+            pass  # the flake8 app wants to always SystemExit :(
+
+    lines = capture.getvalue().splitlines()
+    _acknowledge_existing_errors.acknowledge_lint_errors(lines)