Replace no-jinja-nesting with jinja[invalid] (#2436)

- Modifies jinja rule to also perform the validity testing that was
  previously part of no-jinja-nesting rule
- Remove no-jinja-nesting
- New implementation is far better as it relies on Ansible own
  templating instead of the old regex approach, which was prone to
  false-positives.

.github/workflows/tox.yml

@@ -166,7 +166,7 @@ jobs:
       WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 707
+      PYTEST_REQPASS: 697
 
     steps:
       - name: Activate WSL1

examples/playbooks/rule-jinja-invalid.yml

@@ -0,0 +1,9 @@
+---
+- name: Fixture
+  hosts: localhost
+  tasks:
+    - name: Foo
+      ansible.builtin.debug:
+        msg: "{{ {{ '1' }} }}" # <-- jinja2[invalid]
+# It should be noted that even ansible --syntax-check fails to spot the jinja
+# error above, but ansible will throw a runtime error when running

src/ansiblelint/__main__.py

@@ -170,6 +170,7 @@ def _do_transform(result: LintResult, opts: Namespace) -> None:
     transformer.run()
 
 
+# pylint: disable=too-many-branches
 def main(argv: list[str] | None = None) -> int:  # noqa: C901
     """Linter CLI entry point."""
     # alter PATH if needed (venv support)
@@ -253,6 +254,11 @@ def main(argv: list[str] | None = None) -> int:  # noqa: C901
     if options.cache_dir_lock:
         options.cache_dir_lock.release()
         pathlib.Path(options.cache_dir_lock.lock_file).unlink(missing_ok=True)
+    if options.mock_filters:
+        _logger.warning(
+            "The following filters were mocked during the run: %s",
+            ",".join(options.mock_filters),
+        )
 
     return app.report_outcome(result, mark_as_success=mark_as_success)
 

src/ansiblelint/config.py

@@ -115,6 +115,7 @@
     verbosity=False,
     warn_list=[],
     kinds=DEFAULT_KINDS,
+    mock_filters=[],
     mock_modules=[],
     mock_roles=[],
     loop_var_prefix=None,

src/ansiblelint/constants.py

@@ -77,7 +77,7 @@ def main():
     "203": "no-tabs",
     "205": "playbook-extension",
     "206": "jinja[spacing]",
-    "207": "no-jinja-nesting",
+    "207": "jinja[invalid]",
     "208": "risky-file-permissions",
     "301": "no-changed-when",
     "302": "deprecated-command-syntax",
@@ -105,6 +105,7 @@ def main():
     "unnamed-task": "name[missing]",
     "git-latest": "latest[git]",
     "hg-latest": "latest[hg]",
+    "no-jinja-nesting": "jinja[invalid]",
 }
 
 PLAYBOOK_TASK_KEYWORDS = [

src/ansiblelint/data/profiles.yml

@@ -29,7 +29,6 @@ basic:
     inline-env-var:
     key-order:
     literal-compare:
-    no-jinja-nesting:
     jinja:
     no-jinja-when:
     no-tabs:

src/ansiblelint/rules/jinja.md

@@ -6,7 +6,9 @@ version can report:
 - `jinja[spacing]` when there are no spaces between variables
   and operators, including filters, like `{{ var_name | filter }}`. This
   improves readability and makes it less likely to introduce typos.
-- `jinja[invalid]` when the jinja2 template is invalid
+- `jinja[invalid]` when the jinja2 template is invalid, like `{{ {{ '1' }} }}`,
+  which would result in a runtime error if you try to use it with Ansible, even
+  if it does pass the Ansible syntax check.
 
 As jinja2 syntax is closely following Python one we aim to follow
 [black](https://black.readthedocs.io/en/stable/) formatting rules. If you are

src/ansiblelint/rules/jinja.py

@@ -8,12 +8,14 @@
 
 import black
 import jinja2
+from ansible.errors import AnsibleError
 from ansible.parsing.yaml.objects import AnsibleUnicode
+from yaml.representer import RepresenterError
 
 from ansiblelint.file_utils import Lintable
 from ansiblelint.rules import AnsibleLintRule
 from ansiblelint.skip_utils import get_rule_skips_from_line
-from ansiblelint.utils import LINE_NUMBER_KEY, parse_yaml_from_file
+from ansiblelint.utils import LINE_NUMBER_KEY, parse_yaml_from_file, template
 from ansiblelint.yaml_utils import nested_items_path
 
 if TYPE_CHECKING:
@@ -49,6 +51,22 @@ def matchtask(
     ) -> bool | str | MatchError:
         for key, v, _ in nested_items_path(task):
             if isinstance(v, str):
+
+                try:
+                    template(
+                        basedir=file.dir if file else ".",
+                        value=v,
+                        variables={},
+                        fail_on_error=True,
+                    )
+                except (AnsibleError, ValueError, RepresenterError) as exc:
+                    return self.create_matcherror(
+                        message=str(exc),
+                        linenumber=task[LINE_NUMBER_KEY],
+                        filename=file,
+                        tag=f"{self.id}[invalid]",
+                    )
+
                 reformatted, details, tag = self.check_whitespace(v, key=key)
                 if reformatted != v:
                     return self.create_matcherror(
@@ -455,11 +473,24 @@ def test_jinja_spacing_vars() -> None:
             pytest.param("{{ item.0.user }}", "{{ item.0.user }}", "spacing", id="39"),
             # Not supported by back, while jinja allows ~ to be binary operator:
             pytest.param("{{ a ~ b }}", "{{ a ~ b }}", "spacing", id="40"),
+            pytest.param(
+                "--format='{{'{{'}}.Size{{'}}'}}'",
+                "--format='{{ '{{' }}.Size{{ '}}' }}'",
+                "spacing",
+                id="41",
+            ),
+            pytest.param(
+                "{{ list_one + {{ list_two | max }} }}",
+                "{{ list_one + {{list_two | max}} }}",
+                "spacing",
+                id="42",
+            ),
         ),
     )
     def test_jinja(text: str, expected: str, tag: str) -> None:
         """Tests our ability to spot spacing errors inside jinja2 templates."""
         rule = JinjaRule()
+
         reformatted, details, returned_tag = rule.check_whitespace(text, key="name")
         assert tag == returned_tag, details
         assert expected == reformatted
@@ -490,3 +521,13 @@ def test_jinja_implicit(text: str, expected: str, tag: str) -> None:
         reformatted, details, returned_tag = rule.check_whitespace(text, key="when")
         assert tag == returned_tag, details
         assert expected == reformatted
+
+    def test_jinja_invalid() -> None:
+        """Tests our ability to spot spacing errors inside jinja2 templates."""
+        collection = RulesCollection()
+        collection.register(JinjaRule())
+        success = "examples/playbooks/rule-jinja-invalid.yml"
+        errs = Runner(success, rules=collection).run()
+        assert len(errs) == 1
+        assert errs[0].tag == "jinja[invalid]"
+        assert errs[0].rule.id == "jinja"

src/ansiblelint/rules/risky_file_permissions.py

@@ -266,7 +266,7 @@ def matchtask(
 - hosts: all
   tasks:
     - name: Lineinfile when create is true
-      lineinfile:
+      ansible.builtin.lineinfile:
         path: foo
         create: true
         line: some content here

src/ansiblelint/schemas/ansible-lint-config.json

@@ -110,7 +110,6 @@
           "name",
           "no-changed-when",
           "no-handler",
-          "no-jinja-nesting",
           "no-jinja-when",
           "no-log-password",
           "no-loop-var-prefix",

src/ansiblelint/utils.py

@@ -107,9 +107,24 @@ def ansible_templar(basedir: str, templatevars: Any) -> Templar:
 def ansible_template(
     basedir: str, varname: Any, templatevars: Any, **kwargs: Any
 ) -> Any:
-    """Render a templated string."""
+    """Render a templated string by mocking missing filters."""
     templar = ansible_templar(basedir=basedir, templatevars=templatevars)
-    return templar.template(varname, **kwargs)
+    while True:
+        try:
+            return templar.template(varname, **kwargs)
+        except AnsibleError as exc:
+            if exc.message.startswith(
+                "template error while templating string: No filter named"
+            ):
+                missing_filter = exc.message.split("'")[1]
+                # Mock the filter to avoid and error from Ansible templating
+                # pylint: disable=protected-access
+                templar.environment.filters._delegatee[missing_filter] = lambda x: x
+                # Record the mocked filter so we can warn the user
+                if missing_filter not in options.mock_filters:
+                    options.mock_filters.append(missing_filter)
+                continue
+            raise
 
 
 LINE_NUMBER_KEY = "__line__"
@@ -215,6 +230,7 @@ def template(
     basedir: str,
     value: Any,
     variables: Any,
+    fail_on_error: bool = False,
     fail_on_undefined: bool = False,
     **kwargs: str,
 ) -> Any:
@@ -230,7 +246,8 @@ def template(
         # I guess the filter doesn't like empty vars...
     except (AnsibleError, ValueError, RepresenterError):
         # templating failed, so just keep value as is.
-        pass
+        if fail_on_error:
+            raise
     return value