diff --git a/.github/workflows/lint-policy-in-other-repos.yml b/.github/workflows/lint-policy-in-other-repos.yml index 3e15449..2c5c71e 100644 --- a/.github/workflows/lint-policy-in-other-repos.yml +++ b/.github/workflows/lint-policy-in-other-repos.yml @@ -44,12 +44,15 @@ jobs: python -m pip install --upgrade pip python -m pip install uv make install - - name: Run cfengine lint + - name: Check other repos with cfengine format and cfengine lint working-directory: cfengine-cli run: | - uv run cfengine lint --strict no ../masterfiles - uv run cfengine lint --strict no ../modules - uv run cfengine lint --strict no ../documentation + # Linting should work with no errors on all the other repos: + uv run cfengine lint --strict no ../masterfiles ../documentation ../modules + # Formatting should work with no errors on all the other repos: + uv run cfengine format ../masterfiles ../documentation ../modules + # Linting should still work after formatting: + uv run cfengine lint --strict no ../masterfiles ../documentation ../modules # TODO: Add core when ready # TODO: Do all of them together, with strict, when ready; # uv run cfengine lint ../core ../masterfiles ../documentation ../modules diff --git a/src/cfengine_cli/format.py b/src/cfengine_cli/format.py index 63865c5..2595c6b 100644 --- a/src/cfengine_cli/format.py +++ b/src/cfengine_cli/format.py @@ -236,6 +236,11 @@ def split_generic_list( """Split list elements into one-per-line strings, each pre-indented.""" has_macros = _contains_macro(middle) elements: list[str] = [] + # Indices of the lines that end a top-level value element. For a nested + # element, like a function call inside an slist, this is only the final line, + # outside the function call, so we do not add trailing commas into function + # call argument lists. + value_end_indices: list[int] = [] for element in middle: if elements and element.type == ",": elements[-1] = elements[-1] + "," @@ -243,6 +248,9 @@ def split_generic_list( if element.type == "macro": elements.append(text(element)) continue + if element.type == "comment": + elements.append(" " * indent + text(element)) + continue line = " " * indent + stringify_single_line_node(element) # Strict < reserves 1 char for the comma appended after this check if len(line) < line_length: @@ -251,14 +259,18 @@ def split_generic_list( lines = split_generic_value(element, indent, line_length) elements.append(" " * indent + lines[0]) elements.extend(lines[1:]) + value_end_indices.append(len(elements) - 1) - # Adjust trailing commas: with macros, fix every non-macro element - # (one per branch); without, fix only the last non-comment element. + # Set trailing commas if has_macros: - for i, e in enumerate(elements): - if not e.lstrip().startswith(("@", "#")): - elements[i] = _set_trailing_comma(e, trailing_comma) + # With macros: Use the indices we counted earlier to ensure there + # are trailing commas at the right places (before macro, inside macro, + # after macros), but NOT inside the nested function calls which may + # be multi line + for i in value_end_indices: + elements[i] = _set_trailing_comma(elements[i], trailing_comma) else: + # No macros: Just ensure the last non-comment element has a trailing comma for i in reversed(range(len(elements))): if not elements[i].lstrip().startswith("#"): elements[i] = _set_trailing_comma(elements[i], trailing_comma) diff --git a/tests/format/011_macros.expected.cf b/tests/format/011_macros.expected.cf index 54c59b3..3fc82e5 100644 --- a/tests/format/011_macros.expected.cf +++ b/tests/format/011_macros.expected.cf @@ -200,3 +200,94 @@ bundle agent test(a) "$(a)"; } @endif +bundle agent test_macro_call_param +{ + classes: + "a" -> { "CFE-3927" } + and => { + # The variable must be defined + isvariable("default:def.control_agent_environment_vars"), + # The length of the variable must be greater than 0 (can't be an empty list) + isgreaterthan(length("default:def.control_agent_environment_vars"), 0), + # Each element of the list must be of the form KEY=VALUE + every(".+=.+", "default:def.control_agent_environment_vars"), + # In 3.18 and greater we can validate the type of variable in use +@if minimum_version(3.18.0) + regcmp( + "(policy slist|data array)", + type("default:def.control_agent_environment_vars", "true") + ), +@endif + }; + + "b" -> { "CFE-3927" } + and => { + # The variable must be defined + isvariable("default:def.control_agent_environment_vars"), + isgreaterthan(length("default:def.control_agent_environment_vars"), 0), +@if minimum_version(3.18.0) + regcmp( + "(policy slist|data array)", + type("default:def.control_agent_environment_vars", "true") + ), +@endif + every(".+=.+", "default:def.control_agent_environment_vars"), + }; + + "c" -> { "CFE-3927" } + and => { + # The variable must be defined + isvariable("default:def.control_agent_environment_vars"), + isgreaterthan(length("default:def.control_agent_environment_vars"), 0), +@if minimum_version(3.18.0) + regcmp( + "(policy slist|data array)", + type("default:def.control_agent_environment_vars", "true") + ), + every(".+=.+", "default:def.control_agent_environment_vars"), +@endif + }; + + "d" -> { "CFE-3927" } + and => { +@if minimum_version(3.18.0) + every(".+=.+", "default:def.control_agent_environment_vars"), +@endif + isvariable("default:def.control_agent_environment_vars"), + isgreaterthan(length("default:def.control_agent_environment_vars"), 0), + regcmp( + "(policy slist|data array)", + type("default:def.control_agent_environment_vars", "true") + ), + }; + + "e" -> { "CFE-3927" } + and => { +@if minimum_version(3.18.0) + "a", +@endif + "b", + "c", + "d", + isgreaterthan(length("default:def.control_agent_environment_vars"), 0), + regcmp( + "(policy slist|data array)", + type("default:def.control_agent_environment_vars", "true") + ), + }; + + "f" -> { "CFE-3927" } + and => { + isgreaterthan(length("default:def.control_agent_environment_vars"), 0), + regcmp( + "(policy slist|data array)", + type("default:def.control_agent_environment_vars", "true") + ), + "a", + "b", + "c", +@if minimum_version(3.18.0) + "d", +@endif + }; +} diff --git a/tests/format/011_macros.input.cf b/tests/format/011_macros.input.cf index bdcd477..b32489a 100644 --- a/tests/format/011_macros.input.cf +++ b/tests/format/011_macros.input.cf @@ -193,3 +193,59 @@ reports: "$(a)"; } @endif + +bundle agent test_macro_call_param +{ +classes: +"a" -> { "CFE-3927" } +and => { +# The variable must be defined +isvariable("default:def.control_agent_environment_vars"), +# The length of the variable must be greater than 0 (can't be an empty list) +isgreaterthan(length("default:def.control_agent_environment_vars"), 0), +# Each element of the list must be of the form KEY=VALUE +every(".+=.+", "default:def.control_agent_environment_vars"), +# In 3.18 and greater we can validate the type of variable in use +@if minimum_version(3.18.0) +regcmp("(policy slist|data array)",type("default:def.control_agent_environment_vars", "true")), +@endif +}; +"b" -> { "CFE-3927" } +and => { +# The variable must be defined +isvariable("default:def.control_agent_environment_vars"),isgreaterthan(length("default:def.control_agent_environment_vars"), 0), +@if minimum_version(3.18.0) +regcmp( "(policy slist|data array)", type("default:def.control_agent_environment_vars", "true") ), +@endif +every(".+=.+", "default:def.control_agent_environment_vars"), +}; +"c" -> { "CFE-3927" } +and => { +# The variable must be defined +isvariable("default:def.control_agent_environment_vars"), isgreaterthan(length("default:def.control_agent_environment_vars"), 0), +@if minimum_version(3.18.0) +regcmp("(policy slist|data array)",type("default:def.control_agent_environment_vars", "true")),every(".+=.+", "default:def.control_agent_environment_vars"), +@endif +}; +"d" -> { "CFE-3927" } +and => { +@if minimum_version(3.18.0) +every(".+=.+", "default:def.control_agent_environment_vars"), +@endif +isvariable("default:def.control_agent_environment_vars"),isgreaterthan(length("default:def.control_agent_environment_vars"), 0),regcmp("(policy slist|data array)",type("default:def.control_agent_environment_vars", "true")), +}; +"e" -> { "CFE-3927" } +and => { +@if minimum_version(3.18.0) +"a", +@endif +"b","c","d",isgreaterthan(length("default:def.control_agent_environment_vars"),0),regcmp("(policy slist|data array)",type("default:def.control_agent_environment_vars", "true")), +}; +"f" -> { "CFE-3927" } +and => { +isgreaterthan(length("default:def.control_agent_environment_vars"), 0),regcmp("(policy slist|data array)",type("default:def.control_agent_environment_vars", "true")),"a","b","c", +@if minimum_version(3.18.0) +"d", +@endif +}; +}