Enable progressive mode (#1065)

Adds a new option --progressive that ease adoption of the linter as it will only report failure if it detects an increase number of violations since previous commit.

Author: ssbarnea

.gitignore

@@ -41,3 +41,6 @@ pip-wheel-metadata
 
 # mypy
 .mypy_cache
+
+# .cache is used by progressive mode
+.cache

README.rst

@@ -94,6 +94,8 @@ The following is the output from ``ansible-lint --help``, providing an overview
       -q                    quieter, although not silent output
       -p                    parseable output in the format of pep8
       --parseable-severity  parseable output including severity of rule
+      --progressive         Return success if it detects a reduction in number of violations compared with
+                            previous git commit. This feature works only on git repository clones.
       -r RULESDIR           Specify custom rule directories. Add -R to keep using embedded rules from
                             /usr/local/lib/python3.8/site-packages/ansiblelint/rules
       -R                    Keep default rules when using -r
@@ -111,6 +113,23 @@ The following is the output from ``ansible-lint --help``, providing an overview
       -c CONFIG_FILE        Specify configuration file to use. Defaults to ".ansible-lint"
       --version             show program's version number and exit
 
+Progressive mode
+----------------
+
+In order to ease tool adoption, git users can enable the progressive mode using
+``--progressive`` option. This makes the linter return a success even if
+some failures are found, as long the total number of violations did not
+increase since the previous commit.
+
+As expected, this mode makes the linter run twice if it finds any violations.
+The second run is performed against a temporary git working copy that contains
+the previous commit. All the violations that were already present are removed
+from the list and the final result is displayed.
+
+The most notable benefit introduced by this mode it does not prevent merging
+new code while allowing developer to address historical violation at his own
+speed.
+
 CI/CD
 -----
 

lib/ansiblelint/__main__.py

@@ -24,13 +24,16 @@
 import logging
 import os
 import pathlib
+import subprocess
 import sys
-from typing import TYPE_CHECKING, List, Set, Type
+from contextlib import contextmanager
+from typing import TYPE_CHECKING, Any, List, Set, Type, Union
 
 from rich.markdown import Markdown
 
 from ansiblelint import cli, formatters
 from ansiblelint.color import console, console_stderr
+from ansiblelint.file_utils import cwd
 from ansiblelint.generate_docs import rules_as_rich, rules_as_rst
 from ansiblelint.rules import RulesCollection
 from ansiblelint.runner import Runner
@@ -96,8 +99,9 @@ def report_outcome(matches: List["MatchError"], options) -> int:
 # .ansible-lint
 warn_list:  # or 'skip_list' to silence them completely
 """
+    matches_unignored = [match for match in matches if not match.ignored]
 
-    matched_rules = {match.rule.id: match.rule for match in matches}
+    matched_rules = {match.rule.id: match.rule for match in matches_unignored}
     for id in sorted(matched_rules.keys()):
         if {id, *matched_rules[id].tags}.isdisjoint(options.warn_list):
             msg += f"  - '{id}'  # {matched_rules[id].shortdesc}\n"
@@ -151,6 +155,76 @@ def main() -> int:
         skip.update(str(s).split(','))
     options.skip_list = frozenset(skip)
 
+    matches = _get_matches(rules, options)
+
+    # Assure we do not print duplicates and the order is consistent
+    matches = sorted(set(matches))
+
+    mark_as_success = False
+    if matches and options.progressive:
+        _logger.info(
+            "Matches found, running again on previous revision in order to detect regressions")
+        with _previous_revision():
+            old_matches = _get_matches(rules, options)
+            # remove old matches from current list
+            matches_delta = list(set(matches) - set(old_matches))
+            if len(matches_delta) == 0:
+                _logger.warning(
+                    "Total violations not increased since previous "
+                    "commit, will mark result as success. (%s -> %s)",
+                    len(old_matches), len(matches_delta))
+                mark_as_success = True
+
+            ignored = 0
+            for match in matches:
+                # if match is not new, mark is as ignored
+                if match not in matches_delta:
+                    match.ignored = True
+                    ignored += 1
+            if ignored:
+                _logger.warning(
+                    "Marked %s previously known violation(s) as ignored due to"
+                    " progressive mode.", ignored)
+
+    _render_matches(matches, options, formatter, cwd)
+
+    if matches and not mark_as_success:
+        return report_outcome(matches, options=options)
+    else:
+        return 0
+
+
+def _render_matches(
+        matches: List,
+        options: "Namespace",
+        formatter: Any,
+        cwd: Union[str, pathlib.Path]):
+
+    ignored_matches = [match for match in matches if match.ignored]
+    fatal_matches = [match for match in matches if not match.ignored]
+    # Displayed ignored matches first
+    if ignored_matches:
+        _logger.warning(
+            "Listing %s violation(s) marked as ignored, likely already known",
+            len(ignored_matches))
+        for match in ignored_matches:
+            if match.ignored:
+                print(formatter.format(match, options.colored))
+    if fatal_matches:
+        _logger.warning("Listing %s violation(s) that are fatal", len(fatal_matches))
+        for match in fatal_matches:
+            if not match.ignored:
+                print(formatter.format(match, options.colored))
+
+    # If run under GitHub Actions we also want to emit output recognized by it.
+    if os.getenv('GITHUB_ACTIONS') == 'true' and os.getenv('GITHUB_WORKFLOW'):
+        formatter = formatters.AnnotationsFormatter(cwd, True)
+        for match in matches:
+            print(formatter.format(match))
+
+
+def _get_matches(rules: RulesCollection, options: "Namespace") -> list:
+
     if not options.playbook:
         # no args triggers auto-detection mode
         playbooks = get_playbooks_and_roles(options=options)
@@ -164,23 +238,26 @@ def main() -> int:
                         options.skip_list, options.exclude_paths,
                         options.verbosity, checked_files)
         matches.extend(runner.run())
-
-    # Assure we do not print duplicates and the order is consistent
-    matches = sorted(set(matches))
-
-    for match in matches:
-        print(formatter.format(match, options.colored))
-
-    # If run under GitHub Actions we also want to emit output recognized by it.
-    if os.getenv('GITHUB_ACTIONS') == 'true' and os.getenv('GITHUB_WORKFLOW'):
-        formatter = formatters.AnnotationsFormatter(cwd, True)
-        for match in matches:
-            print(formatter.format(match))
-
-    if matches:
-        return report_outcome(matches, options=options)
-    else:
-        return 0
+    return matches
+
+
+@contextmanager
+def _previous_revision():
+    """Create or update a temporary workdir containing the previous revision."""
+    worktree_dir = ".cache/old-rev"
+    revision = subprocess.run(
+        ["git", "rev-parse", "HEAD^1"],
+        check=True,
+        universal_newlines=True,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.DEVNULL,
+        ).stdout
+    p = pathlib.Path(worktree_dir)
+    p.mkdir(parents=True, exist_ok=True)
+    os.system(f"git worktree add -f {worktree_dir} 2>/dev/null")
+    with cwd(worktree_dir):
+        os.system(f"git checkout {revision}")
+        yield
 
 
 if __name__ == "__main__":

lib/ansiblelint/cli.py

@@ -111,6 +111,12 @@ def get_cli_parser() -> argparse.ArgumentParser:
                         default=False,
                         action='store_true',
                         help="parseable output including severity of rule")
+    parser.add_argument('--progressive', dest='progressive',
+                        default=False,
+                        action='store_true',
+                        help="Return success if it detects a reduction in number"
+                        " of violations compared with previous git commit. This "
+                        "feature works only in git repositories.")
     parser.add_argument('-r', action=AbspathArgAction, dest='rulesdir',
                         default=[], type=Path,
                         help="Specify custom rule directories. Add -R "

lib/ansiblelint/errors.py

@@ -41,6 +41,7 @@ def __init__(
         self.details = details
         self.filename = normpath(filename) if filename else None
         self.rule = rule
+        self.ignored = False  # If set it will be displayed but not counted as failure
 
     def __repr__(self):
         """Return a MatchError instance representation."""

lib/ansiblelint/file_utils.py

@@ -1,5 +1,6 @@
 """Utility functions related to file operations."""
 import os
+from contextlib import contextmanager
 
 
 def normpath(path) -> str:
@@ -11,3 +12,14 @@ def normpath(path) -> str:
     """
     # convertion to string in order to allow receiving non string objects
     return os.path.relpath(str(path))
+
+
+@contextmanager
+def cwd(path):
+    """Context manager for temporary changing current working directory."""
+    old_pwd = os.getcwd()
+    os.chdir(path)
+    try:
+        yield
+    finally:
+        os.chdir(old_pwd)