Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify VariableHasSpacesRule to check for spaces around filters #2180

Merged
merged 3 commits into from Jun 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions examples/playbooks/var-spacing.yml
Expand Up @@ -8,6 +8,12 @@
- name: good variable format
ansible.builtin.debug:
msg: "Value: {{ good_format }}"
- name: good variable filter format
ansible.builtin.debug:
msg: "{{ good_format | filter }}"
- name: good variable filter format
ansible.builtin.debug:
msg: "Value: {{ good_format | filter }}"
- name: jinja escaping allowed
ansible.builtin.debug:
msg: "{{ '{{' }}"
Expand All @@ -30,6 +36,15 @@
- name: bad variable format
ansible.builtin.debug:
msg: "{{bad_format }}"
- name: bad variable filter format
ansible.builtin.debug:
msg: "{{ bad_format|filter }}"
- name: bad variable filter format
ansible.builtin.debug:
msg: "Value: {{ bad_format |filter }}"
- name: bad variable filter format
ansible.builtin.debug:
msg: "{{ bad_format| filter }}"
- name: not a jinja variable # noqa var-spacing
ansible.builtin.debug:
# spell-checker: disable-next-line
Expand Down
5 changes: 5 additions & 0 deletions examples/playbooks/vars/var-spacing.yml
Expand Up @@ -3,6 +3,8 @@
# ".bad_var_1", ".bad_var_2", ".bad_var_3", ".invalid_multiline_nested_json", ".invalid_nested_json"
good_var_1: "{{ good_format }}"
good_var_2: "Value: {{ good_format }}"
good_var_3: "{{ good_format | filter }}"
good_var_4: "Value: {{ good_format | filter }}"
jinja_escape_1: "{{ '{{' }}"
jinja_escape_2: docker info --format '{{ '{{' }}json .Swarm.LocalNodeState{{ '}}' }}' | tr -d '"'
jinja_whitespace_control: |
Expand All @@ -12,6 +14,9 @@ jinja_whitespace_control: |
bad_var_1: "{{bad_format}}"
bad_var_2: "Value: {{ bad_format}}"
bad_var_3: "{{bad_format }}"
bad_var_4: "{{ bad_format|filter }}"
bad_var_5: "Value: {{ bad_format |filter }}"
bad_var_6: "{{ bad_format| filter }}"
# spell-checker: disable-next-line
non_jinja_var: "data = ${lookup{$local_part}lsearch{/etc/aliases}}" # noqa: var-spacing
json_inside_jinja: "{{ {'test': {'subtest': variable}} }}"
Expand Down
19 changes: 19 additions & 0 deletions src/ansiblelint/rules/var_spacing.md
@@ -0,0 +1,19 @@
## var-spacing

Variables and filters in Jinja2 should have spaces before and after, like
`{{ var_name | filter }}.`. This improves readability and makes it
less likely to introduce typos.

### Problematic code

```yaml
---
foo: "{{some|dict2items}}"
```

### Correct code

```yaml
---
foo: "{{ some | dict2items }}"
```
23 changes: 16 additions & 7 deletions src/ansiblelint/rules/var_spacing.py
Expand Up @@ -19,24 +19,28 @@


class VariableHasSpacesRule(AnsibleLintRule):
"""Variables should have spaces before and after: {{ var_name }}."""
"""Jinja2 variables and filters should have spaces before and after."""

id = "var-spacing"
description = "Variables should have spaces before and after: ``{{ var_name }}``"
severity = "LOW"
tags = ["formatting"]
version_added = "v4.0.0"
version_added = "v6.3.0"

bracket_regex = re.compile(r"{{[^{\n' -]|[^ '\n}-]}}", re.MULTILINE | re.DOTALL)
exclude_json_re = re.compile(r"[^{]{'\w+': ?[^{]{.*?}}", re.MULTILINE | re.DOTALL)
pipe_spacing_regex = re.compile(
r"{{.*(?<=[^ \n\t])[|].*|.*[|](?=[^ \n\t]).*}}", re.MULTILINE | re.DOTALL
)

def matchtask(
self, task: Dict[str, Any], file: Optional[Lintable] = None
) -> Union[bool, str]:
for _, v, _ in nested_items_path(task):
if isinstance(v, str):
cleaned = self.exclude_json_re.sub("", v)
if bool(self.bracket_regex.search(cleaned)):
if bool(self.bracket_regex.search(cleaned)) or bool(
self.pipe_spacing_regex.search(cleaned)
):
return self.shortdesc.format(var_name=v)
return False

Expand All @@ -51,7 +55,9 @@ def matchyaml(self, file: Lintable) -> List["MatchError"]:
for k, v, path in nested_items_path(data):
if isinstance(v, AnsibleUnicode):
cleaned = self.exclude_json_re.sub("", v)
if bool(self.bracket_regex.search(cleaned)):
if bool(self.bracket_regex.search(cleaned)) or bool(
self.pipe_spacing_regex.search(cleaned)
):
path_elem = [
f"[{i}]" if isinstance(i, int) else i for i in path + [k]
]
Expand Down Expand Up @@ -85,7 +91,7 @@ def matchyaml(self, file: Lintable) -> List["MatchError"]:
@pytest.fixture(name="error_expected_lines")
def fixture_error_expected_lines() -> List[int]:
"""Return list of expected error lines."""
return [24, 27, 30, 56, 67]
return [30, 33, 36, 39, 42, 45, 71, 82]

@pytest.fixture(name="test_playbook")
def fixture_test_playbook() -> str:
Expand Down Expand Up @@ -119,14 +125,17 @@ def fixture_error_expected_details_varsfile() -> List[str]:
".bad_var_1",
".bad_var_2",
".bad_var_3",
".bad_var_4",
".bad_var_5",
".bad_var_6",
".invalid_multiline_nested_json",
".invalid_nested_json",
]

@pytest.fixture(name="error_expected_lines_varsfile")
def fixture_error_expected_lines_varsfile() -> List[int]:
"""Return list of expected error lines."""
return [12, 13, 14, 27, 33]
return [14, 15, 16, 17, 18, 19, 32, 38]

@pytest.fixture(name="test_varsfile_path")
def fixture_test_varsfile_path() -> str:
Expand Down
9 changes: 7 additions & 2 deletions test/eco/bootstrap.result
@@ -1,17 +1,22 @@
CMD: ansible-lint -f pep8 -x fqcn-builtins

RC: 0
RC: 2

STDERR:
WARNING Listing 3 violation(s) that are fatal
WARNING Listing 7 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
- var-spacing # Jinja2 variables and filters should have spaces before and after



STDOUT:
.github/workflows/galaxy.yml:1: schema: Additional properties are not allowed ('jobs', 'true' were unexpected) (schema[galaxy])
meta/main.yml:1: schema: 7 is not one of ['6.1', '7.1', '7.2', 'all'] (schema[meta])
requirements.yml:1: schema: None is not of type 'array' (schema[requirements])
vars/main.yml:18: var-spacing: Jinja2 variables and filters should have spaces before and after
vars/main.yml:33: var-spacing: Jinja2 variables and filters should have spaces before and after
vars/main.yml:44: var-spacing: Jinja2 variables and filters should have spaces before and after
vars/main.yml:70: var-spacing: Jinja2 variables and filters should have spaces before and after