Add name[play] to highlight plays without name (#2295)

From now on, we will be require plays to have names, same as task,
as these are also displayed by Ansible and help documenting them.

Related: #2171

Author: ssbarnea

.github/workflows/tox.yml

@@ -165,7 +165,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: 693
+      PYTEST_REQPASS: 694
 
     steps:
       - name: Activate WSL1

examples/playbooks/play-name-missing.yml

@@ -0,0 +1,3 @@
+---
+- hosts: localhost # <-- name[missing]
+  tasks: []

examples/playbooks/task-has-name-failure.yml

@@ -1,11 +1,11 @@
 ---
-- hosts: all
+- hosts: all # <-- name[missing]
   tasks:
-    - ansible.builtin.command: echo "no name" # <-- 1
+    - ansible.builtin.command: echo "no name" # <-- name[missing]
       changed_when: false
-    - name: "" # <-- 2
+    - name: "" # <-- name[missing]
       ansible.builtin.command: echo "empty name"
       changed_when: false
-    - ansible.builtin.debug: # <-- 3
+    - ansible.builtin.debug: # <-- name[missing]
         msg: Debug without a name
-    - ansible.builtin.meta: flush_handlers # <-- 4
+    - ansible.builtin.meta: flush_handlers # <-- name[missing]

src/ansiblelint/config.py

@@ -10,7 +10,7 @@
 
 from ansiblelint.loaders import yaml_from_file
 
-DEFAULT_WARN_LIST = ["experimental", "name[casing]", "role-name"]
+DEFAULT_WARN_LIST = ["experimental", "name[casing]", "name[play]", "role-name"]
 
 DEFAULT_KINDS = [
     # Do not sort this list, order matters.

src/ansiblelint/rules/meta_no_tags.py

@@ -105,7 +105,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
     def test_valid_tag_rule(rule_runner: Any) -> None:
         """Test rule matches."""
         results = rule_runner.run_role_meta_main(META_TAG_VALID)
-        assert "Use 'galaxy_tags' rather than 'categories'" in str(results)
+        assert "Use 'galaxy_tags' rather than 'categories'" in str(results), results
         assert "Expected 'categories' to be a list" in str(results)
         assert "invalid: 'my s q l'" in str(results)
         assert "invalid: 'MYTAG'" in str(results)

src/ansiblelint/rules/name.md

@@ -1,24 +1,32 @@
 ## name
 
-This rule identifies several problems related to task naming. This is important
-because these names are the primary way to identify executed tasks on console,
-logs or web interface. Their role is also to document what a task is supposed
-to do.
+This rule identifies several problems related to naming of tasks and plays.
+This is important because these names are the primary way to identify executed
+operations on console, logs or web interface. Their role is also to document
+what Ansible is supposed to do.
 
-- `name[missing]` - All tasks are required to have a name
-- `name[casing]` - All task names should start with a capital letter
+This rule can produce messages such:
+
+- `name[casing]` - All names should start with an uppercase letter.
+- `name[missing]` - All tasks should be named.
+- `name[play]` - All plays should be named.
 
 ### Problematic code
 
 ```yaml
 ---
-- ansible.builtin.command: touch /tmp/.placeholder
+- hosts: localhost
+  tasks:
+    - ansible.builtin.command: touch /tmp/.placeholder
 ```
 
 ### Correct code
 
 ```yaml
 ---
-- name: Create placeholder file
-  ansible.builtin.command: touch /tmp/.placeholder
+- name: Play for creating playholder
+  hosts: localhost
+  tasks:
+    - name: Create placeholder file
+      ansible.builtin.command: touch /tmp/.placeholder
 ```

src/ansiblelint/rules/name.py

@@ -11,20 +11,44 @@
 if TYPE_CHECKING:
     from typing import Optional
 
+    from ansiblelint.constants import odict
     from ansiblelint.file_utils import Lintable
 
 
 class NameRule(AnsibleLintRule):
-    """Rule for checking task names and their content."""
+    """Rule for checking task and play names."""
 
     id = "name"
     description = (
-        "All tasks should have a distinct name for readability "
+        "All tasks and plays should have a distinct name for readability "
         "and for ``--start-at-task`` to work"
     )
     severity = "MEDIUM"
     tags = ["idiom"]
-    version_added = "historic"
+    version_added = "v6.5.0 (last update)"
+
+    def matchplay(self, file: Lintable, data: odict[str, Any]) -> list[MatchError]:
+        """Return matches found for a specific play (entry in playbook)."""
+        if file.kind != "playbook":
+            return []
+        if "name" not in data and not any(
+            key in data
+            for key in ["import_playbook", "ansible.builtin.import_playbook"]
+        ):
+            return [
+                self.create_matcherror(
+                    message="All plays should be named.",
+                    linenumber=data[LINE_NUMBER_KEY],
+                    tag="name[play]",
+                    filename=file,
+                )
+            ]
+        match = self._check_name(
+            data["name"], lintable=file, linenumber=data[LINE_NUMBER_KEY]
+        )
+        if match:
+            return [match]
+        return []
 
     def matchtask(
         self, task: dict[str, Any], file: Lintable | None = None
@@ -37,14 +61,22 @@ def matchtask(
                 tag="name[missing]",
                 filename=file,
             )
+        return (
+            self._check_name(name, lintable=file, linenumber=task[LINE_NUMBER_KEY])
+            or False
+        )
+
+    def _check_name(
+        self, name: str, lintable: Lintable | None, linenumber: int
+    ) -> MatchError | None:
         if not name[0].isupper():
             return self.create_matcherror(
                 message="All names should start with an uppercase letter.",
-                linenumber=task[LINE_NUMBER_KEY],
+                linenumber=linenumber,
                 tag="name[casing]",
-                filename=file,
+                filename=lintable,
             )
-        return False
+        return None
 
 
 if "pytest" in sys.modules:  # noqa: C901
@@ -67,7 +99,7 @@ def test_file_negative() -> None:
         failure = "examples/playbooks/task-has-name-failure.yml"
         bad_runner = Runner(failure, rules=collection)
         errs = bad_runner.run()
-        assert len(errs) == 4
+        assert len(errs) == 5
 
     def test_rule_name_lowercase() -> None:
         """Negative test for a task that starts with lowercase."""
@@ -79,3 +111,13 @@ def test_rule_name_lowercase() -> None:
         assert len(errs) == 1
         assert errs[0].tag == "name[casing]"
         assert errs[0].rule.id == "name"
+
+    def test_name_play() -> None:
+        """Positive test for name[play]."""
+        collection = RulesCollection()
+        collection.register(NameRule())
+        success = "examples/playbooks/play-name-missing.yml"
+        errs = Runner(success, rules=collection).run()
+        assert len(errs) == 1
+        assert errs[0].tag == "name[play]"
+        assert errs[0].rule.id == "name"

test/test_skiputils.py

@@ -19,7 +19,8 @@
 
 PLAYBOOK_WITH_NOQA = """\
 ---
-- hosts: all
+- name: Fixture
+  hosts: all
   vars:
     SOME_VAR_NOQA: "Foo"  # noqa var-naming
     SOME_VAR: "Bar"