diff --git a/.config/dictionary.txt b/.config/dictionary.txt index fa2a4b26c8..488b72f7dc 100644 --- a/.config/dictionary.txt +++ b/.config/dictionary.txt @@ -161,6 +161,7 @@ synchronize sysvinit taskimports taskincludes +templating testmon testns testpath diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index a5d407083a..cb3fc6800d 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -156,7 +156,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: 399 + PYTEST_REQPASS: 550 steps: - name: Activate WSL1 diff --git a/docs/installing.rst b/docs/installing.rst index b083c19713..5e62d3beb1 100644 --- a/docs/installing.rst +++ b/docs/installing.rst @@ -11,17 +11,11 @@ Installing Installing on Windows is not supported because we use symlinks inside Python packages. -While our project does not directly ship a container, the -tool is part of the toolset_ container. Please avoid raising any bugs -related to containers and use the discussions_ forum instead. +Our project does not ship a container. Please avoid raising any bugs +related to containers and use the `discussions `_ forum instead. -.. code-block:: bash - - # replace docker with podman - docker run -h toolset -it quay.io/ansible/toolset ansible-lint --version - -.. _toolset: https://github.com/ansible-community/toolset -.. _discussions: https://github.com/ansible-community/ansible-lint/discussions +We recommend you to try `creator-ee `_, +which is a container that also carries ansible-lint. Using pip or pipx ----------------- diff --git a/examples/playbooks/tasks/empty_blocks.yml b/examples/playbooks/tasks/empty_blocks.yml new file mode 100644 index 0000000000..ce819294e3 --- /dev/null +++ b/examples/playbooks/tasks/empty_blocks.yml @@ -0,0 +1,17 @@ +--- +- name: a named block task + block: + - name: an assertion + ansible.builtin.assert: + fail_msg: foo + rescue: # null + always: {} +- block: + - name: another assertion + ansible.builtin.assert: + fail_msg: bar + rescue: {} + always: + - name: yet another assertion + ansible.builtin.assert: + fail_msg: baz diff --git a/examples/playbooks/tasks/x.yml b/examples/playbooks/tasks/x.yml index c63ea7eee8..856351be4e 100644 --- a/examples/playbooks/tasks/x.yml +++ b/examples/playbooks/tasks/x.yml @@ -4,3 +4,4 @@ action: funny value=clown args: key: value +- # a second null task, validates yaml_utils.get_path_to_task diff --git a/examples/templates/playbooks/playbook.yml b/examples/templates/playbooks/playbook.yml new file mode 100644 index 0000000000..bf852459ac --- /dev/null +++ b/examples/templates/playbooks/playbook.yml @@ -0,0 +1,6 @@ +--- +# even if is hosted under playbooks and is named playbook.yml, this file +# is not a real playbook because it is hosted in a "templates" directory +# and that means we will avoid processing it as a playbook. Templates use +# either jinja2 or another templating engine, so we cannot load them. +foo: bar diff --git a/src/ansiblelint/config.py b/src/ansiblelint/config.py index 7d8be12adb..6d6b8c36f0 100644 --- a/src/ansiblelint/config.py +++ b/src/ansiblelint/config.py @@ -9,6 +9,7 @@ # Do not sort this list, order matters. {"jinja2": "**/*.j2"}, # jinja2 templates are not always parsable as something else {"jinja2": "**/*.j2.*"}, + {"text": "**/templates/**/*.*"}, # templates are likely not validable {"inventory": "**/inventory/**.yml"}, {"requirements": "**/meta/requirements.yml"}, # v1 only # https://docs.ansible.com/ansible/latest/dev_guide/collections_galaxy_meta.html diff --git a/src/ansiblelint/errors.py b/src/ansiblelint/errors.py index d80e699778..5f349637d8 100644 --- a/src/ansiblelint/errors.py +++ b/src/ansiblelint/errors.py @@ -1,6 +1,6 @@ """Exceptions and error representations.""" import functools -from typing import Any, Dict, Optional, Union +from typing import Any, Dict, List, Optional, Union from ansiblelint._internal.rules import BaseRule, RuntimeErrorRule from ansiblelint.file_utils import Lintable, normpath @@ -76,6 +76,8 @@ def __init__( self.match_type: Optional[str] = None # for task matches, save the normalized task object (useful for transforms) self.task: Optional[Dict[str, Any]] = None + # path to the problem area, like: [0,"pre_tasks",3] for [0].pre_tasks[3] + self.yaml_path: List[Union[int, str]] = [] def __repr__(self) -> str: """Return a MatchError instance representation.""" diff --git a/src/ansiblelint/file_utils.py b/src/ansiblelint/file_utils.py index 876791935a..40a8635d88 100644 --- a/src/ansiblelint/file_utils.py +++ b/src/ansiblelint/file_utils.py @@ -51,6 +51,9 @@ def normpath(path: Union[str, BasePathLike]) -> str: Currently it generates a relative path but in the future we may want to make this user configurable. """ + # prevent possible ValueError with relpath(), when input is an empty string + if not path: + path = "." # conversion to string in order to allow receiving non string objects relpath = os.path.relpath(str(path)) path_absolute = os.path.abspath(str(path)) diff --git a/src/ansiblelint/rules/__init__.py b/src/ansiblelint/rules/__init__.py index 4d6c80848d..fed92b3c3c 100644 --- a/src/ansiblelint/rules/__init__.py +++ b/src/ansiblelint/rules/__init__.py @@ -10,7 +10,20 @@ from collections import defaultdict from functools import lru_cache from importlib.abc import Loader -from typing import Any, Dict, Iterator, List, Optional, Set, Union +from typing import ( + Any, + Dict, + Iterator, + List, + MutableMapping, + MutableSequence, + Optional, + Set, + Union, + cast, +) + +from ruamel.yaml.comments import CommentedMap, CommentedSeq import ansiblelint.skip_utils import ansiblelint.utils @@ -200,6 +213,81 @@ def matchyaml(self, file: Lintable) -> List[MatchError]: return matches +class TransformMixin: + """A mixin for AnsibleLintRule to enable transforming files. + + If ansible-lint is started with the ``--write`` option, then the ``Transformer`` + will call the ``transform()`` method for every MatchError identified if the rule + that identified it subclasses this ``TransformMixin``. Only the rule that identified + a MatchError can do transforms to fix that match. + """ + + def transform( + self, + match: MatchError, + lintable: Lintable, + data: Union[CommentedMap, CommentedSeq, str], + ) -> None: + """Transform ``data`` to try to fix the MatchError identified by this rule. + + The ``match`` was generated by this rule in the ``lintable`` file. + When ``transform()`` is called on a rule, the rule should either fix the + issue, if possible, or make modifications that make it easier to fix manually. + + For YAML files, ``data`` is an editable YAML dict/array that preserves + any comments that were in the original file. + + .. code:: python + + data[0]["tasks"][0]["when"] = False + + This is easier with the ``seek()`` utility method: + + .. code :: python + + target_task = self.seek(match.yaml_path, data) + target_task["when"] = False + + For any files that aren't YAML, ``data`` is the loaded file's content as a string. + To edit non-YAML files, save the updated contents in ``lintable.content``: + + .. code:: python + + new_data = self.do_something_to_fix_the_match(data) + lintable.content = new_data + """ + + @staticmethod + def seek( + yaml_path: List[Union[int, str]], + data: Union[MutableMapping[str, Any], MutableSequence[Any], str], + ) -> Any: + """Get the element identified by ``yaml_path`` in ``data``. + + Rules that work with YAML need to seek, or descend, into nested YAML data + structures to perform the relevant transforms. For example: + + .. code:: python + + def transform(self, match, lintable, data): + target_task = self.seek(match.yaml_path, data) + # transform target_task + """ + if isinstance(data, str): + # can't descend into a string + return data + target = data + for segment in yaml_path: + # The cast() calls tell mypy what types we expect. + # Essentially this does: + # target = target[segment] + if isinstance(segment, str): + target = cast(MutableMapping[str, Any], target)[segment] + elif isinstance(segment, int): + target = cast(MutableSequence[Any], target)[segment] + return target + + def is_valid_rule(rule: Any) -> bool: """Check if given rule is valid or not.""" return issubclass(rule, AnsibleLintRule) and bool(rule.id) and bool(rule.shortdesc) diff --git a/src/ansiblelint/rules/yaml.py b/src/ansiblelint/rules/yaml.py index 852c17e47c..3abe3971ec 100644 --- a/src/ansiblelint/rules/yaml.py +++ b/src/ansiblelint/rules/yaml.py @@ -17,6 +17,9 @@ DESCRIPTION = """\ Rule violations reported by YamlLint when this is installed. +Yamllint configuration will be loaded following yamllint's configuration file +resolution order. + You can fully disable all of them by adding 'yaml' to the 'skip_list'. Specific tag identifiers that are printed at the end of rule name, diff --git a/src/ansiblelint/transformer.py b/src/ansiblelint/transformer.py index e4b6391efc..030631c731 100644 --- a/src/ansiblelint/transformer.py +++ b/src/ansiblelint/transformer.py @@ -1,13 +1,14 @@ """Transformer implementation.""" import logging -from typing import Dict, List, Set, Union +from typing import Dict, List, Optional, Set, Union, cast from ruamel.yaml.comments import CommentedMap, CommentedSeq from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable +from ansiblelint.rules import TransformMixin from ansiblelint.runner import LintResult -from ansiblelint.yaml_utils import FormattedYAML +from ansiblelint.yaml_utils import FormattedYAML, get_path_to_play, get_path_to_task __all__ = ["Transformer"] @@ -50,7 +51,7 @@ def __init__(self, result: LintResult): def run(self) -> None: """For each file, read it, execute transforms on it, then write it.""" - for file, _ in self.matches_per_file.items(): + for file, matches in self.matches_per_file.items(): # str() convinces mypy that "text/yaml" is a valid Literal. # Otherwise, it thinks base_kind is one of playbook, meta, tasks, ... file_is_yaml = str(file.base_kind) == "text/yaml" @@ -62,17 +63,48 @@ def run(self) -> None: data = "" file_is_yaml = False + ruamel_data: Optional[Union[CommentedMap, CommentedSeq]] = None if file_is_yaml: # We need a fresh YAML() instance for each load because ruamel.yaml # stores intermediate state during load which could affect loading # any other files. (Based on suggestion from ruamel.yaml author) yaml = FormattedYAML() - ruamel_data: Union[CommentedMap, CommentedSeq] = yaml.loads(data) + ruamel_data = yaml.loads(data) if not isinstance(ruamel_data, (CommentedMap, CommentedSeq)): # This is an empty vars file or similar which loads as None. # It is not safe to write this file or data-loss is likely. # Only maps and sequences can preserve comments. Skip it. continue + + self._do_transforms(file, ruamel_data or data, file_is_yaml, matches) + + if file_is_yaml: + # noinspection PyUnboundLocalVariable file.content = yaml.dumps(ruamel_data) + + if file.updated: file.write() + + @staticmethod + def _do_transforms( + file: Lintable, + data: Union[CommentedMap, CommentedSeq, str], + file_is_yaml: bool, + matches: List[MatchError], + ) -> None: + """Do Rule-Transforms handling any last-minute MatchError inspections.""" + for match in sorted(matches): + if not isinstance(match.rule, TransformMixin): + continue + if file_is_yaml and not match.yaml_path: + data = cast(Union[CommentedMap, CommentedSeq], data) + if match.match_type == "play": + match.yaml_path = get_path_to_play(file, match.linenumber, data) + elif match.task or file.kind in ( + "tasks", + "handlers", + "playbook", + ): + match.yaml_path = get_path_to_task(file, match.linenumber, data) + match.rule.transform(match, file, data) diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 8b034aac48..4e8e9d8b9d 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -334,9 +334,14 @@ def _taskshandlers_children( results.append(children) continue - if ( - "include_role" in task_handler or "import_role" in task_handler - ): # lgtm [py/unreachable-statement] + including_commands = ( + "import_role", + "ansible.builtin.import_role", + "include_role", + "ansible.builtin.include_role", + ) + if any(x in task_handler for x in including_commands): + # lgtm [py/unreachable-statement] task_handler = normalize_task_v2(task_handler) _validate_task_handler_action_for_role(task_handler["action"]) results.extend( diff --git a/src/ansiblelint/yaml_utils.py b/src/ansiblelint/yaml_utils.py index 3534c63d0d..d96e14e690 100644 --- a/src/ansiblelint/yaml_utils.py +++ b/src/ansiblelint/yaml_utils.py @@ -1,10 +1,12 @@ """Utility helpers to simplify working with yaml-based data.""" +# pylint: disable=too-many-lines import functools import logging import os import re from io import StringIO from typing import ( + TYPE_CHECKING, Any, Callable, Dict, @@ -12,12 +14,14 @@ List, Optional, Pattern, + Set, Tuple, Union, cast, ) import ruamel.yaml.events +from ruamel.yaml.comments import CommentedMap, CommentedSeq, Format from ruamel.yaml.constructor import RoundTripConstructor from ruamel.yaml.emitter import Emitter, ScalarAnalysis @@ -31,10 +35,15 @@ from yamllint.config import YamlLintConfig import ansiblelint.skip_utils +from ansiblelint.constants import NESTED_TASK_KEYS, PLAYBOOK_TASK_KEYWORDS from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.utils import get_action_tasks, normalize_task, parse_yaml_linenumbers +if TYPE_CHECKING: + # noinspection PyProtectedMember + from ruamel.yaml.comments import LineCol # pylint: disable=ungrouped-imports + _logger = logging.getLogger(__name__) YAMLLINT_CONFIG = """ @@ -59,7 +68,14 @@ def load_yamllint_config() -> YamlLintConfig: config = YamlLintConfig(content=YAMLLINT_CONFIG) # if we detect local yamllint config we use it but raise a warning # as this is likely to get out of sync with our internal config. - for file in [".yamllint", ".yamllint.yaml", ".yamllint.yml"]: + for file in [ + ".yamllint", + ".yamllint.yaml", + ".yamllint.yml", + os.getenv("YAMLLINT_CONFIG_FILE", ""), + os.getenv("XDG_CONFIG_HOME", os.path.expanduser("~/.config")) + + "/yamllint/config", + ]: if os.path.isfile(file): _logger.warning( "Loading custom %s config file, this extends our " @@ -225,6 +241,215 @@ def _nested_items_path( ) +def get_path_to_play( + lintable: Lintable, + line_number: int, # 1-based + ruamel_data: Union[CommentedMap, CommentedSeq], +) -> List[Union[str, int]]: + """Get the path to the play in the given file at the given line number.""" + if line_number < 1: + raise ValueError(f"expected line_number >= 1, got {line_number}") + if lintable.kind != "playbook" or not isinstance(ruamel_data, CommentedSeq): + return [] + lc: "LineCol" # lc uses 0-based counts # pylint: disable=invalid-name + # line_number is 1-based. Convert to 0-based. + line_index = line_number - 1 + + prev_play_line_index = ruamel_data.lc.line + last_play_index = len(ruamel_data) + for play_index, play in enumerate(ruamel_data): + next_play_index = play_index + 1 + if last_play_index > next_play_index: + next_play_line_index = ruamel_data[next_play_index].lc.line + else: + next_play_line_index = None + + lc = play.lc # pylint: disable=invalid-name + assert isinstance(lc.line, int) + if lc.line == line_index: + return [play_index] + if play_index > 0 and prev_play_line_index < line_index < lc.line: + return [play_index - 1] + # The previous play check (above) can't catch the last play, + # so, handle the last play separately. + if ( + next_play_index == last_play_index + and line_index > lc.line + and (next_play_line_index is None or line_index < next_play_line_index) + ): + # part of this (last) play + return [play_index] + prev_play_line_index = play.lc.line + return [] + + +def get_path_to_task( + lintable: Lintable, + line_number: int, # 1-based + ruamel_data: Union[CommentedMap, CommentedSeq], +) -> List[Union[str, int]]: + """Get the path to the task in the given file at the given line number.""" + if line_number < 1: + raise ValueError(f"expected line_number >= 1, got {line_number}") + if lintable.kind in ("tasks", "handlers"): + assert isinstance(ruamel_data, CommentedSeq) + return _get_path_to_task_in_tasks_block(line_number, ruamel_data) + if lintable.kind == "playbook": + assert isinstance(ruamel_data, CommentedSeq) + return _get_path_to_task_in_playbook(line_number, ruamel_data) + # if lintable.kind in ["yaml", "requirements", "vars", "meta", "reno"]: + + return [] + + +def _get_path_to_task_in_playbook( + line_number: int, # 1-based + ruamel_data: CommentedSeq, +) -> List[Union[str, int]]: + """Get the path to the task in the given playbook data at the given line number.""" + last_play_index = len(ruamel_data) + for play_index, play in enumerate(ruamel_data): + next_play_index = play_index + 1 + if last_play_index > next_play_index: + next_play_line_index = ruamel_data[next_play_index].lc.line + else: + next_play_line_index = None + + play_keys = list(play.keys()) + for tasks_keyword in PLAYBOOK_TASK_KEYWORDS: + if not play.get(tasks_keyword): + continue + + try: + next_keyword = play_keys[play_keys.index(tasks_keyword) + 1] + except IndexError: + next_block_line_index = None + else: + next_block_line_index = play.lc.data[next_keyword][0] + # last_line_number_in_block is 1-based; next_*_line_index is 0-based + # next_*_line_index - 1 to get line before next_*_line_index. + # Then + 1 to make it a 1-based number. + # So, last_line_number_in_block = next_*_line_index - 1 + 1 + if next_block_line_index is not None: + last_line_number_in_block = next_block_line_index + elif next_play_line_index is not None: + last_line_number_in_block = next_play_line_index + else: + last_line_number_in_block = None + + task_path = _get_path_to_task_in_tasks_block( + line_number, play[tasks_keyword], last_line_number_in_block + ) + if task_path: + # mypy gets confused without this typehint + tasks_keyword_path: List[Union[int, str]] = [ + play_index, + tasks_keyword, + ] + return tasks_keyword_path + list(task_path) + # line_number is before first play or no tasks keywords in any of the plays + return [] + + +def _get_path_to_task_in_tasks_block( + line_number: int, # 1-based + tasks_block: CommentedSeq, + last_line_number: Optional[int] = None, # 1-based +) -> List[Union[str, int]]: + """Get the path to the task in the given tasks block at the given line number.""" + task: Optional[CommentedMap] + # line_number and last_line_number are 1-based. Convert to 0-based. + line_index = line_number - 1 + last_line_index = None if last_line_number is None else last_line_number - 1 + + # lc (LineCol) uses 0-based counts + prev_task_line_index = tasks_block.lc.line + last_task_index = len(tasks_block) + for task_index, task in enumerate(tasks_block): + next_task_index = task_index + 1 + if last_task_index > next_task_index: + if tasks_block[next_task_index] is not None: + next_task_line_index = tasks_block[next_task_index].lc.line + else: + next_task_line_index = tasks_block.lc.item(next_task_index)[0] + else: + next_task_line_index = None + + if task is None: + # create a dummy task to represent the null task + task = CommentedMap() + task.lc.line, task.lc.col = tasks_block.lc.item(task_index) + + nested_task_keys = set(task.keys()).intersection(set(NESTED_TASK_KEYS)) + if nested_task_keys: + subtask_path = _get_path_to_task_in_nested_tasks_block( + line_number, task, nested_task_keys, next_task_line_index + ) + if subtask_path: + # mypy gets confused without this typehint + task_path: List[Union[str, int]] = [task_index] + return task_path + list(subtask_path) + + assert isinstance(task.lc.line, int) + if task.lc.line == line_index: + return [task_index] + if task_index > 0 and prev_task_line_index < line_index < task.lc.line: + return [task_index - 1] + # The previous task check can't catch the last task, + # so, handle the last task separately (also after subtask checks). + # pylint: disable=too-many-boolean-expressions + if ( + next_task_index == last_task_index + and line_index > task.lc.line + and (next_task_line_index is None or line_index < next_task_line_index) + and (last_line_index is None or line_index <= last_line_index) + ): + # part of this (last) task + return [task_index] + prev_task_line_index = task.lc.line + # line is not part of this tasks block + return [] + + +def _get_path_to_task_in_nested_tasks_block( + line_number: int, # 1-based + task: CommentedMap, + nested_task_keys: Set[str], + next_task_line_index: Optional[int] = None, # 0-based +) -> List[Union[str, int]]: + """Get the path to the task in the given nested tasks block.""" + # loop through the keys in line order + task_keys = list(task.keys()) + task_keys_by_index = dict(enumerate(task_keys)) + for task_index, task_key in enumerate(task_keys): + nested_task_block = task[task_key] + if task_key not in nested_task_keys or not nested_task_block: + continue + next_task_key = task_keys_by_index.get(task_index + 1, None) + if next_task_key is not None: + next_task_key_line_index = task.lc.data[next_task_key][0] + else: + next_task_key_line_index = None + # last_line_number_in_block is 1-based; next_*_line_index is 0-based + # next_*_line_index - 1 to get line before next_*_line_index. + # Then + 1 to make it a 1-based number. + # So, last_line_number_in_block = next_*_line_index - 1 + 1 + last_line_number_in_block = ( + next_task_key_line_index + if next_task_key_line_index is not None + else next_task_line_index + ) + subtask_path = _get_path_to_task_in_tasks_block( + line_number, + nested_task_block, + last_line_number_in_block, # 1-based + ) + if subtask_path: + return [task_key] + list(subtask_path) + # line is not part of this nested tasks block + return [] + + class OctalIntYAML11(ScalarInt): """OctalInt representation for YAML 1.1.""" @@ -669,6 +894,7 @@ def loads(self, stream: str) -> Any: def dumps(self, data: Any) -> str: """Dump YAML document to string (including its preamble_comment).""" preamble_comment: Optional[str] = getattr(data, "preamble_comment", None) + self._prevent_wrapping_flow_style(data) with StringIO() as stream: if preamble_comment: stream.write(preamble_comment) @@ -676,6 +902,50 @@ def dumps(self, data: Any) -> str: text = stream.getvalue() return self._post_process_yaml(text) + def _prevent_wrapping_flow_style(self, data: Any) -> None: + if not isinstance(data, (CommentedMap, CommentedSeq)): + return + for key, value, parent_path in nested_items_path(data): + if not isinstance(value, (CommentedMap, CommentedSeq)): + continue + fa: Format = value.fa # pylint: disable=invalid-name + if fa.flow_style(): + predicted_indent = self._predict_indent_length(parent_path, key) + predicted_width = len(str(value)) + if predicted_indent + predicted_width > self.width: + # this flow-style map will probably get line-wrapped, + # so, switch it to block style to avoid the line wrap. + fa.set_block_style() + + def _predict_indent_length( + self, parent_path: List[Union[str, int]], key: Any + ) -> int: + indent = 0 + + # each parent_key type tells us what the indent is for the next level. + for parent_key in parent_path: + if isinstance(parent_key, int) and indent == 0: + # root level is a sequence + indent += self.sequence_dash_offset + elif isinstance(parent_key, int): + # next level is a sequence + indent += cast(int, self.sequence_indent) + elif isinstance(parent_key, str): + # next level is a map + indent += cast(int, self.map_indent) + + if isinstance(key, int) and indent == 0: + # flow map is an item in a root-level sequence + indent += self.sequence_dash_offset + elif isinstance(key, int) and indent > 0: + # flow map is in a sequence + indent += cast(int, self.sequence_indent) + elif isinstance(key, str): + # flow map is in a map + indent += len(key + ": ") + + return indent + # ruamel.yaml only preserves empty (no whitespace) blank lines # (ie "/n/n" becomes "/n/n" but "/n /n" becomes "/n"). # So, we need to identify whitespace-only lines to drop spaces before reading. diff --git a/test/eco/debops.result b/test/eco/debops.result index 44bdbebaac..986daad5cc 100644 --- a/test/eco/debops.result +++ b/test/eco/debops.result @@ -4,31 +4,6 @@ RC: 0 STDERR: WARNING Loading custom .yamllint config file, this extends our internal yamllint config. -WARNING Listing 18 violation(s) that are fatal -You can skip specific rules or tags by adding them to your configuration file: -# .config/ansible-lint.yml -warn_list: # or 'skip_list' to silence them completely - - experimental # all rules tagged as experimental - -Finished with 0 failure(s), 18 warning(s) on 4869 files. STDOUT: -ansible/roles/golang/tasks/golang_build_install.yml:82: risky-file-permissions File permissions unset or incorrect. -ansible/roles/hashicorp/tasks/main.yml:106: risky-file-permissions File permissions unset or incorrect. -ansible/roles/hashicorp/tasks/main.yml:121: risky-file-permissions File permissions unset or incorrect. -ansible/roles/ipxe/tasks/debian_netboot.yml:26: risky-file-permissions File permissions unset or incorrect. -ansible/roles/ipxe/tasks/debian_netboot.yml:76: risky-file-permissions File permissions unset or incorrect. -ansible/roles/owncloud/tasks/tarball.yml:75: risky-file-permissions File permissions unset or incorrect. -ansible/roles/owncloud/tasks/tarball.yml:102: risky-file-permissions File permissions unset or incorrect. -ansible/roles/rstudio_server/tasks/main.yml:70: risky-file-permissions File permissions unset or incorrect. -ansible/roles/wpcli/tasks/main.yml:23: risky-file-permissions File permissions unset or incorrect. -roles/golang/tasks/golang_build_install.yml:82: risky-file-permissions File permissions unset or incorrect. -roles/hashicorp/tasks/main.yml:106: risky-file-permissions File permissions unset or incorrect. -roles/hashicorp/tasks/main.yml:121: risky-file-permissions File permissions unset or incorrect. -roles/ipxe/tasks/debian_netboot.yml:26: risky-file-permissions File permissions unset or incorrect. -roles/ipxe/tasks/debian_netboot.yml:76: risky-file-permissions File permissions unset or incorrect. -roles/owncloud/tasks/tarball.yml:75: risky-file-permissions File permissions unset or incorrect. -roles/owncloud/tasks/tarball.yml:102: risky-file-permissions File permissions unset or incorrect. -roles/rstudio_server/tasks/main.yml:70: risky-file-permissions File permissions unset or incorrect. -roles/wpcli/tasks/main.yml:23: risky-file-permissions File permissions unset or incorrect. diff --git a/test/eco/docker-rootless.result b/test/eco/docker-rootless.result index 20dfdc066b..06be1a549d 100644 --- a/test/eco/docker-rootless.result +++ b/test/eco/docker-rootless.result @@ -4,15 +4,6 @@ RC: 0 STDERR: WARNING Loading custom .yamllint.yml config file, this extends our internal yamllint config. -WARNING Listing 2 violation(s) that are fatal -You can skip specific rules or tags by adding them to your configuration file: -# .config/ansible-lint.yml -warn_list: # or 'skip_list' to silence them completely - - experimental # all rules tagged as experimental - -Finished with 0 failure(s), 2 warning(s) on 29 files. STDOUT: -tasks/docker_install_rootless.yml:22: risky-file-permissions File permissions unset or incorrect. -tasks/docker_install_rootless.yml:32: risky-file-permissions File permissions unset or incorrect. diff --git a/test/eco/hardening.result b/test/eco/hardening.result index a7807374c7..f76258f16f 100644 --- a/test/eco/hardening.result +++ b/test/eco/hardening.result @@ -6,7 +6,7 @@ STDERR: WARNING Loading custom .yamllint.yml config file, this extends our internal yamllint config. WARNING Listing 4 violation(s) that are fatal -Finished with 0 failure(s), 4 warning(s) on 121 files. +Finished with 0 failure(s), 4 warning(s) on 118 files. STDOUT: diff --git a/test/include-import-role.yml b/test/include-import-role.yml deleted file mode 100644 index aa866b691d..0000000000 --- a/test/include-import-role.yml +++ /dev/null @@ -1,18 +0,0 @@ ---- -- hosts: all - vars: - var_is_set: false - - tasks: - - import_role: - name: test-role - -- hosts: all - vars: - var_is_set: false - - tasks: - - include_role: - name: test-role - tasks_from: world - when: "{{ var_is_set }}" diff --git a/test/test_file_utils.py b/test/test_file_utils.py index bfa472c983..6eca570c5f 100644 --- a/test/test_file_utils.py +++ b/test/test_file_utils.py @@ -25,15 +25,17 @@ @pytest.mark.parametrize( - "path", + ("path", "expected"), ( - pytest.param(Path("a/b/../"), id="pathlib.Path"), - pytest.param("a/b/../", id="str"), + pytest.param(Path("a/b/../"), "a", id="pathlib.Path"), + pytest.param("a/b/../", "a", id="str"), + pytest.param("", ".", id="empty"), + pytest.param(".", ".", id="empty"), ), ) -def test_normpath_with_path_object(path: str) -> None: +def test_normpath(path: str, expected: str) -> None: """Ensure that relative parent dirs are normalized in paths.""" - assert normpath(path) == "a" + assert normpath(path) == expected def test_expand_path_vars(monkeypatch: MonkeyPatch) -> None: diff --git a/test/test_import_include_role.py b/test/test_import_include_role.py index 7b4f8da289..b2f4e8cdfb 100644 --- a/test/test_import_include_role.py +++ b/test/test_import_include_role.py @@ -8,44 +8,76 @@ from ansiblelint.rules import RulesCollection from ansiblelint.runner import Runner -ROLE_TASKS_MAIN = """ +ROLE_TASKS_MAIN = """\ +--- - name: shell instead of command shell: echo hello world + changed_when: false """ -ROLE_TASKS_WORLD = """ -- command: echo this is a task without a name +ROLE_TASKS_WORLD = """\ +--- +- debug: msg="this is a task without a name" """ -PLAY_IMPORT_ROLE = """ +PLAY_IMPORT_ROLE = """\ +--- - hosts: all tasks: - - import_role: + - name: some import + import_role: name: test-role """ -PLAY_IMPORT_ROLE_INLINE = """ +PLAY_IMPORT_ROLE_FQCN = """\ +--- - hosts: all tasks: - - import_role: name=test-role + - name: some import + ansible.builtin.import_role: + name: test-role +""" + +PLAY_IMPORT_ROLE_INLINE = """\ +--- +- hosts: all + + tasks: + - name: some import + import_role: name=test-role """ -PLAY_INCLUDE_ROLE = """ +PLAY_INCLUDE_ROLE = """\ +--- - hosts: all tasks: - - include_role: + - name: some import + include_role: name: test-role tasks_from: world """ -PLAY_INCLUDE_ROLE_INLINE = """ +PLAY_INCLUDE_ROLE_FQCN = """\ +--- - hosts: all tasks: - - include_role: name=test-role tasks_from=world + - name: some import + ansible.builtin.include_role: + name: test-role + tasks_from: world +""" + +PLAY_INCLUDE_ROLE_INLINE = """\ +--- +- hosts: all + + tasks: + - name: some import + include_role: name=test-role tasks_from=world """ @@ -67,12 +99,17 @@ def fixture_playbook_path(request: SubRequest, tmp_path: Path) -> str: ( pytest.param( PLAY_IMPORT_ROLE, - ["only when shell functionality is required"], + ["only when shell functionality is required", "All tasks should be named"], id="IMPORT_ROLE", ), + pytest.param( + PLAY_IMPORT_ROLE_FQCN, + ["only when shell functionality is required", "All tasks should be named"], + id="IMPORT_ROLE_FQCN", + ), pytest.param( PLAY_IMPORT_ROLE_INLINE, - ["only when shell functionality is require"], + ["only when shell functionality is require", "All tasks should be named"], id="IMPORT_ROLE_INLINE", ), pytest.param( @@ -80,6 +117,11 @@ def fixture_playbook_path(request: SubRequest, tmp_path: Path) -> str: ["only when shell functionality is require", "All tasks should be named"], id="INCLUDE_ROLE", ), + pytest.param( + PLAY_INCLUDE_ROLE_FQCN, + ["only when shell functionality is require", "All tasks should be named"], + id="INCLUDE_ROLE_FQCN", + ), pytest.param( PLAY_INCLUDE_ROLE_INLINE, ["only when shell functionality is require", "All tasks should be named"], @@ -92,7 +134,11 @@ def test_import_role2( default_rules_collection: RulesCollection, playbook_path: str, messages: List[str] ) -> None: """Test that include_role digs deeper than import_role.""" - runner = Runner(playbook_path, rules=default_rules_collection) + runner = Runner( + playbook_path, rules=default_rules_collection, skip_list=["fqcn-builtins"] + ) results = runner.run() for message in messages: assert message in str(results) + # Ensure no other unexpected messages are present + assert len(messages) == len(results), results diff --git a/test/test_transform_mixin.py b/test/test_transform_mixin.py new file mode 100644 index 0000000000..e14cf22dee --- /dev/null +++ b/test/test_transform_mixin.py @@ -0,0 +1,128 @@ +"""Tests for TransformMixin.""" + +from typing import Any, Dict, List, MutableMapping, MutableSequence, Type, Union + +import pytest + +from ansiblelint.rules import TransformMixin + +DUMMY_MAP: Dict[str, Any] = { + "foo": "text", + "bar": {"some": "text2"}, + "fruits": ["apple", "orange"], + "answer": [{"forty-two": ["life", "universe", "everything"]}], +} +DUMMY_LIST: List[Dict[str, Any]] = [ + {"foo": "text"}, + {"bar": {"some": "text2"}, "fruits": ["apple", "orange"]}, + {"answer": [{"forty-two": ["life", "universe", "everything"]}]}, +] + + +@pytest.mark.parametrize( + ("yaml_path", "data", "expected_error"), + ( + ([0], DUMMY_MAP, KeyError), + (["bar", 0], DUMMY_MAP, KeyError), + (["fruits", 100], DUMMY_MAP, IndexError), + (["answer", 1], DUMMY_MAP, IndexError), + (["answer", 0, 42], DUMMY_MAP, KeyError), + (["answer", 0, "42"], DUMMY_MAP, KeyError), + ([100], DUMMY_LIST, IndexError), + ([0, 0], DUMMY_LIST, KeyError), + ([0, "wrong key"], DUMMY_LIST, KeyError), + ([1, "bar", "wrong key"], DUMMY_LIST, KeyError), + ([1, "fruits", "index should be int"], DUMMY_LIST, TypeError), + ([1, "fruits", 100], DUMMY_LIST, IndexError), + ), +) +def test_seek_with_bad_path( + yaml_path: List[Union[int, str]], + data: Union[MutableMapping[str, Any], MutableSequence[Any], str], + expected_error: Type[Exception], +) -> None: + """Verify that TransformMixin.seek() propagates errors.""" + with pytest.raises(expected_error): + TransformMixin.seek(yaml_path, data) + + +@pytest.mark.parametrize( + ("yaml_path", "data", "expected"), + ( + ([], DUMMY_MAP, DUMMY_MAP), + (["foo"], DUMMY_MAP, DUMMY_MAP["foo"]), + (["bar"], DUMMY_MAP, DUMMY_MAP["bar"]), + (["bar", "some"], DUMMY_MAP, DUMMY_MAP["bar"]["some"]), + (["fruits"], DUMMY_MAP, DUMMY_MAP["fruits"]), + (["fruits", 0], DUMMY_MAP, DUMMY_MAP["fruits"][0]), + (["fruits", 1], DUMMY_MAP, DUMMY_MAP["fruits"][1]), + (["answer"], DUMMY_MAP, DUMMY_MAP["answer"]), + (["answer", 0], DUMMY_MAP, DUMMY_MAP["answer"][0]), + (["answer", 0, "forty-two"], DUMMY_MAP, DUMMY_MAP["answer"][0]["forty-two"]), + ( + ["answer", 0, "forty-two", 0], + DUMMY_MAP, + DUMMY_MAP["answer"][0]["forty-two"][0], + ), + ( + ["answer", 0, "forty-two", 1], + DUMMY_MAP, + DUMMY_MAP["answer"][0]["forty-two"][1], + ), + ( + ["answer", 0, "forty-two", 2], + DUMMY_MAP, + DUMMY_MAP["answer"][0]["forty-two"][2], + ), + ([], DUMMY_LIST, DUMMY_LIST), + ([0], DUMMY_LIST, DUMMY_LIST[0]), + ([0, "foo"], DUMMY_LIST, DUMMY_LIST[0]["foo"]), + ([1], DUMMY_LIST, DUMMY_LIST[1]), + ([1, "bar"], DUMMY_LIST, DUMMY_LIST[1]["bar"]), + ([1, "bar", "some"], DUMMY_LIST, DUMMY_LIST[1]["bar"]["some"]), + ([1, "fruits"], DUMMY_LIST, DUMMY_LIST[1]["fruits"]), + ([1, "fruits", 0], DUMMY_LIST, DUMMY_LIST[1]["fruits"][0]), + ([1, "fruits", 1], DUMMY_LIST, DUMMY_LIST[1]["fruits"][1]), + ([2], DUMMY_LIST, DUMMY_LIST[2]), + ([2, "answer"], DUMMY_LIST, DUMMY_LIST[2]["answer"]), + ([2, "answer", 0], DUMMY_LIST, DUMMY_LIST[2]["answer"][0]), + ( + [2, "answer", 0, "forty-two"], + DUMMY_LIST, + DUMMY_LIST[2]["answer"][0]["forty-two"], + ), + ( + [2, "answer", 0, "forty-two", 0], + DUMMY_LIST, + DUMMY_LIST[2]["answer"][0]["forty-two"][0], + ), + ( + [2, "answer", 0, "forty-two", 1], + DUMMY_LIST, + DUMMY_LIST[2]["answer"][0]["forty-two"][1], + ), + ( + [2, "answer", 0, "forty-two", 2], + DUMMY_LIST, + DUMMY_LIST[2]["answer"][0]["forty-two"][2], + ), + ( + [], + "this is a string that should be returned as is, ignoring path.", + "this is a string that should be returned as is, ignoring path.", + ), + ( + [2, "answer", 0, "forty-two", 2], + "this is a string that should be returned as is, ignoring path.", + "this is a string that should be returned as is, ignoring path.", + ), + ), +) +def test_seek( + yaml_path: List[Union[int, str]], + data: Union[MutableMapping[str, Any], MutableSequence[Any], str], + expected: Any, +) -> None: + """Ensure TransformMixin.seek() retrieves the correct data.""" + actual = TransformMixin.seek(yaml_path, data) + assert actual == expected diff --git a/test/test_yaml_utils.py b/test/test_yaml_utils.py index 9f07c8ae88..53c0539ae3 100644 --- a/test/test_yaml_utils.py +++ b/test/test_yaml_utils.py @@ -1,9 +1,10 @@ """Tests for yaml-related utility functions.""" from io import StringIO from pathlib import Path -from typing import Any, Optional, Tuple +from typing import Any, List, Optional, Tuple, Union import pytest +from ruamel.yaml.comments import CommentedMap, CommentedSeq from ruamel.yaml.emitter import Emitter from ruamel.yaml.main import YAML @@ -212,6 +213,7 @@ def fixture_yaml_formatting_fixtures(fixture_filename: str) -> Tuple[str, str, s ( "fmt-1.yml", "fmt-2.yml", + "fmt-3.yml", ), ) def test_formatted_yaml_loader_dumper( @@ -245,3 +247,634 @@ def test_formatted_yaml_loader_dumper( # # Instead, `pytest --regenerate-formatting-fixtures` will fail if prettier would # change any files in test/fixtures/formatting-after + + +@pytest.fixture(name="lintable") +def fixture_lintable(file_path: str) -> Lintable: + """Return a playbook Lintable for use in ``get_path_to_*`` tests.""" + return Lintable(file_path) + + +@pytest.fixture(name="ruamel_data") +def fixture_ruamel_data(lintable: Lintable) -> Union[CommentedMap, CommentedSeq]: + """Return the loaded YAML data for the Lintable.""" + yaml = ansiblelint.yaml_utils.FormattedYAML() + data: Union[CommentedMap, CommentedSeq] = yaml.loads(lintable.content) + return data + + +@pytest.mark.parametrize( + ("file_path", "line_number", "expected_path"), + ( + # ignored lintables + pytest.param( + "examples/playbooks/tasks/passing_task.yml", 2, [], id="ignore_tasks_file" + ), + pytest.param( + "examples/roles/more_complex/handlers/main.yml", + 2, + [], + id="ignore_handlers_file", + ), + pytest.param("examples/playbooks/vars/other.yml", 2, [], id="ignore_vars_file"), + pytest.param( + "examples/host_vars/localhost.yml", 2, [], id="ignore_host_vars_file" + ), + pytest.param("examples/group_vars/all.yml", 2, [], id="ignore_group_vars_file"), + pytest.param( + "examples/inventory/inventory.yml", 2, [], id="ignore_inventory_file" + ), + pytest.param( + "examples/roles/dependency_in_meta/meta/main.yml", + 2, + [], + id="ignore_meta_file", + ), + pytest.param( + "examples/reqs_v1/requirements.yml", 2, [], id="ignore_requirements_v1_file" + ), + pytest.param( + "examples/reqs_v2/requirements.yml", 2, [], id="ignore_requirements_v2_file" + ), + # we don't have any release notes examples. Oh well. + # pytest.param("examples/", 2, [], id="ignore_release_notes_file"), + pytest.param( + ".pre-commit-config.yaml", 2, [], id="ignore_unrecognized_yaml_file" + ), + # playbook lintables + pytest.param( + "examples/playbooks/become.yml", + 1, + [], + id="1_play_playbook-line_before_play", + ), + pytest.param( + "examples/playbooks/become.yml", + 2, + [0], + id="1_play_playbook-first_line_in_play", + ), + pytest.param( + "examples/playbooks/become.yml", + 10, + [0], + id="1_play_playbook-middle_line_in_play", + ), + pytest.param( + "examples/playbooks/become.yml", + 100, + [0], + id="1_play_playbook-line_after_eof", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 1, + [], + id="4_play_playbook-line_before_play_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 2, + [0], + id="4_play_playbook-first_line_in_play_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 5, + [0], + id="4_play_playbook-middle_line_in_play_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 9, + [0], + id="4_play_playbook-last_line_in_play_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 10, + [1], + id="4_play_playbook-first_line_in_play_2", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 14, + [1], + id="4_play_playbook-middle_line_in_play_2", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 18, + [1], + id="4_play_playbook-last_line_in_play_2", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 19, + [2], + id="4_play_playbook-first_line_in_play_3", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 23, + [2], + id="4_play_playbook-middle_line_in_play_3", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 27, + [2], + id="4_play_playbook-last_line_in_play_3", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 28, + [3], + id="4_play_playbook-first_line_in_play_4", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 31, + [3], + id="4_play_playbook-middle_line_in_play_4", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 35, + [3], + id="4_play_playbook-last_line_in_play_4", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 100, + [3], + id="4_play_playbook-line_after_eof", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 1, + [], + id="import_playbook-line_before_play_1", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 2, + [0], + id="import_playbook-first_line_in_play_1", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 3, + [0], + id="import_playbook-middle_line_in_play_1", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 4, + [0], + id="import_playbook-last_line_in_play_1", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 5, + [1], + id="import_playbook-first_line_in_play_2", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 6, + [1], + id="import_playbook-middle_line_in_play_2", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 7, + [1], + id="import_playbook-last_line_in_play_2", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 8, + [2], + id="import_playbook-first_line_in_play_3", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 9, + [2], + id="import_playbook-last_line_in_play_3", + ), + pytest.param( + "examples/playbooks/playbook-parent.yml", + 15, + [2], + id="import_playbook-line_after_eof", + ), + ), +) +def test_get_path_to_play( + lintable: Lintable, + line_number: int, + ruamel_data: Union[CommentedMap, CommentedSeq], + expected_path: List[Union[int, str]], +) -> None: + """Ensure ``get_path_to_play`` returns the expected path given a file + line.""" + path_to_play = ansiblelint.yaml_utils.get_path_to_play( + lintable, line_number, ruamel_data + ) + assert path_to_play == expected_path + + +@pytest.mark.parametrize( + ("file_path", "line_number", "expected_path"), + ( + # ignored lintables + pytest.param("examples/playbooks/vars/other.yml", 2, [], id="ignore_vars_file"), + pytest.param( + "examples/host_vars/localhost.yml", 2, [], id="ignore_host_vars_file" + ), + pytest.param("examples/group_vars/all.yml", 2, [], id="ignore_group_vars_file"), + pytest.param( + "examples/inventory/inventory.yml", 2, [], id="ignore_inventory_file" + ), + pytest.param( + "examples/roles/dependency_in_meta/meta/main.yml", + 2, + [], + id="ignore_meta_file", + ), + pytest.param( + "examples/reqs_v1/requirements.yml", 2, [], id="ignore_requirements_v1_file" + ), + pytest.param( + "examples/reqs_v2/requirements.yml", 2, [], id="ignore_requirements_v2_file" + ), + # we don't have any release notes examples. Oh well. + # pytest.param("examples/", 2, [], id="ignore_release_notes_file"), + pytest.param( + ".pre-commit-config.yaml", 2, [], id="ignore_unrecognized_yaml_file" + ), + # tasks-containing lintables + pytest.param( + "examples/playbooks/become.yml", + 4, + [], + id="1_task_playbook-line_before_tasks", + ), + pytest.param( + "examples/playbooks/become.yml", + 5, + [0, "tasks", 0], + id="1_task_playbook-first_line_in_task_1", + ), + pytest.param( + "examples/playbooks/become.yml", + 10, + [0, "tasks", 0], + id="1_task_playbook-middle_line_in_task_1", + ), + pytest.param( + "examples/playbooks/become.yml", + 15, + [0, "tasks", 0], + id="1_task_playbook-last_line_in_task_1", + ), + pytest.param( + "examples/playbooks/become.yml", + 100, + [0, "tasks", 0], + id="1_task_playbook-line_after_eof_without_anything_after_task", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 1, + [], + id="4_play_playbook-play_1_line_before_tasks", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 7, + [0, "tasks", 0], + id="4_play_playbook-play_1_first_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 9, + [0, "tasks", 0], + id="4_play_playbook-play_1_last_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 10, + [], + id="4_play_playbook-play_2_line_before_tasks", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 12, + [], + id="4_play_playbook-play_2_line_before_tasks", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 13, + [1, "tasks", 0], + id="4_play_playbook-play_2_first_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 18, + [1, "tasks", 0], + id="4_play_playbook-play_2_middle_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 18, + [1, "tasks", 0], + id="4_play_playbook-play_2_last_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 19, + [], + id="4_play_playbook-play_3_line_before_tasks", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 22, + [], + id="4_play_playbook-play_3_line_before_tasks", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 23, + [2, "tasks", 0], + id="4_play_playbook-play_3_first_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 25, + [2, "tasks", 0], + id="4_play_playbook-play_3_middle_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 27, + [2, "tasks", 0], + id="4_play_playbook-play_3_last_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 28, + [], + id="4_play_playbook-play_4_line_before_tasks", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 31, + [], + id="4_play_playbook-play_4_line_before_tasks", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 32, + [3, "tasks", 0], + id="4_play_playbook-play_4_first_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 33, + [3, "tasks", 0], + id="4_play_playbook-play_4_middle_line_task_1", + ), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 35, + [3, "tasks", 0], + id="4_play_playbook-play_4_last_line_task_1", + ), + # playbook with multiple tasks + tasks blocks in a play + pytest.param( + # must have at least one key after one of the tasks blocks + "examples/playbooks/include.yml", + 6, + [0, "pre_tasks", 0], + id="playbook-multi_tasks_blocks-pre_tasks_last_task_before_roles", + ), + pytest.param( + "examples/playbooks/include.yml", + 7, + [], + id="playbook-multi_tasks_blocks-roles_after_pre_tasks", + ), + pytest.param( + "examples/playbooks/include.yml", + 10, + [], + id="playbook-multi_tasks_blocks-roles_before_tasks", + ), + pytest.param( + "examples/playbooks/include.yml", + 12, + [0, "tasks", 0], + id="playbook-multi_tasks_blocks-tasks_first_task", + ), + pytest.param( + "examples/playbooks/include.yml", + 14, + [0, "tasks", 1], + id="playbook-multi_tasks_blocks-tasks_last_task_before_handlers", + ), + pytest.param( + "examples/playbooks/include.yml", + 16, + [0, "handlers", 0], + id="playbook-multi_tasks_blocks-handlers_task", + ), + # playbook with subtasks blocks + pytest.param( + "examples/playbooks/blockincludes.yml", + 13, + [0, "tasks", 0, "block", 1, "block", 0, "block", 1, "block", 0], + id="playbook-deeply_nested_task", + ), + pytest.param( + "examples/playbooks/block.yml", + 12, + [0, "tasks", 0, "block", 1], + id="playbook-subtasks-block_task_2", + ), + pytest.param( + "examples/playbooks/block.yml", + 22, + [0, "tasks", 0, "rescue", 2], + id="playbook-subtasks-rescue_task_3", + ), + pytest.param( + "examples/playbooks/block.yml", + 25, + [0, "tasks", 0, "always", 0], + id="playbook-subtasks-always_task_3", + ), + # tasks files + pytest.param("examples/playbooks/tasks/x.yml", 2, [0], id="tasks-null_task"), + pytest.param( + "examples/playbooks/tasks/x.yml", 6, [1], id="tasks-null_task_next" + ), + pytest.param( + "examples/playbooks/tasks/empty_blocks.yml", + 7, + [0], # this IS part of the first task and "rescue" does not have subtasks. + id="tasks-null_rescue", + ), + pytest.param( + "examples/playbooks/tasks/empty_blocks.yml", + 8, + [0], # this IS part of the first task and "always" does not have subtasks. + id="tasks-empty_always", + ), + pytest.param( + "examples/playbooks/tasks/empty_blocks.yml", + 16, + [1, "always", 0], + id="tasks-task_beyond_empty_blocks", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 1, + [], + id="tasks-line_before_tasks", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 2, + [0], + id="tasks-first_line_in_task_1", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 3, + [0], + id="tasks-middle_line_in_task_1", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 4, + [0], + id="tasks-last_line_in_task_1", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 5, + [1], + id="tasks-first_line_in_task_2", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 6, + [1], + id="tasks-middle_line_in_task_2", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 7, + [1], + id="tasks-last_line_in_task_2", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 8, + [2], + id="tasks-first_line_in_task_3", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 9, + [2], + id="tasks-last_line_in_task_3", + ), + pytest.param( + "examples/roles/more_complex/tasks/main.yml", + 100, + [2], + id="tasks-line_after_eof", + ), + # handlers + pytest.param( + "examples/roles/more_complex/handlers/main.yml", + 1, + [], + id="handlers-line_before_tasks", + ), + pytest.param( + "examples/roles/more_complex/handlers/main.yml", + 2, + [0], + id="handlers-first_line_in_task_1", + ), + pytest.param( + "examples/roles/more_complex/handlers/main.yml", + 3, + [0], + id="handlers-last_line_in_task_1", + ), + pytest.param( + "examples/roles/more_complex/handlers/main.yml", + 100, + [0], + id="handlers-line_after_eof", + ), + ), +) +def test_get_path_to_task( + lintable: Lintable, + line_number: int, + ruamel_data: Union[CommentedMap, CommentedSeq], + expected_path: List[Union[int, str]], +) -> None: + """Ensure ``get_task_to_play`` returns the expected path given a file + line.""" + path_to_task = ansiblelint.yaml_utils.get_path_to_task( + lintable, line_number, ruamel_data + ) + assert path_to_task == expected_path + + +@pytest.mark.parametrize( + ("file_path", "line_number"), + ( + pytest.param("examples/playbooks/become.yml", 0, id="1_play_playbook"), + pytest.param( + "examples/playbooks/become-user-without-become-success.yml", + 0, + id="4_play_playbook", + ), + pytest.param("examples/playbooks/playbook-parent.yml", 0, id="import_playbook"), + pytest.param("examples/playbooks/become.yml", 0, id="1_task_playbook"), + ), +) +def test_get_path_to_play_raises_value_error_for_bad_line_number( + lintable: Lintable, + line_number: int, + ruamel_data: Union[CommentedMap, CommentedSeq], +) -> None: + """Ensure ``get_path_to_play`` raises ValueError for line_number < 1.""" + with pytest.raises( + ValueError, match=f"expected line_number >= 1, got {line_number}" + ): + ansiblelint.yaml_utils.get_path_to_play(lintable, line_number, ruamel_data) + + +@pytest.mark.parametrize( + ("file_path", "line_number"), + (pytest.param("examples/roles/more_complex/tasks/main.yml", 0, id="tasks"),), +) +def test_get_path_to_task_raises_value_error_for_bad_line_number( + lintable: Lintable, + line_number: int, + ruamel_data: Union[CommentedMap, CommentedSeq], +) -> None: + """Ensure ``get_task_to_play`` raises ValueError for line_number < 1.""" + with pytest.raises( + ValueError, match=f"expected line_number >= 1, got {line_number}" + ): + ansiblelint.yaml_utils.get_path_to_task(lintable, line_number, ruamel_data)