diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index 2bb376e9b8..e06c5e8320 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -59,7 +59,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: 796 + PYTEST_REQPASS: 797 steps: - name: Activate WSL1 if: "contains(matrix.shell, 'wsl')" diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 84fc1e3667..374e1d23f8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -21,6 +21,8 @@ exclude: > examples/broken/encoding.j2| examples/broken/yaml-with-tabs/invalid-due-tabs.yaml| examples/playbooks/collections/.*| + examples/playbooks/vars/empty.transformed.yml| + examples/playbooks/vars/empty.yml| src/ansiblelint/schemas/rulebook.json| test/schemas/data/licenses.json| test/schemas/negative_test| @@ -40,17 +42,19 @@ repos: (?x)^( .*\.md$| examples/other/some.j2.yaml| + examples/playbooks/collections/.*| examples/playbooks/example.yml| examples/playbooks/multiline-brackets.*| examples/playbooks/templates/not-valid.yaml| - examples/playbooks/with-umlaut-.*| + examples/playbooks/vars/empty.transformed.yml| + examples/playbooks/vars/empty.yml| examples/playbooks/with-skip-tag-id.yml| + examples/playbooks/with-umlaut-.*| examples/yamllint/.*| - examples/playbooks/collections/.*| + src/ansiblelint/schemas/(molecule|tasks|playbook|rulebook).json| test/fixtures/formatting-before/.*| - test/schemas/data/.*| test/schemas/(negative_test|test)/.*\.md| - src/ansiblelint/schemas/(molecule|tasks|playbook|rulebook).json| + test/schemas/data/.*| src/ansiblelint/schemas/ansible-navigator-config.json )$ always_run: true diff --git a/examples/playbooks/vars/empty.transformed.yml b/examples/playbooks/vars/empty.transformed.yml new file mode 100644 index 0000000000..10abfa5486 --- /dev/null +++ b/examples/playbooks/vars/empty.transformed.yml @@ -0,0 +1,3 @@ + +--- +# broken vars files due to spacing at the start of the file. diff --git a/examples/playbooks/vars/empty.yml b/examples/playbooks/vars/empty.yml new file mode 100644 index 0000000000..10abfa5486 --- /dev/null +++ b/examples/playbooks/vars/empty.yml @@ -0,0 +1,3 @@ + +--- +# broken vars files due to spacing at the start of the file. diff --git a/src/ansiblelint/file_utils.py b/src/ansiblelint/file_utils.py index 973de39b0e..cf1ef9c295 100644 --- a/src/ansiblelint/file_utils.py +++ b/src/ansiblelint/file_utils.py @@ -248,10 +248,14 @@ def __init__( def _guess_kind(self) -> None: if self.kind == "yaml": - if isinstance(self.data, list) and ( - "hosts" in self.data[0] - or "import_playbook" in self.data[0] - or "ansible.builtin.import_playbook" in self.data[0] + if ( + isinstance(self.data, list) + and len(self.data) > 0 + and ( + "hosts" in self.data[0] + or "import_playbook" in self.data[0] + or "ansible.builtin.import_playbook" in self.data[0] + ) ): if "rules" not in self.data[0]: self.kind = "playbook" diff --git a/src/ansiblelint/transformer.py b/src/ansiblelint/transformer.py index a87b0c6db5..3716ef9595 100644 --- a/src/ansiblelint/transformer.py +++ b/src/ansiblelint/transformer.py @@ -100,6 +100,10 @@ def run(self) -> None: # This is an empty vars file or similar which loads as None. # It is not safe to write this file or data-loss is likely. # Only maps and sequences can preserve comments. Skip it. + _logger.debug( + "Ignored reformatting %s because current implementation in ruamel.yaml would drop comments. See https://sourceforge.net/p/ruamel-yaml/tickets/460/", + file, + ) continue if self.write_set != {"none"}: diff --git a/src/ansiblelint/yaml_utils.py b/src/ansiblelint/yaml_utils.py index d4db18b2f4..dd3a0f648a 100644 --- a/src/ansiblelint/yaml_utils.py +++ b/src/ansiblelint/yaml_utils.py @@ -924,10 +924,20 @@ def loads(self, stream: str) -> Any: if not isinstance(stream, str): msg = f"expected a str but got {type(stream)}" raise NotImplementedError(msg) + # As ruamel drops comments for any document that is not a mapping or sequence, + # we need to avoid using it to reformat those documents. + # https://sourceforge.net/p/ruamel-yaml/tickets/460/ + text, preamble_comment = self._pre_process_yaml(stream) data = self.load(stream=text) - if preamble_comment is not None: - data.preamble_comment = preamble_comment + if preamble_comment is not None and isinstance( + data, + (CommentedMap, CommentedSeq), + ): + data.preamble_comment = preamble_comment # type: ignore[union-attr] + # Because data can validly also be None for empty documents, we cannot + # really annotate the return type here, so we need to remember to + # never save None or scalar data types when reformatting. return data def dumps(self, data: Any) -> str: diff --git a/test/test_transformer.py b/test/test_transformer.py index 9dd9a76f81..78dd121059 100644 --- a/test/test_transformer.py +++ b/test/test_transformer.py @@ -81,6 +81,7 @@ def fixture_runner_result( id="empty_vars", ), pytest.param("examples/playbooks/vars/strings.yml", 0, True, id="strings"), + pytest.param("examples/playbooks/vars/empty.yml", 1, False, id="empty"), pytest.param("examples/playbooks/name-case.yml", 1, True, id="name_case"), pytest.param("examples/playbooks/fqcn.yml", 3, True, id="fqcn"), ),