Skip to content

Commit

Permalink
Fix exception when using reformatting on scalars
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea committed May 10, 2023
1 parent 3473542 commit 92d2f33
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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')"
Expand Down
12 changes: 8 additions & 4 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions examples/playbooks/vars/empty.transformed.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

---
# broken vars files due to spacing at the start of the file.
3 changes: 3 additions & 0 deletions examples/playbooks/vars/empty.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@

---
# broken vars files due to spacing at the start of the file.
12 changes: 8 additions & 4 deletions src/ansiblelint/file_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 4 additions & 0 deletions src/ansiblelint/transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"}:
Expand Down
14 changes: 12 additions & 2 deletions src/ansiblelint/yaml_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions test/test_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
),
Expand Down

0 comments on commit 92d2f33

Please sign in to comment.