Skip to content

feat: Enable processing of empty values in multiple sections (variables, keywords, test cases)#1719

Open
MobyNL wants to merge 1 commit intoMarketSquare:mainfrom
MobyNL:enable-empty-values-in-multiple-sections
Open

feat: Enable processing of empty values in multiple sections (variables, keywords, test cases)#1719
MobyNL wants to merge 1 commit intoMarketSquare:mainfrom
MobyNL:enable-empty-values-in-multiple-sections

Conversation

@MobyNL
Copy link
Copy Markdown

@MobyNL MobyNL commented Mar 31, 2026

Hey,

No idea if there was a specific reason to only enable the ReplaceEmptyValues rule in the variables section. So I created this PR with the functionality to also have it in Test Cases, and Keywords.

For now, I created it with only variables as the default section, so core behaviour doesn't change. You can now add a specific setting that enables you to choose which sections as a comma separated list (e.g. "variables,keywords"), and including "all".

Please let me know any changes you would like to see!

BR,
Mark

@MobyNL
Copy link
Copy Markdown
Author

MobyNL commented Mar 31, 2026

I see there are some errors for older versions, I'll look into that

@bhirsz
Copy link
Copy Markdown
Member

bhirsz commented Mar 31, 2026

No reason it wasn't enabled for other sections. Since it has sibling rule empty-variable, which detects in all sections, this change makes sense. In the future we could replace the formatter with fix for empty-variable.

configure = [
"ReplaceEmptyValues.sections=all",
# or
"ReplaceEmptyValues.sections=['variables','keywords']",
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.

Our common approach for such parameter was to use csv and then process it later (split on comma). In this case it's not clear what will be the final type - most likely the string anyway, so using the 'string in form of list' is not ideal. The parsing will be also the same for both toml and cli config.

ReplaceEmptyValues.sections=variables,keywords


@skip_if_disabled
def visit_Variable(self, node: Variable) -> Variable: # noqa: N802
if self.current_section != "variables":
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.

Do we need to track whether we're inside variables? VAR would be visit_Var, so the visit_Variable should only apply to variables section. But I'm not 100 % sure

def visit_Var(self, node: Var) -> Var: # noqa: N802
"""Handle inline VAR statements to replace empty values with proper EMPTY variables."""
if self.current_section not in ("testcases", "keywords"):
return node
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.

VAR may only exist in testcases / keywords section so this statement will be always false.

"""Handle inline VAR statements to replace empty values with proper EMPTY variables."""
if self.current_section not in ("testcases", "keywords"):
return node
if Var is None or node.errors:
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.

Var is None probably will never be true (because visit_Var is only handled by version which supports Var) but it may be required for mypy checker.


args = node.get_tokens(Token.ARGUMENT)
if args:
return node
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.

What about:

VAR    ${name}
...

There is arg, but it's empty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants