Skip to content

Commit 19fe733

Browse files
olehermanseolehermanse
authored
Merge pull request #161 from olehermanse/main
cfengine format: Fixed issue with multilining function calls inside macros
2 parents 93ccbf0 + 2723895 commit 19fe733

4 files changed

Lines changed: 171 additions & 9 deletions

File tree

‎.github/workflows/lint-policy-in-other-repos.yml‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,15 @@ jobs:
4444
python -m pip install --upgrade pip
4545
python -m pip install uv
4646
make install
47-
- name: Run cfengine lint
47+
- name: Check other repos with cfengine format and cfengine lint
4848
working-directory: cfengine-cli
4949
run: |
50-
uv run cfengine lint --strict no ../masterfiles
51-
uv run cfengine lint --strict no ../modules
52-
uv run cfengine lint --strict no ../documentation
50+
# Linting should work with no errors on all the other repos:
51+
uv run cfengine lint --strict no ../masterfiles ../documentation ../modules
52+
# Formatting should work with no errors on all the other repos:
53+
uv run cfengine format ../masterfiles ../documentation ../modules
54+
# Linting should still work after formatting:
55+
uv run cfengine lint --strict no ../masterfiles ../documentation ../modules
5356
# TODO: Add core when ready
5457
# TODO: Do all of them together, with strict, when ready;
5558
# uv run cfengine lint ../core ../masterfiles ../documentation ../modules

‎src/cfengine_cli/format.py‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -236,13 +236,21 @@ def split_generic_list(
236236
"""Split list elements into one-per-line strings, each pre-indented."""
237237
has_macros = _contains_macro(middle)
238238
elements: list[str] = []
239+
# Indices of the lines that end a top-level value element. For a nested
240+
# element, like a function call inside an slist, this is only the final line,
241+
# outside the function call, so we do not add trailing commas into function
242+
# call argument lists.
243+
value_end_indices: list[int] = []
239244
for element in middle:
240245
if elements and element.type == ",":
241246
elements[-1] = elements[-1] + ","
242247
continue
243248
if element.type == "macro":
244249
elements.append(text(element))
245250
continue
251+
if element.type == "comment":
252+
elements.append(" " * indent + text(element))
253+
continue
246254
line = " " * indent + stringify_single_line_node(element)
247255
# Strict < reserves 1 char for the comma appended after this check
248256
if len(line) < line_length:
@@ -251,14 +259,18 @@ def split_generic_list(
251259
lines = split_generic_value(element, indent, line_length)
252260
elements.append(" " * indent + lines[0])
253261
elements.extend(lines[1:])
262+
value_end_indices.append(len(elements) - 1)
254263

255-
# Adjust trailing commas: with macros, fix every non-macro element
256-
# (one per branch); without, fix only the last non-comment element.
264+
# Set trailing commas
257265
if has_macros:
258-
for i, e in enumerate(elements):
259-
if not e.lstrip().startswith(("@", "#")):
260-
elements[i] = _set_trailing_comma(e, trailing_comma)
266+
# With macros: Use the indices we counted earlier to ensure there
267+
# are trailing commas at the right places (before macro, inside macro,
268+
# after macros), but NOT inside the nested function calls which may
269+
# be multi line
270+
for i in value_end_indices:
271+
elements[i] = _set_trailing_comma(elements[i], trailing_comma)
261272
else:
273+
# No macros: Just ensure the last non-comment element has a trailing comma
262274
for i in reversed(range(len(elements))):
263275
if not elements[i].lstrip().startswith("#"):
264276
elements[i] = _set_trailing_comma(elements[i], trailing_comma)

‎tests/format/011_macros.expected.cf‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,3 +200,94 @@ bundle agent test(a)
200200
"$(a)";
201201
}
202202
@endif
203+
bundle agent test_macro_call_param
204+
{
205+
classes:
206+
"a" -> { "CFE-3927" }
207+
and => {
208+
# The variable must be defined
209+
isvariable("default:def.control_agent_environment_vars"),
210+
# The length of the variable must be greater than 0 (can't be an empty list)
211+
isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
212+
# Each element of the list must be of the form KEY=VALUE
213+
every(".+=.+", "default:def.control_agent_environment_vars"),
214+
# In 3.18 and greater we can validate the type of variable in use
215+
@if minimum_version(3.18.0)
216+
regcmp(
217+
"(policy slist|data array)",
218+
type("default:def.control_agent_environment_vars", "true")
219+
),
220+
@endif
221+
};
222+
223+
"b" -> { "CFE-3927" }
224+
and => {
225+
# The variable must be defined
226+
isvariable("default:def.control_agent_environment_vars"),
227+
isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
228+
@if minimum_version(3.18.0)
229+
regcmp(
230+
"(policy slist|data array)",
231+
type("default:def.control_agent_environment_vars", "true")
232+
),
233+
@endif
234+
every(".+=.+", "default:def.control_agent_environment_vars"),
235+
};
236+
237+
"c" -> { "CFE-3927" }
238+
and => {
239+
# The variable must be defined
240+
isvariable("default:def.control_agent_environment_vars"),
241+
isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
242+
@if minimum_version(3.18.0)
243+
regcmp(
244+
"(policy slist|data array)",
245+
type("default:def.control_agent_environment_vars", "true")
246+
),
247+
every(".+=.+", "default:def.control_agent_environment_vars"),
248+
@endif
249+
};
250+
251+
"d" -> { "CFE-3927" }
252+
and => {
253+
@if minimum_version(3.18.0)
254+
every(".+=.+", "default:def.control_agent_environment_vars"),
255+
@endif
256+
isvariable("default:def.control_agent_environment_vars"),
257+
isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
258+
regcmp(
259+
"(policy slist|data array)",
260+
type("default:def.control_agent_environment_vars", "true")
261+
),
262+
};
263+
264+
"e" -> { "CFE-3927" }
265+
and => {
266+
@if minimum_version(3.18.0)
267+
"a",
268+
@endif
269+
"b",
270+
"c",
271+
"d",
272+
isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
273+
regcmp(
274+
"(policy slist|data array)",
275+
type("default:def.control_agent_environment_vars", "true")
276+
),
277+
};
278+
279+
"f" -> { "CFE-3927" }
280+
and => {
281+
isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
282+
regcmp(
283+
"(policy slist|data array)",
284+
type("default:def.control_agent_environment_vars", "true")
285+
),
286+
"a",
287+
"b",
288+
"c",
289+
@if minimum_version(3.18.0)
290+
"d",
291+
@endif
292+
};
293+
}

‎tests/format/011_macros.input.cf‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,3 +193,59 @@ reports:
193193
"$(a)";
194194
}
195195
@endif
196+
197+
bundle agent test_macro_call_param
198+
{
199+
classes:
200+
"a" -> { "CFE-3927" }
201+
and => {
202+
# The variable must be defined
203+
isvariable("default:def.control_agent_environment_vars"),
204+
# The length of the variable must be greater than 0 (can't be an empty list)
205+
isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
206+
# Each element of the list must be of the form KEY=VALUE
207+
every(".+=.+", "default:def.control_agent_environment_vars"),
208+
# In 3.18 and greater we can validate the type of variable in use
209+
@if minimum_version(3.18.0)
210+
regcmp("(policy slist|data array)",type("default:def.control_agent_environment_vars", "true")),
211+
@endif
212+
};
213+
"b" -> { "CFE-3927" }
214+
and => {
215+
# The variable must be defined
216+
isvariable("default:def.control_agent_environment_vars"),isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
217+
@if minimum_version(3.18.0)
218+
regcmp( "(policy slist|data array)", type("default:def.control_agent_environment_vars", "true") ),
219+
@endif
220+
every(".+=.+", "default:def.control_agent_environment_vars"),
221+
};
222+
"c" -> { "CFE-3927" }
223+
and => {
224+
# The variable must be defined
225+
isvariable("default:def.control_agent_environment_vars"), isgreaterthan(length("default:def.control_agent_environment_vars"), 0),
226+
@if minimum_version(3.18.0)
227+
regcmp("(policy slist|data array)",type("default:def.control_agent_environment_vars", "true")),every(".+=.+", "default:def.control_agent_environment_vars"),
228+
@endif
229+
};
230+
"d" -> { "CFE-3927" }
231+
and => {
232+
@if minimum_version(3.18.0)
233+
every(".+=.+", "default:def.control_agent_environment_vars"),
234+
@endif
235+
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")),
236+
};
237+
"e" -> { "CFE-3927" }
238+
and => {
239+
@if minimum_version(3.18.0)
240+
"a",
241+
@endif
242+
"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")),
243+
};
244+
"f" -> { "CFE-3927" }
245+
and => {
246+
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",
247+
@if minimum_version(3.18.0)
248+
"d",
249+
@endif
250+
};
251+
}

0 commit comments

Comments
 (0)