From 231a72061cf6a7a81deea34bde59bf80939420ab Mon Sep 17 00:00:00 2001 From: Ajinkya Udgirkar Date: Thu, 5 Oct 2023 13:48:57 +0530 Subject: [PATCH 1/5] Add play level autofix for key-order rule --- .github/workflows/tox.yml | 2 +- .../transform-key-order-play.transformed.yml | 10 +++++ .../playbooks/transform-key-order-play.yml | 10 +++++ src/ansiblelint/rules/key_order.py | 37 ++++++++++++++++--- src/ansiblelint/schemas/__store__.json | 2 +- test/test_transformer.py | 6 +++ 6 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 examples/playbooks/transform-key-order-play.transformed.yml create mode 100644 examples/playbooks/transform-key-order-play.yml diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index 74a036f202..fc64dd5b07 100644 --- a/.github/workflows/tox.yml +++ b/.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: 831 + PYTEST_REQPASS: 832 steps: - uses: actions/checkout@v4 with: diff --git a/examples/playbooks/transform-key-order-play.transformed.yml b/examples/playbooks/transform-key-order-play.transformed.yml new file mode 100644 index 0000000000..030364d8a7 --- /dev/null +++ b/examples/playbooks/transform-key-order-play.transformed.yml @@ -0,0 +1,10 @@ +--- +- name: This is a playbook # <-- name key should be the first one + hosts: localhost + tasks: + - name: A block + when: true + block: + - name: Display a message + ansible.builtin.debug: + msg: Hello world! diff --git a/examples/playbooks/transform-key-order-play.yml b/examples/playbooks/transform-key-order-play.yml new file mode 100644 index 0000000000..e61920dca6 --- /dev/null +++ b/examples/playbooks/transform-key-order-play.yml @@ -0,0 +1,10 @@ +--- +- hosts: localhost + name: This is a playbook # <-- name key should be the first one + tasks: + - name: A block + when: true + block: + - name: Display a message + ansible.builtin.debug: + msg: Hello world! diff --git a/src/ansiblelint/rules/key_order.py b/src/ansiblelint/rules/key_order.py index 16b058914c..4c7aef553e 100644 --- a/src/ansiblelint/rules/key_order.py +++ b/src/ansiblelint/rules/key_order.py @@ -4,15 +4,15 @@ import functools import sys from dataclasses import dataclass -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any -from ansiblelint.errors import RuleMatchTransformMeta +from ansiblelint.constants import ANNOTATION_KEYS, LINE_NUMBER_KEY +from ansiblelint.errors import MatchError, RuleMatchTransformMeta from ansiblelint.rules import AnsibleLintRule, TransformMixin if TYPE_CHECKING: from ruamel.yaml.comments import CommentedMap, CommentedSeq - from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.utils import Task @@ -77,6 +77,25 @@ class KeyOrderRule(AnsibleLintRule, TransformMixin): "key-order[task]": "You can improve the task key order", } + def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: + """Return matches found for a specific play (entry in playbook).""" + result: list[MatchError] = [] + if file.kind != "playbook": + return result + keys = [str(key) for key, val in data.items() if key not in ANNOTATION_KEYS] + sorted_keys = sorted(keys, key=functools.cmp_to_key(task_property_sorter)) + if keys != sorted_keys: + result.append( + self.create_matcherror( + f"You can improve the play key order to: {', '.join(sorted_keys)}", + filename=file, + tag=f"{self.id}[play]", + lineno=data[LINE_NUMBER_KEY], + transform_meta=KeyOrderTMeta(fixed=tuple(sorted_keys)), + ), + ) + return result + def matchtask( self, task: Task, @@ -103,9 +122,15 @@ def transform( lintable: Lintable, data: CommentedMap | CommentedSeq | str, ) -> None: - if match.tag == "key-order[task]": - if not isinstance(match.transform_meta, KeyOrderTMeta): - return + if not isinstance(match.transform_meta, KeyOrderTMeta): + return + + if match.tag == f"{self.id}[play]": + play = self.seek(match.yaml_path, data) + for key in match.transform_meta.fixed: + play[key] = play.pop(key) + match.fixed = True + if match.tag == f"{self.id}[task]": task = self.seek(match.yaml_path, data) for key in match.transform_meta.fixed: task[key] = task.pop(key) diff --git a/src/ansiblelint/schemas/__store__.json b/src/ansiblelint/schemas/__store__.json index 7d7a01dd0a..d2d0c84585 100644 --- a/src/ansiblelint/schemas/__store__.json +++ b/src/ansiblelint/schemas/__store__.json @@ -24,7 +24,7 @@ "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/inventory.json" }, "meta": { - "etag": "d1f66f20b45ecb814d9be900f01451d8035059b312de4a19edc64e29811a7f39", + "etag": "80d83d5c61044aa67496ea22c74aef08b0216c20e318400d3c939927a2761d51", "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/meta.json" }, "meta-runtime": { diff --git a/test/test_transformer.py b/test/test_transformer.py index f06302484f..fbaf6ae0cb 100644 --- a/test/test_transformer.py +++ b/test/test_transformer.py @@ -125,6 +125,12 @@ def fixture_runner_result( True, id="partial_become", ), + pytest.param( + "examples/playbooks/transform-key-order-play.yml", + 1, + True, + id="key_order_play_transform", + ), ), ) def test_transformer( # pylint: disable=too-many-arguments, too-many-locals From e8f53586117bbc5ddb1b55fb50803501749a2cdd Mon Sep 17 00:00:00 2001 From: Ajinkya Udgirkar Date: Mon, 9 Oct 2023 15:46:07 +0530 Subject: [PATCH 2/5] Fix mac os ci test failures --- test/test_runner.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_runner.py b/test/test_runner.py index 2c0df47b90..3eda1d8453 100644 --- a/test/test_runner.py +++ b/test/test_runner.py @@ -22,6 +22,7 @@ from pathlib import Path from typing import TYPE_CHECKING, Any +from ansiblelint.config import options import pytest @@ -92,6 +93,7 @@ def test_runner_exclude_globs( exclude_path: str, ) -> None: """Test that globs work.""" + options.lintables = [] runner = Runner( "examples/playbooks", rules=default_rules_collection, From af5772fbfc06b6d6cfd6c49dc6b7494b06bd99ac Mon Sep 17 00:00:00 2001 From: Ajinkya Udgirkar Date: Mon, 9 Oct 2023 16:01:16 +0530 Subject: [PATCH 3/5] Fix mac os ci test failures --- test/test_runner.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_runner.py b/test/test_runner.py index 3eda1d8453..1132c3d6ee 100644 --- a/test/test_runner.py +++ b/test/test_runner.py @@ -22,11 +22,11 @@ from pathlib import Path from typing import TYPE_CHECKING, Any -from ansiblelint.config import options import pytest from ansiblelint import formatters +from ansiblelint.config import options from ansiblelint.file_utils import Lintable from ansiblelint.runner import Runner From 226ce1938f5e697cf70441e69a7de2be68c3a035 Mon Sep 17 00:00:00 2001 From: Ajinkya Udgirkar Date: Mon, 9 Oct 2023 16:03:25 +0530 Subject: [PATCH 4/5] Revert "Add play level autofix for key-order rule" This reverts commit 231a72061cf6a7a81deea34bde59bf80939420ab. --- .github/workflows/tox.yml | 2 +- .../transform-key-order-play.transformed.yml | 10 ----- .../playbooks/transform-key-order-play.yml | 10 ----- src/ansiblelint/rules/key_order.py | 37 +++---------------- src/ansiblelint/schemas/__store__.json | 2 +- test/test_transformer.py | 6 --- 6 files changed, 8 insertions(+), 59 deletions(-) delete mode 100644 examples/playbooks/transform-key-order-play.transformed.yml delete mode 100644 examples/playbooks/transform-key-order-play.yml diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index 45edc4dc32..1a3903a6b0 100644 --- a/.github/workflows/tox.yml +++ b/.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: 832 + PYTEST_REQPASS: 831 steps: - uses: actions/checkout@v4 with: diff --git a/examples/playbooks/transform-key-order-play.transformed.yml b/examples/playbooks/transform-key-order-play.transformed.yml deleted file mode 100644 index 030364d8a7..0000000000 --- a/examples/playbooks/transform-key-order-play.transformed.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -- name: This is a playbook # <-- name key should be the first one - hosts: localhost - tasks: - - name: A block - when: true - block: - - name: Display a message - ansible.builtin.debug: - msg: Hello world! diff --git a/examples/playbooks/transform-key-order-play.yml b/examples/playbooks/transform-key-order-play.yml deleted file mode 100644 index e61920dca6..0000000000 --- a/examples/playbooks/transform-key-order-play.yml +++ /dev/null @@ -1,10 +0,0 @@ ---- -- hosts: localhost - name: This is a playbook # <-- name key should be the first one - tasks: - - name: A block - when: true - block: - - name: Display a message - ansible.builtin.debug: - msg: Hello world! diff --git a/src/ansiblelint/rules/key_order.py b/src/ansiblelint/rules/key_order.py index 2cef09df37..06b8b53d3c 100644 --- a/src/ansiblelint/rules/key_order.py +++ b/src/ansiblelint/rules/key_order.py @@ -4,15 +4,15 @@ import functools import sys from dataclasses import dataclass -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING -from ansiblelint.constants import ANNOTATION_KEYS, LINE_NUMBER_KEY -from ansiblelint.errors import MatchError, RuleMatchTransformMeta +from ansiblelint.errors import RuleMatchTransformMeta from ansiblelint.rules import AnsibleLintRule, TransformMixin if TYPE_CHECKING: from ruamel.yaml.comments import CommentedMap, CommentedSeq + from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.utils import Task @@ -77,25 +77,6 @@ class KeyOrderRule(AnsibleLintRule, TransformMixin): "key-order[task]": "You can improve the task key order", } - def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: - """Return matches found for a specific play (entry in playbook).""" - result: list[MatchError] = [] - if file.kind != "playbook": - return result - keys = [str(key) for key, val in data.items() if key not in ANNOTATION_KEYS] - sorted_keys = sorted(keys, key=functools.cmp_to_key(task_property_sorter)) - if keys != sorted_keys: - result.append( - self.create_matcherror( - f"You can improve the play key order to: {', '.join(sorted_keys)}", - filename=file, - tag=f"{self.id}[play]", - lineno=data[LINE_NUMBER_KEY], - transform_meta=KeyOrderTMeta(fixed=tuple(sorted_keys)), - ), - ) - return result - def matchtask( self, task: Task, @@ -122,15 +103,9 @@ def transform( lintable: Lintable, data: CommentedMap | CommentedSeq | str, ) -> None: - if not isinstance(match.transform_meta, KeyOrderTMeta): - return - - if match.tag == f"{self.id}[play]": - play = self.seek(match.yaml_path, data) - for key in match.transform_meta.fixed: - play[key] = play.pop(key) - match.fixed = True - if match.tag == f"{self.id}[task]": + if match.tag == "key-order[task]": + if not isinstance(match.transform_meta, KeyOrderTMeta): + return task = self.seek(match.yaml_path, data) for key in match.transform_meta.fixed: task[key] = task.pop(key) diff --git a/src/ansiblelint/schemas/__store__.json b/src/ansiblelint/schemas/__store__.json index 4ea5192da4..e46c3bed70 100644 --- a/src/ansiblelint/schemas/__store__.json +++ b/src/ansiblelint/schemas/__store__.json @@ -24,7 +24,7 @@ "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/inventory.json" }, "meta": { - "etag": "80d83d5c61044aa67496ea22c74aef08b0216c20e318400d3c939927a2761d51", + "etag": "d1f66f20b45ecb814d9be900f01451d8035059b312de4a19edc64e29811a7f39", "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/meta.json" }, "meta-runtime": { diff --git a/test/test_transformer.py b/test/test_transformer.py index fbaf6ae0cb..f06302484f 100644 --- a/test/test_transformer.py +++ b/test/test_transformer.py @@ -125,12 +125,6 @@ def fixture_runner_result( True, id="partial_become", ), - pytest.param( - "examples/playbooks/transform-key-order-play.yml", - 1, - True, - id="key_order_play_transform", - ), ), ) def test_transformer( # pylint: disable=too-many-arguments, too-many-locals From 2df20d86af6ebbb8e460bec88d5b7801e066abca Mon Sep 17 00:00:00 2001 From: Ajinkya Udgirkar Date: Mon, 9 Oct 2023 16:19:39 +0530 Subject: [PATCH 5/5] schema meta etag update --- src/ansiblelint/schemas/__store__.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ansiblelint/schemas/__store__.json b/src/ansiblelint/schemas/__store__.json index e46c3bed70..4ea5192da4 100644 --- a/src/ansiblelint/schemas/__store__.json +++ b/src/ansiblelint/schemas/__store__.json @@ -24,7 +24,7 @@ "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/inventory.json" }, "meta": { - "etag": "d1f66f20b45ecb814d9be900f01451d8035059b312de4a19edc64e29811a7f39", + "etag": "80d83d5c61044aa67496ea22c74aef08b0216c20e318400d3c939927a2761d51", "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/meta.json" }, "meta-runtime": {