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 01/23] 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 02/23] 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 03/23] 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 04/23] 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 05/23] 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 06/23] 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": From 3ecd314f60ba35a7a678ae22214aac5fc7ee66f7 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 14:13:53 +0200 Subject: [PATCH 07/23] Added linting tests for namespaces Signed-off-by: Ole Herman Schumacher Elgesem --- tests/lint/008_namespace.cf | 16 ++++++++++++++++ tests/lint/008_namespace.output.txt | 7 +++++++ tests/lint/008_namespace.x.cf | 16 ++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 tests/lint/008_namespace.cf create mode 100644 tests/lint/008_namespace.output.txt create mode 100644 tests/lint/008_namespace.x.cf diff --git a/tests/lint/008_namespace.cf b/tests/lint/008_namespace.cf new file mode 100644 index 0000000..301e8e1 --- /dev/null +++ b/tests/lint/008_namespace.cf @@ -0,0 +1,16 @@ +body file control +{ + namespace => "mylib"; +} + +bundle agent target(a) +{ + reports: + "Hello, $(a)"; +} + +bundle agent helper +{ + vars: + "x" string => mylib:target("arg"); +} diff --git a/tests/lint/008_namespace.output.txt b/tests/lint/008_namespace.output.txt new file mode 100644 index 0000000..abfbef5 --- /dev/null +++ b/tests/lint/008_namespace.output.txt @@ -0,0 +1,7 @@ + + vars: + "x" string => other_ns:unknown_func("arg"); + ^-------------------^ +Error: Call to unknown function / bundle / body 'other_ns:unknown_func' at at tests/lint/008_namespace.x.cf:9:19 +FAIL: tests/lint/008_namespace.x.cf (1 errors) +Failure, 1 errors in total. diff --git a/tests/lint/008_namespace.x.cf b/tests/lint/008_namespace.x.cf new file mode 100644 index 0000000..1876c0a --- /dev/null +++ b/tests/lint/008_namespace.x.cf @@ -0,0 +1,16 @@ +body file control +{ + namespace => "mylib"; +} + +bundle agent target(a) +{ + reports: + "Hello, $(a)"; +} + +bundle agent helper +{ + vars: + "x" string => default:target("arg"); +} From 3d2d54bb90692c03e5729da4e82dfadc94397e87 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 14:14:56 +0200 Subject: [PATCH 08/23] Renamed user_definition variable to improve readability Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 51 +++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 38882c9..14f0714 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -154,7 +154,7 @@ def _find_node_type(filename, lines, node, node_type): return matches -def _node_checks(filename, lines, node, user_definition, strict, state: _State): +def _node_checks(filename, lines, node, user_definitions, strict, state: _State): """Checks we run on each node in the syntax tree, utilizes state for checks which require context.""" line = node.range.start_point[0] + 1 @@ -178,7 +178,7 @@ def _node_checks(filename, lines, node, user_definition, strict, state: _State): ( promise_type not in BUILTIN_PROMISE_TYPES.union( - user_definition.get("custom_promise_types", set()) + user_definitions.get("custom_promise_types", set()) ) ) ): @@ -212,8 +212,9 @@ def _node_checks(filename, lines, node, user_definition, strict, state: _State): if ( strict and state.qualify(_text(node), state.namespace) - in user_definition.get("all_bundle_names", set()) - and state.promise_type in user_definition.get("custom_promise_types", set()) + in user_definitions.get("all_bundle_names", set()) + and state.promise_type + in user_definitions.get("custom_promise_types", set()) ): _highlight_range(node, lines) print( @@ -223,8 +224,8 @@ def _node_checks(filename, lines, node, user_definition, strict, state: _State): if strict and ( state.qualify(_text(node), state.namespace) not in set.union( - user_definition.get("all_bundle_names", set()), - user_definition.get("all_body_names", set()), + user_definitions.get("all_bundle_names", set()), + user_definitions.get("all_body_names", set()), ) and _text(node) not in BUILTIN_FUNCTIONS ): @@ -237,22 +238,24 @@ def _node_checks(filename, lines, node, user_definition, strict, state: _State): def _stateful_walk( - filename, lines, node, user_definition, strict, state: _State | None = None + filename, lines, node, user_definitions, strict, state: _State | None = None ) -> int: if state is None: state = _State() - errors = _node_checks(filename, lines, node, user_definition, strict, state) + errors = _node_checks(filename, lines, node, user_definitions, strict, state) state.update(node) for child in node.children: - errors += _stateful_walk(filename, lines, child, user_definition, strict, state) + errors += _stateful_walk( + filename, lines, child, user_definitions, strict, state + ) return errors -def _walk(filename, lines, node, user_definition=None, strict=True) -> int: - if user_definition is None: - user_definition = {} +def _walk(filename, lines, node, user_definitions=None, strict=True) -> int: + if user_definitions is None: + user_definitions = {} error_nodes = _find_node_type(filename, lines, node, "ERROR") if error_nodes: @@ -267,12 +270,12 @@ def _walk(filename, lines, node, user_definition=None, strict=True) -> int: column = node.range.start_point[1] + 1 state = _State() - ret = _stateful_walk(filename, lines, node, user_definition, strict, state=state) + ret = _stateful_walk(filename, lines, node, user_definitions, strict, state=state) state = _State() # Clear state return ret -def _parse_user_definition(filename, lines, root_node): +def _parse_user_definitions(filename, lines, root_node): ns = "default" promise_blocks = set() bundle_blocks = set() @@ -337,7 +340,7 @@ def lint_policy_file( original_line=None, snippet=None, prefix=None, - user_definition=None, + user_definitions=None, strict=True, ): assert original_filename is None or type(original_filename) is str @@ -354,8 +357,8 @@ def lint_policy_file( assert os.path.isfile(filename) assert filename.endswith((".cf", ".cfengine3", ".cf3", ".cf.sub")) - if user_definition is None: - user_definition = {} + if user_definitions is None: + user_definitions = {} tree, lines, original_data = _parse_policy_file(filename) root_node = tree.root_node @@ -387,7 +390,7 @@ def lint_policy_file( else: print(f"Error: Empty policy file '{filename}'") errors += 1 - errors += _walk(filename, lines, root_node, user_definition, strict) + errors += _walk(filename, lines, root_node, user_definitions, strict) if prefix: print(prefix, end="") if errors == 0: @@ -426,33 +429,33 @@ def lint_folder(folder, strict=True): else: errors += lint_single_file(filename) - user_definition = {} + user_definitions = {} # First pass: Gather custom types for filename in policy_files if strict else []: tree, lines, _ = _parse_policy_file(filename) if tree.root_node.type == "source_file": - for key, val in _parse_user_definition( + for key, val in _parse_user_definitions( filename, lines, tree.root_node ).items(): - user_definition[key] = user_definition.get(key, set()).union(val) + user_definitions[key] = user_definitions.get(key, set()).union(val) # Second pass: lint all policy files for filename in policy_files: errors += lint_policy_file( - filename, user_definition=user_definition, strict=strict + filename, user_definitions=user_definitions, strict=strict ) return errors -def lint_single_file(file, user_definition=None, strict=True): +def lint_single_file(file, user_definitions=None, strict=True): assert os.path.isfile(file) if file.endswith("/cfbs.json"): return lint_cfbs_json(file) if file.endswith(".json"): return lint_json(file) assert file.endswith(".cf") - return lint_policy_file(file, user_definition=user_definition, strict=strict) + return lint_policy_file(file, user_definitions=user_definitions, strict=strict) def lint_single_arg(arg, strict=True): From 767a53049779fbd2b26f9571944be973ec39f2b0 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 14:16:07 +0200 Subject: [PATCH 09/23] Removed unnecessary state objects Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 14f0714..6604589 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -269,10 +269,9 @@ def _walk(filename, lines, node, user_definitions=None, strict=True) -> int: line = node.range.start_point[0] + 1 column = node.range.start_point[1] + 1 - state = _State() - ret = _stateful_walk(filename, lines, node, user_definitions, strict, state=state) - state = _State() # Clear state - return ret + return _stateful_walk( + filename, lines, node, user_definitions, strict, state=_State() + ) def _parse_user_definitions(filename, lines, root_node): From 9c6f1e94bb26f68c52b15cb8f359af6dabd2ffdc Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 14:19:22 +0200 Subject: [PATCH 10/23] Renamed / rescoped qualify function Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 6604589..42add99 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -26,6 +26,11 @@ BUILTIN_FUNCTIONS, ) +def _qualify(name: str, namespace: str) -> str: + """If name is already qualified (contains ':'), return as-is. Otherwise prepend namespace.""" + if ":" in name: + return name + return f"{namespace}:{name}" @dataclass class _State: @@ -81,11 +86,6 @@ def update(self, node): return return - @staticmethod - def qualify(name: str, namespace: str) -> str: - """If name is already qualified (contains ':'), return as-is. Otherwise prepend namespace.""" - return name if ":" in name else f"{namespace}:{name}" - def lint_cfbs_json(filename) -> int: assert os.path.isfile(filename) @@ -211,7 +211,7 @@ def _node_checks(filename, lines, node, user_definitions, strict, state: _State) if node.type == "calling_identifier": if ( strict - and state.qualify(_text(node), state.namespace) + and _qualify(_text(node), state.namespace) in user_definitions.get("all_bundle_names", set()) and state.promise_type in user_definitions.get("custom_promise_types", set()) @@ -222,7 +222,7 @@ def _node_checks(filename, lines, node, user_definitions, strict, state: _State) ) return 1 if strict and ( - state.qualify(_text(node), state.namespace) + _qualify(_text(node), state.namespace) not in set.union( user_definitions.get("all_bundle_names", set()), user_definitions.get("all_body_names", set()), @@ -297,14 +297,14 @@ def _parse_user_definitions(filename, lines, root_node): if ns_attr is not None: ns = _text(ns_attr.next_named_sibling).strip("\"'") elif name_node is not None: - body_blocks.add(_State.qualify(_text(name_node), ns)) + body_blocks.add(_qualify(_text(name_node), ns)) elif child.type == "bundle_block": name_node = next( (c for c in child.named_children if c.type == "bundle_block_name"), None, ) if name_node is not None: - bundle_blocks.add(_State.qualify(_text(name_node), ns)) + bundle_blocks.add(_qualify(_text(name_node), ns)) elif child.type == "promise_block": name_node = next( (c for c in child.named_children if c.type == "promise_block_name"), From a3b5e5233c24a30cbe57b271a91a1b1fee0c544a Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 14:19:37 +0200 Subject: [PATCH 11/23] Added end_of_file function to clear state after each file Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 42add99..1b375dc 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -39,6 +39,12 @@ class _State: attribute_name: str | None = None # "if" | "string" | "slist" | ... | None namespace: str = "default" # "ns" | "default" | ... | + def end_of_file(self): + assert self.block_type is None + assert self.promise_type is None + assert self.attribute_name is None + self.namespace = "default" + def update(self, node): """Updates and returns the state that should apply to the children of `node`.""" if node.type == "}": From c2f46d83bb295ac5bf440cb19ab0b1b599a67a69 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 14:28:52 +0200 Subject: [PATCH 12/23] Provided a single entry point for linting so state can persist Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/commands.py | 12 +++--------- src/cfengine_cli/lint.py | 11 ++++++++++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/cfengine_cli/commands.py b/src/cfengine_cli/commands.py index ead79a1..423eb17 100644 --- a/src/cfengine_cli/commands.py +++ b/src/cfengine_cli/commands.py @@ -4,7 +4,7 @@ import json from cfengine_cli.profile import profile_cfengine, generate_callstack from cfengine_cli.dev import dispatch_dev_subcommand -from cfengine_cli.lint import lint_folder, lint_single_arg +from cfengine_cli.lint import lint_args from cfengine_cli.shell import user_command from cfengine_cli.paths import bin from cfengine_cli.version import cfengine_cli_version_string @@ -96,14 +96,8 @@ def format(names, line_length) -> int: def _lint(files, strict) -> int: if not files: - return lint_folder(".", strict) - - errors = 0 - - for file in files: - errors += lint_single_arg(file, strict) - - return errors + return lint_args(["."], strict) + return lint_args(files, strict) def lint(files, strict) -> int: diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 1b375dc..b7ae6bc 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -26,12 +26,14 @@ BUILTIN_FUNCTIONS, ) + def _qualify(name: str, namespace: str) -> str: """If name is already qualified (contains ':'), return as-is. Otherwise prepend namespace.""" if ":" in name: return name return f"{namespace}:{name}" + @dataclass class _State: block_type: str | None = None # "bundle" | "body" | "promise" | None @@ -463,9 +465,16 @@ def lint_single_file(file, user_definitions=None, strict=True): return lint_policy_file(file, user_definitions=user_definitions, strict=strict) -def lint_single_arg(arg, strict=True): +def _lint_single_arg(arg, strict=True): if os.path.isdir(arg): return lint_folder(arg, strict) assert os.path.isfile(arg) return lint_single_file(arg, strict=strict) + + +def lint_args(args, strict): + errors = 0 + for arg in args: + errors += _lint_single_arg(arg, strict) + return errors From 10bd48a286dbb0f2933c615a57b8ea3efd835770 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 15:03:34 +0200 Subject: [PATCH 13/23] Fixed linting test for namespaces expected output Signed-off-by: Ole Herman Schumacher Elgesem --- tests/lint/008_namespace.output.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/lint/008_namespace.output.txt b/tests/lint/008_namespace.output.txt index abfbef5..0ed3872 100644 --- a/tests/lint/008_namespace.output.txt +++ b/tests/lint/008_namespace.output.txt @@ -1,7 +1,7 @@ vars: - "x" string => other_ns:unknown_func("arg"); - ^-------------------^ -Error: Call to unknown function / bundle / body 'other_ns:unknown_func' at at tests/lint/008_namespace.x.cf:9:19 + "x" string => default:target("arg"); + ^------------^ +Error: Call to unknown function / bundle / body 'default:target' at at tests/lint/008_namespace.x.cf:15:19 FAIL: tests/lint/008_namespace.x.cf (1 errors) Failure, 1 errors in total. From ea473479a4d2db2b4d5de93fca7a1c061d667e02 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 15:05:49 +0200 Subject: [PATCH 14/23] lint: Fix a bug by refactoring global state and user_definitions in state The user_definitions code for detecting namespace, unknown bundles, etc. did not work previously when you gave a filename as an argument. It was only working for folders. This commit fixes that, by refactoring the code and making the handling of args vs files vs folders more clear and consistent. Need to make the entry point functions create state, and then never be called recursively. I'm making state global now to make the refactoring easier, but can revert that later. I will do further refactoring commits after this one, but at this point I have something working, which passes tests, and fixes a bug. Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/docs.py | 4 +- src/cfengine_cli/lint.py | 194 +++++++++++++++++++++++++-------------- 2 files changed, 128 insertions(+), 70 deletions(-) diff --git a/src/cfengine_cli/docs.py b/src/cfengine_cli/docs.py index e2f0dcf..d21a3e8 100644 --- a/src/cfengine_cli/docs.py +++ b/src/cfengine_cli/docs.py @@ -16,7 +16,7 @@ from cfbs.pretty import pretty_file from cfbs.utils import find -from cfengine_cli.lint import lint_folder, lint_policy_file +from cfengine_cli.lint import lint_args, lint_policy_file from cfengine_cli.utils import UserError IGNORED_DIRS = [".git"] @@ -409,7 +409,7 @@ def check_docs() -> int: Run by the command: cfengine dev lint-docs""" - r = lint_folder(".", strict=False) + r = lint_args(["."], strict=False) if r != 0: return r _process_markdown_code_blocks( diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index b7ae6bc..a16bc20 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -40,6 +40,7 @@ class _State: promise_type: str | None = None # "vars" | "files" | "classes" | ... | None attribute_name: str | None = None # "if" | "string" | "slist" | ... | None namespace: str = "default" # "ns" | "default" | ... | + user_definitions = {} def end_of_file(self): assert self.block_type is None @@ -95,33 +96,8 @@ def update(self, node): return -def lint_cfbs_json(filename) -> int: - assert os.path.isfile(filename) - assert filename.endswith("cfbs.json") - - config = CFBSConfig.get_instance(filename=filename, non_interactive=True) - r = validate_config(config) - - if r == 0: - print(f"PASS: {filename}") - return 0 - print(f"FAIL: {filename}") - return r - - -def lint_json(filename) -> int: - assert os.path.isfile(filename) - - with open(filename, "r") as f: - data = f.read() - - try: - data = json.loads(data) - except: - print(f"FAIL: {filename} (invalid JSON)") - return 1 - print(f"PASS: {filename}") - return 0 +# TODO: Will remove this global in the future. +state = None def _highlight_range(node, lines): @@ -162,9 +138,11 @@ def _find_node_type(filename, lines, node, node_type): return matches -def _node_checks(filename, lines, node, user_definitions, strict, state: _State): +def _node_checks(filename, lines, node, strict): """Checks we run on each node in the syntax tree, utilizes state for checks which require context.""" + global state + assert state line = node.range.start_point[0] + 1 column = node.range.start_point[1] + 1 if node.type == "attribute_name" and _text(node) == "ifvarclass": @@ -182,11 +160,12 @@ def _node_checks(filename, lines, node, user_definitions, strict, state: _State) f"Deprecation: Promise type '{promise_type}' is deprecated at {filename}:{line}:{column}" ) return 1 + assert state if strict and ( ( promise_type not in BUILTIN_PROMISE_TYPES.union( - user_definitions.get("custom_promise_types", set()) + state.user_definitions.get("custom_promise_types", set()) ) ) ): @@ -220,9 +199,9 @@ def _node_checks(filename, lines, node, user_definitions, strict, state: _State) if ( strict and _qualify(_text(node), state.namespace) - in user_definitions.get("all_bundle_names", set()) + in state.user_definitions.get("all_bundle_names", set()) and state.promise_type - in user_definitions.get("custom_promise_types", set()) + in state.user_definitions.get("custom_promise_types", set()) ): _highlight_range(node, lines) print( @@ -232,8 +211,8 @@ def _node_checks(filename, lines, node, user_definitions, strict, state: _State) if strict and ( _qualify(_text(node), state.namespace) not in set.union( - user_definitions.get("all_bundle_names", set()), - user_definitions.get("all_body_names", set()), + state.user_definitions.get("all_bundle_names", set()), + state.user_definitions.get("all_body_names", set()), ) and _text(node) not in BUILTIN_FUNCTIONS ): @@ -245,19 +224,16 @@ def _node_checks(filename, lines, node, user_definitions, strict, state: _State) return 0 -def _stateful_walk( - filename, lines, node, user_definitions, strict, state: _State | None = None -) -> int: +def _stateful_walk(filename, lines, node, strict) -> int: + global state if state is None: state = _State() - errors = _node_checks(filename, lines, node, user_definitions, strict, state) + errors = _node_checks(filename, lines, node, strict) state.update(node) for child in node.children: - errors += _stateful_walk( - filename, lines, child, user_definitions, strict, state - ) + errors += _stateful_walk(filename, lines, child, strict) return errors @@ -277,9 +253,7 @@ def _walk(filename, lines, node, user_definitions=None, strict=True) -> int: line = node.range.start_point[0] + 1 column = node.range.start_point[1] + 1 - return _stateful_walk( - filename, lines, node, user_definitions, strict, state=_State() - ) + return _stateful_walk(filename, lines, node, strict) def _parse_user_definitions(filename, lines, root_node): @@ -341,15 +315,16 @@ def _parse_policy_file(filename): return tree, lines, original_data -def lint_policy_file( +def _lint_policy_file( filename, original_filename=None, original_line=None, snippet=None, prefix=None, - user_definitions=None, strict=True, ): + global state + assert state assert original_filename is None or type(original_filename) is str assert original_line is None or type(original_line) is int assert snippet is None or type(snippet) is int @@ -364,9 +339,6 @@ def lint_policy_file( assert os.path.isfile(filename) assert filename.endswith((".cf", ".cfengine3", ".cf3", ".cf.sub")) - if user_definitions is None: - user_definitions = {} - tree, lines, original_data = _parse_policy_file(filename) root_node = tree.root_node if root_node.type != "source_file": @@ -397,7 +369,7 @@ def lint_policy_file( else: print(f"Error: Empty policy file '{filename}'") errors += 1 - errors += _walk(filename, lines, root_node, user_definitions, strict) + errors += _walk(filename, lines, root_node, state, strict) if prefix: print(prefix, end="") if errors == 0: @@ -418,7 +390,7 @@ def lint_policy_file( return errors -def lint_folder(folder, strict=True): +def _lint_folder(folder, strict=True): errors = 0 policy_files = [] while folder.endswith(("/.", "/")): @@ -436,45 +408,131 @@ def lint_folder(folder, strict=True): else: errors += lint_single_file(filename) - user_definitions = {} - - # First pass: Gather custom types - for filename in policy_files if strict else []: - tree, lines, _ = _parse_policy_file(filename) - if tree.root_node.type == "source_file": - for key, val in _parse_user_definitions( - filename, lines, tree.root_node - ).items(): - user_definitions[key] = user_definitions.get(key, set()).union(val) - # Second pass: lint all policy files for filename in policy_files: - errors += lint_policy_file( - filename, user_definitions=user_definitions, strict=strict - ) + errors += _lint_policy_file(filename, strict=strict) return errors -def lint_single_file(file, user_definitions=None, strict=True): +def _lint_single_file(file, strict=True): assert os.path.isfile(file) if file.endswith("/cfbs.json"): return lint_cfbs_json(file) if file.endswith(".json"): return lint_json(file) assert file.endswith(".cf") - return lint_policy_file(file, user_definitions=user_definitions, strict=strict) + return lint_policy_file(file, strict=strict) def _lint_single_arg(arg, strict=True): if os.path.isdir(arg): - return lint_folder(arg, strict) + return _lint_folder(arg, strict) assert os.path.isfile(arg) - return lint_single_file(arg, strict=strict) + return _lint_single_file(arg, strict=strict) + + +def _discovery_file(filename): + global state + assert state + tree, lines, _ = _parse_policy_file(filename) + if tree.root_node.type == "source_file": + for key, val in _parse_user_definitions( + filename, lines, tree.root_node + ).items(): + state.user_definitions[key] = state.user_definitions.get(key, set()).union( + val + ) + + +def _discovery_folder(folder): + global state + assert state + assert os.path.isdir(folder) + for filename in os.listdir(folder): + _discovery_file(folder + "/" + filename) + + +def _discovery_args(args): + global state + assert state + for arg in args: + if ( + arg in ("/", ".", "./", "~", "~/") + or arg.endswith("/") + or os.path.isdir(arg) + ): + _discovery_folder(arg) + else: + _discovery_file(arg) + + +# Interface: These are the functions we want to be called from outside +# They create _State() and should not be called recursively inside lint.py + + +def lint_single_file(file, strict=True): + global state + state = _State() + _discovery_file(file) + return _lint_single_file(file, strict) def lint_args(args, strict): + global state + state = _State() + _discovery_args(args) errors = 0 for arg in args: errors += _lint_single_arg(arg, strict) return errors + + +def lint_policy_file( + filename, + original_filename=None, + original_line=None, + snippet=None, + prefix=None, + strict=True, +): + global state + state = _State() + _discovery_file(filename) + return _lint_policy_file( + filename=filename, + original_filename=original_filename, + original_line=original_line, + snippet=snippet, + prefix=prefix, + strict=strict, + ) + + +def lint_cfbs_json(filename) -> int: + assert os.path.isfile(filename) + assert filename.endswith("cfbs.json") + + config = CFBSConfig.get_instance(filename=filename, non_interactive=True) + r = validate_config(config) + + if r == 0: + print(f"PASS: {filename}") + return 0 + print(f"FAIL: {filename}") + return r + + +def lint_json(filename) -> int: + assert os.path.isfile(filename) + + with open(filename, "r") as f: + data = f.read() + + try: + data = json.loads(data) + except: + print(f"FAIL: {filename} (invalid JSON)") + return 1 + print(f"PASS: {filename}") + return 0 From 255c9a9c88b23c121c0fb2095f6955b9f30913d0 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 15:14:33 +0200 Subject: [PATCH 15/23] Partially cleared state after each file discovery Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index a16bc20..99e640a 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -436,13 +436,10 @@ def _discovery_file(filename): global state assert state tree, lines, _ = _parse_policy_file(filename) - if tree.root_node.type == "source_file": - for key, val in _parse_user_definitions( - filename, lines, tree.root_node - ).items(): - state.user_definitions[key] = state.user_definitions.get(key, set()).union( - val - ) + assert tree.root_node.type == "source_file" + for key, val in _parse_user_definitions(filename, lines, tree.root_node).items(): + state.user_definitions[key] = state.user_definitions.get(key, set()).union(val) + state.end_of_file() def _discovery_folder(folder): From 17f07ad53c2fcf031060b9fd08c52197e310ee84 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 16:53:31 +0200 Subject: [PATCH 16/23] Improved readability by renaming, asserting, fixing pyright errors Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 60 ++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 27 deletions(-) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 99e640a..75a87ef 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -6,8 +6,19 @@ - cfbs.json (CFEngine Build project files) - *.json (basic JSON syntax checking) +Linting is performed in 3 steps: +1. Parsing - Read the .cf code and convert it into syntax trees +2. Discovery - Walk the syntax trees and record what is defined +3. Checking - Walk the syntax tree again and check for errors + +By default, linting is strict about bundles and bodies being +defined somewhere in the supplied files / folders. This +can be disabled using the `--strict=no` flag. + Usage: $ cfengine lint +$ cfengine lint ./core/ ./masterfiles/ +$ cfengine lint --strict=no main.cf """ import os @@ -35,7 +46,7 @@ def _qualify(name: str, namespace: str) -> str: @dataclass -class _State: +class State: block_type: str | None = None # "bundle" | "body" | "promise" | None promise_type: str | None = None # "vars" | "files" | "classes" | ... | None attribute_name: str | None = None # "if" | "string" | "slist" | ... | None @@ -48,9 +59,16 @@ def end_of_file(self): assert self.attribute_name is None self.namespace = "default" - def update(self, node): - """Updates and returns the state that should apply to the children of `node`.""" + def navigate(self, node): + """This function is called whenever we move to a node, to update the + state accordingly. + + For example: + - When we encounter a closing } for a bundle, we want to set + block_type from "bundle" to None + """ if node.type == "}": + assert self.attribute_name is None # Should already be cleared by ; assert node.parent assert node.parent.type in [ "bundle_block_body", @@ -58,11 +76,11 @@ def update(self, node): "body_block_body", "list", ] - if node.parent.type != "list": - # We just ended a block - self.block_type = None - self.promise_type = None - self.attribute_name = None + if node.parent.type == "list": + return + # We just ended a block + self.block_type = None + self.promise_type = None return if node.type == ";": self.attribute_name = None @@ -78,12 +96,8 @@ def update(self, node): return if node.type == "bundle_section": # A bundle_section is always: promise_guard, [promises], [class_guarded_promises...] - # The promise_guard is guaranteed to exist by the grammar guard = next((c for c in node.children if c.type == "promise_guard"), None) - if guard is None: # Should never happen - print("ERROR: Bundle section without a promise guard") - return - + assert guard # guaranteed to exist by the grammar self.promise_type = _text(guard)[:-1] # strip trailing ':' return if node.type == "attribute": @@ -141,7 +155,6 @@ def _find_node_type(filename, lines, node, node_type): def _node_checks(filename, lines, node, strict): """Checks we run on each node in the syntax tree, utilizes state for checks which require context.""" - global state assert state line = node.range.start_point[0] + 1 column = node.range.start_point[1] + 1 @@ -160,7 +173,6 @@ def _node_checks(filename, lines, node, strict): f"Deprecation: Promise type '{promise_type}' is deprecated at {filename}:{line}:{column}" ) return 1 - assert state if strict and ( ( promise_type @@ -227,11 +239,11 @@ def _node_checks(filename, lines, node, strict): def _stateful_walk(filename, lines, node, strict) -> int: global state if state is None: - state = _State() + state = State() errors = _node_checks(filename, lines, node, strict) - state.update(node) + state.navigate(node) for child in node.children: errors += _stateful_walk(filename, lines, child, strict) return errors @@ -323,7 +335,6 @@ def _lint_policy_file( prefix=None, strict=True, ): - global state assert state assert original_filename is None or type(original_filename) is str assert original_line is None or type(original_line) is int @@ -433,7 +444,6 @@ def _lint_single_arg(arg, strict=True): def _discovery_file(filename): - global state assert state tree, lines, _ = _parse_policy_file(filename) assert tree.root_node.type == "source_file" @@ -443,16 +453,12 @@ def _discovery_file(filename): def _discovery_folder(folder): - global state - assert state assert os.path.isdir(folder) for filename in os.listdir(folder): _discovery_file(folder + "/" + filename) def _discovery_args(args): - global state - assert state for arg in args: if ( arg in ("/", ".", "./", "~", "~/") @@ -465,19 +471,19 @@ def _discovery_args(args): # Interface: These are the functions we want to be called from outside -# They create _State() and should not be called recursively inside lint.py +# They create State() and should not be called recursively inside lint.py def lint_single_file(file, strict=True): global state - state = _State() + state = State() _discovery_file(file) return _lint_single_file(file, strict) def lint_args(args, strict): global state - state = _State() + state = State() _discovery_args(args) errors = 0 for arg in args: @@ -494,7 +500,7 @@ def lint_policy_file( strict=True, ): global state - state = _State() + state = State() _discovery_file(filename) return _lint_policy_file( filename=filename, From 1dd4246799775c8f64cbcf7899861debf32b0ebf Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 17:03:37 +0200 Subject: [PATCH 17/23] Added mode Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 75a87ef..28bb098 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -21,6 +21,7 @@ $ cfengine lint --strict=no main.cf """ +from enum import Enum import os import json import itertools @@ -45,12 +46,19 @@ def _qualify(name: str, namespace: str) -> str: return f"{namespace}:{name}" +class Mode(Enum): + NONE = None + DISCOVERY = 1 + CHECKING = 2 + + @dataclass class State: block_type: str | None = None # "bundle" | "body" | "promise" | None promise_type: str | None = None # "vars" | "files" | "classes" | ... | None attribute_name: str | None = None # "if" | "string" | "slist" | ... | None namespace: str = "default" # "ns" | "default" | ... | + mode: Mode = Mode.NONE user_definitions = {} def end_of_file(self): @@ -237,10 +245,7 @@ def _node_checks(filename, lines, node, strict): def _stateful_walk(filename, lines, node, strict) -> int: - global state - if state is None: - state = State() - + assert state errors = _node_checks(filename, lines, node, strict) state.navigate(node) From 2bb92d6f04431ce955fc091ee287de919d06bd4f Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 17:03:56 +0200 Subject: [PATCH 18/23] Added makefile to make running tests easier Signed-off-by: Ole Herman Schumacher Elgesem --- Makefile | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 Makefile diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..d2fbaff --- /dev/null +++ b/Makefile @@ -0,0 +1,22 @@ +.PHONY: check format lint install + +default: check + +format: + uv tool run black . + +lint: + uv tool run black --check . + uv tool run flake8 src/ --ignore=E203,W503,E722,E731 --max-complexity=100 --max-line-length=160 + uv tool run pyflakes src/ + uv tool run pyright src/ + +check: format lint install + uv tool run black --check . + uv tool run flake8 src/ --ignore=E203,W503,E722,E731 --max-complexity=100 --max-line-length=160 + uv tool run pyflakes src/ + uv tool run pyright src/ + uv run pytest + bash tests/run-lint-tests.sh + bash tests/run-format-tests.sh + bash tests/run-shell-tests.sh From 15b7243019b63e74b85f7ca016615bab843c2409 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 17:50:46 +0200 Subject: [PATCH 19/23] HACKING.md: Fixed outdated information Signed-off-by: Ole Herman Schumacher Elgesem --- HACKING.md | 98 ++++++++++++++++++++++++++---------------------------- Makefile | 5 +++ 2 files changed, 52 insertions(+), 51 deletions(-) diff --git a/HACKING.md b/HACKING.md index f97bc94..df9a2bf 100644 --- a/HACKING.md +++ b/HACKING.md @@ -4,6 +4,53 @@ This document aims to have relevant information for people contributing to and m It is not necessary for users of the tool to know about these processes. For general user information, see the [README](./README.md). +## Code formatting + +We use automated code formatters to ensure consistent code style / indentation. +Please format Python code with [black](https://pypi.org/project/black/), and YAML and markdown files with [Prettier](https://prettier.io/). +For simplicity's sake, we don't have a custom configuration, we use the tool's defaults. + +If your editor does not do this automatically, you can run these tools from the command line: + +```bash +make format +``` + +## Installing from source: + +For developers working on CFEngine CLI, it is recommended to install an editable version of the tool: + +```bash +make install +``` + +Some of the tests require that you have the CLI installed (they run `cfengine` commands). + +## Running commands without installing + +You can also run commands without installing, using `uv`: + +```bash +uv run cfengine format +``` + +## Running tests + +Use the makefile command to run all linting and tests: + +```bash +make check +``` + +Running individual test suites: + +```bash +uv run pytest +bash tests/run-lint-tests.sh +bash tests/run-format-tests.sh +bash tests/run-shell-tests.sh +``` + ## Releasing new versions Releases are [automated using a GH Action](https://github.com/cfengine/cfengine-cli/blob/main/.github/workflows/pypi-publish.yml) @@ -79,62 +126,11 @@ Copy the token and paste it into the GitHub Secret named `PYPI_PASSWORD`. `PYPI_USERNAME` should be there already, you don't have to edit it, it is simply `__token__`. Don't store the token anywhere else - we generate new tokens if necessary. -## Code formatting - -We use automated code formatters to ensure consistent code style / indentation. -Please format Python code with [black](https://pypi.org/project/black/), and YAML and markdown files with [Prettier](https://prettier.io/). -For simplicity's sake, we don't have a custom configuration, we use the tool's defaults. - -If your editor does not do this automatically, you can run these tools from the command line: - -```bash -black . && prettier . --write -``` - -## Running commands during development - -This project uses `uv`. -This makes it easy to run commands without installing the project, for example: - -```bash -uv run cfengine format -``` - -## Installing from source: - -```bash -git fetch --all --tags -pip3 install . -``` - -## Running tests - -Unit tests: - -```bash -py.test -``` - -Shell tests (requires installing first): - -```bash -cat tests/shell/*.sh | bash -``` - ## Not implemented yet / TODOs - `cfengine run` - The command could automatically detect that you have CFEngine installed on a remote hub, and run it there instead (using `cf-remote`). - Handle when `cf-agent` is not installed, help users install. - Prompt / help users do what they meant (i.e. build and deploy and run). -- `cfengine format` - - Automatically break up and indent method calls, function calls, and nested function calls. - - Smarter placement of comments based on context. - - The command should be able to take a filename as an argument, and also operate using stdin and stdout. - (Receive file content on stdin, file type using command line arg, output formatted file to stdout). - - We can add a shortcut, `cfengine fmt`, since that matches other tools, like `deno`. -- `cfengine lint` - - The command should be able to take a filename as an argument, and also take file content from stdin. - - It would be nice if we refactored `validate_config()` in `cfbs` so it would take a simple dictionary (JSON) instead of a special CFBSConfig object. - Missing commands: - `cfengine install` - Install CFEngine packages / binaries (Wrapping `cf-remote install`). diff --git a/Makefile b/Makefile index d2fbaff..f955a47 100644 --- a/Makefile +++ b/Makefile @@ -4,6 +4,7 @@ default: check format: uv tool run black . + prettier . --write lint: uv tool run black --check . @@ -20,3 +21,7 @@ check: format lint install bash tests/run-lint-tests.sh bash tests/run-format-tests.sh bash tests/run-shell-tests.sh + +install: + git fetch --all --tags + pipx install --force --editable . From 90effd6f624529dbb5c3069e11b76e20724d7f61 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Thu, 2 Apr 2026 18:41:38 +0200 Subject: [PATCH 20/23] lint: Big refactoring complete Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/commands.py | 2 + src/cfengine_cli/docs.py | 4 +- src/cfengine_cli/lint.py | 687 +++++++++++++++++++++-------------- 3 files changed, 416 insertions(+), 277 deletions(-) diff --git a/src/cfengine_cli/commands.py b/src/cfengine_cli/commands.py index 423eb17..6db55f2 100644 --- a/src/cfengine_cli/commands.py +++ b/src/cfengine_cli/commands.py @@ -104,6 +104,8 @@ def lint(files, strict) -> int: errors = _lint(files, strict) if errors == 0: print("Success, no errors found.") + elif errors == 1: + print("Failure, 1 error in total.") else: print(f"Failure, {errors} errors in total.") return errors diff --git a/src/cfengine_cli/docs.py b/src/cfengine_cli/docs.py index d21a3e8..7f37c54 100644 --- a/src/cfengine_cli/docs.py +++ b/src/cfengine_cli/docs.py @@ -16,7 +16,7 @@ from cfbs.pretty import pretty_file from cfbs.utils import find -from cfengine_cli.lint import lint_args, lint_policy_file +from cfengine_cli.lint import lint_args, lint_policy_file_snippet from cfengine_cli.utils import UserError IGNORED_DIRS = [".git"] @@ -138,7 +138,7 @@ def fn_check_syntax( match language: case "cf": - r = lint_policy_file( + r = lint_policy_file_snippet( snippet_abs_path, origin_path, first_line + 1, snippet_number, prefix ) if r != 0: diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 28bb098..d60d38d 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -24,7 +24,6 @@ from enum import Enum import os import json -import itertools import tree_sitter_cfengine as tscfengine from dataclasses import dataclass from tree_sitter import Language, Parser @@ -41,6 +40,8 @@ def _qualify(name: str, namespace: str) -> str: """If name is already qualified (contains ':'), return as-is. Otherwise prepend namespace.""" + assert '"' not in namespace + assert '"' not in name if ":" in name: return name return f"{namespace}:{name}" @@ -48,24 +49,74 @@ def _qualify(name: str, namespace: str) -> str: class Mode(Enum): NONE = None - DISCOVERY = 1 - CHECKING = 2 + SYNTAX = 1 + DISCOVER = 2 + LINT = 3 @dataclass class State: - block_type: str | None = None # "bundle" | "body" | "promise" | None + block_keyword: str | None = None # "bundle" | "body" | "promise" | None + block_type: str | None = None + block_name: str | None = None promise_type: str | None = None # "vars" | "files" | "classes" | ... | None attribute_name: str | None = None # "if" | "string" | "slist" | ... | None namespace: str = "default" # "ns" | "default" | ... | mode: Mode = Mode.NONE - user_definitions = {} + walking: bool = False + strict: bool = True + bundles = {} + bodies = {} + custom_promise_types = {} + policy_file = None + + def print_summary(self): + print("Bundles") + print(self.bundles) + print("Bodies") + print(self.bodies) + print("Custom promise types") + print(self.custom_promise_types) + + def block_string(self) -> str | None: + if not (self.block_keyword and self.block_type and self.block_name): + return None + + return " ".join((self.block_keyword, self.block_type, self.block_name)) + + def start_file(self, policy_file): + assert not self.walking + assert self.mode != Mode.NONE + self.policy_file = policy_file + self.namespace = "default" + self.walking = True def end_of_file(self): - assert self.block_type is None + assert self.walking + assert self.mode != Mode.NONE + assert self.block_keyword is None assert self.promise_type is None assert self.attribute_name is None - self.namespace = "default" + self.walking = False + self.policy_file = None + + def add_bundle(self, name: str): + name = _qualify(name, self.namespace) + # TODO: In the future we will record more information than True, like: + # - Can be a list / dict of all places a bundle with that + # qualified name is defined in cases there are duplicates. + # - Can record the location of each definition + # - Can record the parameters / signature + # - Can record whether the bundle is inside a macro + # - Can have a list of classes and vars defined inside + self.bundles[name] = True + + def add_body(self, name: str): + name = _qualify(name, self.namespace) + self.bodies[name] = True + + def add_promise_type(self, name: str): + self.custom_promise_types[name] = True def navigate(self, node): """This function is called whenever we move to a node, to update the @@ -73,49 +124,248 @@ def navigate(self, node): For example: - When we encounter a closing } for a bundle, we want to set - block_type from "bundle" to None + block_keyword from "bundle" to None """ - if node.type == "}": + assert self.mode != Mode.NONE + assert self.walking + + # Beginnings of blocks: + if node.type in ( + "bundle_block_keyword", + "body_block_keyword", + "promise_block_keyword", + ): + self.block_keyword = _text(node) + assert self.block_keyword in ("bundle", "body", "promise") + return + if node.type in ( + "bundle_block_type", + "body_block_type", + "promise_block_type", + ): + self.block_type = _text(node) + assert self.block_type + return + if node.type in ( + "bundle_block_name", + "body_block_name", + "promise_block_name", + ): + self.block_name = _text(node) + assert self.block_name + return + + # Update namespace inside body file control: + if ( + self.block_string() == "body file control" + and self.attribute_name == "namespace" + and node.type == "quoted_string" + ): + self.namespace = _text(node)[1:-1] + return + + # New promise type (bundle section) inside a bundle: + if node.type == "promise_guard": + self.promise_type = _text(node)[:-1] # strip trailing ':' + return + + if node.type == "attribute_name": + self.attribute_name = _text(node) + return + + # Attributes always end with ; in all 3 block types + if node.type == ";": + self.attribute_name = None + return + + # Clear things when ending a top level block: + if node.type == "}" and node.parent.type != "list": assert self.attribute_name is None # Should already be cleared by ; assert node.parent assert node.parent.type in [ "bundle_block_body", "promise_block_body", "body_block_body", - "list", ] - if node.parent.type == "list": - return - # We just ended a block + self.block_keyword = None self.block_type = None + self.block_name = None self.promise_type = None return - if node.type == ";": - self.attribute_name = None - return - if node.type == "bundle_block": - self.block_type = "bundle" - return - if node.type == "body_block": - self.block_type = "body" - return - if node.type == "promise_block": - self.block_type = "promise" - return - if node.type == "bundle_section": - # A bundle_section is always: promise_guard, [promises], [class_guarded_promises...] - guard = next((c for c in node.children if c.type == "promise_guard"), None) - assert guard # guaranteed to exist by the grammar - self.promise_type = _text(guard)[:-1] # strip trailing ':' - return - if node.type == "attribute": - for child in node.children: - if child.type == "attribute_name": - self.attribute_name = _text(child) - if self.attribute_name == "namespace": - self.namespace = _text(child.next_named_sibling).strip("\"'") - return - return + + +class PolicyFile: + def __init__(self, filename): + self.filename = filename + tree, lines, original_data = _parse_policy_file(filename) + self.tree = tree + self.lines = lines + self.original_data = original_data + + # Flatten tree so it is easier to iterate over: + self.nodes = [] + + def visit(x): + self.nodes.append(x) + return 0 + + _walk_callback(tree.root_node, visit) + + +def _check_syntax(policy_file: PolicyFile) -> int: + assert state + assert state.mode == Mode.SYNTAX + filename = policy_file.filename + lines = policy_file.lines + errors = 0 + if not policy_file.tree.root_node.children: + print(f"Error: Empty policy file '{filename}'") + return 1 + + assert policy_file.tree.root_node.type == "source_file" + + state.start_file(policy_file) + for node in policy_file.nodes: + state.navigate(node) + _discover_node(node) + if node.type != "ERROR": + continue + line = node.range.start_point[0] + 1 + column = node.range.start_point[1] + 1 + _highlight_range(node, lines) + print(f"Error: Syntax error at {filename}:{line}:{column}") + errors += 1 + state.end_of_file() + return errors + + +def _discover_node(node) -> int: + assert state + # Define bodies: + if node.type == "body_block_name": + name = _text(node) + if name == "control": + return 0 # No need to define control blocks + state.add_body(name) + return 0 + + # Define bundles: + if node.type == "bundle_block_name": + name = _text(node) + state.add_bundle(name) + return 0 + + # Define custom promise types: + if node.type == "promise_block_name": + state.add_promise_type(_text(node)) + return 0 + + return 0 + + +def _discover(policy_file: PolicyFile) -> int: + assert state + assert state.mode == Mode.DISCOVER + state.start_file(policy_file) + for node in policy_file.nodes: + state.navigate(node) + _discover_node(node) + state.end_of_file() + return 0 + + +def _lint_node(node, policy_file): + return _node_checks(policy_file.filename, policy_file.lines, node) + + +def _lint(policy_file: PolicyFile) -> int: + assert state + assert state.mode == Mode.LINT + errors = 0 + state.start_file(policy_file) + for node in policy_file.nodes: + state.navigate(node) + errors += _lint_node(node, policy_file) + state.end_of_file() + if errors == 0: + print(f"PASS: {policy_file.filename}") + else: + print( + f"FAIL: {policy_file.filename} ({errors} error{'s' if errors > 1 else ''})" + ) + return errors + + +def _find_policy_files(args): + for arg in args: + if os.path.isdir(arg): + while arg.endswith(("/.", "/")): + arg = arg[0:-1] + for result in find(arg, extension=".cf"): + yield result + elif arg.endswith(".cf"): + yield arg + + +def _find_json_files(args): + for arg in args: + if os.path.isdir(arg): + for result in find(arg, extension=".json"): + yield result + elif arg.endswith(".json"): + yield arg + + +def filter_filenames(filenames): + for filename in filenames: + if "/out/" in filename or "/." in filename: + continue + if filename.startswith(".") and not filename.startswith("./"): + continue + if filename.startswith("out/"): + continue + yield filename + + +def _lint_main(args: list, strict: bool) -> int: + errors = 0 + + global state + state = State() + state.strict = strict + state.mode = Mode.SYNTAX + + json_filenames = filter_filenames(_find_json_files(args)) + policy_filenames = filter_filenames(_find_policy_files(args)) + + # TODO: JSON checking could be split into parse + # and additional checks for cfbs.json. + # The second step could happen after discovery for consistency. + for file in json_filenames: + errors += _lint_json(file) + + policy_files = [] + for filename in policy_filenames: + policy_file = PolicyFile(filename) + r = _check_syntax(policy_file) + errors += r + if r != 0: + print(f"FAIL: {filename} ({errors} error{'s' if errors > 1 else ''})") + continue + policy_files.append(policy_file) + if errors != 0: + return errors + state.mode = Mode.DISCOVER + + for policy_file in policy_files: + errors += _discover(policy_file) + + state.mode = Mode.LINT + + for policy_file in policy_files: + errors += _lint(policy_file) + + return errors # TODO: Will remove this global in the future. @@ -147,20 +397,18 @@ def _text(node): return node.text.decode() -def _walk_generic(filename, lines, node, visitor): - visitor(node) - for node in node.children: - _walk_generic(filename, lines, node, visitor) +def _walk_callback(node, callback) -> int: + assert node + assert callback - -def _find_node_type(filename, lines, node, node_type): - matches = [] - visitor = lambda x: matches.extend([x] if x.type == node_type else []) - _walk_generic(filename, lines, node, visitor) - return matches + errors = 0 + errors += callback(node) + for child in node.children: + _walk_callback(child, callback) + return errors -def _node_checks(filename, lines, node, strict): +def _node_checks(filename, lines, node): """Checks we run on each node in the syntax tree, utilizes state for checks which require context.""" assert state @@ -181,13 +429,10 @@ def _node_checks(filename, lines, node, strict): f"Deprecation: Promise type '{promise_type}' is deprecated at {filename}:{line}:{column}" ) return 1 - if strict and ( - ( - promise_type - not in BUILTIN_PROMISE_TYPES.union( - state.user_definitions.get("custom_promise_types", set()) - ) - ) + if ( + state.strict + and promise_type not in BUILTIN_PROMISE_TYPES + and promise_type not in state.custom_promise_types ): _highlight_range(node, lines) print( @@ -216,107 +461,48 @@ def _node_checks(filename, lines, node, strict): ) return 1 if node.type == "calling_identifier": + name = _text(node) + qualified_name = _qualify(name, state.namespace) if ( - strict - and _qualify(_text(node), state.namespace) - in state.user_definitions.get("all_bundle_names", set()) - and state.promise_type - in state.user_definitions.get("custom_promise_types", set()) + state.strict + and qualified_name in state.bundles + and state.promise_type in state.custom_promise_types ): _highlight_range(node, lines) print( - f"Error: Call to bundle '{_text(node)}' inside custom promise: '{state.promise_type}' at {filename}:{line}:{column}" + f"Error: Call to bundle '{name}' inside custom promise: '{state.promise_type}' at {filename}:{line}:{column}" ) return 1 - if strict and ( - _qualify(_text(node), state.namespace) - not in set.union( - state.user_definitions.get("all_bundle_names", set()), - state.user_definitions.get("all_body_names", set()), - ) - and _text(node) not in BUILTIN_FUNCTIONS + if state.strict and ( + qualified_name not in state.bundles + and qualified_name not in state.bodies + and name not in BUILTIN_FUNCTIONS ): _highlight_range(node, lines) print( - f"Error: Call to unknown function / bundle / body '{_text(node)}' at at {filename}:{line}:{column}" + f"Error: Call to unknown function / bundle / body '{name}' at at {filename}:{line}:{column}" ) return 1 return 0 -def _stateful_walk(filename, lines, node, strict) -> int: +def _stateful_walk(filename, lines, node) -> int: assert state - errors = _node_checks(filename, lines, node, strict) - - state.navigate(node) - for child in node.children: - errors += _stateful_walk(filename, lines, child, strict) - return errors - - -def _walk(filename, lines, node, user_definitions=None, strict=True) -> int: - if user_definitions is None: - user_definitions = {} - - error_nodes = _find_node_type(filename, lines, node, "ERROR") - if error_nodes: - for node in error_nodes: - line = node.range.start_point[0] + 1 - column = node.range.start_point[1] + 1 - _highlight_range(node, lines) - print(f"Error: Syntax error at {filename}:{line}:{column}") - return len(error_nodes) + errors = 0 - line = node.range.start_point[0] + 1 - column = node.range.start_point[1] + 1 + def visit(x): + nonlocal errors + assert state + state.navigate(node) + if state.mode == Mode.LINT: + errors += _node_checks(filename, lines, node) + return errors - return _stateful_walk(filename, lines, node, strict) + return _walk_callback(node, visit) -def _parse_user_definitions(filename, lines, root_node): - ns = "default" - promise_blocks = set() - bundle_blocks = set() - body_blocks = set() - - for child in root_node.children: - if child.type == "body_block": - name_node = next( - (c for c in child.named_children if c.type == "body_block_name"), - None, - ) - ns_attr = next( - ( - c - for c in _find_node_type(filename, lines, child, "attribute_name") - if _text(c) == "namespace" - ), - None, - ) - if ns_attr is not None: - ns = _text(ns_attr.next_named_sibling).strip("\"'") - elif name_node is not None: - body_blocks.add(_qualify(_text(name_node), ns)) - elif child.type == "bundle_block": - name_node = next( - (c for c in child.named_children if c.type == "bundle_block_name"), - None, - ) - if name_node is not None: - bundle_blocks.add(_qualify(_text(name_node), ns)) - elif child.type == "promise_block": - name_node = next( - (c for c in child.named_children if c.type == "promise_block_name"), - None, - ) - if name_node is not None: - promise_blocks.add(_text(name_node)) - - return { - "custom_promise_types": promise_blocks, - "all_bundle_names": bundle_blocks, - "all_body_names": body_blocks, - } +def _walk(filename, lines, node) -> int: + return _stateful_walk(filename, lines, node) def _parse_policy_file(filename): @@ -332,147 +518,97 @@ def _parse_policy_file(filename): return tree, lines, original_data -def _lint_policy_file( +def _lint_policy_file_snippet( filename, - original_filename=None, - original_line=None, - snippet=None, - prefix=None, - strict=True, + original_filename, + original_line, + snippet, + prefix, ): assert state - assert original_filename is None or type(original_filename) is str - assert original_line is None or type(original_line) is int - assert snippet is None or type(snippet) is int - if ( - original_filename is not None - or original_line is not None - or snippet is not None - ): - assert original_filename and os.path.isfile(original_filename) - assert original_line and original_line > 0 - assert snippet and snippet > 0 + assert prefix + assert type(original_filename) is str + assert type(original_line) is int + assert type(snippet) is int + assert type(prefix) is str + assert original_filename and os.path.isfile(original_filename) + assert original_line and original_line > 0 + assert snippet and snippet > 0 assert os.path.isfile(filename) assert filename.endswith((".cf", ".cfengine3", ".cf3", ".cf.sub")) tree, lines, original_data = _parse_policy_file(filename) root_node = tree.root_node - if root_node.type != "source_file": - if snippet: - assert original_filename and original_line - print( - f"Error: Failed to parse policy snippet {snippet} at '{original_filename}:{original_line}'" - ) - else: - print(f" Empty policy file '{filename}'") - print(" Is this valid CFEngine policy?") - print("") - lines = original_data.decode().split("\n") - if not len(lines) <= 5: - lines = lines[:4] + ["..."] - for line in lines: - print(" " + line) - print("") - return 1 assert root_node.type == "source_file" errors = 0 if not root_node.children: - if snippet: - assert original_filename and original_line - print( - f"Error: Empty policy snippet {snippet} at '{original_filename}:{original_line}'" - ) - else: - print(f"Error: Empty policy file '{filename}'") + assert original_filename and original_line + print( + f"Error: Empty policy snippet {snippet} at '{original_filename}:{original_line}'" + ) errors += 1 - errors += _walk(filename, lines, root_node, state, strict) - if prefix: - print(prefix, end="") + state.start_file(filename) + errors += _walk(filename, lines, root_node) + state.end_of_file() + print(prefix, end="") if errors == 0: - if snippet: - assert original_filename and original_line - print( - f"PASS: Snippet {snippet} at '{original_filename}:{original_line}' (cf3)" - ) - else: - print(f"PASS: {filename}") + assert original_filename and original_line + print(f"PASS: Snippet {snippet} at '{original_filename}:{original_line}' (cf3)") return 0 - if snippet: - assert original_filename and original_line - print(f"FAIL: Snippet {snippet} at '{original_filename}:{original_line}' (cf3)") - else: - print(f"FAIL: {filename} ({errors} error{'s' if errors > 0 else ''})") + assert original_filename and original_line + print(f"FAIL: Snippet {snippet} at '{original_filename}:{original_line}' (cf3)") return errors -def _lint_folder(folder, strict=True): - errors = 0 - policy_files = [] - while folder.endswith(("/.", "/")): - folder = folder[0:-1] - for filename in itertools.chain( - find(folder, extension=".json"), find(folder, extension=".cf") - ): - if filename.startswith(("./.", "./out/", folder + "/.", folder + "/out/")): - continue - if filename.startswith(".") and not filename.startswith("./"): - continue +def _lint_policy_file( + filename, + prefix=None, +): + assert state + assert os.path.isfile(filename) + assert filename.endswith((".cf", ".cfengine3", ".cf3", ".cf.sub")) - if filename.endswith((".cf", ".cfengine3", ".cf3", ".cf.sub")): - policy_files.append(filename) - else: - errors += lint_single_file(filename) + tree, lines, original_data = _parse_policy_file(filename) + root_node = tree.root_node + assert root_node.type == "source_file" + errors = 0 + if not root_node.children: + print(f"Error: Empty policy file '{filename}'") + errors += 1 + state.start_file(filename) + errors += _walk(filename, lines, root_node) + state.end_of_file() + if prefix: + print(prefix, end="") + if errors == 0: + print(f"PASS: {filename}") + return 0 - # Second pass: lint all policy files - for filename in policy_files: - errors += _lint_policy_file(filename, strict=strict) + print(f"FAIL: {filename} ({errors} error{'s' if errors > 1 else ''})") return errors -def _lint_single_file(file, strict=True): +def _lint_json(file): assert os.path.isfile(file) if file.endswith("/cfbs.json"): return lint_cfbs_json(file) - if file.endswith(".json"): - return lint_json(file) - assert file.endswith(".cf") - return lint_policy_file(file, strict=strict) - - -def _lint_single_arg(arg, strict=True): - if os.path.isdir(arg): - return _lint_folder(arg, strict) - assert os.path.isfile(arg) - - return _lint_single_file(arg, strict=strict) + assert file.endswith(".json") + return lint_json(file) def _discovery_file(filename): assert state - tree, lines, _ = _parse_policy_file(filename) - assert tree.root_node.type == "source_file" - for key, val in _parse_user_definitions(filename, lines, tree.root_node).items(): - state.user_definitions[key] = state.user_definitions.get(key, set()).union(val) - state.end_of_file() - + assert state.mode == Mode.DISCOVER -def _discovery_folder(folder): - assert os.path.isdir(folder) - for filename in os.listdir(folder): - _discovery_file(folder + "/" + filename) - - -def _discovery_args(args): - for arg in args: - if ( - arg in ("/", ".", "./", "~", "~/") - or arg.endswith("/") - or os.path.isdir(arg) - ): - _discovery_folder(arg) - else: - _discovery_file(arg) + tree, lines, original_data = _parse_policy_file(filename) + root_node = tree.root_node + assert root_node.type == "source_file" + errors = 0 + errors += _lint_policy_file(filename) + # errors += _walk(filename, lines, root_node) + state.end_of_file() + return errors # Interface: These are the functions we want to be called from outside @@ -480,41 +616,42 @@ def _discovery_args(args): def lint_single_file(file, strict=True): - global state - state = State() - _discovery_file(file) - return _lint_single_file(file, strict) + return _lint_main([file], strict) -def lint_args(args, strict): - global state - state = State() - _discovery_args(args) - errors = 0 - for arg in args: - errors += _lint_single_arg(arg, strict) - return errors +def lint_args(args, strict=True) -> int: + return _lint_main(args, strict) -def lint_policy_file( +def lint_policy_file_snippet( filename, - original_filename=None, - original_line=None, - snippet=None, - prefix=None, + original_filename, + original_line, + snippet, + prefix, strict=True, ): global state state = State() - _discovery_file(filename) - return _lint_policy_file( - filename=filename, - original_filename=original_filename, - original_line=original_line, - snippet=snippet, - prefix=prefix, - strict=strict, - ) + state.strict = strict + state.mode = Mode.DISCOVER + errors = _discovery_file(filename) + if errors: + return errors + state.mode = Mode.LINT + if snippet: + return _lint_policy_file_snippet( + filename=filename, + original_filename=original_filename, + original_line=original_line, + snippet=snippet, + prefix=prefix, + ) + assert not snippet + assert not original_filename + assert not prefix + assert not original_line + return _lint_policy_file(filename=filename) def lint_cfbs_json(filename) -> int: From f9752380a9c5339b43fd2aeda8a68821bcf1527e Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 3 Apr 2026 00:17:23 +0200 Subject: [PATCH 21/23] tests: Corrected plural s in output Signed-off-by: Ole Herman Schumacher Elgesem --- tests/lint/002_ifvarclass.output.txt | 4 ++-- tests/lint/003_deprecated_promise_type.output.txt | 4 ++-- tests/lint/004_bundle_name_lowercase.output.txt | 4 ++-- tests/lint/005_bundle_type.output.txt | 4 ++-- tests/lint/006_syntax_error.output.txt | 4 ++-- tests/lint/007_empty_file.output.txt | 4 ++-- tests/lint/008_namespace.output.txt | 4 ++-- 7 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/lint/002_ifvarclass.output.txt b/tests/lint/002_ifvarclass.output.txt index af068bf..6ffa1cb 100644 --- a/tests/lint/002_ifvarclass.output.txt +++ b/tests/lint/002_ifvarclass.output.txt @@ -3,5 +3,5 @@ ifvarclass => "cfengine"; ^--------^ Deprecation: Use 'if' instead of 'ifvarclass' at tests/lint/002_ifvarclass.x.cf:5:7 -FAIL: tests/lint/002_ifvarclass.x.cf (1 errors) -Failure, 1 errors in total. +FAIL: tests/lint/002_ifvarclass.x.cf (1 error) +Failure, 1 error in total. diff --git a/tests/lint/003_deprecated_promise_type.output.txt b/tests/lint/003_deprecated_promise_type.output.txt index 94f2fbd..36f78b3 100644 --- a/tests/lint/003_deprecated_promise_type.output.txt +++ b/tests/lint/003_deprecated_promise_type.output.txt @@ -3,5 +3,5 @@ defaults: ^-------^ Deprecation: Promise type 'defaults' is deprecated at tests/lint/003_deprecated_promise_type.x.cf:3:3 -FAIL: tests/lint/003_deprecated_promise_type.x.cf (1 errors) -Failure, 1 errors in total. +FAIL: tests/lint/003_deprecated_promise_type.x.cf (1 error) +Failure, 1 error in total. diff --git a/tests/lint/004_bundle_name_lowercase.output.txt b/tests/lint/004_bundle_name_lowercase.output.txt index 7e5e3f5..484d04e 100644 --- a/tests/lint/004_bundle_name_lowercase.output.txt +++ b/tests/lint/004_bundle_name_lowercase.output.txt @@ -2,5 +2,5 @@ bundle agent MyBundle ^------^ Convention: Bundle name should be lowercase at tests/lint/004_bundle_name_lowercase.x.cf:1:14 -FAIL: tests/lint/004_bundle_name_lowercase.x.cf (1 errors) -Failure, 1 errors in total. +FAIL: tests/lint/004_bundle_name_lowercase.x.cf (1 error) +Failure, 1 error in total. diff --git a/tests/lint/005_bundle_type.output.txt b/tests/lint/005_bundle_type.output.txt index a1f4412..89b1b44 100644 --- a/tests/lint/005_bundle_type.output.txt +++ b/tests/lint/005_bundle_type.output.txt @@ -2,5 +2,5 @@ bundle notavalidtype my_bundle ^-----------^ Error: Bundle type must be one of (agent, common, monitor, server, edit_line, edit_xml), not 'notavalidtype' at tests/lint/005_bundle_type.x.cf:1:8 -FAIL: tests/lint/005_bundle_type.x.cf (1 errors) -Failure, 1 errors in total. +FAIL: tests/lint/005_bundle_type.x.cf (1 error) +Failure, 1 error in total. diff --git a/tests/lint/006_syntax_error.output.txt b/tests/lint/006_syntax_error.output.txt index 5e55fa8..de31969 100644 --- a/tests/lint/006_syntax_error.output.txt +++ b/tests/lint/006_syntax_error.output.txt @@ -3,5 +3,5 @@ reports ^-----^ Error: Syntax error at tests/lint/006_syntax_error.x.cf:3:3 -FAIL: tests/lint/006_syntax_error.x.cf (1 errors) -Failure, 1 errors in total. +FAIL: tests/lint/006_syntax_error.x.cf (1 error) +Failure, 1 error in total. diff --git a/tests/lint/007_empty_file.output.txt b/tests/lint/007_empty_file.output.txt index f4ab399..8d17cd1 100644 --- a/tests/lint/007_empty_file.output.txt +++ b/tests/lint/007_empty_file.output.txt @@ -1,3 +1,3 @@ Error: Empty policy file 'tests/lint/007_empty_file.x.cf' -FAIL: tests/lint/007_empty_file.x.cf (1 errors) -Failure, 1 errors in total. +FAIL: tests/lint/007_empty_file.x.cf (1 error) +Failure, 1 error in total. diff --git a/tests/lint/008_namespace.output.txt b/tests/lint/008_namespace.output.txt index 0ed3872..c4ce8ac 100644 --- a/tests/lint/008_namespace.output.txt +++ b/tests/lint/008_namespace.output.txt @@ -3,5 +3,5 @@ "x" string => default:target("arg"); ^------------^ Error: Call to unknown function / bundle / body 'default:target' at at tests/lint/008_namespace.x.cf:15:19 -FAIL: tests/lint/008_namespace.x.cf (1 errors) -Failure, 1 errors in total. +FAIL: tests/lint/008_namespace.x.cf (1 error) +Failure, 1 error in total. From 88f62e490e18e2a85aaf8ae89ac7261e83ac22b9 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 3 Apr 2026 00:22:53 +0200 Subject: [PATCH 22/23] Adjusted lint tests to use similar file name conventions as format tests Signed-off-by: Ole Herman Schumacher Elgesem --- .gitignore | 1 + ...{002_ifvarclass.output.txt => 002_ifvarclass.expected.txt} | 0 ...pe.output.txt => 003_deprecated_promise_type.expected.txt} | 0 ...case.output.txt => 004_bundle_name_lowercase.expected.txt} | 0 ...05_bundle_type.output.txt => 005_bundle_type.expected.txt} | 0 ..._syntax_error.output.txt => 006_syntax_error.expected.txt} | 0 ...{007_empty_file.output.txt => 007_empty_file.expected.txt} | 0 .../{008_namespace.output.txt => 008_namespace.expected.txt} | 0 tests/run-lint-tests.sh | 4 ++-- 9 files changed, 3 insertions(+), 2 deletions(-) rename tests/lint/{002_ifvarclass.output.txt => 002_ifvarclass.expected.txt} (100%) rename tests/lint/{003_deprecated_promise_type.output.txt => 003_deprecated_promise_type.expected.txt} (100%) rename tests/lint/{004_bundle_name_lowercase.output.txt => 004_bundle_name_lowercase.expected.txt} (100%) rename tests/lint/{005_bundle_type.output.txt => 005_bundle_type.expected.txt} (100%) rename tests/lint/{006_syntax_error.output.txt => 006_syntax_error.expected.txt} (100%) rename tests/lint/{007_empty_file.output.txt => 007_empty_file.expected.txt} (100%) rename tests/lint/{008_namespace.output.txt => 008_namespace.expected.txt} (100%) diff --git a/.gitignore b/.gitignore index 178253e..2395d9f 100644 --- a/.gitignore +++ b/.gitignore @@ -11,6 +11,7 @@ __pycache__ /tmp/ *.log *.output.cf +*.output.txt git_diff_exists commit_message.txt black_output.txt diff --git a/tests/lint/002_ifvarclass.output.txt b/tests/lint/002_ifvarclass.expected.txt similarity index 100% rename from tests/lint/002_ifvarclass.output.txt rename to tests/lint/002_ifvarclass.expected.txt diff --git a/tests/lint/003_deprecated_promise_type.output.txt b/tests/lint/003_deprecated_promise_type.expected.txt similarity index 100% rename from tests/lint/003_deprecated_promise_type.output.txt rename to tests/lint/003_deprecated_promise_type.expected.txt diff --git a/tests/lint/004_bundle_name_lowercase.output.txt b/tests/lint/004_bundle_name_lowercase.expected.txt similarity index 100% rename from tests/lint/004_bundle_name_lowercase.output.txt rename to tests/lint/004_bundle_name_lowercase.expected.txt diff --git a/tests/lint/005_bundle_type.output.txt b/tests/lint/005_bundle_type.expected.txt similarity index 100% rename from tests/lint/005_bundle_type.output.txt rename to tests/lint/005_bundle_type.expected.txt diff --git a/tests/lint/006_syntax_error.output.txt b/tests/lint/006_syntax_error.expected.txt similarity index 100% rename from tests/lint/006_syntax_error.output.txt rename to tests/lint/006_syntax_error.expected.txt diff --git a/tests/lint/007_empty_file.output.txt b/tests/lint/007_empty_file.expected.txt similarity index 100% rename from tests/lint/007_empty_file.output.txt rename to tests/lint/007_empty_file.expected.txt diff --git a/tests/lint/008_namespace.output.txt b/tests/lint/008_namespace.expected.txt similarity index 100% rename from tests/lint/008_namespace.output.txt rename to tests/lint/008_namespace.expected.txt diff --git a/tests/run-lint-tests.sh b/tests/run-lint-tests.sh index bf4deb4..624f6ff 100644 --- a/tests/run-lint-tests.sh +++ b/tests/run-lint-tests.sh @@ -21,12 +21,12 @@ for file in tests/lint/*.cf; do # - Fail (non-zero exit code) # - Output the correct error message - expected="$(echo $file | sed 's/\.x\.cf$/.output.txt/')" + expected="$(echo $file | sed 's/\.x\.cf$/.expected.txt/')" if [ ! -f "$expected" ]; then echo "FAIL: Missing expected output file: $expected" exit 1 fi - output="tmp/$(basename $file .x.cf).lint-output.txt" + output="tests/lint/$(basename $file .x.cf).output.txt" if cfengine lint "$file" > "$output" 2>&1; then echo "FAIL: $file - expected lint failure but got success" exit 1 From aa88e2041b74baf0ce32939584cebd313af29a3d Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Fri, 3 Apr 2026 00:27:51 +0200 Subject: [PATCH 23/23] README.md: Adjusted plural s Signed-off-by: Ole Herman Schumacher Elgesem --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2b80121..48d5fb8 100644 --- a/README.md +++ b/README.md @@ -60,8 +60,8 @@ When it finds a mistake, it points out where the problem is like this; ifvarclass => "cfengine"; ^--------^ Deprecation: Use 'if' instead of 'ifvarclass' at main.cf:5:7 -FAIL: main.cf (1 errors) -Failure, 1 errors in total. +FAIL: main.cf (1 error) +Failure, 1 error in total. ``` Note that since we use a different parser than `cf-agent` / `cf-promises`, they are not 100% in sync.