Modify VariableHasSpacesRule to check for spaces around filters (#2180)

* Modify VariableHasSpacesRule to check for spaces around |

* More

* Update version_added to reflect new behavior

Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>

examples/playbooks/var-spacing.yml

@@ -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: "{{ '{{' }}"
@@ -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

examples/playbooks/vars/var-spacing.yml

@@ -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: |
@@ -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}} }}"

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 }}"
+```

src/ansiblelint/rules/var_spacing.py

@@ -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
 
@@ -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]
                         ]
@@ -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:
@@ -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:

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