Skip to content

ENT-13824/13826: Errors on userdefined bundles that shadow built-ins & function-calls inside vars that are not built-in#46

Merged
olehermanse merged 2 commits intocfengine:mainfrom
SimonThalvorsen:main
Apr 8, 2026
Merged

ENT-13824/13826: Errors on userdefined bundles that shadow built-ins & function-calls inside vars that are not built-in#46
olehermanse merged 2 commits intocfengine:mainfrom
SimonThalvorsen:main

Conversation

@SimonThalvorsen
Copy link
Copy Markdown
Contributor

No description provided.

@SimonThalvorsen SimonThalvorsen marked this pull request as draft April 7, 2026 14:44
@olehermanse
Copy link
Copy Markdown
Member

@SimonThalvorsen The failing test is adjusted in #47 - rebase this PR and please add some more tests to test your new functionality :)

@SimonThalvorsen SimonThalvorsen force-pushed the main branch 3 times, most recently from 8a6ca2a to cb5c862 Compare April 8, 2026 11:48
@SimonThalvorsen SimonThalvorsen marked this pull request as ready for review April 8, 2026 11:48
…ars-promise

Signed-off-by: Simon Halvorsen <simon.halvorsen@northern.tech>
…-in func

Signed-off-by: Simon Halvorsen <simon.halvorsen@northern.tech>
Comment on lines +527 to +531
if (
name not in BUILTIN_FUNCTIONS
and state.promise_type == "vars"
and state.attribute_name not in ("action", "classes")
):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SimonThalvorsen Small nitpick: I think it makes a bit more sense to start "top-level" and go further specific in the condition. I would do:

Suggested change
if (
name not in BUILTIN_FUNCTIONS
and state.promise_type == "vars"
and state.attribute_name not in ("action", "classes")
):
if (
state.promise_type == "vars"
and state.attribute_name not in ("action", "classes")
and name not in BUILTIN_FUNCTIONS
):

It could even make sense to add state.block_keyword == "bundle" at the start for readability.

Also, if name not in BUILTIN_FUNCTIONS might be a slow operation, makes sense to save some time by doing the fast checks first.

@olehermanse olehermanse merged commit e451097 into cfengine:main Apr 8, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants