Prune reported errors after autofix (#3774)

Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>

.github/workflows/tox.yml

@@ -71,7 +71,7 @@ jobs:
       WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 827
+      PYTEST_REQPASS: 828
     steps:
       - name: Activate WSL1
         if: "contains(matrix.shell, 'wsl')"

pyproject.toml

@@ -145,6 +145,7 @@ max-statements = 60
 disable = [
   # Disabled on purpose:
   "line-too-long", # covered by black
+  "protected-access", # covered by ruff SLF001
   "too-many-branches", # covered by ruff C901
   # TODO(ssbarnea): remove temporary skips adding during initial adoption:
   "duplicate-code",

src/ansiblelint/__main__.py

@@ -55,6 +55,7 @@
 )
 from ansiblelint.constants import RC
 from ansiblelint.loaders import load_ignore_txt
+from ansiblelint.runner import get_matches
 from ansiblelint.skip_utils import normalize_tag
 from ansiblelint.version import __version__
 
@@ -202,6 +203,74 @@ def support_banner() -> None:
         )
 
 
+def fix(runtime_options: Options, result: LintResult, rules: RulesCollection) -> None:
+    """Fix the linting errors.
+
+    :param options: Options object
+    :param result: LintResult object
+    """
+    match_count = len(result.matches)
+    _logger.debug("Begin fixing: %s matches", match_count)
+    ruamel_safe_version = "0.17.26"
+
+    # pylint: disable=import-outside-toplevel
+    from packaging.version import Version
+    from ruamel.yaml import __version__ as ruamel_yaml_version_str
+
+    # pylint: enable=import-outside-toplevel
+
+    if Version(ruamel_safe_version) > Version(ruamel_yaml_version_str):
+        _logger.warning(
+            "We detected use of `--fix` feature with a buggy ruamel-yaml %s library instead of >=%s, upgrade it before reporting any bugs like dropped comments.",
+            ruamel_yaml_version_str,
+            ruamel_safe_version,
+        )
+    acceptable_tags = {"all", "none", *rules.known_tags()}
+    unknown_tags = set(options.write_list).difference(acceptable_tags)
+
+    if unknown_tags:
+        _logger.error(
+            "Found invalid value(s) (%s) for --fix arguments, must be one of: %s",
+            ", ".join(unknown_tags),
+            ", ".join(acceptable_tags),
+        )
+        sys.exit(RC.INVALID_CONFIG)
+    _do_transform(result, options)
+
+    rerun = ["yaml"]
+    resolved = []
+    for idx, match in reversed(list(enumerate(result.matches))):
+        _logger.debug("Fixing: (%s of %s) %s", match_count - idx, match_count, match)
+        if match.fixed:
+            _logger.debug("Fixed, removed: %s", match)
+            result.matches.pop(idx)
+            continue
+        if match.rule.id not in rerun:
+            _logger.debug("Not rerun eligible: %s", match)
+            continue
+
+        uid = (match.rule.id, match.filename)
+        if uid in resolved:
+            _logger.debug("Previously resolved: %s", match)
+            result.matches.pop(idx)
+            continue
+        _logger.debug("Rerunning: %s", match)
+        runtime_options.tags = [match.rule.id]
+        runtime_options.lintables = [match.filename]
+        runtime_options._skip_ansible_syntax_check = True  # noqa: SLF001
+        new_results = get_matches(rules, runtime_options)
+        if not new_results.matches:
+            _logger.debug("Newly resolved: %s", match)
+            result.matches.pop(idx)
+            resolved.append(uid)
+            continue
+        if match in new_results.matches:
+            _logger.debug("Still found: %s", match)
+            continue
+        _logger.debug("Fixed, removed: %s", match)
+        result.matches.pop(idx)
+
+
 # pylint: disable=too-many-statements,too-many-locals
 def main(argv: list[str] | None = None) -> int:
     """Linter CLI entry point."""
@@ -244,7 +313,6 @@ def main(argv: list[str] | None = None) -> int:
 
     # pylint: disable=import-outside-toplevel
     from ansiblelint.rules import RulesCollection
-    from ansiblelint.runner import _get_matches
 
     if options.list_profiles:
         from ansiblelint.generate_docs import profiles_as_rich
@@ -265,30 +333,7 @@ def main(argv: list[str] | None = None) -> int:
 
     if isinstance(options.tags, str):
         options.tags = options.tags.split(",")  # pragma: no cover
-    result = _get_matches(rules, options)
-
-    if options.write_list:
-        ruamel_safe_version = "0.17.26"
-        from packaging.version import Version
-        from ruamel.yaml import __version__ as ruamel_yaml_version_str
-
-        if Version(ruamel_safe_version) > Version(ruamel_yaml_version_str):
-            _logger.warning(
-                "We detected use of `--fix` feature with a buggy ruamel-yaml %s library instead of >=%s, upgrade it before reporting any bugs like dropped comments.",
-                ruamel_yaml_version_str,
-                ruamel_safe_version,
-            )
-        acceptable_tags = {"all", "none", *rules.known_tags()}
-        unknown_tags = set(options.write_list).difference(acceptable_tags)
-
-        if unknown_tags:
-            _logger.error(
-                "Found invalid value(s) (%s) for --fix arguments, must be one of: %s",
-                ", ".join(unknown_tags),
-                ", ".join(acceptable_tags),
-            )
-            sys.exit(RC.INVALID_CONFIG)
-        _do_transform(result, options)
+    result = get_matches(rules, options)
 
     mark_as_success = True
 
@@ -302,6 +347,10 @@ def main(argv: list[str] | None = None) -> int:
     for match in result.matches:
         if match.tag in ignore_map[match.filename]:
             match.ignored = True
+            _logger.debug("Ignored: %s", match)
+
+    if options.write_list:
+        fix(runtime_options=options, result=result, rules=rules)
 
     app.render_matches(result.matches)
 

src/ansiblelint/config.py

@@ -107,6 +107,10 @@
 class Options:  # pylint: disable=too-many-instance-attributes,too-few-public-methods
     """Store ansible-lint effective configuration options."""
 
+    # Private attributes
+    _skip_ansible_syntax_check: bool = False
+
+    # Public attributes
     cache_dir: Path | None = None
     colored: bool = True
     configured: bool = False

src/ansiblelint/errors.py

@@ -134,6 +134,10 @@ def __repr__(self) -> str:
             self.details,
         )
 
+    def __str__(self) -> str:
+        """Return a MatchError instance string representation."""
+        return self.__repr__()
+
     @property
     def position(self) -> str:
         """Return error positioning, with column number if available."""

src/ansiblelint/rules/__init__.py

@@ -486,10 +486,13 @@ def run(
                 or rule.has_dynamic_tags
                 or not set(rule.tags).union([rule.id]).isdisjoint(tags)
             ):
+                _logger.debug("Running rule %s", rule.id)
                 rule_definition = set(rule.tags)
                 rule_definition.add(rule.id)
                 if set(rule_definition).isdisjoint(skip_list):
                     matches.extend(rule.getmatches(file))
+            else:
+                _logger.debug("Skipping rule %s", rule.id)
 
         # some rules can produce matches with tags that are inside our
         # skip_list, so we need to cleanse the matches

src/ansiblelint/rules/deprecated_local_action.py

@@ -9,7 +9,7 @@
 from typing import TYPE_CHECKING
 
 from ansiblelint.rules import AnsibleLintRule, TransformMixin
-from ansiblelint.runner import _get_matches
+from ansiblelint.runner import get_matches
 from ansiblelint.transformer import Transformer
 
 if TYPE_CHECKING:
@@ -107,7 +107,7 @@ def test_local_action_transform(
         config_options.write_list = ["all"]
 
         config_options.lintables = [playbook]
-        runner_result = _get_matches(
+        runner_result = get_matches(
             rules=default_rules_collection,
             options=config_options,
         )

src/ansiblelint/rules/jinja.py

@@ -18,6 +18,7 @@
 from ansiblelint.errors import RuleMatchTransformMeta
 from ansiblelint.file_utils import Lintable
 from ansiblelint.rules import AnsibleLintRule, TransformMixin
+from ansiblelint.runner import get_matches
 from ansiblelint.skip_utils import get_rule_skips_from_line
 from ansiblelint.text import has_jinja
 from ansiblelint.utils import parse_yaml_from_file, template
@@ -482,10 +483,7 @@ def blacken(text: str) -> str:
     import pytest
 
     from ansiblelint.rules import RulesCollection  # pylint: disable=ungrouped-imports
-    from ansiblelint.runner import (
-        Runner,
-        _get_matches,
-    )
+    from ansiblelint.runner import Runner
 
     # pylint: disable=ungrouped-imports
     from ansiblelint.transformer import Transformer  # pylint: disable=ungrouped-imports
@@ -840,7 +838,7 @@ def test_jinja_transform(
         config_options.write_list = ["all"]
 
         config_options.lintables = [playbook]
-        runner_result = _get_matches(
+        runner_result = get_matches(
             rules=default_rules_collection,
             options=config_options,
         )

src/ansiblelint/rules/no_log_password.py

@@ -19,7 +19,7 @@
 from typing import TYPE_CHECKING
 
 from ansiblelint.rules import AnsibleLintRule, RulesCollection, TransformMixin
-from ansiblelint.runner import _get_matches
+from ansiblelint.runner import get_matches
 from ansiblelint.transformer import Transformer
 from ansiblelint.utils import Task, convert_to_boolean
 
@@ -336,7 +336,7 @@ def test_no_log_password_transform(
         rules.register(NoLogPasswordsRule())
 
         config_options.lintables = [playbook]
-        runner_result = _get_matches(rules=rules, options=config_options)
+        runner_result = get_matches(rules=rules, options=config_options)
         transformer = Transformer(result=runner_result, options=config_options)
         transformer.run()
 

src/ansiblelint/runner.py

@@ -79,11 +79,13 @@ def __init__(
         verbosity: int = 0,
         checked_files: set[Lintable] | None = None,
         project_dir: str | None = None,
+        _skip_ansible_syntax_check: bool = False,
     ) -> None:
         """Initialize a Runner instance."""
         self.rules = rules
         self.lintables: set[Lintable] = set()
         self.project_dir = os.path.abspath(project_dir) if project_dir else None
+        self.skip_ansible_syntax_check = _skip_ansible_syntax_check
 
         if skip_list is None:
             skip_list = []
@@ -234,32 +236,36 @@ def _run(self) -> list[MatchError]:
                 )
 
         # -- phase 1 : syntax check in parallel --
-        app = get_app(offline=True)
-
-        def worker(lintable: Lintable) -> list[MatchError]:
-            # pylint: disable=protected-access
-            return self._get_ansible_syntax_check_matches(
-                lintable=lintable,
-                app=app,
-            )
+        if not self.skip_ansible_syntax_check:
+            app = get_app(offline=True)
+
+            def worker(lintable: Lintable) -> list[MatchError]:
+                # pylint: disable=protected-access
+                return self._get_ansible_syntax_check_matches(
+                    lintable=lintable,
+                    app=app,
+                )
 
-        for lintable in self.lintables:
-            if lintable.kind not in ("playbook", "role") or lintable.stop_processing:
-                continue
-            files.append(lintable)
+            for lintable in self.lintables:
+                if (
+                    lintable.kind not in ("playbook", "role")
+                    or lintable.stop_processing
+                ):
+                    continue
+                files.append(lintable)
 
-        # avoid resource leak warning, https://github.com/python/cpython/issues/90549
-        # pylint: disable=unused-variable
-        global_resource = multiprocessing.Semaphore()  # noqa: F841
+            # avoid resource leak warning, https://github.com/python/cpython/issues/90549
+            # pylint: disable=unused-variable
+            global_resource = multiprocessing.Semaphore()  # noqa: F841
 
-        pool = multiprocessing.pool.ThreadPool(processes=threads())
-        return_list = pool.map(worker, files, chunksize=1)
-        pool.close()
-        pool.join()
-        for data in return_list:
-            matches.extend(data)
+            pool = multiprocessing.pool.ThreadPool(processes=threads())
+            return_list = pool.map(worker, files, chunksize=1)
+            pool.close()
+            pool.join()
+            for data in return_list:
+                matches.extend(data)
 
-        matches = self._filter_excluded_matches(matches)
+            matches = self._filter_excluded_matches(matches)
         # -- phase 2 ---
         if not matches:
             # do our processing only when ansible syntax check passed in order
@@ -585,7 +591,13 @@ def threads() -> int:
     return os_cpu_count
 
 
-def _get_matches(rules: RulesCollection, options: Options) -> LintResult:
+def get_matches(rules: RulesCollection, options: Options) -> LintResult:
+    """Get matches for given rules and options.
+
+    :param rules: Rules to use for linting.
+    :param options: Options to use for linting.
+    :returns: LintResult containing matches and checked files.
+    """
     lintables = ansiblelint.utils.get_lintables(opts=options, args=options.lintables)
 
     for rule in rules:
@@ -605,6 +617,7 @@ def _get_matches(rules: RulesCollection, options: Options) -> LintResult:
         verbosity=options.verbosity,
         checked_files=checked_files,
         project_dir=options.project_dir,
+        _skip_ansible_syntax_check=options._skip_ansible_syntax_check,  # noqa: SLF001
     )
     matches.extend(runner.run())
 

src/ansiblelint/transformer.py

@@ -44,8 +44,8 @@ def __init__(self, result: LintResult, options: Options):
         self.matches_per_file: dict[Lintable, list[MatchError]] = {
             file: [] for file in result.files
         }
-
-        for match in self.matches:
+        not_ignored = [match for match in self.matches if not match.ignored]
+        for match in not_ignored:
             try:
                 lintable = lintables[match.filename]
             except KeyError:

test/test_formatter_base.py

@@ -36,7 +36,6 @@ def test_base_formatter_when_base_dir(
 
     # Then
     assert isinstance(output_path, (str, Path))
-    # pylint: disable=protected-access
     assert base_formatter.base_dir is None or isinstance(
         base_formatter.base_dir,
         (str, Path),