fix: jinja[spacing] rule creating invalid syntax for minus modifiers (#5102)

Signed-off-by: Dotify71 <222976357+Dotify71@users.noreply.github.com>
Co-authored-by: Dotify71 <222976357+Dotify71@users.noreply.github.com>
Co-authored-by: Sathya pramod <8750601+sathyapramod@users.noreply.github.com>

Author: Dotify71

src/ansiblelint/rules/jinja.py

@@ -6,10 +6,10 @@
 import os
 import re
 import sys
-from collections.abc import Mapping
+from collections.abc import Callable, Mapping
 from dataclasses import dataclass
 from pathlib import Path
-from typing import TYPE_CHECKING, NamedTuple
+from typing import TYPE_CHECKING, Any, NamedTuple
 
 import black
 import jinja2
@@ -105,6 +105,98 @@ def __str__(self) -> str:
         return f"{self.key}={self.value} at {self.path} fixed to {self.fixed}"
 
 
+def _is_broken_rewrite(
+    env: jinja2.Environment, original: str, reformatted: str
+) -> bool:
+    """Return True if the reformatted text has a syntax error while the original does not."""
+    try:
+        env.parse(reformatted)
+    except jinja2.exceptions.TemplateSyntaxError:
+        try:
+            env.parse(original)
+        except jinja2.exceptions.TemplateSyntaxError:
+            return False
+        else:
+            return True
+    else:
+        return False
+
+
+def _cook(value: str, *, implicit: bool = False) -> str:
+    """Prepare an implicit string for jinja parsing when needed."""
+    if not implicit:
+        return value
+    if value.startswith("{{") and value.endswith("}}"):
+        # maybe we should make this an error?
+        return value
+    return f"{{{{ {value} }}}}"
+
+
+def _uncook(value: str, *, implicit: bool = False) -> str:
+    """Restore an string to original form when it was an implicit one."""
+    if not implicit:
+        return value
+    return value[3:-3]
+
+
+def _is_verb_token(expr_type: str | None, token: Any, verb_skipped: bool) -> bool:
+    return bool(
+        expr_type
+        and expr_type.startswith("{%")
+        and token.token_type in ("name", "whitespace")
+        and not verb_skipped
+    )
+
+
+def _format_expression(expr_str: str, last_token: Any) -> str:
+    if isinstance(expr_str, str) and "\n" in expr_str:
+        raise NotImplementedError
+    leading_spaces = " " * (len(expr_str) - len(expr_str.lstrip()))
+    expr_str = leading_spaces + blacken(expr_str.lstrip())
+    if last_token.token_type != "whitespace" and not expr_str.startswith(" "):
+        expr_str = " " + expr_str
+    if not expr_str.endswith(" "):
+        expr_str += " "
+    return expr_str
+
+
+def _process_jinja_tokens(lex: Callable[[str], Any], text: str) -> list[Token]:
+    tokens: list[Token] = []
+    begin_types = ("variable_begin", "comment_begin", "block_begin")
+    end_types = ("variable_end", "comment_end", "block_end")
+
+    expr_str = None
+    expr_type = None
+    verb_skipped = True
+    lineno = 1
+
+    for token in lex(text):
+        if _is_verb_token(expr_type, token, verb_skipped):
+            # on {% blocks we do not take first word as part of the expression
+            tokens.append(token)
+            if token.token_type != "whitespace":
+                verb_skipped = True
+        elif token.token_type in begin_types:
+            tokens.append(token)
+            expr_type = token.value  # such {#, {{, {%
+            expr_str = ""
+            verb_skipped = False
+        elif token.token_type in end_types and expr_str is not None:
+            # process expression
+            formatted = _format_expression(expr_str, tokens[-1])
+            tokens.append(Token(lineno, "data", formatted))
+            tokens.append(token)
+            expr_str = None
+            expr_type = None
+        elif expr_str is not None:
+            expr_str += token.value
+        else:
+            tokens.append(token)
+        lineno = token.lineno
+
+    return tokens
+
+
 class JinjaRule(AnsibleLintRule, TransformMixin):
     """Rule that looks inside jinja2 templates."""
 
@@ -351,7 +443,6 @@ def unlex(
 
         return result
 
-    # pylint: disable=too-many-locals
     def check_whitespace(
         self,
         text: str,
@@ -365,33 +456,13 @@ def check_whitespace(
 
         :returns: (string, string, string)  reformatted text, detailed error, error tag
         """
-
-        def cook(value: str, *, implicit: bool = False) -> str:
-            """Prepare an implicit string for jinja parsing when needed."""
-            if not implicit:
-                return value
-            if value.startswith("{{") and value.endswith("}}"):
-                # maybe we should make this an error?
-                return value
-            return f"{{{{ {value} }}}}"
-
-        def uncook(value: str, *, implicit: bool = False) -> str:
-            """Restore an string to original form when it was an implicit one."""
-            if not implicit:
-                return value
-            return value[3:-3]
-
         # Detect original line ending style to preserve it
         # Only preserve \r\n (Windows CRLF), let \r (Mac classic) be normalized to \n
         # as per jinja 3.0.0 behavior (see test case 44)
         original_line_ending = None
         if "\r\n" in text:
             original_line_ending = "\r\n"
 
-        tokens = []
-        details = ""
-        begin_types = ("variable_begin", "comment_begin", "block_begin")
-        end_types = ("variable_end", "comment_end", "block_end")
         implicit = False
 
         # implicit templates do not have the {{ }} wrapping
@@ -405,55 +476,14 @@ def uncook(value: str, *, implicit: bool = False) -> str:
             )
         ):
             implicit = True
-            text = cook(text, implicit=implicit)
+            text = _cook(text, implicit=implicit)
 
         # don't try to lex strings that have no jinja inside them
         if not has_jinja(text):
             return text, "", "spacing"
 
-        expr_str = None
-        expr_type = None
-        verb_skipped = True
-        lineno = 1
-        try:  # noqa: PLW0717
-            for token in self.lex(text):
-                if (
-                    expr_type
-                    and expr_type.startswith("{%")
-                    and token.token_type in ("name", "whitespace")
-                    and not verb_skipped
-                ):
-                    # on {% blocks we do not take first word as part of the expression
-                    tokens.append(token)
-                    if token.token_type != "whitespace":
-                        verb_skipped = True
-                elif token.token_type in begin_types:
-                    tokens.append(token)
-                    expr_type = token.value  # such {#, {{, {%
-                    expr_str = ""
-                    verb_skipped = False
-                elif token.token_type in end_types and expr_str is not None:
-                    # process expression
-                    # pylint: disable=unsupported-membership-test
-                    if isinstance(expr_str, str) and "\n" in expr_str:
-                        raise NotImplementedError  # noqa: TRY301
-                    leading_spaces = " " * (len(expr_str) - len(expr_str.lstrip()))
-                    expr_str = leading_spaces + blacken(expr_str.lstrip())
-                    if tokens[
-                        -1
-                    ].token_type != "whitespace" and not expr_str.startswith(" "):
-                        expr_str = " " + expr_str
-                    if not expr_str.endswith(" "):
-                        expr_str += " "
-                    tokens.append(Token(lineno, "data", expr_str))
-                    tokens.append(token)
-                    expr_str = None
-                    expr_type = None
-                elif expr_str is not None:
-                    expr_str += token.value
-                else:
-                    tokens.append(token)
-                lineno = token.lineno
+        try:
+            tokens = _process_jinja_tokens(self.lex, text)
 
         except jinja2.exceptions.TemplateSyntaxError as exc:
             return "", str(exc.message), "invalid"
@@ -464,12 +494,16 @@ def uncook(value: str, *, implicit: bool = False) -> str:
             # newlines, as we decided to not touch them yet.
             # These both are documented as known limitations.
             _logger.debug("Ignored jinja internal error %s", exc)
-            return uncook(text, implicit=implicit), "", "spacing"
+            return _uncook(text, implicit=implicit), "", "spacing"
 
         # finalize
         reformatted = self.unlex(tokens, original_line_ending)
         failed = reformatted != text
-        reformatted = uncook(reformatted, implicit=implicit)
+
+        if failed and _is_broken_rewrite(self.env, text, reformatted):
+            return _uncook(text, implicit=implicit), "", "spacing"
+
+        reformatted = _uncook(reformatted, implicit=implicit)
         details = (
             f"Jinja2 template rewrite recommendation: `{reformatted}`."
             if failed
@@ -809,6 +843,12 @@ def test_jinja_spacing_vars(empty_rule_collection: RulesCollection) -> None:
                 "spacing",
                 id="46",
             ),
+            pytest.param(
+                "'{ {{- item.limits -}} }'",
+                "'{ {{- item.limits -}} }'",
+                "spacing",
+                id="47",
+            ),
         ),
     )
     def test_jinja(text: str, expected: str, tag: str) -> None:

src/ansiblelint/schemas/__store__.json

@@ -40,7 +40,7 @@
     "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/play-argspec.json"
   },
   "playbook": {
-    "etag": "09d087fd7fe850790857de197c778bbce80ec360234e3240e2eff964f773aa8c",
+    "etag": "3a703ff75b757c2f62a9322ce67cd10f6d53e3c9b6d2a44c80f6c368b136ed3b",
     "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/playbook.json"
   },
   "requirements": {
@@ -56,7 +56,7 @@
     "url": "https://raw.githubusercontent.com/ansible/ansible-rulebook/main/ansible_rulebook/schema/ruleset_schema.json"
   },
   "tasks": {
-    "etag": "0e94cba9067e983083040bd661515801b5b1d4fd5660019f34234bba7ec8de1a",
+    "etag": "d95fa51d1e73fa96d28ca009822e158c5cf38d5e0b2329938674da3b356a5cee",
     "url": "https://raw.githubusercontent.com/ansible/ansible-lint/main/src/ansiblelint/schemas/tasks.json"
   },
   "vars": {