From b685341429f31ea7b5e96b5811c3c15f5612d33b Mon Sep 17 00:00:00 2001 From: Conner Crosby Date: Wed, 8 May 2024 10:20:19 -0400 Subject: [PATCH] Allow subdirectories to be part of the task prefix (#4066) --- .../playbooks/tasks/partial_prefix/foo.yml | 10 ++++ src/ansiblelint/rules/name.md | 5 ++ src/ansiblelint/rules/name.py | 57 ++++++++++++++++++- test/test_file_path_evaluation.py | 8 +-- 4 files changed, 73 insertions(+), 7 deletions(-) create mode 100644 examples/playbooks/tasks/partial_prefix/foo.yml 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/src/ansiblelint/rules/name.md b/src/ansiblelint/rules/name.md index 7e31319d71..d88e4e9cbd 100644 --- a/src/ansiblelint/rules/name.md +++ b/src/ansiblelint/rules/name.md @@ -28,6 +28,11 @@ 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. +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`. + 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 b6a6d6ffaf..35e16d4eb5 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 @@ -121,9 +124,13 @@ 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, - ) + stems = [ + self._collection.options.task_name_prefix.format(stem=stem) + for stem in wcmatch.pathlib.PurePath( + self._find_full_stem(lintable), + ).parts + ] + prefix = "".join(stems) if lintable.kind == "tasks" and lintable.path.stem != "main": if not name.startswith(prefix): # For the moment in order to raise errors this rule needs to be @@ -166,6 +173,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, @@ -257,6 +293,21 @@ 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_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...