Extend key-order key to sort more keys than the name

- refactor key ordering to allow us to sort all keys
- ensure `block`/`rescue`/`always` are sorted `last`
- ensure `when` is sorted before `block`
- ensure `action` is sorted after `name`

.github/workflows/tox.yml

@@ -166,7 +166,7 @@ jobs:
       WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 697
+      PYTEST_REQPASS: 709
 
     steps:
       - name: Activate WSL1

examples/playbooks/test_skip_inside_yaml.yml

@@ -10,11 +10,11 @@
       action: ansible.builtin.hg
 
     - name: Test latest[git] and partial-become
-      become_user: alice
       action: ansible.builtin.git
-    - name: Test latest[git] and partial-become (skipped) # noqa latest[git] partial-become
       become_user: alice
+    - name: Test latest[git] and partial-become (skipped) # noqa latest[git] partial-become
       action: ansible.builtin.git
+      become_user: alice
 
     - name: Test YAML # <-- 1 jinja[spacing]
       ansible.builtin.get_url:

src/ansiblelint/rules/key_order.md

@@ -1,13 +1,14 @@
 # key-order
 
-This rule recommends reordering key names in ansible content in order to make
-code easier to maintain and avoid mistakes.
+This rule recommends reordering key names in ansible content to make
+code easier to maintain and less prone to errors.
 
-Here are some examples of common ordering checks done:
+Here are some examples of common ordering checks done for tasks and handlers:
 
 - `name` must always be the first key for plays, tasks and handlers
+- `action` should be the second key, just after `name`
 - when present, the `block` key must be the last, avoid accidental indentation
-  bugs moving keys between block and the last task within the block.
+  bugs moving keys between `block` and the last task within the block.
 
 ## Problematic code
 

src/ansiblelint/rules/key_order.py

@@ -1,13 +1,43 @@
 """All tasks should be have name come first."""
 from __future__ import annotations
 
+import functools
 import sys
 from typing import Any
 
 from ansiblelint.file_utils import Lintable
 from ansiblelint.rules import AnsibleLintRule
 from ansiblelint.testing import RunFromText
 
+SORTER = (
+    "name",
+    "action",
+    None,  # this is where all the unknowns go
+    "when",
+    "notify",
+    "block",
+    "rescue",
+    "always",
+)
+
+
+def get_property_sort_index(a: str) -> int:
+    """Return the index of the property in the sorter."""
+    a_index = -1
+    for i, v in enumerate(SORTER):
+        if v == a:
+            return i
+        elif v is None:
+            a_index = i
+    return a_index
+
+
+def task_property_sorter(a: str, b: str) -> int:
+    """Sort task properties based on SORTER."""
+    x = get_property_sort_index(a)
+    y = get_property_sort_index(b)
+    return (x > y) - (x < y)
+
 
 class KeyOrderRule(AnsibleLintRule):
     """Ensure specific order of keys in mappings."""
@@ -16,17 +46,17 @@ class KeyOrderRule(AnsibleLintRule):
     shortdesc = __doc__
     severity = "LOW"
     tags = ["formatting", "experimental"]
-    version_added = "v6.2.0"
+    version_added = "v6.6.1"
     needs_raw_task = True
 
     def matchtask(
         self, task: dict[str, Any], file: Lintable | None = None
     ) -> bool | str:
         raw_task = task["__raw_task__"]
-        if "name" in raw_task:
-            attribute_list = [*raw_task]
-            if bool(attribute_list[0] != "name"):
-                return "'name' key is not first"
+        keys = [key for key in raw_task.keys() if not key.startswith("_")]
+        sorted_keys = sorted(keys, key=functools.cmp_to_key(task_property_sorter))
+        if keys != sorted_keys:
+            return f"You can improve the task key order to: {', '.join(sorted_keys)}"
         return False
 
 
@@ -86,3 +116,45 @@ def test_task_name_has_name_first_rule_fail(rule_runner: RunFromText) -> None:
         """Test rule matches."""
         results = rule_runner.run_playbook(PLAY_FAIL)
         assert len(results) == 6
+
+    @pytest.mark.parametrize(
+        "input,expected",
+        [
+            pytest.param([], []),
+            pytest.param(["block", "name"], ["name", "block"]),
+            pytest.param(
+                ["block", "name", "action", "..."], ["name", "action", "...", "block"]
+            ),
+        ],
+    )
+    def test_property_sorter(input: list[str], expected: list[str]) -> None:
+        """Test the task property sorter."""
+        result = sorted(input, key=functools.cmp_to_key(task_property_sorter))
+        assert expected == result
+
+    @pytest.mark.parametrize(
+        "key,order",
+        [
+            pytest.param("name", 0),
+            pytest.param("action", 1),
+            pytest.param("foobar", SORTER.index(None)),
+            pytest.param("block", len(SORTER) - 3),
+            pytest.param("rescue", len(SORTER) - 2),
+            pytest.param("always", len(SORTER) - 1),
+        ],
+    )
+    def test_property_sort_index(key: str, order: int) -> None:
+        """Test sorting index"""
+        assert get_property_sort_index(key) == order
+
+    @pytest.mark.parametrize(
+        "a,b,result",
+        [
+            pytest.param("name", "block", -1),
+            pytest.param("block", "name", 1),
+            pytest.param("block", "block", 0),
+        ],
+    )
+    def test_property_sortfunc(a: str, b: str, result: int) -> None:
+        "Test sorting function"
+        assert task_property_sorter(a, b) == result