cfengine format: Fixed formatting around macros

src/cfengine_cli/format.py

@@ -66,6 +66,9 @@ def __init__(self) -> None:
         self.empty: bool = True
         self.previous: Node | None = None
         self.buffer: str = ""
+        # Indentation level saved when @if is encountered, restored after
+        # @else so that both branches are formatted at the same depth.
+        self.macro_indent: int = 0
 
     def _write(self, message: str, end: str = "\n") -> None:
         """Append text to the buffer with the given line ending."""
@@ -432,6 +435,8 @@ def can_single_line_promise(node: Node, indent: int, line_length: int) -> bool:
     children = node.children
     attrs = [c for c in children if c.type == "attribute"]
     next_sib = node.next_named_sibling
+    while next_sib and next_sib.type == "macro":
+        next_sib = next_sib.next_named_sibling
     if len(attrs) > 1:
         # We always want to multiline a promise with multiple attributes
         # even if it would fit on one line, i.e this should be split:
@@ -479,7 +484,6 @@ def _format_promise(
     fmt: Formatter,
     indent: int,
     line_length: int,
-    macro_indent: int,
 ) -> bool:
     """Format a promise node. Returns True if handled, False to fall through."""
     # Single-line promise
@@ -513,7 +517,7 @@ def _format_promise(
         close_indent = indent + 2
         if attrs:
             fmt.print("}", close_indent)
-            _format_remaining_children(children, fmt, indent, line_length, macro_indent)
+            _format_remaining_children(children, fmt, indent, line_length)
         else:
             fmt.print("};", close_indent)
         return True
@@ -522,7 +526,7 @@ def _format_promise(
     prefix = _promiser_line_with_stakeholder(children)
     if prefix:
         fmt.print(prefix, indent)
-        _format_remaining_children(children, fmt, indent, line_length, macro_indent)
+        _format_remaining_children(children, fmt, indent, line_length)
         return True
 
     return False
@@ -533,13 +537,12 @@ def _format_remaining_children(
     fmt: Formatter,
     indent: int,
     line_length: int,
-    macro_indent: int,
 ) -> None:
     """Format promise children, skipping promiser/arrow/stakeholder parts."""
     for child in children:
         if child.type in PROMISER_PARTS:
             continue
-        autoformat(child, fmt, line_length, macro_indent, indent)
+        autoformat(child, fmt, line_length, indent)
 
 
 # ---------------------------------------------------------------------------
@@ -570,7 +573,10 @@ def _format_block_header(node: Node, fmt: Formatter) -> list[Node]:
         # Skip over preceding empty comments since they will be removed
         while prev_sib and prev_sib.type == "comment" and _is_empty_comment(prev_sib):
             prev_sib = prev_sib.prev_named_sibling
-        if not (prev_sib and prev_sib.type == "comment"):
+        is_macro_wrapped = (
+            prev_sib and prev_sib.type == "macro" and text(prev_sib).startswith("@if")
+        )
+        if not (prev_sib and prev_sib.type == "comment") and not is_macro_wrapped:
             fmt.blank_line()
     fmt.print(line, 0)
     for i, comment in enumerate(header_comments):
@@ -599,14 +605,20 @@ def _needs_blank_line_before(child: Node, indent: int, line_length: int) -> bool
     if child.type == "bundle_section":
         return prev.type == "bundle_section"
 
-    if child.type == "promise" and prev.type in {"promise", "half_promise"}:
-        promise_indent = indent + 2
-        both_single = (
-            prev.type == "promise"
-            and can_single_line_promise(prev, promise_indent, line_length)
-            and can_single_line_promise(child, promise_indent, line_length)
-        )
-        return not both_single
+    if child.type == "promise":
+        # Skip past macros to find the content-bearing previous sibling,
+        # so we detect promise groups separated by macro-split halves.
+        prev_content = prev
+        while prev_content and prev_content.type == "macro":
+            prev_content = prev_content.prev_named_sibling
+        if prev_content and prev_content.type in {"promise", "half_promise"}:
+            promise_indent = indent + 2
+            both_single = (
+                prev_content.type == "promise"
+                and can_single_line_promise(prev_content, promise_indent, line_length)
+                and can_single_line_promise(child, promise_indent, line_length)
+            )
+            return not both_single
 
     if child.type in CLASS_GUARD_TYPES:
         return prev.type in {"promise", "half_promise", "class_guarded_promises"}
@@ -637,8 +649,8 @@ def _is_empty_comment(node: Node) -> bool:
 
 
 def _skip_comments(sibling: Node | None, direction: str = "next") -> Node | None:
-    """Walk past adjacent comment siblings to find the nearest non-comment."""
-    while sibling and sibling.type == "comment":
+    """Walk past adjacent comment and macro siblings to find the nearest content node."""
+    while sibling and sibling.type in ("comment", "macro"):
         sibling = (
             sibling.next_named_sibling
             if direction == "next"
@@ -670,21 +682,25 @@ def autoformat(
     node: Node,
     fmt: Formatter,
     line_length: int,
-    macro_indent: int,
     indent: int = 0,
 ) -> None:
     """Recursively format a tree-sitter node tree into the Formatter buffer."""
     previous = fmt.update_previous(node)
 
     # Macro handling
     if previous and previous.type == "macro" and text(previous).startswith("@else"):
-        indent = macro_indent
+        if node.type != "half_promise":
+            indent = fmt.macro_indent
     if node.type == "macro":
-        fmt.print(node, 0)
         if text(node).startswith("@if"):
-            macro_indent = indent
+            # Add blank line before @if at top level if preceded by a block
+            prev_sib = node.prev_named_sibling
+            if prev_sib and prev_sib.type in BLOCK_TYPES and not fmt.empty:
+                fmt.blank_line()
+            fmt.macro_indent = indent
         elif text(node).startswith("@else"):
-            indent = macro_indent
+            indent = fmt.macro_indent
+        fmt.print(node, 0)
         return
 
     # Block header (bundle/body/promise blocks)
@@ -703,15 +719,15 @@ def autoformat(
 
     # Promise — delegate to promise formatter
     if node.type == "promise":
-        if _format_promise(node, children, fmt, indent, line_length, macro_indent):
+        if _format_promise(node, children, fmt, indent, line_length):
             return
 
     # Interior node with children — recurse
     if children:
         for child in children:
             if _needs_blank_line_before(child, indent, line_length):
                 fmt.blank_line()
-            autoformat(child, fmt, line_length, macro_indent, indent)
+            autoformat(child, fmt, line_length, indent)
         return
 
     # Leaf nodes
@@ -739,15 +755,14 @@ def format_policy_file(filename: str, line_length: int, check: bool) -> int:
     PY_LANGUAGE = Language(tscfengine.language())
     parser = Parser(PY_LANGUAGE)
 
-    macro_indent = 0
     fmt = Formatter()
     with open(filename, "rb") as f:
         original_data = f.read()
     tree = parser.parse(original_data)
 
     root_node = tree.root_node
     assert root_node.type == "source_file"
-    autoformat(root_node, fmt, line_length, macro_indent)
+    autoformat(root_node, fmt, line_length)
 
     new_data = fmt.buffer + "\n"
     if new_data != original_data.decode("utf-8"):
@@ -768,14 +783,13 @@ def format_policy_fin_fout(
     PY_LANGUAGE = Language(tscfengine.language())
     parser = Parser(PY_LANGUAGE)
 
-    macro_indent = 0
     fmt = Formatter()
     original_data = fin.read().encode("utf-8")
     tree = parser.parse(original_data)
 
     root_node = tree.root_node
     assert root_node.type == "source_file"
-    autoformat(root_node, fmt, line_length, macro_indent)
+    autoformat(root_node, fmt, line_length)
 
     new_data = fmt.buffer + "\n"
     fout.write(new_data)

tests/format/011_macros.expected.cf

@@ -0,0 +1,85 @@
+bundle agent main
+{
+  vars:
+    "v" string => "hello";
+@if minimum_version(3.18)
+    "new_var" string => "only in 3.18+";
+@endif
+  reports:
+    "Hello!";
+@if minimum_version(3.20)
+    "New report";
+@endif
+}
+
+bundle agent other
+{
+@if minimum_version(3.18)
+  vars:
+    "v" string => "value";
+@else
+  vars:
+    "v" string => "old_value";
+@endif
+}
+
+@if minimum_version(3.18)
+bundle agent new_bundle
+{
+  reports:
+    "Only available in 3.18+";
+}
+@endif
+
+bundle agent half_promises
+{
+  vars:
+    "promiser"
+@if minimum_version(3.18)
+      string => "new_value";
+@else
+      string => "old_value";
+@endif
+
+    "another"
+@if minimum_version(3.18)
+      string => "new";
+@else
+      string => "old";
+@endif
+}
+
+bundle agent comments_and_macros
+{
+  vars:
+    # Comment before macro
+@if minimum_version(3.18)
+    "v" string => "new";
+@endif
+    # Comment after macro
+    "w" string => "always";
+@if minimum_version(3.18)
+    # Comment inside macro block
+    "x" string => "new_with_comment";
+@endif
+  reports:
+    # Report comment
+@if minimum_version(3.20)
+    # Another comment
+    "New report";
+@endif
+    "Always report";
+}
+
+bundle agent half_with_comment
+{
+  vars:
+    # Comment before half promise
+    "promiser"
+      # Comment before macro in half promise
+@if minimum_version(3.18)
+      string => "new_value";
+@else
+      string => "old_value";
+@endif
+}

tests/format/011_macros.input.cf

@@ -0,0 +1,84 @@
+bundle agent main
+{
+vars:
+"v" string => "hello";
+@if minimum_version(3.18)
+"new_var" string => "only in 3.18+";
+@endif
+reports:
+"Hello!";
+@if minimum_version(3.20)
+"New report";
+@endif
+}
+
+bundle agent other
+{
+@if minimum_version(3.18)
+vars:
+"v" string => "value";
+@else
+vars:
+"v" string => "old_value";
+@endif
+}
+
+@if minimum_version(3.18)
+bundle agent new_bundle
+{
+reports:
+"Only available in 3.18+";
+}
+@endif
+
+bundle agent half_promises
+{
+vars:
+"promiser"
+@if minimum_version(3.18)
+string => "new_value";
+@else
+string => "old_value";
+@endif
+"another"
+@if minimum_version(3.18)
+string => "new";
+@else
+string => "old";
+@endif
+}
+
+bundle agent comments_and_macros
+{
+vars:
+# Comment before macro
+@if minimum_version(3.18)
+"v" string => "new";
+@endif
+# Comment after macro
+"w" string => "always";
+@if minimum_version(3.18)
+# Comment inside macro block
+"x" string => "new_with_comment";
+@endif
+reports:
+# Report comment
+@if minimum_version(3.20)
+# Another comment
+"New report";
+@endif
+"Always report";
+}
+
+bundle agent half_with_comment
+{
+vars:
+# Comment before half promise
+"promiser"
+# Comment before macro in half promise
+@if minimum_version(3.18)
+string => "new_value";
+@else
+string => "old_value";
+@endif
+}