Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 100 additions & 1 deletion src/robocop/formatter/formatters/ReplaceEmptyValues.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from typing import TYPE_CHECKING

from robot.api.parsing import Token
from robot.api.parsing import Keyword, KeywordSection, TestCase, TestCaseSection, Token, Var

from robocop.formatter.disablers import skip_if_disabled, skip_section_if_disabled
from robocop.formatter.formatters import Formatter
Expand Down Expand Up @@ -41,16 +41,78 @@ class ReplaceEmptyValues(Formatter):
... ${EMPTY}
... value3
```

By default, this formatter only processes the Variables section. You can configure
which sections to process using the ``sections`` parameter:
- ``variables`` (default) - only Variables section
- ``keywords`` - only Keywords section
- ``testcases`` - only Test Cases section
- ``all`` - all sections
- List of sections - e.g., ``["variables", "keywords"]``

Configuration example in pyproject.toml:
```toml
[tool.robocop.format]
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

@MobyNL MobyNL Apr 21, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated to csv style! Good to stay consistent. I was under the assumption in my head that convention was a list, but clearly it's not. Although, I still have to update documentation

]
```
"""

HANDLES_SKIP = frozenset({"skip_sections"})

def __init__(self, sections: str | list[str] = "variables") -> None:
super().__init__()
if isinstance(sections, str):
if sections == "all":
self.enabled_sections = {"variables", "keywords", "testcases"}
else:
self.enabled_sections = {s.strip().lower() for s in sections.split(",")}
else:
self.enabled_sections = {s.lower() for s in sections}
self.current_section: str | None = None # Track which section we're currently visiting

@skip_section_if_disabled
def visit_VariableSection(self, node: VariableSection) -> VariableSection: # noqa: N802
if "variables" not in self.enabled_sections:
return node
self.current_section = "variables"
result = self.generic_visit(node)
self.current_section = None
return result

@skip_section_if_disabled
def visit_TestCaseSection(self, node: TestCaseSection) -> TestCaseSection: # noqa: N802
if "testcases" not in self.enabled_sections:
return node
self.current_section = "testcases"
result = self.generic_visit(node)
self.current_section = None
return result

@skip_section_if_disabled
def visit_KeywordSection(self, node: KeywordSection) -> KeywordSection: # noqa: N802
if "keywords" not in self.enabled_sections:
return node
self.current_section = "keywords"
result = self.generic_visit(node)
self.current_section = None
return result

@skip_if_disabled
def visit_TestCase(self, node: TestCase) -> TestCase: # noqa: N802
return self.generic_visit(node)

@skip_if_disabled
def visit_Keyword(self, node: Keyword) -> Keyword: # noqa: N802
return self.generic_visit(node)

@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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This was for some debugging I did while building it. I removed it

return node
if node.errors or not node.name:
return node
args = node.get_tokens(Token.ARGUMENT)
Expand All @@ -73,3 +135,40 @@ def visit_Variable(self, node: Variable) -> Variable: # noqa: N802
tokens = [node.tokens[0], sep, Token(Token.ARGUMENT, node.name[0] + "{EMPTY}"), *node.tokens[1:]]
node.tokens = tokens
return node

@skip_if_disabled
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.

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.

return node

variable_token = node.get_token(Token.VARIABLE)
if not variable_token:
return node

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good one. I didn't think of multilines. It is now implemented!


var_name = variable_token.value
if var_name.startswith("${"):
empty_value = "${EMPTY}"
elif var_name.startswith("@{"):
empty_value = "@{EMPTY}"
elif var_name.startswith("&{"):
empty_value = "&{EMPTY}"
else:
return node

sep = Token(Token.SEPARATOR, self.formatting_config.separator)
tokens = []
for token in node.tokens:
tokens.append(token)
if token == variable_token:
tokens.append(sep)
tokens.append(Token(Token.ARGUMENT, empty_value))

node.tokens = tokens
return node
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
*** Variables ***
${EMPTY_SCALAR} ${EMPTY}
@{EMPTY_LIST} @{EMPTY}
&{EMPTY_DICT} &{EMPTY}


*** Test Cases ***
Test With Empty Vars
VAR ${empty_in_test} ${EMPTY}
VAR @{empty_list_test} @{EMPTY}
Log ${empty_in_test}


*** Keywords ***
Keyword With Empty Vars
VAR ${empty_in_kw} ${EMPTY}
VAR &{empty_dict_kw} &{EMPTY}
Log ${empty_in_kw}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
*** Keywords ***
Keyword With Empty Vars
VAR ${empty_scalar} ${EMPTY}
VAR @{empty_list} @{EMPTY}
VAR &{empty_dict} &{EMPTY}
VAR ${scalar} value
VAR @{list} item1 item2
VAR &{dict} key=value
Log ${empty_scalar}

Keyword With Traditional VAR
[Documentation] Test with traditional Set Variable
${empty} Set Variable
${filled} Set Variable value
RETURN ${empty}

Keyword With Empty Assignment
${var1} ${var2} ${var3} Get Multiple Values
Log Many ${var1} ${var2} ${var3}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
*** Variables ***
${VAR_EMPTY} ${EMPTY}
@{VAR_LIST} @{EMPTY}


*** Test Cases ***
Test Should Not Be Modified
VAR ${empty_in_test}
Log ${empty_in_test}


*** Keywords ***
Keyword With Empty Vars
VAR ${empty_in_kw} ${EMPTY}
Log ${empty_in_kw}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
*** Test Cases ***
Test With Empty Vars
VAR ${empty_scalar} ${EMPTY}
VAR @{empty_list} @{EMPTY}
VAR &{empty_dict} &{EMPTY}
VAR ${scalar} value
Log ${empty_scalar}

Test With Scoped VAR
VAR ${empty_test} ${EMPTY} scope=TEST
VAR ${empty_suite} ${EMPTY} scope=SUITE
VAR ${empty_global} ${EMPTY} scope=GLOBAL
VAR @{empty_list} @{EMPTY} scope=TEST
Log ${empty_test}

Test Traditional Assignment
${empty} Set Variable
${filled} Set Variable value
Log ${empty}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
*** Variables ***
${EMPTY_SCALAR} ${EMPTY}
@{EMPTY_LIST} @{EMPTY}
&{EMPTY_DICT} &{EMPTY}


*** Test Cases ***
Test With Empty Vars
VAR ${empty_in_test}
VAR @{empty_list_test}
Log ${empty_in_test}


*** Keywords ***
Keyword With Empty Vars
VAR ${empty_in_kw}
VAR &{empty_dict_kw}
Log ${empty_in_kw}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
*** Keywords ***
Keyword With Empty Vars
VAR ${empty_scalar}
VAR @{empty_list}
VAR &{empty_dict}
VAR ${scalar} value
VAR @{list} item1 item2
VAR &{dict} key=value
Log ${empty_scalar}

Keyword With Traditional VAR
[Documentation] Test with traditional Set Variable
${empty} Set Variable
${filled} Set Variable value
RETURN ${empty}

Keyword With Empty Assignment
${var1} ${var2} ${var3} Get Multiple Values
Log Many ${var1} ${var2} ${var3}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
*** Variables ***
${VAR_EMPTY} ${EMPTY}
@{VAR_LIST} @{EMPTY}


*** Test Cases ***
Test Should Not Be Modified
VAR ${empty_in_test}
Log ${empty_in_test}


*** Keywords ***
Keyword With Empty Vars
VAR ${empty_in_kw}
Log ${empty_in_kw}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
*** Test Cases ***
Test With Empty Vars
VAR ${empty_scalar}
VAR @{empty_list}
VAR &{empty_dict}
VAR ${scalar} value
Log ${empty_scalar}

Test With Scoped VAR
VAR ${empty_test} scope=TEST
VAR ${empty_suite} scope=SUITE
VAR ${empty_global} scope=GLOBAL
VAR @{empty_list} scope=TEST
Log ${empty_test}

Test Traditional Assignment
${empty} Set Variable
${filled} Set Variable value
Log ${empty}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
*** Variables ***
${EMPTY_VAR} ${EMPTY}
@{EMPTY_LIST} @{EMPTY}
&{EMPTY_DICT} &{EMPTY}
54 changes: 54 additions & 0 deletions tests/formatter/formatters/ReplaceEmptyValues/test_formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,57 @@ def test_formatter(self):

def test_skip_section(self):
self.compare(source="test.robot", skip_sections=["variables"], not_modified=True)

def test_keywords_section(self):
"""Test formatting empty values in Keywords section only."""
configure = [f"{self.FORMATTER_NAME}.sections=keywords"]
self.compare(
source="keywords.robot",
expected="keywords.robot",
configure=configure,
)

def test_testcases_section(self):
"""Test formatting empty values in Test Cases section only."""
configure = [f"{self.FORMATTER_NAME}.sections=testcases"]
self.compare(
source="testcases.robot",
expected="testcases.robot",
configure=configure,
)

def test_all_sections(self):
"""Test formatting empty values in all sections (variables, keywords, testcases)."""
configure = [f"{self.FORMATTER_NAME}.sections=all"]
self.compare(
source="all_sections.robot",
expected="all_sections.robot",
configure=configure,
)

def test_mixed_sections_list(self):
"""Test formatting with a list of specific sections (variables and keywords)."""
configure = [f"{self.FORMATTER_NAME}.sections=variables,keywords"]
self.compare(
source="mixed_sections.robot",
expected="mixed_sections.robot",
configure=configure,
)

def test_keywords_section_only_does_not_modify_variables(self):
"""Test that when configured for keywords only, variables section is not modified."""
configure = [f"{self.FORMATTER_NAME}.sections=keywords"]
self.compare(
source="variables_only.robot",
not_modified=True,
configure=configure,
)

def test_testcases_section_only_does_not_modify_variables(self):
"""Test that when configured for testcases only, variables section is not modified."""
configure = [f"{self.FORMATTER_NAME}.sections=testcases"]
self.compare(
source="variables_only.robot",
not_modified=True,
configure=configure,
)
Loading