Enable YAML 1.2 support for non-ansible files (#3809)

Author: ssbarnea

.config/dictionary.txt

@@ -208,6 +208,7 @@ matchtasks
 matchvar
 matchyaml
 maxdepth
+maxsplit
 minversion
 mkdir
 mkdocs

.github/workflows/tox.yml

@@ -70,7 +70,7 @@ jobs:
     env:
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 834
+      PYTEST_REQPASS: 845
     steps:
       - uses: actions/checkout@v4
         with:

src/ansiblelint/transformer.py

@@ -8,7 +8,11 @@
 
 from ansiblelint.file_utils import Lintable
 from ansiblelint.rules import AnsibleLintRule, TransformMixin
-from ansiblelint.yaml_utils import FormattedYAML, get_path_to_play, get_path_to_task
+from ansiblelint.yaml_utils import (
+    FormattedYAML,
+    get_path_to_play,
+    get_path_to_task,
+)
 
 if TYPE_CHECKING:
     from ansiblelint.config import Options
@@ -92,7 +96,12 @@ def run(self) -> None:
                 # We need a fresh YAML() instance for each load because ruamel.yaml
                 # stores intermediate state during load which could affect loading
                 # any other files. (Based on suggestion from ruamel.yaml author)
-                yaml = FormattedYAML()
+                yaml = FormattedYAML(
+                    # Ansible only uses YAML 1.1, but others files should use newer 1.2 (ruamel.yaml defaults to 1.2)
+                    version=(1, 1)
+                    if file.is_owned_by_ansible()
+                    else None,
+                )
 
                 ruamel_data = yaml.load(data)
                 if not isinstance(ruamel_data, (CommentedMap, CommentedSeq)):
@@ -108,11 +117,6 @@ def run(self) -> None:
             if self.write_set != {"none"}:
                 self._do_transforms(file, ruamel_data or data, file_is_yaml, matches)
 
-            if not file.is_owned_by_ansible():
-                # Do nothing until we enable support for YAML 1.2
-                # https://github.com/ansible/ansible-lint/issues/3811
-                continue
-
             if file_is_yaml:
                 _logger.debug("Dumping %s using YAML (%s)", file, yaml.version)
                 # noinspection PyUnboundLocalVariable

src/ansiblelint/yaml_utils.py

@@ -35,7 +35,7 @@
 if TYPE_CHECKING:
     # noinspection PyProtectedMember
     from ruamel.yaml.comments import LineCol
-    from ruamel.yaml.compat import StreamTextType
+    from ruamel.yaml.compat import StreamTextType, VersionType
     from ruamel.yaml.nodes import ScalarNode
     from ruamel.yaml.representer import RoundTripRepresenter
     from ruamel.yaml.tokens import CommentToken
@@ -44,6 +44,7 @@
 
 _logger = logging.getLogger(__name__)
 
+
 YAMLLINT_CONFIG = """
 extends: default
 rules:
@@ -750,13 +751,14 @@ def write_version_directive(self, version_text: Any) -> None:
 class FormattedYAML(YAML):
     """A YAML loader/dumper that handles ansible content better by default."""
 
-    def __init__(
+    def __init__(  # pylint: disable=too-many-arguments
         self,
         *,
         typ: str | None = None,
         pure: bool = False,
         output: Any = None,
         plug_ins: list[str] | None = None,
+        version: VersionType | None = None,
     ):
         """Return a configured ``ruamel.yaml.YAML`` instance.
 
@@ -816,10 +818,12 @@ def __init__(
               tasks:
                 - name: Task
         """
-        # Default to reading/dumping YAML 1.1 (ruamel.yaml defaults to 1.2)
-        self._yaml_version_default: tuple[int, int] = (1, 1)
-        self._yaml_version: str | tuple[int, int] = self._yaml_version_default
-
+        if version:
+            if isinstance(version, str):
+                x, y = version.split(".", maxsplit=1)
+                version = (int(x), int(y))
+            self._yaml_version_default: VersionType = version
+            self._yaml_version: VersionType = self._yaml_version_default
         super().__init__(typ=typ, pure=pure, output=output, plug_ins=plug_ins)
 
         # NB: We ignore some mypy issues because ruamel.yaml typehints are not great.
@@ -920,16 +924,18 @@ def _defaults_from_yamllint_config() -> dict[str, bool | int | str]:
 
         return cast(dict[str, Union[bool, int, str]], config)
 
-    @property  # type: ignore[override]
-    def version(self) -> str | tuple[int, int]:
+    @property
+    def version(self) -> VersionType | None:
         """Return the YAML version used to parse or dump.
 
         Ansible uses PyYAML which only supports YAML 1.1. ruamel.yaml defaults to 1.2.
         So, we have to make sure we dump yaml files using YAML 1.1.
         We can relax the version requirement once ansible uses a version of PyYAML
         that includes this PR: https://github.com/yaml/pyyaml/pull/555
         """
-        return self._yaml_version
+        if hasattr(self, "_yaml_version"):
+            return self._yaml_version
+        return None
 
     @version.setter
     def version(self, value: str | tuple[int, int] | None) -> None:
@@ -939,7 +945,11 @@ def version(self, value: str | tuple[int, int] | None) -> None:
         So, if a file does not include the directive, it sets this to None.
         But, None effectively resets the parsing version to YAML 1.2 (ruamel's default).
         """
-        self._yaml_version = value if value is not None else self._yaml_version_default
+        if value is not None:
+            self._yaml_version = value
+        elif hasattr(self, "_yaml_version_default"):
+            self._yaml_version = self._yaml_version_default
+        # We do nothing if the object did not have a previous default version defined
 
     def load(self, stream: Path | StreamTextType) -> Any:
         """Load YAML content from a string while avoiding known ruamel.yaml issues."""
@@ -977,7 +987,11 @@ def dumps(self, data: Any) -> str:
                 stream.write(preamble_comment)
             self.dump(data, stream)
             text = stream.getvalue()
-        return self._post_process_yaml(text)
+        strip_version_directive = hasattr(self, "_yaml_version_default")
+        return self._post_process_yaml(
+            text,
+            strip_version_directive=strip_version_directive,
+        )
 
     def _prevent_wrapping_flow_style(self, data: Any) -> None:
         if not isinstance(data, (CommentedMap, CommentedSeq)):
@@ -1065,7 +1079,7 @@ def _pre_process_yaml(self, text: str) -> tuple[str, str | None]:
         return text, "".join(preamble_comments) or None
 
     @staticmethod
-    def _post_process_yaml(text: str) -> str:
+    def _post_process_yaml(text: str, *, strip_version_directive: bool = False) -> str:
         """Handle known issues with ruamel.yaml dumping.
 
         Make sure there's only one newline at the end of the file.
@@ -1077,6 +1091,10 @@ def _post_process_yaml(text: str) -> str:
 
         Make sure null list items don't end in a space.
         """
+        # remove YAML directive
+        if strip_version_directive and text.startswith("%YAML"):
+            text = text.split("\n", 1)[1]
+
         text = text.rstrip("\n") + "\n"
 
         lines = text.splitlines(keepends=True)

test/test_yaml_utils.py

@@ -15,6 +15,7 @@
 
 if TYPE_CHECKING:
     from ruamel.yaml.comments import CommentedMap, CommentedSeq
+    from ruamel.yaml.compat import VersionType
     from ruamel.yaml.emitter import Emitter
 
 fixtures_dir = Path(__file__).parent / "fixtures"
@@ -202,8 +203,7 @@ def test_custom_ruamel_yaml_emitter(
     assert output == expected_output
 
 
-@pytest.fixture(name="yaml_formatting_fixtures")
-def fixture_yaml_formatting_fixtures(fixture_filename: str) -> tuple[str, str, str]:
+def load_yaml_formatting_fixtures(fixture_filename: str) -> tuple[str, str, str]:
     """Get the contents for the formatting fixture files.
 
     To regenerate these fixtures, please run ``pytest --regenerate-formatting-fixtures``.
@@ -220,26 +220,61 @@ def fixture_yaml_formatting_fixtures(fixture_filename: str) -> tuple[str, str, s
 
 
 @pytest.mark.parametrize(
-    "fixture_filename",
+    ("before", "after", "version"),
     (
-        pytest.param("fmt-1.yml", id="1"),
-        pytest.param("fmt-2.yml", id="2"),
-        pytest.param("fmt-3.yml", id="3"),
-        pytest.param("fmt-4.yml", id="4"),
-        pytest.param("fmt-5.yml", id="5"),
+        pytest.param("---\nfoo: bar\n", "---\nfoo: bar\n", None, id="1"),
+        # verify that 'on' is not translated to bool (1.2 behavior)
+        pytest.param("---\nfoo: on\n", "---\nfoo: on\n", None, id="2"),
+        # When version is manually mentioned by us, we expect to output without version directive
+        pytest.param("---\nfoo: on\n", "---\nfoo: on\n", (1, 2), id="3"),
+        pytest.param("---\nfoo: on\n", "---\nfoo: true\n", (1, 1), id="4"),
+        pytest.param("%YAML 1.1\n---\nfoo: on\n", "---\nfoo: true\n", (1, 1), id="5"),
+        # verify that in-line directive takes precedence but dumping strips if we mention a specific version
+        pytest.param("%YAML 1.1\n---\nfoo: on\n", "---\nfoo: true\n", (1, 2), id="6"),
+        # verify that version directive are kept if present
+        pytest.param("%YAML 1.1\n---\nfoo: on\n", "---\nfoo: true\n", None, id="7"),
+        pytest.param(
+            "%YAML 1.2\n---\nfoo: on\n",
+            "%YAML 1.2\n---\nfoo: on\n",
+            None,
+            id="8",
+        ),
+        pytest.param("---\nfoo: YES\n", "---\nfoo: true\n", (1, 1), id="9"),
+        pytest.param("---\nfoo: YES\n", "---\nfoo: YES\n", (1, 2), id="10"),
+        pytest.param("---\nfoo: YES\n", "---\nfoo: YES\n", None, id="11"),
+    ),
+)
+def test_fmt(before: str, after: str, version: VersionType) -> None:
+    """Tests behavior of formatter in regards to different YAML versions, specified or not."""
+    yaml = ansiblelint.yaml_utils.FormattedYAML(version=version)
+    data = yaml.load(before)
+    result = yaml.dumps(data)
+    assert result == after
+
+
+@pytest.mark.parametrize(
+    ("fixture_filename", "version"),
+    (
+        pytest.param("fmt-1.yml", (1, 1), id="1"),
+        pytest.param("fmt-2.yml", (1, 1), id="2"),
+        pytest.param("fmt-3.yml", (1, 1), id="3"),
+        pytest.param("fmt-4.yml", (1, 1), id="4"),
+        pytest.param("fmt-5.yml", (1, 1), id="5"),
     ),
 )
 def test_formatted_yaml_loader_dumper(
-    yaml_formatting_fixtures: tuple[str, str, str],
-    fixture_filename: str,  # noqa: ARG001
+    fixture_filename: str,
+    version: tuple[int, int],
 ) -> None:
     """Ensure that FormattedYAML loads/dumps formatting fixtures consistently."""
     # pylint: disable=unused-argument
-    before_content, prettier_content, after_content = yaml_formatting_fixtures
+    before_content, prettier_content, after_content = load_yaml_formatting_fixtures(
+        fixture_filename,
+    )
     assert before_content != prettier_content
     assert before_content != after_content
 
-    yaml = ansiblelint.yaml_utils.FormattedYAML()
+    yaml = ansiblelint.yaml_utils.FormattedYAML(version=version)
 
     data_before = yaml.load(before_content)
     dump_from_before = yaml.dumps(data_before)