Skip to content

Commit

Permalink
Relax loop-var-prefix checking
Browse files Browse the repository at this point in the history
Fixes: #2838
  • Loading branch information
ssbarnea committed Feb 15, 2023
1 parent cacaa9a commit 5cc0e77
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .ansible-lint
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ mock_roles:
- fake_namespace.fake_collection.fake_role # role within a collection

# Enable checking of loop variable prefixes in roles
loop_var_prefix: "{role}_"
loop_var_prefix: "^(__|{role}_)"

# Enforce variable names to follow pattern below, in addition to Ansible own
# requirements, like avoiding python identifiers. To disable add `var-naming`
Expand Down
10 changes: 10 additions & 0 deletions examples/roles/loop_var_prefix/tasks/pass.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,3 +18,13 @@
- bar
loop_control:
loop_var: loop_var_prefix_item
- name: pass | Using alternative double dunder prefix
block:
- name: pass | That should also pass
ansible.builtin.debug:
var: __some_item
loop:
- foo
- bar
loop_control:
loop_var: __some_item
2 changes: 2 additions & 0 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@

PROFILES = yaml_from_file(Path(__file__).parent / "data" / "profiles.yml")

LOOP_VAR_PREFIX = "^(__|{role}_)"

options = Namespace(
cache_dir=None,
colored=True,
Expand Down
34 changes: 27 additions & 7 deletions src/ansiblelint/rules/loop_var_prefix.md
Original file line number Diff line number Diff line change
@@ -1,16 +1,33 @@
# loop-var-prefix

This rule avoids conflicts with nested looping tasks by configuring a variable prefix with `loop_var`.
Ansible sets `item` as the loop variable.
You can use `loop_var` to specify a prefix for loop variables and ensure they are unique to each task.
This rule avoids conflicts with nested looping tasks by configuring a variable
prefix with `loop_var`. Ansible sets `item` as the loop variable. You can use
`loop_var` to specify a prefix for loop variables and ensure they are unique to
each task.

This rule can produce the following messages:

- `loop-var-prefix[missing]` - Replace any unsafe implicit `item` loop variable by adding `loop_var: <loop_var_prefix>...`.
- `loop-var-prefix[wrong]` - Ensure loop variables start with `<loop_var_prefix>`.
- `loop-var-prefix[missing]` - Replace any unsafe implicit `item` loop variable
by adding `loop_var: <loop_var_prefix>...`.
- `loop-var-prefix[wrong]` - Ensure loop variables start with
`<loop_var_prefix>`.

This is an opt-in rule.
You must enable it in your Ansible-lint configuration as follows:
This rule originates from the [Naming parameters section of Ansible Best
Practices guide][cop314].

## Settings

You can change the behavior of this rule by overriding its default regular
expression used to check loop variable naming. Keep in mind that the `{role}`
part is replaced with the inferred role name when applicable.

```yaml
# .ansible-lint
loop_var_prefix: "^(__|{role}_)"
```

This is an opt-in rule. You must enable it in your Ansible-lint configuration as
follows:

```yaml
enable_list:
Expand Down Expand Up @@ -56,3 +73,6 @@ enable_list:
loop_control:
loop_var: my_prefix # <- Specifies a unique prefix for loop variables.
```

[cop314]:
https://redhat-cop.github.io/automation-good-practices/#_naming_parameters
19 changes: 10 additions & 9 deletions src/ansiblelint/rules/loop_var_prefix.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
"""Optional Ansible-lint rule to enforce use of prefix on role loop vars."""
from __future__ import annotations

import re
import sys
from typing import TYPE_CHECKING, Any

from ansiblelint.config import options
from ansiblelint.config import LOOP_VAR_PREFIX, options
from ansiblelint.errors import MatchError
from ansiblelint.rules import AnsibleLintRule
from ansiblelint.text import toidentifier
Expand All @@ -26,7 +27,7 @@ class RoleLoopVarPrefix(AnsibleLintRule):
"""

tags = ["idiom"]
prefix = ""
prefix = re.compile("")
severity = "MEDIUM"

def matchtask(
Expand All @@ -36,9 +37,9 @@ def matchtask(
if not file or not file.role or not options.loop_var_prefix:
return []

self.prefix = options.loop_var_prefix.format(role=toidentifier(file.role))
# self.prefix becomes `loop_var_prefix_` because role name is loop_var_prefix

self.prefix = re.compile(
options.loop_var_prefix.format(role=toidentifier(file.role))
)
has_loop = "loop" in task
for key in task.keys():
if key.startswith("with_"):
Expand All @@ -49,18 +50,18 @@ def matchtask(
loop_var = loop_control.get("loop_var", "")

if loop_var:
if not loop_var.startswith(self.prefix):
if not self.prefix.match(loop_var):
return [
self.create_matcherror(
message=f"Loop variable should start with {self.prefix}",
message=f"Loop variable name does not match /{options.loop_var_prefix}/ regex, where role={toidentifier(file.role)}.",
filename=file,
tag="loop-var-prefix[wrong]",
)
]
else:
return [
self.create_matcherror(
message=f"Replace unsafe implicit `item` loop variable by adding `loop_var: {self.prefix}...`.",
message=f"Replace unsafe implicit `item` loop variable by adding a `loop_var` that is matching /{options.loop_var_prefix}/ regex.",
filename=file,
tag="loop-var-prefix[missing]",
)
Expand Down Expand Up @@ -92,7 +93,7 @@ def test_loop_var_prefix(
) -> None:
"""Test rule matches."""
# Enable checking of loop variable prefixes in roles
options.loop_var_prefix = "{role}_"
options.loop_var_prefix = LOOP_VAR_PREFIX
results = Runner(test_file, rules=default_rules_collection).run()
for result in results:
assert result.rule.id == RoleLoopVarPrefix().id
Expand Down

0 comments on commit 5cc0e77

Please sign in to comment.