From f4688c6312336c053f989793f13238c8cfb43238 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 28 Apr 2026 15:28:50 +0200 Subject: [PATCH 1/2] cfengine lint: Started recording and using all definitions of bodies and bundles in case there are multiple Co-authored-by: Claude Opus 4.6 (1M context) Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 67 +++++++++++++------ tests/lint/016_macro_multi_def_bundle.cf | 25 +++++++ .../016_macro_multi_def_bundle.expected.txt | 12 ++++ tests/lint/016_macro_multi_def_bundle.x.cf | 25 +++++++ 4 files changed, 107 insertions(+), 22 deletions(-) create mode 100644 tests/lint/016_macro_multi_def_bundle.cf create mode 100644 tests/lint/016_macro_multi_def_bundle.expected.txt create mode 100644 tests/lint/016_macro_multi_def_bundle.x.cf diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index e5bd818..16244db 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -291,24 +291,30 @@ def end_file(self) -> None: self.walking = False self.policy_file = None - def add_bundle(self, name: str) -> None: + def add_bundle(self, name: str, node: Node) -> None: """This is called during discovery wherever a bundle is defined. For example: bundle agent my_bundle {} + We create a dictionary where the key is the qualified name, + and the value is a list of all the definitions. This is because the + same qualified name can appear multiple times (e.g. inside macro + if/else branches). Each definition records the file, line number, + and parameter list. """ 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] = {"is_defined": True} - - def add_body(self, name: str) -> None: + assert self.policy_file + line = node.range.start_point[0] + 1 + definition = { + "filename": self.policy_file.filename, + "line": line, + } + if name not in self.bundles: + self.bundles[name] = [] + self.bundles[name].append(definition) + + def add_body(self, name: str, node: Node) -> None: """This is called during discovery wherever a body is defined. For example: @@ -316,9 +322,20 @@ def add_body(self, name: str) -> None: Control bundles are a special case, so would not be called for: body file control {} + + Similar to add_bundles, we record a list of definitions, since + each body can be defined multiple times (inside different macros). """ name = _qualify(name, self.namespace) - self.bodies[name] = {"is_defined": True} + assert self.policy_file + line = node.range.start_point[0] + 1 + definition = { + "filename": self.policy_file.filename, + "line": line, + } + if name not in self.bodies: + self.bodies[name] = [] + self.bodies[name].append(definition) def add_promise_type(self, name: str) -> None: """This is called during discovery wherever a custom promise type is @@ -479,23 +496,23 @@ def _discover_node(node: Node, state: State) -> int: name = _text(node) if name == "control": return 0 # No need to define control blocks - state.add_body(name) + state.add_body(name, node) qualified_name = _qualify(name, state.namespace) if (n := node.next_named_sibling) and n.type == "parameter_list": _, *args, _ = n.children args = list(filter(",".__ne__, iter(_text(x) for x in args))) - state.bodies[qualified_name].update({"parameters": args}) + state.bodies[qualified_name][-1]["parameters"] = args return 0 # Define bundles: if node.type == "bundle_block_name": name = _text(node) qualified_name = _qualify(name, state.namespace) - state.add_bundle(name) + state.add_bundle(name, node) if (n := node.next_named_sibling) and n.type == "parameter_list": _, *args, _ = n.children args = list(filter(",".__ne__, iter(_text(x) for x in args))) - state.bundles[qualified_name].update({"parameters": args}) + state.bundles[qualified_name][-1]["parameters"] = args return 0 # Define custom promise types: @@ -727,19 +744,25 @@ def _lint_node( qualified_name = _qualify(call, state.namespace) if qualified_name in state.bundles: - max_args = len(state.bundles[qualified_name].get("parameters", [])) - if max_args != len(args): + definitions = state.bundles[qualified_name] + valid_counts = {len(d.get("parameters", [])) for d in definitions} + if len(args) not in valid_counts: _highlight_range(node, lines) + counts = sorted(valid_counts) + expected = " or ".join(str(c) for c in counts) print( - f"Error: Expected {max_args} arguments, received {len(args)} for bundle '{call}' {location}" + f"Error: Expected {expected} arguments, received {len(args)} for bundle '{call}' {location}" ) return 1 if qualified_name in state.bodies: - max_args = len(state.bodies[qualified_name].get("parameters", [])) - if max_args != len(args): + definitions = state.bodies[qualified_name] + valid_counts = {len(d.get("parameters", [])) for d in definitions} + if len(args) not in valid_counts: _highlight_range(node, lines) + counts = sorted(valid_counts) + expected = " or ".join(str(c) for c in counts) print( - f"Error: Expected {max_args} arguments, received {len(args)} for body '{call}' {location}" + f"Error: Expected {expected} arguments, received {len(args)} for body '{call}' {location}" ) return 1 diff --git a/tests/lint/016_macro_multi_def_bundle.cf b/tests/lint/016_macro_multi_def_bundle.cf new file mode 100644 index 0000000..4e00ace --- /dev/null +++ b/tests/lint/016_macro_multi_def_bundle.cf @@ -0,0 +1,25 @@ +@if minimum_version(3.24) +bundle agent test(a, b) +{ + reports: + "$(a) and $(b)"; +} +@else +bundle agent test(a) +{ + reports: + "$(a)"; +} +@endif + +bundle agent main +{ + methods: +@if minimum_version(3.24) + "test1" + usebundle => test("hello", "world"); +@else + "test2" + usebundle => test("hello"); +@endif +} diff --git a/tests/lint/016_macro_multi_def_bundle.expected.txt b/tests/lint/016_macro_multi_def_bundle.expected.txt new file mode 100644 index 0000000..23ae3f7 --- /dev/null +++ b/tests/lint/016_macro_multi_def_bundle.expected.txt @@ -0,0 +1,12 @@ + + "test1" + usebundle => test(); + ^----^ +Error: Expected 1 or 2 arguments, received 0 for bundle 'test' at tests/lint/016_macro_multi_def_bundle.x.cf:20:20 + + "test2" + usebundle => test("hello", "world", "!"); + ^-------------------------^ +Error: Expected 1 or 2 arguments, received 3 for bundle 'test' at tests/lint/016_macro_multi_def_bundle.x.cf:23:20 +FAIL: tests/lint/016_macro_multi_def_bundle.x.cf (2 errors) +Failure, 2 errors in total. diff --git a/tests/lint/016_macro_multi_def_bundle.x.cf b/tests/lint/016_macro_multi_def_bundle.x.cf new file mode 100644 index 0000000..594a7a4 --- /dev/null +++ b/tests/lint/016_macro_multi_def_bundle.x.cf @@ -0,0 +1,25 @@ +@if minimum_version(3.24) +bundle agent test(a, b) +{ + reports: + "$(a) and $(b)"; +} +@else +bundle agent test(a) +{ + reports: + "$(a)"; +} +@endif + +bundle agent main +{ + methods: +@if minimum_version(3.24) + "test1" + usebundle => test(); +@else + "test2" + usebundle => test("hello", "world", "!"); +@endif +} From acfce433b7dce993dadf21d9ed0609621f4da1c7 Mon Sep 17 00:00:00 2001 From: Ole Herman Schumacher Elgesem Date: Tue, 28 Apr 2026 15:39:59 +0200 Subject: [PATCH 2/2] Simplified the code for adding definitions Co-authored-by: Claude Opus 4.6 (1M context) Signed-off-by: Ole Herman Schumacher Elgesem --- src/cfengine_cli/lint.py | 69 +++++++++++++++------------------------- 1 file changed, 25 insertions(+), 44 deletions(-) diff --git a/src/cfengine_cli/lint.py b/src/cfengine_cli/lint.py index 16244db..981146f 100644 --- a/src/cfengine_cli/lint.py +++ b/src/cfengine_cli/lint.py @@ -291,51 +291,41 @@ def end_file(self) -> None: self.walking = False self.policy_file = None - def add_bundle(self, name: str, node: Node) -> None: - """This is called during discovery wherever a bundle is defined. - - For example: - bundle agent my_bundle {} + def _add_definition(self, name: str, node: Node, definitions: dict) -> None: + """Add a definition (bundle or body) to the given dictionary. - We create a dictionary where the key is the qualified name, - and the value is a list of all the definitions. This is because the - same qualified name can appear multiple times (e.g. inside macro - if/else branches). Each definition records the file, line number, - and parameter list. + The value for each qualified name is a list of definitions, since + the same name can appear multiple times (e.g. inside macro if/else + branches). Each definition records the file, line, and parameter + list. """ name = _qualify(name, self.namespace) assert self.policy_file - line = node.range.start_point[0] + 1 + n = node.next_named_sibling + if n and n.type == "parameter_list": + _, *args, _ = n.children + parameters = list(filter(",".__ne__, iter(_text(x) for x in args))) + else: + parameters = [] definition = { "filename": self.policy_file.filename, - "line": line, + "line": node.range.start_point[0] + 1, + "parameters": parameters, } - if name not in self.bundles: - self.bundles[name] = [] - self.bundles[name].append(definition) + if name not in definitions: + definitions[name] = [] + definitions[name].append(definition) + + def add_bundle(self, name: str, node: Node) -> None: + """This is called during discovery wherever a bundle is defined.""" + self._add_definition(name, node, self.bundles) def add_body(self, name: str, node: Node) -> None: """This is called during discovery wherever a body is defined. - For example: - body contain my_body {} - - Control bundles are a special case, so would not be called for: - body file control {} - - Similar to add_bundles, we record a list of definitions, since - each body can be defined multiple times (inside different macros). + Control bodies are a special case and should not be passed here. """ - name = _qualify(name, self.namespace) - assert self.policy_file - line = node.range.start_point[0] + 1 - definition = { - "filename": self.policy_file.filename, - "line": line, - } - if name not in self.bodies: - self.bodies[name] = [] - self.bodies[name].append(definition) + self._add_definition(name, node, self.bodies) def add_promise_type(self, name: str) -> None: """This is called during discovery wherever a custom promise type is @@ -497,27 +487,18 @@ def _discover_node(node: Node, state: State) -> int: if name == "control": return 0 # No need to define control blocks state.add_body(name, node) - qualified_name = _qualify(name, state.namespace) - if (n := node.next_named_sibling) and n.type == "parameter_list": - _, *args, _ = n.children - args = list(filter(",".__ne__, iter(_text(x) for x in args))) - state.bodies[qualified_name][-1]["parameters"] = args return 0 # Define bundles: if node.type == "bundle_block_name": name = _text(node) - qualified_name = _qualify(name, state.namespace) state.add_bundle(name, node) - if (n := node.next_named_sibling) and n.type == "parameter_list": - _, *args, _ = n.children - args = list(filter(",".__ne__, iter(_text(x) for x in args))) - state.bundles[qualified_name][-1]["parameters"] = args return 0 # Define custom promise types: if node.type == "promise_block_name": - state.add_promise_type(_text(node)) + name = _text(node) + state.add_promise_type(name) return 0 return 0