From 4ff6c0edb5a27ba166b9d280d2adc786abe715d4 Mon Sep 17 00:00:00 2001 From: Simon Halvorsen Date: Wed, 22 Apr 2026 17:24:09 +0200 Subject: [PATCH 1/2] ENT-13880: Added check for half-promises outside of macros Missing semicolons not handled, leave this to the formatter. --- src/cfengine_cli/lint.py | 49 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 47 insertions(+), 2 deletions(-) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 981146f..7d7fe7a 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -27,12 +27,13 @@ are a bit awkward. Could make iteration nicer. """ +from copy import deepcopy from enum import Enum import os import json from typing import Callable, Iterable import tree_sitter_cfengine as tscfengine -from dataclasses import dataclass +from dataclasses import dataclass, field from tree_sitter import Language, Node, Parser, Tree from cfbs.validate import validate_config from cfbs.cfbs_config import CFBSConfig @@ -179,6 +180,13 @@ def visit(x): _walk_callback(tree.root_node, visit) + def __deepcopy__(self, _): + """Overrides deepcopy for state-snapshotting. + treesitter-tree is not pickleable, and PolicyFile + should not change across state snapshots + """ + return self + @dataclass class State: @@ -199,6 +207,8 @@ class State: promise_type: str | None = None # "vars" | "files" | "classes" | ... | None attribute_name: str | None = None # "if" | "string" | "slist" | ... | None namespace: str = DEFAULT_NAMESPACE # "ns" | "default" | ... | + macro: str | None = None # "minimum_version()", "else", "between_versions()" + old_state: dict = field(default_factory=dict) mode: Mode = Mode.NONE walking: bool = False strict: bool = True @@ -307,11 +317,14 @@ def _add_definition(self, name: str, node: Node, definitions: dict) -> None: parameters = list(filter(",".__ne__, iter(_text(x) for x in args))) else: parameters = [] + definition = { "filename": self.policy_file.filename, "line": node.range.start_point[0] + 1, "parameters": parameters, } + if self.macro: + definition["macro"] = self.macro if name not in definitions: definitions[name] = [] definitions[name].append(definition) @@ -429,6 +442,22 @@ def navigate(self, node) -> None: self.promise_type = None return + if node.type == "macro": + macro_type = _text(node) + if macro_type.startswith("@if"): + self.old_state = deepcopy(self.__dict__) + self.macro = macro_type.split(" ")[-1] + elif macro_type.startswith("@else"): + self.macro = "else" + self.old_state = deepcopy(self.__dict__) + self.__dict__.update(self.old_state) + elif macro_type.startswith("@endif"): + self.macro = None + self.__dict__.update(self.old_state) + # NOTE: this or that? maybe a dict of states based on the macro-type? + self.old_state = {} + return + def _check_syntax(policy_file: PolicyFile, state: State) -> int: """Iterate over a syntax tree and print errors if it is empty or has syntax @@ -746,7 +775,23 @@ def _lint_node( f"Error: Expected {expected} arguments, received {len(args)} for body '{call}' {location}" ) return 1 - + if node.type == "half_promise": + prev_sib = node.prev_named_sibling + while prev_sib and prev_sib.type == "comment": + prev_sib = prev_sib.prev_named_sibling + prev_type = prev_sib.type if prev_sib else None + if not state.macro: + _highlight_range(node, lines) + print( + f"Error: Found promise attribute with no parent-promiser outside of a macro {location}" + ) + return 1 + elif prev_type != "macro": + _highlight_range(node, lines) + print( + f"Error: Multiple promise attributes with ending semicolon found inside macro '{state.macro}' {location}" + ) + return 1 return 0 From bfed5d1e87250ed3b1256ca20a24b7eb10a2175a Mon Sep 17 00:00:00 2001 From: Simon Halvorsen Date: Wed, 22 Apr 2026 17:27:34 +0200 Subject: [PATCH 2/2] Added tests for checking validity of half-promises/macros --- tests/lint/017_half_promises.cf | 12 ++++++++++ tests/lint/017_half_promises.expected.txt | 17 +++++++++++++ tests/lint/017_half_promises.x.cf | 29 +++++++++++++++++++++++ 3 files changed, 58 insertions(+) create mode 100644 tests/lint/017_half_promises.cf create mode 100644 tests/lint/017_half_promises.expected.txt create mode 100644 tests/lint/017_half_promises.x.cf diff --git a/tests/lint/017_half_promises.cf b/tests/lint/017_half_promises.cf new file mode 100644 index 0000000..7d80f9f --- /dev/null +++ b/tests/lint/017_half_promises.cf @@ -0,0 +1,12 @@ +bundle agent main +{ + vars: + "string" string => $(bar); + + "string" +@if minimum_version(3.18) + string => $($(baz)); +@else + string => $($(baz)); +@endif +} diff --git a/tests/lint/017_half_promises.expected.txt b/tests/lint/017_half_promises.expected.txt new file mode 100644 index 0000000..412b429 --- /dev/null +++ b/tests/lint/017_half_promises.expected.txt @@ -0,0 +1,17 @@ + + string => $(bar); + string => $(bar); + ^---------------^ +Error: Found promise attribute with no parent-promiser outside of a macro at tests/lint/017_half_promises.x.cf:6:7 + + string => $($(baz)); + string => $($(baz)); + ^------------------^ +Error: Multiple promise attributes with ending semicolon found inside macro 'minimum_version(3.18)' at tests/lint/017_half_promises.x.cf:9:7 + + # comment + string => $($(baz)); + ^------------------^ +Error: Multiple promise attributes with ending semicolon found inside macro 'else' at tests/lint/017_half_promises.x.cf:26:7 +FAIL: tests/lint/017_half_promises.x.cf (3 errors) +Failure, 3 errors in total. diff --git a/tests/lint/017_half_promises.x.cf b/tests/lint/017_half_promises.x.cf new file mode 100644 index 0000000..d9a85ff --- /dev/null +++ b/tests/lint/017_half_promises.x.cf @@ -0,0 +1,29 @@ +bundle agent main +{ + vars: + "string" + string => $(bar); + string => $(bar); +@if minimum_version(3.18) + string => $($(baz)); + string => $($(baz)); +@else + string => $($(baz)); +@endif + + "string" + # comment + # comment + # comment +@if minimum_version(3.20) + string => $($(baz)); + + # comment +@else + string => $($(baz)); + + # comment + string => $($(baz)); + # comment +@endif +}