Fix acknowledge comments (#126)

* debugging: give test name based on input folder

* change LintError to typing.NamedTuple for type info

* add some more test ids

* update format

* handle new way of dealing with multiple violations on a line

* handle that comments can trigger W505.

* format

* update tests

* handle lint errors

* these aren't valid code files that can pass black

* update tests

* this doesn't get used like we thought it did

* make spacing consistent

* bump patch version

* cleanup leading spaces

* add test that old comment stile is removed on --aggressive

Author: mshafer-NI

ni_python_styleguide/_acknowledge_existing_errors/__init__.py

@@ -3,8 +3,7 @@
 import re
 from collections import defaultdict
 
-from ni_python_styleguide import _format
-from ni_python_styleguide import _utils
+from ni_python_styleguide import _format, _utils
 
 _module_logger = logging.getLogger(__name__)
 
@@ -18,16 +17,25 @@ def _add_noqa_to_line(lineno, code_lines, error_code, explanation):
     old_line_ending = "\n" if line.endswith("\n") else ""
     line = line.rstrip("\n")
 
-    if f"noqa {error_code}" not in line:
+    if f"noqa: {error_code}" not in line:
         prefix = "  " if line.strip() else ""
-        line += f"{prefix}# noqa {error_code}: {explanation} (auto-generated noqa)"
+        code, _, existing = line.partition("# noqa:")
+
+        existing_codes, _, existing_explanations = existing.partition(" - ")
+        if existing_codes:
+            error_code = existing_codes + ", " + error_code
+            explanation = f"{existing_explanations}, {explanation} (auto-generated noqa)"
+        else:
+            explanation = explanation + " (auto-generated noqa)"
+
+        line = f"{code.rstrip()}{prefix}# noqa: {error_code.lstrip()} - {explanation}"
 
     code_lines[lineno] = line + old_line_ending
 
 
 def _filter_suppresion_from_line(line: str):
     if "(auto-generated noqa)" in line:
-        return re.sub(r"# noqa .+\(auto-generated noqa\)", "", line).rstrip()
+        return re.sub(r"# noqa:? .+\(auto-generated noqa\)$", "", line).rstrip()
     else:
         return line
 
@@ -56,6 +64,8 @@ def acknowledge_lint_errors(
     for bad_file, errors_in_file in lint_errors_by_file.items():
         _suppress_errors_in_file(bad_file, errors_in_file, encoding=_utils.DEFAULT_ENCODING)
 
+        _handle_emergent_violations(exclude, app_import_names, extend_ignore, file_or_dir, bad_file)
+
         if aggressive:
             # some cases are expected to take up to 4 passes, making this 2x rounded
             per_file_format_iteration_limit = 10
@@ -81,8 +91,16 @@ def acknowledge_lint_errors(
                     bad_file, current_lint_errors, encoding=_utils.DEFAULT_ENCODING
                 )
 
+                remaining_errors = _handle_emergent_violations(
+                    exclude=exclude,
+                    app_import_names=app_import_names,
+                    extend_ignore=extend_ignore,
+                    file_or_dir=file_or_dir,
+                    bad_file=bad_file,
+                )
+
                 changed = _format.format_check(bad_file)
-                if not changed:  # are we done?
+                if not changed and not remaining_errors:  # are we done?
                     break
             else:
                 failed_files.append(
@@ -93,6 +111,27 @@ def acknowledge_lint_errors(
         raise RuntimeError("Could not handle some files:\n" + "\n\n".join(failed_files) + "\n\n\n")
 
 
+def _handle_emergent_violations(exclude, app_import_names, extend_ignore, file_or_dir, bad_file):
+    """Some errors can be created by adding the acknowledge comments handle those now.
+
+    Example emergent violations:
+      W505 -> due to adding comment to docstring
+    """
+    current_lint_errors = set(
+        _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,
+        )
+    )
+    errors_to_process_now = set(filter(lambda o: o.code in {"W505"}, current_lint_errors))
+    _suppress_errors_in_file(bad_file, errors_to_process_now, encoding=_utils.DEFAULT_ENCODING)
+    remaining_errors = current_lint_errors - errors_to_process_now
+    return remaining_errors
+
+
 def remove_auto_suppressions_from_file(file: pathlib.Path):
     """Removes auto-suppressions from file."""
     lines = file.read_text(encoding=_utils.DEFAULT_ENCODING).splitlines()

ni_python_styleguide/_utils/lint.py

@@ -1,6 +1,6 @@
 import logging
 import re
-from collections import namedtuple
+import typing
 
 from ni_python_styleguide import _lint
 
@@ -23,7 +23,14 @@ def get_errors_to_process(exclude, app_import_names, extend_ignore, file_or_dir,
     return lint_errors_to_process
 
 
-LintError = namedtuple("LintError", ["file", "line", "column", "code", "explanation"])
+class LintError(typing.NamedTuple):
+    """Class defining a lint error."""
+
+    file: str
+    line: int
+    column: int
+    code: str
+    explanation: str
 
 
 def parse(line):

pyproject.toml

@@ -3,14 +3,15 @@ name = "ni-python-styleguide"
 # The -alpha.0 here denotes a source based version
 # This is removed when released through the Publish-Package.yml GitHub action
 # Official PyPI releases follow Major.Minor.Patch
-version = "0.4.0-alpha.0"
+version = "0.4.1-alpha.0"
 description = "NI's internal and external Python linter rules and plugins"
 authors = ["NI <[email protected]>"]
 readme = "README.md" # apply the repo readme to the package as well
 repository = "https://github.com/ni/python-styleguide"
 license = "MIT"
 include = ["ni_python_styleguide/config.toml"]
 
+
 [tool.poetry.dependencies]
 python = "^3.7"
 
@@ -24,7 +25,6 @@ click = ">=7.1.2"
 toml = ">=0.10.1"
 isort = ">=5.10"
 
-
 # flake8 plugins should be listed here (in alphabetical order)
 flake8-black = ">=0.2.1"
 flake8-docstrings = ">=1.5.0"
@@ -40,29 +40,30 @@ pep8-naming = ">=0.11.1"
 # Tooling that we're locking so our tool can run
 importlib-metadata = {version= "<5.0", python="<3.8"}
 
+
 [tool.poetry.dev-dependencies]
 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'
 
+
 [tool.black]
 line-length = 100
-extend-exclude = '''
-(
-/.*__snapshots/.*output\.py # exclude the simple snapshot outputs
-)
-'''
+
 
 [tool.pytest.ini_options]
 addopts = "--doctest-modules"
 norecursedirs = "*__snapshots"
 
+
 [tool.ni-python-styleguide]
 extend_exclude = "*__snapshots/*/*input.py"
 
+
 [build-system]
 requires = ["poetry>=0.12"]
 build-backend = "poetry.masonry.api"

tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/blank_file_tests/output.py

@@ -0,0 +1 @@
+# noqa: D100 - Missing docstring in public module (auto-generated noqa)

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

@@ -1 +1 @@
-# noqa D100: Missing docstring in public module (auto-generated noqa)
+# noqa: D100 - Missing docstring in public module (auto-generated noqa)

tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/blank_file_tests/output__aggressive.py

@@ -1,24 +1,24 @@
-def method1():  # noqa D100: Missing docstring in public module (auto-generated noqa)  # noqa D103: Missing docstring in public function (auto-generated noqa)
+def method1():  # noqa: D100, D103 - Missing docstring in public module (auto-generated noqa), Missing docstring in public function (auto-generated noqa)
     return 7
 
 
 def method2():
     """Provide an examples of doc strings that are too long.
 
     Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
-    """  # noqa W505: doc line too long (127 > 100 characters) (auto-generated noqa)
+    """  # noqa: W505 - doc line too long (127 > 100 characters) (auto-generated noqa)
     return 7  # Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
 
 
-class Foo:  # noqa D101: Missing docstring in public class (auto-generated noqa)
-    def __init__(self):  # noqa D107: Missing docstring in __init__ (auto-generated noqa)
+class Foo:  # noqa: D101 - Missing docstring in public class (auto-generated noqa)
+    def __init__(self):  # noqa: D107 - Missing docstring in __init__ (auto-generated noqa)
         pass
 
     def add(self, o):
         """Provide an examples of doc strings that are too long.
 
         Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
-        """  # noqa W505: doc line too long (131 > 100 characters) (auto-generated noqa)
+        """  # noqa: W505 - doc line too long (131 > 100 characters) (auto-generated noqa)
         return 7  # Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
 
 
@@ -30,5 +30,5 @@ def add(self, o):
         """Provide an examples of doc strings that are too long.
 
         Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
-        """  # noqa W505: doc line too long (131 > 100 characters) (auto-generated noqa)
+        """  # noqa: W505 - doc line too long (131 > 100 characters) (auto-generated noqa)
         return 7  # Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

…ases__snapshots/doc_line_tests/output.py …__snapshots/doc_line_tests/output.py.txttests/test_cli/acknowledge_existing_errors_test_cases__snapshots/doc_line_tests/output.py renamed to tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/doc_line_tests/output.py.txt tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/doc_line_tests/output.py renamed to tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/doc_line_tests/output.py.txt

@@ -1,24 +1,24 @@
-def method1():  # noqa D100: Missing docstring in public module (auto-generated noqa)  # noqa D103: Missing docstring in public function (auto-generated noqa)
+def method1():  # noqa: D100, D103 - Missing docstring in public module (auto-generated noqa), Missing docstring in public function (auto-generated noqa)
     return 7
 
 
 def method2():
     """Provide an examples of doc strings that are too long.
 
     Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
-    """  # noqa W505: doc line too long (127 > 100 characters) (auto-generated noqa)
+    """  # noqa: W505 - doc line too long (127 > 100 characters) (auto-generated noqa)
     return 7  # Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
 
 
-class Foo:  # noqa D101: Missing docstring in public class (auto-generated noqa)
-    def __init__(self):  # noqa D107: Missing docstring in __init__ (auto-generated noqa)
+class Foo:  # noqa: D101 - Missing docstring in public class (auto-generated noqa)
+    def __init__(self):  # noqa: D107 - Missing docstring in __init__ (auto-generated noqa)
         pass
 
     def add(self, o):
         """Provide an examples of doc strings that are too long.
 
         Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
-        """  # noqa W505: doc line too long (131 > 100 characters) (auto-generated noqa)
+        """  # noqa: W505 - doc line too long (131 > 100 characters) (auto-generated noqa)
         return 7  # Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
 
 
@@ -30,5 +30,5 @@ def add(self, o):
         """Provide an examples of doc strings that are too long.
 
         Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
-        """  # noqa W505: doc line too long (131 > 100 characters) (auto-generated noqa)
+        """  # noqa: W505 - doc line too long (131 > 100 characters) (auto-generated noqa)
         return 7  # Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.

tests/test_cli/acknowledge_existing_errors_test_cases__snapshots/doc_line_tests/output__aggressive.py

@@ -35,68 +35,68 @@ def method_with_parameters_on_multiple_lines(x, y):
     return x + y
 
 
-def method_with_bad_names_on_single_line(myBadlyNamedParam, my_other_Bad_name):  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
+def method_with_bad_names_on_single_line(myBadlyNamedParam, my_other_Bad_name):  # noqa: N803 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
     """Provide parameters with bad names on single line."""
     return myBadlyNamedParam + my_other_Bad_name
 
 
 def method_with_bad_names_on_multiple_lines_1(
-    myBadlyNamedParam,  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
+    myBadlyNamedParam,  # noqa: N803 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
 ):
     """Provide parameters with bad names on multiple lines."""
     return myBadlyNamedParam + 5
 
 
 def method_with_bad_names_on_multiple_lines_2(
-    myBadlyNamedParam,  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
+    myBadlyNamedParam,  # noqa: N803 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
     my_other_Bad_name,
 ):
     """Provide parameters with bad names on multiple lines."""
     return myBadlyNamedParam + my_other_Bad_name
 
 
-def method_withBadName_with_shadow(input):  # noqa N802: function name 'method_withBadName_with_shadow' should be lowercase (auto-generated noqa)
+def method_withBadName_with_shadow(input):  # noqa: N802 - function name 'method_withBadName_with_shadow' should be lowercase (auto-generated noqa)
     """Shadow a builtin."""
     return input
 
 
-def method_withBadName_with_unused_param(unused_input):  # noqa N802: function name 'method_withBadName_with_unused_param' should be lowercase (auto-generated noqa)
+def method_withBadName_with_unused_param(unused_input):  # noqa: N802 - function name 'method_withBadName_with_unused_param' should be lowercase (auto-generated noqa)
     """Provide and unused param."""
     return 5
 
 
-def method_withBadName_with_parameters_on_multiple_lines(x, y):  # noqa N802: function name 'method_withBadName_with_parameters_on_multiple_lines' should be lowercase (auto-generated noqa)
+def method_withBadName_with_parameters_on_multiple_lines(x, y):  # noqa: N802 - function name 'method_withBadName_with_parameters_on_multiple_lines' should be lowercase (auto-generated noqa)
     """Provide parameters on multiple lines test case."""
     return x + y
 
 
-def method_withBadName_with_bad_params_on_single_line(myBadlyNamedParam, my_other_Bad_name):  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)  # noqa N802: function name 'method_withBadName_with_bad_params_on_single_line' should be lowercase (auto-generated noqa)
+def method_withBadName_with_bad_params_on_single_line(myBadlyNamedParam, my_other_Bad_name):  # noqa: N803, N802 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa), function name 'method_withBadName_with_bad_params_on_single_line' should be lowercase (auto-generated noqa)
     """Provide parameters with bad names on single line."""
     return myBadlyNamedParam + my_other_Bad_name
 
 
-def method_withBadName_with_bad_params_on_multiple_lines_1(  # noqa N802: function name 'method_withBadName_with_bad_params_on_multiple_lines_1' should be lowercase (auto-generated noqa)
-    myBadlyNamedParam,  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
+def method_withBadName_with_bad_params_on_multiple_lines_1(  # noqa: N802 - function name 'method_withBadName_with_bad_params_on_multiple_lines_1' should be lowercase (auto-generated noqa)
+    myBadlyNamedParam,  # noqa: N803 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
 ):
     """Provide parameters with bad names on multiple lines."""
     return myBadlyNamedParam + 5
 
 
-def method_withBadName_with_bad_params_on_multiple_lines_2(  # noqa N802: function name 'method_withBadName_with_bad_params_on_multiple_lines_2' should be lowercase (auto-generated noqa)
-    myBadlyNamedParam,  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
+def method_withBadName_with_bad_params_on_multiple_lines_2(  # noqa: N802 - function name 'method_withBadName_with_bad_params_on_multiple_lines_2' should be lowercase (auto-generated noqa)
+    myBadlyNamedParam,  # noqa: N803 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
     my_other_Bad_name,
 ):
     """Provide parameters with bad names on multiple lines."""
     return myBadlyNamedParam + my_other_Bad_name
 
 
-def method_withBadName_andParams(my_normal_param, myBadlyNamedParam, my_other_Bad_param):  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)  # noqa N802: function name 'method_withBadName_andParams' should be lowercase (auto-generated noqa)
+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
 
 
-def method_withBadName_and_bad_param_with_long_name(  # noqa N802: function name 'method_withBadName_and_bad_param_with_long_name' should be lowercase (auto-generated noqa)
-    my_normal_param, myBadlyNamedParam, my_other_Bad_param  # noqa N803: argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
+def method_withBadName_and_bad_param_with_long_name(  # noqa: N802 - function name 'method_withBadName_and_bad_param_with_long_name' should be lowercase (auto-generated noqa)
+    my_normal_param, myBadlyNamedParam, my_other_Bad_param  # noqa: N803 - argument name 'myBadlyNamedParam' should be lowercase (auto-generated noqa)
 ):
-    """Provide example where black will want to split out result even more"""  # noqa D415: First line should end with a period, question mark, or exclamation point (auto-generated noqa)
+    """Provide example where black will want to split out result even more"""  # noqa: D415, W505 - First line should end with a period, question mark, or exclamation point (auto-generated noqa), doc line too long (188 > 100 characters) (auto-generated noqa)
     return 5 + 7