diff --git a/.config/requirements-test.txt b/.config/requirements-test.txt index 5ac970b68f..b18fa187b3 100644 --- a/.config/requirements-test.txt +++ b/.config/requirements-test.txt @@ -12,3 +12,4 @@ pytest-plus >= 0.2 # for PYTEST_REQPASS pytest-xdist >= 2.1.0 spdx-tools >= 0.7.1 # Apache types-pyyaml # IDE support +types-jsonschema # IDE support diff --git a/.config/requirements.txt b/.config/requirements.txt index 38ed89c7c9..85268767f0 100644 --- a/.config/requirements.txt +++ b/.config/requirements.txt @@ -101,6 +101,7 @@ text-unidecode==1.3 tinycss2==1.2.1 tomli==2.0.1 tomlkit==0.11.6 +types-jsonschema==4.17.0.6 types-pyyaml==6.0.12.8 typing-extensions==4.5.0 uritools==4.0.1 diff --git a/.github/workflows/tox.yml b/.github/workflows/tox.yml index 9e72340e19..3af673c9ac 100644 --- a/.github/workflows/tox.yml +++ b/.github/workflows/tox.yml @@ -59,7 +59,7 @@ jobs: WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:GITHUB_STEP_SUMMARY # Number of expected test passes, safety measure for accidental skip of # tests. Update value if you add/remove tests. - PYTEST_REQPASS: 802 + PYTEST_REQPASS: 803 steps: - name: Activate WSL1 if: "contains(matrix.shell, 'wsl')" diff --git a/examples/playbooks/command-check-success.yml b/examples/playbooks/command-check-success.yml index b855b8a4b1..2a8e43dd50 100644 --- a/examples/playbooks/command-check-success.yml +++ b/examples/playbooks/command-check-success.yml @@ -1,5 +1,6 @@ --- -- hosts: localhost +- name: Fixture for no-changed-when + hosts: localhost tasks: - name: Command with creates check ansible.builtin.command: echo blah @@ -15,46 +16,48 @@ ansible.builtin.command: echo blah changed_when: false - - name: Command with inline creates + - name: Command with inline creates # noqa: no-free-form ansible.builtin.command: creates=Z echo blah - - name: Command with inline removes + - name: Command with inline removes # noqa: no-free-form ansible.builtin.command: removes=Z echo blah - - name: Command with cmd + - name: Command with cmd # noqa: fqcn[action-core] command: cmd: echo blah args: creates: Z - - name: Use shell with creates check + - name: Use shell with creates check # noqa: fqcn[action-core] command-instead-of-shell shell: echo blah args: creates: Z - - name: Use shell with removes check + - name: Use shell with removes check # noqa: fqcn[action-core] command-instead-of-shell shell: echo blah args: removes: Z - - name: Use shell with changed_when + - name: Use shell with changed_when # noqa: fqcn[action-core] command-instead-of-shell shell: echo blah changed_when: false - - name: Use shell with inline creates + - name: Use shell with inline creates # noqa: fqcn[action-core] no-free-form command-instead-of-shell shell: creates=Z echo blah - - name: Use shell with inline removes + - name: Use shell with inline removes # noqa: fqcn[action-core] no-free-form command-instead-of-shell shell: removes=Z echo blah - - name: Use shell with cmd + - name: Use shell with cmd # noqa: fqcn[action-core] command-instead-of-shell shell: cmd: echo blah args: creates: Z -- hosts: localhost +- name: Fixture + hosts: localhost handlers: - - name: Restart something + - name: Restart something # noqa: fqcn[action-core] no-changed-when command: do something - - include: handlers/included-handlers.yml + - name: Foo # noqa: fqcn[action-core] deprecated-module + include: handlers/included-handlers.yml diff --git a/examples/playbooks/handlers/included-handlers.yml b/examples/playbooks/handlers/included-handlers.yml index 77e744023b..6dec22dbcf 100644 --- a/examples/playbooks/handlers/included-handlers.yml +++ b/examples/playbooks/handlers/included-handlers.yml @@ -1,6 +1,6 @@ --- -- name: Restart xyz +- name: Restart xyz # noqa: no-free-form fqcn[action-core] service: name=xyz state=restarted # see Issue #165 -- name: Command handler issue 165 +- name: Command handler issue 165 # noqa: fqcn[action-core] no-changed-when command: do something diff --git a/examples/playbooks/rule-var-naming-fail.yml b/examples/playbooks/rule-var-naming-fail.yml index 6d812e9962..2d99fc7deb 100644 --- a/examples/playbooks/rule-var-naming-fail.yml +++ b/examples/playbooks/rule-var-naming-fail.yml @@ -2,11 +2,11 @@ - name: Fixture hosts: localhost vars: - CamelCaseIsBad: false # invalid + CamelCaseIsBad: false # invalid 1 this_is_valid: # valid because content is a dict, not a variable CamelCase: ... ALL_CAPS: ... - ALL_CAPS_ARE_BAD_TOO: ... # invalid + ALL_CAPS_ARE_BAD_TOO: ... # invalid 2 CamelCaseButErrorIgnored: true # noqa: var-naming tasks: @@ -18,15 +18,15 @@ CamelCaseButErrorIgnored: true # noqa: var-naming - name: Test in a block vars: - BAD: false # invalid - MoreBad: ... # invalid + BAD: false # invalid 3 + MoreBad: ... # invalid 4 block: - name: Foo vars: - ALL_CAPS_ARE_BAD_TOO: "{{ MoreBad }}" # invalid + ALL_CAPS_ARE_BAD_TOO: "{{ MoreBad }}" # invalid 5 ansible.builtin.set_fact: - CamelCaseIsBad: "{{ BAD }}" # invalid + CamelCaseIsBad: "{{ BAD }}" # invalid 6 - name: Test on register ansible.builtin.debug: var: test_var - register: CamelCaseIsBad # invalid + register: CamelCaseIsBad # invalid 7 diff --git a/examples/playbooks/task_in_list-0.yml b/examples/playbooks/task_in_list-0.yml new file mode 100644 index 0000000000..7956dbb11f --- /dev/null +++ b/examples/playbooks/task_in_list-0.yml @@ -0,0 +1,28 @@ +--- +- name: Fixture for task_in_list + hosts: localhost + tasks: + - name: A + ansible.builtin.debug: + msg: "A" + - name: B + ansible.builtin.debug: + msg: "C" + pre_tasks: + - name: C + ansible.builtin.debug: + msg: "C" + post_tasks: + - name: D + block: + - name: E + ansible.builtin.debug: + msg: "E" + rescue: + - name: F + ansible.builtin.debug: + msg: "F" + always: + - name: G + ansible.builtin.debug: + msg: "G" diff --git a/pyproject.toml b/pyproject.toml index c3aae1a2ab..d97055c596 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -95,6 +95,7 @@ line_length = 88 [tool.mypy] python_version = 3.9 +strict = true color_output = true error_summary = true disallow_untyped_calls = true @@ -105,10 +106,16 @@ disallow_any_generics = true # warn_return_any = True # warn_unused_configs = True # site-packages is here to help vscode mypy integration getting confused -exclude = "(build|dist|test/local-content|site-packages|~/.pyenv)" +exclude = "(build|dist|test/local-content|site-packages|~/.pyenv|examples/playbooks/collections|plugins/modules)" [[tool.mypy.overrides]] -module = ["ansible.*", "yamllint.*", "ansiblelint._version", "spdx.*"] +module = [ + "ansible.*", + "yamllint.*", + "ansiblelint._version", + "ruamel.yaml", + "spdx.*" +] ignore_missing_imports = true ignore_errors = true diff --git a/src/ansiblelint/rules/__init__.py b/src/ansiblelint/rules/__init__.py index db95f22a4c..659bb6e976 100644 --- a/src/ansiblelint/rules/__init__.py +++ b/src/ansiblelint/rules/__init__.py @@ -155,7 +155,9 @@ def matchtasks(self, file: Lintable) -> list[MatchError]: # noqa: C901 ): return matches - for task in ansiblelint.yaml_utils.iter_tasks_in_file(file): + for task in ansiblelint.utils.task_in_list( + data=file.data, kind=file.kind, filename=str(file.path) + ): if task.error is not None: # normalize_task converts AnsibleParserError to MatchError return [task.error] diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 7261d86db2..7515526f8e 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -27,7 +27,8 @@ import os import re from argparse import Namespace -from collections.abc import ItemsView, Mapping, Sequence +from collections.abc import ItemsView, Iterator, Mapping, Sequence +from dataclasses import _MISSING_TYPE, dataclass, field from functools import lru_cache from pathlib import Path from typing import Any, Callable @@ -665,6 +666,115 @@ def extract_from_list( return results +@dataclass +class Task: + """Class that represents a task from linter point of view. + + raw_task: + When looping through the tasks in the file, each "raw_task" is minimally + processed to include these special keys: __line__, __file__, skipped_rules. + normalized_task: + When each raw_task is "normalized", action shorthand (strings) get parsed + by ansible into python objects and the action key gets normalized. If the task + should be skipped (skipped is True) or normalizing it fails (error is not None) + then this is just the raw_task instead of a normalized copy. + skip_tags: + List of tags found to be skipped, from tags block or noqa comments + error: + This is normally None. It will be a MatchError when the raw_task cannot be + normalized due to an AnsibleParserError. + position: Any + """ + + raw_task: dict[str, Any] + filename: str = "" + _normalized_task: dict[str, Any] | _MISSING_TYPE = field(init=False, repr=False) + error: MatchError | None = None + position: Any = None + + @property + def name(self) -> str | None: + """Return the name of the task.""" + return self.raw_task.get("name", None) + + @property + def normalized_task(self) -> dict[str, Any]: + """Return the name of the task.""" + if not hasattr(self, "_normalized_task"): + try: + self._normalized_task = normalize_task( + self.raw_task, filename=self.filename + ) + except MatchError as err: + self.error = err + # When we cannot normalize it, we just use the raw task instead + # to avoid adding extra complexity to the rules. + self._normalized_task = self.raw_task + if isinstance(self._normalized_task, _MISSING_TYPE): + raise RuntimeError("Task was not normalized") + return self._normalized_task + + @property + def skip_tags(self) -> list[str]: + """Return the list of tags to skip.""" + skip_tags: list[str] = self.raw_task.get(SKIPPED_RULES_KEY, []) + return skip_tags + + def __repr__(self) -> str: + """Return a string representation of the task.""" + return f"Task('{self.name}' [{self.position}])" + + +def task_in_list( + data: AnsibleBaseYAMLObject, + filename: str, + kind: str, + position: str = ".", +) -> Iterator[Task]: + """Get action tasks from block structures.""" + + def each_entry(data: AnsibleBaseYAMLObject, position: str) -> Iterator[Task]: + if not data: + return + for entry_index, entry in enumerate(data): + if not entry: + continue + _pos = f"{position}[{entry_index}]" + if isinstance(entry, dict): + yield Task( + entry, + position=_pos, + ) + for block in [k for k in entry if k in NESTED_TASK_KEYS]: + yield from task_in_list( + data=entry[block], + filename=filename, + kind="tasks", + position=f"{_pos}.{block}", + ) + + if not isinstance(data, list): + return + if kind == "playbook": + attributes = ["tasks", "pre_tasks", "post_tasks", "handlers"] + for item_index, item in enumerate(data): + for attribute in attributes: + if not isinstance(item, dict): + continue + if attribute in item: + if isinstance(item[attribute], list): + yield from each_entry( + item[attribute], + f"{position }[{item_index}].{attribute}", + ) + elif item[attribute] is not None: + raise RuntimeError( + f"Key '{attribute}' defined, but bad value: '{str(item[attribute])}'" + ) + else: + yield from each_entry(data, position) + + def add_action_type(actions: AnsibleBaseYAMLObject, action_type: str) -> list[Any]: """Add action markers to task objects.""" results = [] diff --git a/src/ansiblelint/yaml_utils.py b/src/ansiblelint/yaml_utils.py index 42721f075a..e0f7c0fc09 100644 --- a/src/ansiblelint/yaml_utils.py +++ b/src/ansiblelint/yaml_utils.py @@ -7,7 +7,6 @@ import os import re from collections.abc import Iterator, Sequence -from dataclasses import dataclass from io import StringIO from re import Pattern from typing import TYPE_CHECKING, Any, Callable, Union, cast @@ -32,9 +31,7 @@ PLAYBOOK_TASK_KEYWORDS, SKIPPED_RULES_KEY, ) -from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable -from ansiblelint.utils import get_action_tasks, normalize_task if TYPE_CHECKING: # noinspection PyProtectedMember @@ -109,42 +106,6 @@ def load_yamllint_config() -> YamlLintConfig: return config -def iter_tasks_in_file( - lintable: Lintable, -) -> Iterator[Task]: - """Iterate over tasks in file. - - :param lintable: The playbook or tasks/handlers yaml file to get tasks from - - Yields a Task object - """ - data = lintable.data - if not data: - return - - raw_tasks = get_action_tasks(data, lintable) - - for raw_task in raw_tasks: - err: MatchError | None = None - - skip_tags: list[str] = raw_task.get(SKIPPED_RULES_KEY, []) - - try: - normalized_task = normalize_task(raw_task, str(lintable.path)) - except MatchError as err: - # normalize_task converts AnsibleParserError to MatchError - yield Task(raw_task, raw_task, skip_tags, err) - return - - if "skip_ansible_lint" in raw_task.get("tags", []): - skip_tags.append("skip_ansible_lint") - if skip_tags: - yield Task(raw_task, normalized_task, skip_tags, None) - continue - - yield Task(raw_task, normalized_task, skip_tags, None) - - def nested_items_path( data_collection: dict[Any, Any] | list[Any], ignored_keys: Sequence[str] = (), @@ -1120,28 +1081,3 @@ def clean_json( # neither a dict nor a list, do nothing pass return obj - - -@dataclass -class Task: - """Class that represents a task from linter point of view. - - raw_task: - When looping through the tasks in the file, each "raw_task" is minimally - processed to include these special keys: __line__, __file__, skipped_rules. - normalized_task: - When each raw_task is "normalized", action shorthand (strings) get parsed - by ansible into python objects and the action key gets normalized. If the task - should be skipped (skipped is True) or normalizing it fails (error is not None) - then this is just the raw_task instead of a normalized copy. - skip_tags: - List of tags found to be skipped, from tags block or noqa comments - error: - This is normally None. It will be a MatchError when the raw_task cannot be - normalized due to an AnsibleParserError. - """ - - raw_task: tuple[dict[str, Any]] - normalized_task: dict[str, Any] - skip_tags: list[str] - error: MatchError | None = None diff --git a/test/test_utils.py b/test/test_utils.py index 5fee3a2a4f..102834bb81 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -387,3 +387,35 @@ def test_find_children_in_task(default_rules_collection: RulesCollection) -> Non Lintable("examples/playbooks/tasks/bug-2875.yml"), rules=default_rules_collection, ).run() + + +@pytest.mark.parametrize( + ("file", "names", "positions"), + ( + pytest.param( + "examples/playbooks/task_in_list-0.yml", + ["A", "B", "C", "D", "E", "F", "G"], + [ + ".[0].tasks[0]", + ".[0].tasks[1]", + ".[0].pre_tasks[0]", + ".[0].post_tasks[0]", + ".[0].post_tasks[0].block[0]", + ".[0].post_tasks[0].rescue[0]", + ".[0].post_tasks[0].always[0]", + ], + id="0", + ), + ), +) +def test_task_in_list(file: str, names: list[str], positions: list[str]) -> None: + """Check that tasks get extracted from blocks if present.""" + lintable = Lintable(file) + assert lintable.kind + tasks = list( + utils.task_in_list(data=lintable.data, filename=file, kind=lintable.kind) + ) + assert len(tasks) == len(names) + for index, task in enumerate(tasks): + assert task.name == names[index] + assert task.position == positions[index] diff --git a/test/test_yaml_utils.py b/test/test_yaml_utils.py index 8af486b2c5..567cabb276 100644 --- a/test/test_yaml_utils.py +++ b/test/test_yaml_utils.py @@ -13,6 +13,7 @@ import ansiblelint.yaml_utils from ansiblelint.file_utils import Lintable +from ansiblelint.utils import task_in_list fixtures_dir = Path(__file__).parent / "fixtures" formatting_before_fixtures_dir = fixtures_dir / "formatting-before" @@ -27,9 +28,17 @@ def fixture_empty_lintable() -> Lintable: return lintable -def test_iter_tasks_in_file_with_empty_file(empty_lintable: Lintable) -> None: - """Make sure that iter_tasks_in_file returns early when files are empty.""" - res = list(ansiblelint.yaml_utils.iter_tasks_in_file(empty_lintable)) +def test_tasks_in_list_empty_file(empty_lintable: Lintable) -> None: + """Make sure that task_in_list returns early when files are empty.""" + assert empty_lintable.kind + assert empty_lintable.path + res = list( + task_in_list( + data=empty_lintable, + filename=str(empty_lintable.path), + kind=empty_lintable.kind, + ) + ) assert not res