From 52d9e0fcc1b3111e832d8d4cbc5e307bf5f13b8b Mon Sep 17 00:00:00 2001
From: Conner Crosby <cavcrosby@gmail.com>
Date: Wed, 8 May 2024 10:20:19 -0400
Subject: [PATCH] Add subdirectories to be part of the task prefix (#4066)

---
 examples/playbooks/tasks/main.yml             |  7 ++
 .../playbooks/tasks/partial_prefix/foo.yml    | 10 +++
 .../playbooks/tasks/partial_prefix/main.yml   | 10 +++
 src/ansiblelint/rules/name.md                 | 15 +++-
 src/ansiblelint/rules/name.py                 | 88 ++++++++++++++++++-
 test/test_file_path_evaluation.py             |  8 +-
 6 files changed, 127 insertions(+), 11 deletions(-)
 create mode 100644 examples/playbooks/tasks/main.yml
 create mode 100644 examples/playbooks/tasks/partial_prefix/foo.yml
 create mode 100644 examples/playbooks/tasks/partial_prefix/main.yml

diff --git a/examples/playbooks/tasks/main.yml b/examples/playbooks/tasks/main.yml
new file mode 100644
index 0000000000..b44604b196
--- /dev/null
+++ b/examples/playbooks/tasks/main.yml
@@ -0,0 +1,7 @@
+---
+- name: This is correct
+  ansible.builtin.assert:
+    that: true
+- name: A phony prefix | This is also correct
+  ansible.builtin.assert:
+    that: true
diff --git a/examples/playbooks/tasks/partial_prefix/foo.yml b/examples/playbooks/tasks/partial_prefix/foo.yml
new file mode 100644
index 0000000000..5dfb8e9109
--- /dev/null
+++ b/examples/playbooks/tasks/partial_prefix/foo.yml
@@ -0,0 +1,10 @@
+---
+- name: foo | This prefix is incomplete
+  ansible.builtin.assert:
+    that: true
+- name: partial_prefix | This prefix is incomplete
+  ansible.builtin.assert:
+    that: true
+- name: partial_prefix | foo | This is correct
+  ansible.builtin.assert:
+    that: true
diff --git a/examples/playbooks/tasks/partial_prefix/main.yml b/examples/playbooks/tasks/partial_prefix/main.yml
new file mode 100644
index 0000000000..5c141a8f9f
--- /dev/null
+++ b/examples/playbooks/tasks/partial_prefix/main.yml
@@ -0,0 +1,10 @@
+---
+- name: partial_prefix | main | This is correct
+  ansible.builtin.assert:
+    that: true
+- name: main | This prefix is incomplete
+  ansible.builtin.assert:
+    that: true
+- name: partial_prefix | This prefix is incomplete
+  ansible.builtin.assert:
+    that: true
diff --git a/src/ansiblelint/rules/name.md b/src/ansiblelint/rules/name.md
index 7e31319d71..0c5080a0f4 100644
--- a/src/ansiblelint/rules/name.md
+++ b/src/ansiblelint/rules/name.md
@@ -21,12 +21,21 @@ If you want to ignore some of the messages above, you can add any of them to the
 
 ## name[prefix]
 
-This rule applies only to included task files that are not named `main.yml`. It
-suggests adding the stem of the file as a prefix to the task name.
+This rule applies only to included task files that are not named `main.yml` or
+are embedded within subdirectories. It suggests adding the stems of the file
+path as a prefix to the task name.
 
 For example, if you have a task named `Restart server` inside a file named
 `tasks/deploy.yml`, this rule suggests renaming it to `deploy | Restart server`,
-so it would be easier to identify where it comes from.
+so it would be easier to identify where it comes from. If the file was named
+`tasks/main.yml`, then the rule would have no effect.
+
+For task files that are embedded within subdirectories, these subdirectories
+will also be appended as part of the prefix. For example, if you have a task
+named `Terminate server` inside a file named `tasks/foo/destroy.yml`, this rule
+suggests renaming it to `foo | destroy | Terminate server`. If the file was
+named `tasks/foo/main.yml` then the rule would recommend renaming the task to
+`foo | main | Terminate server`.
 
 For the moment, this sub-rule is just an **opt-in**, so you need to add it to
 your `enable_list` to activate it.
diff --git a/src/ansiblelint/rules/name.py b/src/ansiblelint/rules/name.py
index 4e930a7053..fe38e74995 100644
--- a/src/ansiblelint/rules/name.py
+++ b/src/ansiblelint/rules/name.py
@@ -6,6 +6,9 @@
 import sys
 from typing import TYPE_CHECKING, Any
 
+import wcmatch.pathlib
+import wcmatch.wcmatch
+
 from ansiblelint.constants import LINE_NUMBER_KEY
 from ansiblelint.file_utils import Lintable
 from ansiblelint.rules import AnsibleLintRule, TransformMixin
@@ -125,10 +128,15 @@ def _check_name(
         # stage one check prefix
         effective_name = name
         if self._collection and lintable:
-            prefix = self._collection.options.task_name_prefix.format(
-                stem=lintable.path.stem,
-            )
-            if lintable.kind == "tasks" and lintable.path.stem != "main":
+            full_stem = self._find_full_stem(lintable)
+            stems = [
+                self._collection.options.task_name_prefix.format(stem=stem)
+                for stem in wcmatch.pathlib.PurePath(
+                    full_stem,
+                ).parts
+            ]
+            prefix = "".join(stems)
+            if lintable.kind == "tasks" and full_stem != "main":
                 if not name.startswith(prefix):
                     # For the moment in order to raise errors this rule needs to be
                     # enabled manually. Still, we do allow use of prefixes even without
@@ -170,6 +178,35 @@ def _check_name(
             )
         return results
 
+    def _find_full_stem(self, lintable: Lintable) -> str:
+        lintable_dir = wcmatch.pathlib.PurePath(lintable.dir)
+        stem = lintable.path.stem
+        kind = str(lintable.kind)
+
+        stems = [lintable_dir.name]
+        lintable_dir = lintable_dir.parent
+        pathex = lintable_dir / stem
+        for entry in self.options.kinds:
+            for key, value in entry.items():
+                if kind == key:
+                    glob = value
+
+        while pathex.globmatch(
+            glob,
+            flags=(
+                wcmatch.pathlib.GLOBSTAR
+                | wcmatch.pathlib.BRACE
+                | wcmatch.pathlib.DOTGLOB
+            ),
+        ):
+            stems.insert(0, lintable_dir.name)
+            lintable_dir = lintable_dir.parent
+            pathex = lintable_dir / stem
+
+        if stems[0].startswith(kind):
+            del stems[0]
+        return str(wcmatch.pathlib.PurePath(*stems, stem))
+
     def transform(
         self,
         match: MatchError,
@@ -244,6 +281,19 @@ def test_file_negative() -> None:
         errs = bad_runner.run()
         assert len(errs) == 5
 
+    def test_name_prefix_positive(config_options: Options) -> None:
+        """Positive test for name[prefix]."""
+        config_options.enable_list = ["name[prefix]"]
+        collection = RulesCollection(options=config_options)
+        collection.register(NameRule())
+        success = Lintable(
+            "examples/playbooks/tasks/main.yml",
+            kind="tasks",
+        )
+        good_runner = Runner(success, rules=collection)
+        results = good_runner.run()
+        assert len(results) == 0
+
     def test_name_prefix_negative(config_options: Options) -> None:
         """Negative test for name[missing]."""
         config_options.enable_list = ["name[prefix]"]
@@ -261,6 +311,36 @@ def test_name_prefix_negative(config_options: Options) -> None:
         assert results[1].tag == "name[prefix]"
         assert results[2].tag == "name[prefix]"
 
+    def test_name_prefix_negative_2(config_options: Options) -> None:
+        """Negative test for name[prefix]."""
+        config_options.enable_list = ["name[prefix]"]
+        collection = RulesCollection(options=config_options)
+        collection.register(NameRule())
+        failure = Lintable(
+            "examples/playbooks/tasks/partial_prefix/foo.yml",
+            kind="tasks",
+        )
+        bad_runner = Runner(failure, rules=collection)
+        results = bad_runner.run()
+        assert len(results) == 2
+        assert results[0].tag == "name[prefix]"
+        assert results[1].tag == "name[prefix]"
+
+    def test_name_prefix_negative_3(config_options: Options) -> None:
+        """Negative test for name[prefix]."""
+        config_options.enable_list = ["name[prefix]"]
+        collection = RulesCollection(options=config_options)
+        collection.register(NameRule())
+        failure = Lintable(
+            "examples/playbooks/tasks/partial_prefix/main.yml",
+            kind="tasks",
+        )
+        bad_runner = Runner(failure, rules=collection)
+        results = bad_runner.run()
+        assert len(results) == 2
+        assert results[0].tag == "name[prefix]"
+        assert results[1].tag == "name[prefix]"
+
     def test_rule_name_lowercase() -> None:
         """Negative test for a task that starts with lowercase."""
         collection = RulesCollection()
diff --git a/test/test_file_path_evaluation.py b/test/test_file_path_evaluation.py
index 088593f91d..69f02bb15f 100644
--- a/test/test_file_path_evaluation.py
+++ b/test/test_file_path_evaluation.py
@@ -43,14 +43,14 @@
     "tasks/subtasks/subtask_1.yml": textwrap.dedent(
         """\
         ---
-        - name: subtask_1 | From subtask 1 import subtask 2
+        - name: subtasks | subtask_1 | From subtask 1 import subtask 2
           ansible.builtin.import_tasks: tasks/subtasks/subtask_2.yml
         """,
     ),
     "tasks/subtasks/subtask_2.yml": textwrap.dedent(
         """\
         ---
-        - name: subtask_2 | From subtask 2 do something
+        - name: subtasks | subtask_2 | From subtask 2 do something
           debug:  # <-- expected to raise fqcn[action-core]
             msg: |
               Something...
@@ -87,14 +87,14 @@
     "tasks/subtasks/subtask_1.yml": textwrap.dedent(
         """\
         ---
-        - name: subtask_1 | From subtask 1 import subtask 2
+        - name: subtasks | subtask_1 | From subtask 1 import subtask 2
           ansible.builtin.include_tasks: tasks/subtasks/subtask_2.yml
         """,
     ),
     "tasks/subtasks/subtask_2.yml": textwrap.dedent(
         """\
         ---
-        - name: subtask_2 | From subtask 2 do something
+        - name: subtasks | subtask_2 | From subtask 2 do something
           debug:  # <-- expected to raise fqcn[action-core]
             msg: |
               Something...