diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index aed46a9e29..245aeafc8e 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -73,7 +73,7 @@ jobs: env: # Number of expected test passes, safety measure for accidental skip of # tests. Update value if you add/remove tests. - PYTEST_REQPASS: 865 + PYTEST_REQPASS: 868 steps: - uses: actions/checkout@v4 with: 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...