Add optional rule to check for loop var prefix

Based on original custom rule written by Albin Vass on
https://opendev.org/zuul/zuul-jobs/blame/branch/master/.rules/ZuulJobsNamespaceLoopVar.py

This new rule becomes active only when loop_var_prefix is defined
inside the config and allows user to use dynamic prefix based
on role folder name.

.ansible-lint

@@ -4,3 +4,5 @@ mock_modules:
   - zuul_return
 mock_roles:
   - mocked_role
+# Enable checking of loop variable prefixes in roles
+loop_var_prefix: "{role}_"

examples/playbooks/pass-loop-var-prefix.yml

@@ -0,0 +1,17 @@
+- hosts: localhost
+  tasks:
+    # validate that we did not trigger loop-var-prefix on playbooks
+    - name: that should pass
+      debug:
+        var: item
+      loop:
+        - foo
+        - bar
+    - name: a block
+      block:
+        - name: that should also pass
+          debug:
+            var: item
+          loop:
+            - apples
+            - oranges

examples/roles/fail_loop_var_prefix/tasks/main.yml

@@ -0,0 +1,23 @@
+# 3 expected no-loop-var-prefix failures at 2, 8, 18
+- name: that should trigger no-loop-var-prefix
+  debug:
+    var: item
+  loop:
+    - foo
+    - bar
+- name: that should fail due to wrong custom
+  debug:
+    var: zz_item
+  loop:
+    - foo
+    - bar
+  loop_control:
+    loop_var: zz_item
+- name: Using a block
+  block:
+    - name: that should also not pass
+      debug:
+        var: item
+      loop:
+        - apples
+        - oranges

src/ansiblelint/cli.py

@@ -208,7 +208,11 @@ def merge_config(file_config, cli_config: Namespace) -> Namespace:
         'tags': [],
         'warn_list': ['experimental'],
         'mock_modules': [],
-        'mock_roles': []
+        'mock_roles': [],
+    }
+
+    scalar_map = {
+        "loop_var_prefix": None
     }
 
     if not file_config:
@@ -223,6 +227,10 @@ def merge_config(file_config, cli_config: Namespace) -> Namespace:
         x = getattr(cli_config, entry) or file_config.get(entry, False)
         setattr(cli_config, entry, x)
 
+    for entry, default in scalar_map.items():
+        x = getattr(cli_config, entry, None) or file_config.get(entry, default)
+        setattr(cli_config, entry, x)
+
     # if either commandline parameter or config file option is set merge
     # with the other, if neither is set use the default
     for entry, default in lists_map.items():

src/ansiblelint/config.py

@@ -38,5 +38,6 @@
     warn_list=[],
     kinds=DEFAULT_KINDS,
     mock_modules=[],
-    mock_roles=[]
+    mock_roles=[],
+    loop_var_prefix=None
 )

src/ansiblelint/file_utils.py

@@ -106,6 +106,16 @@ def __init__(
             self.path = name
         self._content = content
 
+        # if the lintable is part of a role, we save role folder name
+        self.role = ""
+        parts = self.path.parent.parts
+        if 'roles' in parts:
+            role = self.path
+            while role.parent.name != "roles" and role.name:
+                role = role.parent
+            if role.exists:
+                self.role = role.name
+
         self.kind = kind or kind_from_path(self.path)
         # We store absolute directory in dir
         if self.kind == "role":

src/ansiblelint/rules/RoleLoopVarPrefix.py

@@ -0,0 +1,92 @@
+"""Optional Ansible-lint rule to enforce use of prefix on role loop vars."""
+from typing import TYPE_CHECKING, Any, List
+
+from ansiblelint.config import options
+from ansiblelint.errors import MatchError
+from ansiblelint.file_utils import Lintable
+from ansiblelint.rules import AnsibleLintRule
+from ansiblelint.text import toidentifier
+
+if TYPE_CHECKING:
+    from ansiblelint.constants import odict
+
+
+class RoleLoopVarPrefix(AnsibleLintRule):
+    """Role loop_var should use configured prefix."""
+
+    id = 'no-loop-var-prefix'
+    shortdesc = __doc__
+    url = "https://docs.ansible.com/ansible/latest/user_guide/playbooks_loops.html#defining-inner-and-outer-variable-names-with-loop-var"  # noqa 501
+    description = """\
+Looping inside roles has the risk of clashing with loops from user-playbooks.
+This is recommended to always prefix them.
+
+See: %s
+""" % url
+
+    tags = ['no-loop-var-prefix', 'safety']
+    prefix = ""
+    severity = 'MEDIUM'
+
+    def matchplay(
+            self,
+            file: Lintable,
+            data: "odict[str, Any]") -> List[MatchError]:
+        """Return matches found for a specific playbook."""
+        results: List[MatchError] = []
+
+        if not options.loop_var_prefix:
+            return results
+        self.prefix = options.loop_var_prefix.format(role=toidentifier(file.role))
+        self.shortdesc = f"{self.__class__.shortdesc}: {self.prefix}"
+
+        if file.kind not in ('tasks', 'handlers'):
+            return results
+
+        results.extend(self.handle_play(file, data))
+        return results
+
+    def handle_play(
+            self,
+            lintable: Lintable,
+            task: "odict[str, Any]") -> List[MatchError]:
+        """Return matches for a playlist."""
+        results = []
+        if 'block' in task:
+            results.extend(self.handle_tasks(lintable, task['block']))
+        else:
+            results.extend(self.handle_task(lintable, task))
+        return results
+
+    def handle_tasks(
+            self,
+            lintable: Lintable,
+            tasks: List["odict[str, Any]"]) -> List[MatchError]:
+        """Return matches for a list of tasks."""
+        results = []
+        for play in tasks:
+            results.extend(self.handle_play(lintable, play))
+        return results
+
+    def handle_task(
+            self,
+            lintable: Lintable,
+            task: "odict[str, Any]") -> List[MatchError]:
+        """Return matches for a specific task."""
+        results = []
+        has_loop = 'loop' in task
+        for key in task.keys():
+            if key.startswith('with_'):
+                has_loop = True
+
+        if has_loop:
+            loop_control = task.get('loop_control', {})
+            loop_var = loop_control.get('loop_var', "")
+
+            if not loop_var or not loop_var.startswith(self.prefix):
+                results.append(
+                    self.create_matcherror(
+                        filename=lintable,
+                        linenumber=task['__line__']
+                    ))
+        return results

src/ansiblelint/text.py

@@ -13,3 +13,11 @@ def strip_ansi_escape(data: Union[str, bytes]) -> str:
         data = data.decode("utf-8")
 
     return re.sub(r"\x1b[^m]*m", "", data)
+
+
+def toidentifier(text: str) -> str:
+    """Convert unsafe chars to ones allowed in variables."""
+    result = re.sub(r"[\s-]+", '_', text)
+    if not result.isidentifier:
+        raise RuntimeError("Unable to convert %s to valid variable name.", text)
+    return result