Refactor task iterator (part 2) (#3212)

.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

.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

.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')"

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

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

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

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"

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
 

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]

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 = []