Add support for reformatting YAML files

docs/conf.py

@@ -301,6 +301,10 @@
     ("py:class", "ruamel.yaml.representer.RoundTripRepresenter"),
     ("py:class", "ruamel.yaml.scalarint.ScalarInt"),
     ("py:class", "ruamel.yaml.tokens.CommentToken"),
+    ("py:class", "ruamel.yaml.comments.CommentedMap"),
+    ("py:class", "ruamel.yaml.comments.CommentedSeq"),
+    ("py:class", "CommentedMap"),
+    ("py:class", "CommentedSeq"),
     ("py:obj", "Any"),
     ("py:obj", "ansiblelint.formatters.T"),
 ]

examples/playbooks/become.transformed.yml

@@ -0,0 +1,15 @@
+---
+- hosts: all
+
+  tasks:
+    - name: clone content repository
+      ansible.builtin.git:
+        repo: "{{ archive_services_repo_url }}"
+        dest: /home/www
+        accept_hostkey: true
+        version: master
+        update: false
+      become: true
+      become_user: nobody
+      notify:
+        - restart apache2

examples/playbooks/contains_secrets.transformed.yml

@@ -0,0 +1,16 @@
+---
+- hosts: localhost
+  vars:
+    plain: hello123
+    # spell-checker: disable-next-line
+    # just 'hello123' encrypted with 'letmein' for test purposes
+    secret: !vault |
+      $ANSIBLE_VAULT;1.1;AES256
+      63346434613163653866303630313238626164313961613935373137323639636333393338386232
+      3735313061316666343839343665383036623237353263310a623639336530383433343833653138
+      30393032393534316164613834393864616566646164363830316664623636643731383164376163
+      3736653037356435310a303533383533353739323834343637366438633766666163656330343631
+      3066
+  tasks:
+    - name: just a debug task
+      ansible.builtin.debug: msg="hello world"

examples/playbooks/nomatchestest.transformed.yml

@@ -0,0 +1,9 @@
+---
+- hosts: whatever
+
+  tasks:
+    - name: hello world
+      action: debug msg="Hello!" # noqa: fqcn-builtins
+
+    - name: this should be fine too
+      action: file state=touch dest=./wherever mode=0600 # noqa: fqcn-builtins

examples/playbooks/unicode.transformed.yml

@@ -0,0 +1,6 @@
+---
+- hosts: all
+  tasks:
+    # cspell:disable-next-line
+    - name: тест
+      ansible.builtin.command: uname

src/ansiblelint/app.py

@@ -9,9 +9,10 @@
 
 if TYPE_CHECKING:
     from argparse import Namespace
-    from typing import Dict  # pylint: disable=ungrouped-imports
+    from typing import Dict, Set  # pylint: disable=ungrouped-imports
 
     from ansiblelint._internal.rules import BaseRule
+    from ansiblelint.file_utils import Lintable
     from ansiblelint.runner import LintResult
 
 
@@ -78,6 +79,13 @@ def count_results(self, matches: List[MatchError]) -> Tuple[int, int]:
                 warnings += 1
         return failures, warnings
 
+    @staticmethod
+    def count_lintables(files: "Set[Lintable]") -> Tuple[int, int]:
+        """Count total and modified files."""
+        files_count = len(files)
+        changed_files_count = len([file for file in files if file.updated])
+        return files_count, changed_files_count
+
     @staticmethod
     def _get_matched_skippable_rules(
         matches: List[MatchError],
@@ -101,6 +109,7 @@ def report_outcome(
         msg = ""
 
         failures, warnings = self.count_results(result.matches)
+        files_count, changed_files_count = self.count_lintables(result.files)
 
         matched_rules = self._get_matched_skippable_rules(result.matches)
 
@@ -140,9 +149,11 @@ def report_outcome(
 
         if result.matches and not self.options.quiet:
             console_stderr.print(render_yaml(msg))
+            if changed_files_count:
+                console_stderr.print(f"Modified {changed_files_count} files.")
             console_stderr.print(
                 f"Finished with {failures} failure(s), {warnings} warning(s) "
-                f"on {len(result.files)} files."
+                f"on {files_count} files."
             )
 
         if mark_as_success or not failures:

src/ansiblelint/transformer.py

@@ -1,10 +1,13 @@
 """Transformer implementation."""
 import logging
-from typing import List, Set
+from typing import Dict, List, Set, Union
+
+from ruamel.yaml.comments import CommentedMap, CommentedSeq
 
 from ansiblelint.errors import MatchError
 from ansiblelint.file_utils import Lintable
 from ansiblelint.runner import LintResult
+from ansiblelint.yaml_utils import FormattedYAML
 
 __all__ = ["Transformer"]
 
@@ -17,12 +20,48 @@ class Transformer:
     The Transformer is similar to the ``ansiblelint.runner.Runner`` which manages
     running each of the rules. We only expect there to be one ``Transformer`` instance
     which should be instantiated from the main entrypoint function.
+
+    In the future, the transformer will be responsible for running transforms for each
+    of the rule matches. For now, it just reads/writes YAML files which is a
+    pre-requisite for the planned rule-specific transforms.
     """
 
     def __init__(self, result: LintResult):
         """Initialize a Transformer instance."""
         self.matches: List[MatchError] = result.matches
         self.files: Set[Lintable] = result.files
 
+        file: Lintable
+        lintables: Dict[str, Lintable] = {file.filename: file for file in result.files}
+        self.matches_per_file: Dict[Lintable, List[MatchError]] = {
+            file: [] for file in result.files
+        }
+
+        for match in self.matches:
+            try:
+                lintable = lintables[match.filename]
+            except KeyError:
+                # we shouldn't get here, but this is easy to recover from so do that.
+                lintable = Lintable(match.filename)
+                self.matches_per_file[lintable] = []
+            self.matches_per_file[lintable].append(match)
+
     def run(self) -> None:
         """For each file, read it, execute transforms on it, then write it."""
+        yaml = FormattedYAML()
+        for file, _ in self.matches_per_file.items():
+            # str() convinces mypy that "text/yaml" is a valid Literal.
+            # Otherwise, it thinks base_kind is one of playbook, meta, tasks, ...
+            file_is_yaml = str(file.base_kind) == "text/yaml"
+
+            try:
+                data: str = file.content
+            except (UnicodeDecodeError, IsADirectoryError):
+                # we hit a binary file (eg a jar or tar.gz) or a directory
+                data = ""
+                file_is_yaml = False
+
+            if file_is_yaml:
+                ruamel_data: Union[CommentedMap, CommentedSeq] = yaml.loads(data)
+                file.content = yaml.dumps(ruamel_data)
+                file.write()

src/ansiblelint/yaml_utils.py

@@ -374,8 +374,8 @@ def write_comment(self, comment: CommentToken, pre: bool = False) -> None:
             value = self._re_repeat_blank_lines.sub("\n\n", value)
         comment.value = value
 
-        # make sure that the comment only has one space before it.
-        if comment.column > self.column + 1:
+        # make sure that the eol comment only has one space before it.
+        if comment.column > self.column + 1 and not self.whitespace:
             comment.column = self.column + 1
         return super().write_comment(comment, pre)
 

test/test_transformer.py

@@ -0,0 +1,90 @@
+"""Tests for Transformer."""
+
+from argparse import Namespace
+from typing import Iterator, Tuple
+
+import py
+import pytest
+
+from ansiblelint.rules import RulesCollection
+
+# noinspection PyProtectedMember
+from ansiblelint.runner import LintResult, _get_matches
+from ansiblelint.transformer import Transformer
+
+
+@pytest.fixture
+def copy_examples_dir(
+    tmpdir: py.path.local, config_options: Namespace
+) -> Iterator[Tuple[py.path.local, py.path.local]]:
+    """Fixture that copies the examples/ dir into a tmpdir."""
+    examples_dir = py.path.local("examples")
+    examples_dir.copy(tmpdir / "examples")
+    old_cwd = tmpdir.chdir()
+    config_options.cwd = tmpdir
+    yield old_cwd, tmpdir
+    old_cwd.chdir()
+
+
+@pytest.fixture
+def runner_result(
+    config_options: Namespace,
+    default_rules_collection: RulesCollection,
+    playbook: str,
+) -> LintResult:
+    """Fixture that runs the Runner to populate a LintResult for a given file."""
+    config_options.lintables = [playbook]
+    result = _get_matches(rules=default_rules_collection, options=config_options)
+    return result
+
+
+@pytest.mark.parametrize(
+    ("playbook", "matches_count", "transformed"),
+    (
+        # reuse TestRunner::test_runner test cases to ensure transformer does not mangle matches
+        pytest.param(
+            "examples/playbooks/nomatchestest.yml", 0, False, id="nomatchestest"
+        ),
+        pytest.param("examples/playbooks/unicode.yml", 1, False, id="unicode"),
+        pytest.param(
+            "examples/playbooks/lots_of_warnings.yml", 992, False, id="lots_of_warnings"
+        ),
+        pytest.param("examples/playbooks/become.yml", 0, True, id="become"),
+        pytest.param(
+            "examples/playbooks/contains_secrets.yml", 0, True, id="contains_secrets"
+        ),
+    ),
+)
+def test_transformer(
+    copy_examples_dir: Tuple[py.path.local, py.path.local],
+    playbook: str,
+    runner_result: LintResult,
+    transformed: bool,
+    matches_count: int,
+) -> None:
+    """
+    Test that transformer can go through any corner cases.
+
+    Based on TestRunner::test_runner
+    """
+    transformer = Transformer(result=runner_result)
+    transformer.run()
+
+    matches = runner_result.matches
+    assert len(matches) == matches_count
+
+    orig_dir, tmp_dir = copy_examples_dir
+    orig_playbook = orig_dir / playbook
+    expected_playbook = orig_dir / playbook.replace(".yml", ".transformed.yml")
+    transformed_playbook = tmp_dir / playbook
+
+    orig_playbook_content = orig_playbook.read()
+    expected_playbook_content = expected_playbook.read()
+    transformed_playbook_content = transformed_playbook.read()
+
+    if transformed:
+        assert orig_playbook_content != transformed_playbook_content
+    else:
+        assert orig_playbook_content == transformed_playbook_content
+
+    assert transformed_playbook_content == expected_playbook_content