feat: Enable processing of empty values in multiple sections (variables, keywords, test cases)#1719
Conversation
…es, keywords, test cases)
|
I see there are some errors for older versions, I'll look into that |
|
No reason it wasn't enabled for other sections. Since it has sibling rule |
| configure = [ | ||
| "ReplaceEmptyValues.sections=all", | ||
| # or | ||
| "ReplaceEmptyValues.sections=['variables','keywords']", |
There was a problem hiding this comment.
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": |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
What about:
VAR ${name}
...
There is arg, but it's empty.
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