From c2cfd2f78d6e696d03e5c815cd75b8fa3180ae38 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Wed, 1 Apr 2026 21:13:01 +0200 Subject: [PATCH 1/6] format.py: Added unit tests and docstrings Co-authored-by: Claude Opus 4.6 (1M context) Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/format.py | 24 +++++++++++++ tests/unit/test_format.py | 70 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) create mode 100644 tests/unit/test_format.py diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index 033a2c2..5dbfcc5 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -50,6 +50,17 @@ def update_previous(self, node): def stringify_children_from_strings(parts): + """Join pre-extracted string tokens into a formatted parameter list. + + Used when formatting bundle/body headers. Comments are + stripped from the parameter_list node before this function is called, + so `parts` contains only the structural tokens: "(", identifiers, "," + separators, and ")". The function removes any trailing comma before + ")", then joins the tokens with appropriate spacing (space after each + comma, no space after "(" or before ")"). + + Example: ["(", "a", ",", "b", ",", ")"] -> "(a, b)" + """ # Remove trailing comma before closing paren cleaned = [] for i, part in enumerate(parts): @@ -69,6 +80,19 @@ def stringify_children_from_strings(parts): def stringify_children(children): + """Join a list of tree-sitter nodes into a single-line string. + + Operates on the direct child nodes of a CFEngine syntax construct + (e.g. a list, call, or attribute). Each child is recursively + flattened via stringify_single_line(). Spacing rules: + - A space is inserted after each "," separator. + - A space is inserted before and after "=>" (fat arrow). + - No extra space otherwise (e.g. no space after "(" or before ")"). + + Used by stringify_single_line() to recursively flatten any node with + children, and by maybe_split_generic_list() to attempt a single-line + rendering before falling back to multi-line splitting. + """ result = "" previous = None for child in children: diff --git a/tests/unit/test_format.py b/tests/unit/test_format.py new file mode 100644 index 0000000..c54a160 --- /dev/null +++ b/tests/unit/test_format.py @@ -0,0 +1,70 @@ +from cfengine_cli.format import stringify_children_from_strings, stringify_children + + +class MockNode: + """Minimal stand-in for a tree-sitter Node used by stringify_children.""" + + def __init__(self, node_type, node_text=None, children=None): + self.type = node_type + self.text = node_text.encode("utf-8") if node_text is not None else None + self.children = children or [] + +def _leaf(node_type, node_text=None): + return MockNode(node_type, node_text or node_type) + +def test_stringify_children_from_strings(): + assert stringify_children_from_strings([]) == "" + assert stringify_children_from_strings(["foo"]) == "foo" + assert stringify_children_from_strings(["(", "a", ")"]) == "(a)" + assert stringify_children_from_strings(["(", "a", ",", "b", ")"]) == "(a, b)" + assert stringify_children_from_strings(["(", "a", ",", ")"]) == "(a)" + assert ( + stringify_children_from_strings(["(", "a", ",", "b", ",", ")"]) + == "(a, b)" + ) + assert stringify_children_from_strings(["a", "b", "c"]) == "a b c" + assert stringify_children_from_strings(["a", ",", "b"]) == "a, b" + assert stringify_children_from_strings(["(", ")"]) == "()" + parts = ["(", "x", ",", "y", ",", "z", ")"] + assert stringify_children_from_strings(parts) == "(x, y, z)" + +def test_stringify_children(): + assert stringify_children([]) == "" + assert stringify_children([_leaf("identifier", "foo")]) == "foo" + + nodes = [_leaf("string", '"a"'), _leaf(","), _leaf("string", '"b"')] + assert stringify_children(nodes) == '"a", "b"' + + nodes = [_leaf("identifier", "lval"), _leaf("=>"), _leaf("string", '"rval"')] + assert stringify_children(nodes) == 'lval => "rval"' + + nodes = [_leaf("("), _leaf("identifier", "x"), _leaf(")")] + assert stringify_children(nodes) == "(x)" + + nodes = [ + _leaf("{"), + _leaf("string", '"a"'), + _leaf(","), + _leaf("string", '"b"'), + _leaf("}"), + ] + assert stringify_children(nodes) == '{"a", "b"}' + nodes = [ + _leaf("identifier", "package_name"), + _leaf("=>"), + _leaf("string", '"nginx"'), + ] + + assert stringify_children(nodes) == 'package_name => "nginx"' + inner = MockNode( + "call", + children=[ + _leaf("calling_identifier", "func"), + _leaf("("), + _leaf("string", '"arg"'), + _leaf(")"), + ], + ) + + nodes = [_leaf("identifier", "x"), _leaf("=>"), inner] + assert stringify_children(nodes) == 'x => func("arg")' From 0cd3ebc715d262f68b9505c2af4ddb38944d26e4 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Wed, 1 Apr 2026 21:21:04 +0200 Subject: [PATCH 2/6] format.py: Renamed functions / parameters / variables to improve readability Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/format.py | 38 ++++++++++++++--------------- tests/unit/test_format.py | 50 +++++++++++++++++++------------------- 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index 5dbfcc5..c34ad8d 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -49,7 +49,7 @@ def update_previous(self, node): return tmp -def stringify_children_from_strings(parts): +def stringify_parameter_list(parts): """Join pre-extracted string tokens into a formatted parameter list. Used when formatting bundle/body headers. Comments are @@ -79,39 +79,39 @@ def stringify_children_from_strings(parts): return result -def stringify_children(children): +def stringify_single_line_nodes(nodes): """Join a list of tree-sitter nodes into a single-line string. Operates on the direct child nodes of a CFEngine syntax construct (e.g. a list, call, or attribute). Each child is recursively - flattened via stringify_single_line(). Spacing rules: + flattened via stringify_single_line_node(). Spacing rules: - A space is inserted after each "," separator. - A space is inserted before and after "=>" (fat arrow). - No extra space otherwise (e.g. no space after "(" or before ")"). - Used by stringify_single_line() to recursively flatten any node with + Used by stringify_single_line_node() to recursively flatten any node with children, and by maybe_split_generic_list() to attempt a single-line rendering before falling back to multi-line splitting. """ result = "" previous = None - for child in children: - string = stringify_single_line(child) + for node in nodes: + string = stringify_single_line_node(node) if previous and previous.type == ",": result += " " - if previous and child.type == "=>": + if previous and node.type == "=>": result += " " if previous and previous.type == "=>": result += " " result += string - previous = child + previous = node return result -def stringify_single_line(node): +def stringify_single_line_node(node): if not node.children: return text(node) - return stringify_children(node.children) + return stringify_single_line_nodes(node.children) def split_generic_value(node, indent, line_length): @@ -119,7 +119,7 @@ def split_generic_value(node, indent, line_length): return split_rval_call(node, indent, line_length) if node.type == "list": return split_rval_list(node, indent, line_length) - return [stringify_single_line(node)] + return [stringify_single_line_node(node)] def split_generic_list(middle, indent, line_length): @@ -128,7 +128,7 @@ def split_generic_list(middle, indent, line_length): if elements and element.type == ",": elements[-1] = elements[-1] + "," continue - line = " " * indent + stringify_single_line(element) + line = " " * indent + stringify_single_line_node(element) if len(line) < line_length: elements.append(line) else: @@ -139,7 +139,7 @@ def split_generic_list(middle, indent, line_length): def maybe_split_generic_list(nodes, indent, line_length): - string = " " * indent + stringify_children(nodes) + string = " " * indent + stringify_single_line_nodes(nodes) if len(string) < line_length: return [string] return split_generic_list(nodes, indent, line_length) @@ -171,11 +171,11 @@ def split_rval(node, indent, line_length): return split_rval_list(node, indent, line_length) if node.type == "call": return split_rval_call(node, indent, line_length) - return [stringify_single_line(node)] + return [stringify_single_line_node(node)] def maybe_split_rval(node, indent, offset, line_length): - line = stringify_single_line(node) + line = stringify_single_line_node(node) if len(line) + offset < line_length: return [line] return split_rval(node, indent, line_length) @@ -193,11 +193,11 @@ def attempt_split_attribute(node, indent, line_length): lines = maybe_split_rval(rval, indent, offset, line_length) lines[0] = prefix + lines[0] return lines - return [" " * indent + stringify_single_line(node)] + return [" " * indent + stringify_single_line_node(node)] def stringify(node, indent, line_length): - single_line = " " * indent + stringify_single_line(node) + single_line = " " * indent + stringify_single_line_node(node) # Reserve 1 char for trailing ; or , after attributes effective_length = line_length - 1 if node.type == "attribute" else line_length if len(single_line) < effective_length: @@ -233,7 +233,7 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0): else: parts.append(text(p)) # Append directly to previous part (no space before parens) - header_parts[-1] = header_parts[-1] + stringify_children_from_strings( + header_parts[-1] = header_parts[-1] + stringify_parameter_list( parts ) else: @@ -273,7 +273,7 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0): line = ( text(promiser_node) + " " - + stringify_single_line(attr_children[0]) + + stringify_single_line_node(attr_children[0]) + ";" ) if indent + len(line) <= line_length: diff --git a/tests/unit/test_format.py b/tests/unit/test_format.py index c54a160..b0b346b 100644 --- a/tests/unit/test_format.py +++ b/tests/unit/test_format.py @@ -1,45 +1,45 @@ -from cfengine_cli.format import stringify_children_from_strings, stringify_children +from cfengine_cli.format import stringify_parameter_list, stringify_single_line_nodes class MockNode: - """Minimal stand-in for a tree-sitter Node used by stringify_children.""" + """Minimal stand-in for a tree-sitter Node used by stringify_single_line_nodes.""" def __init__(self, node_type, node_text=None, children=None): self.type = node_type self.text = node_text.encode("utf-8") if node_text is not None else None self.children = children or [] + def _leaf(node_type, node_text=None): return MockNode(node_type, node_text or node_type) -def test_stringify_children_from_strings(): - assert stringify_children_from_strings([]) == "" - assert stringify_children_from_strings(["foo"]) == "foo" - assert stringify_children_from_strings(["(", "a", ")"]) == "(a)" - assert stringify_children_from_strings(["(", "a", ",", "b", ")"]) == "(a, b)" - assert stringify_children_from_strings(["(", "a", ",", ")"]) == "(a)" - assert ( - stringify_children_from_strings(["(", "a", ",", "b", ",", ")"]) - == "(a, b)" - ) - assert stringify_children_from_strings(["a", "b", "c"]) == "a b c" - assert stringify_children_from_strings(["a", ",", "b"]) == "a, b" - assert stringify_children_from_strings(["(", ")"]) == "()" + +def test_stringify_parameter_list(): + assert stringify_parameter_list([]) == "" + assert stringify_parameter_list(["foo"]) == "foo" + assert stringify_parameter_list(["(", "a", ")"]) == "(a)" + assert stringify_parameter_list(["(", "a", ",", "b", ")"]) == "(a, b)" + assert stringify_parameter_list(["(", "a", ",", ")"]) == "(a)" + assert stringify_parameter_list(["(", "a", ",", "b", ",", ")"]) == "(a, b)" + assert stringify_parameter_list(["a", "b", "c"]) == "a b c" + assert stringify_parameter_list(["a", ",", "b"]) == "a, b" + assert stringify_parameter_list(["(", ")"]) == "()" parts = ["(", "x", ",", "y", ",", "z", ")"] - assert stringify_children_from_strings(parts) == "(x, y, z)" + assert stringify_parameter_list(parts) == "(x, y, z)" + -def test_stringify_children(): - assert stringify_children([]) == "" - assert stringify_children([_leaf("identifier", "foo")]) == "foo" +def test_stringify_single_line_nodes(): + assert stringify_single_line_nodes([]) == "" + assert stringify_single_line_nodes([_leaf("identifier", "foo")]) == "foo" nodes = [_leaf("string", '"a"'), _leaf(","), _leaf("string", '"b"')] - assert stringify_children(nodes) == '"a", "b"' + assert stringify_single_line_nodes(nodes) == '"a", "b"' nodes = [_leaf("identifier", "lval"), _leaf("=>"), _leaf("string", '"rval"')] - assert stringify_children(nodes) == 'lval => "rval"' + assert stringify_single_line_nodes(nodes) == 'lval => "rval"' nodes = [_leaf("("), _leaf("identifier", "x"), _leaf(")")] - assert stringify_children(nodes) == "(x)" + assert stringify_single_line_nodes(nodes) == "(x)" nodes = [ _leaf("{"), @@ -48,14 +48,14 @@ def test_stringify_children(): _leaf("string", '"b"'), _leaf("}"), ] - assert stringify_children(nodes) == '{"a", "b"}' + assert stringify_single_line_nodes(nodes) == '{"a", "b"}' nodes = [ _leaf("identifier", "package_name"), _leaf("=>"), _leaf("string", '"nginx"'), ] - assert stringify_children(nodes) == 'package_name => "nginx"' + assert stringify_single_line_nodes(nodes) == 'package_name => "nginx"' inner = MockNode( "call", children=[ @@ -67,4 +67,4 @@ def test_stringify_children(): ) nodes = [_leaf("identifier", "x"), _leaf("=>"), inner] - assert stringify_children(nodes) == 'x => func("arg")' + assert stringify_single_line_nodes(nodes) == 'x => func("arg")' From becc66445bcd0e93bf324f0fef8bf04d4de66d40 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 04:16:18 +0200 Subject: [PATCH 3/6] Formatting test 002: Added an example of multiple promise types We want to insert newlines before promise types (promise guards). So we need a test for this. Will fix the implementation in a subsequent commit. Signed-off-by: Ole Herman Schumacher Elgesem --- tests/format/002_basics.expected.cf | 4 ++++ tests/format/002_basics.input.cf | 3 +++ 2 files changed, 7 insertions(+) diff --git a/tests/format/002_basics.expected.cf b/tests/format/002_basics.expected.cf index 64a236f..9d0696d 100644 --- a/tests/format/002_basics.expected.cf +++ b/tests/format/002_basics.expected.cf @@ -29,4 +29,8 @@ bundle agent main if => "bar" # Comment at atttribute level string => "some_value"; + + classes: + # Comment before promise + "a" if => "b"; } diff --git a/tests/format/002_basics.input.cf b/tests/format/002_basics.input.cf index 6ac1977..54aafee 100644 --- a/tests/format/002_basics.input.cf +++ b/tests/format/002_basics.input.cf @@ -26,4 +26,7 @@ baz:: if => "bar" # Comment at atttribute level string => "some_value"; +classes: +# Comment before promise +"a" if => "b"; } From 5c86a364a6367d1595b647292ca4f80b671f6c39 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 04:18:52 +0200 Subject: [PATCH 4/6] Added formatting test for removing empty coomments Signed-off-by: Ole Herman Schumacher Elgesem --- .../006_remove_empty_comments.expected.cf | 24 +++++++++++++++ .../format/006_remove_empty_comments.input.cf | 30 +++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 tests/format/006_remove_empty_comments.expected.cf create mode 100644 tests/format/006_remove_empty_comments.input.cf diff --git a/tests/format/006_remove_empty_comments.expected.cf b/tests/format/006_remove_empty_comments.expected.cf new file mode 100644 index 0000000..3dd3d0a --- /dev/null +++ b/tests/format/006_remove_empty_comments.expected.cf @@ -0,0 +1,24 @@ +bundle agent main +{ + # comment + reports: + "hello"; +} + +bundle agent b +{ + # Some long comment here + # + # With more explanation here + reports: + "hello"; +} + +bundle agent c +# Some long comment here +# +# With more explanation here +{ + reports: + "hello"; +} diff --git a/tests/format/006_remove_empty_comments.input.cf b/tests/format/006_remove_empty_comments.input.cf new file mode 100644 index 0000000..43172d6 --- /dev/null +++ b/tests/format/006_remove_empty_comments.input.cf @@ -0,0 +1,30 @@ +bundle agent main +{ +# +# comment +reports: +"hello"; +} + + +bundle agent b +{ +# Some long comment here +# +# With more explanation here +# +reports: +"hello"; +} + + + +bundle agent c +# Some long comment here +# +# With more explanation here +# +{ +reports: +"hello"; +} From cc6bc94ee36c01b42fe670247caf20b1ebc07d40 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 04:20:50 +0200 Subject: [PATCH 5/6] Added formatting test for multiple class guarded promises We want to ensure empty newlines are added before each class guard (except the first one in each bundle section). Signed-off-by: Ole Herman Schumacher Elgesem --- .../format/007_class_guarded_empty_lines.expected.cf | 11 +++++++++++ tests/format/007_class_guarded_empty_lines.input.cf | 10 ++++++++++ 2 files changed, 21 insertions(+) create mode 100644 tests/format/007_class_guarded_empty_lines.expected.cf create mode 100644 tests/format/007_class_guarded_empty_lines.input.cf diff --git a/tests/format/007_class_guarded_empty_lines.expected.cf b/tests/format/007_class_guarded_empty_lines.expected.cf new file mode 100644 index 0000000..d340c89 --- /dev/null +++ b/tests/format/007_class_guarded_empty_lines.expected.cf @@ -0,0 +1,11 @@ +bundle agent main +{ + vars: + hpux:: + "package_dir" + string => "$(sys.flavour)_$(sys.arch)"; + + !hpux:: + "package_dir" + string => "$(sys.class)_$(sys.arch)"; +} diff --git a/tests/format/007_class_guarded_empty_lines.input.cf b/tests/format/007_class_guarded_empty_lines.input.cf new file mode 100644 index 0000000..46487e2 --- /dev/null +++ b/tests/format/007_class_guarded_empty_lines.input.cf @@ -0,0 +1,10 @@ +bundle agent main +{ + vars: + hpux:: + "package_dir" + string => "$(sys.flavour)_$(sys.arch)"; + !hpux:: + "package_dir" + string => "$(sys.class)_$(sys.arch)"; +} From 4c73e8a306615c31537e609f4603d483d1b92120 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 04:25:38 +0200 Subject: [PATCH 6/6] Adjusted formatter according to test expectations The formatter now: - Removes empty leading / trailing comments. - Inserts empty lines before each class guard except the first one. - Inserts empty lines before each promise guard except the first one. Co-authored-by: Claude Opus 4.6 (1M context) Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/format.py | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index c34ad8d..d89735d 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -233,9 +233,7 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0): else: parts.append(text(p)) # Append directly to previous part (no space before parens) - header_parts[-1] = header_parts[-1] + stringify_parameter_list( - parts - ) + header_parts[-1] = header_parts[-1] + stringify_parameter_list(parts) else: header_parts.append(text(x)) line = " ".join(header_parts) @@ -244,7 +242,15 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0): if not (prev_sib and prev_sib.type == "comment"): fmt.print("", 0) fmt.print(line, 0) - for comment in header_comments: + for i, comment in enumerate(header_comments): + if comment.strip() == "#": + prev_is_comment = i > 0 and header_comments[i - 1].strip() != "#" + next_is_comment = ( + i + 1 < len(header_comments) + and header_comments[i + 1].strip() != "#" + ) + if not (prev_is_comment and next_is_comment): + continue fmt.print(comment, 0) children = node.children[-1].children if node.type in [ @@ -263,11 +269,17 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0): return if node.type == "promise": # Single-line promise: if exactly 1 attribute, no half_promise continuation, - # and the whole line fits in line_length + # not inside a class guard, and the whole line fits in line_length attr_children = [c for c in children if c.type == "attribute"] next_sib = node.next_named_sibling has_continuation = next_sib and next_sib.type == "half_promise" - if len(attr_children) == 1 and not has_continuation: + parent = node.parent + in_class_guard = parent and parent.type in [ + "class_guarded_promises", + "class_guarded_body_attributes", + "class_guarded_promise_block_attributes", + ] + if len(attr_children) == 1 and not has_continuation and not in_class_guard: promiser_node = next((c for c in children if c.type == "promiser"), None) if promiser_node: line = ( @@ -281,8 +293,13 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0): return if children: for child in children: + # Blank line between bundle sections + if child.type == "bundle_section": + prev = child.prev_named_sibling + if prev and prev.type == "bundle_section": + fmt.print("", 0) # Blank line between promises in a section - if child.type == "promise": + elif child.type == "promise": prev = child.prev_named_sibling if prev and prev.type in ["promise", "half_promise"]: fmt.print("", 0) @@ -295,6 +312,7 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0): if prev and prev.type in [ "promise", "half_promise", + "class_guarded_promises", ]: fmt.print("", 0) elif child.type == "comment": @@ -312,6 +330,11 @@ def autoformat(node, fmt, line_length, macro_indent, indent=0): fmt.print_same_line(node) return if node.type == "comment": + if text(node).strip() == "#": + prev = node.prev_named_sibling + nxt = node.next_named_sibling + if not (prev and prev.type == "comment" and nxt and nxt.type == "comment"): + return comment_indent = indent next_sib = node.next_named_sibling while next_sib and next_sib.type == "comment":