Skip to content

Commit

Permalink
Fix Set Local Variable not supported in RenameVariables (#611)
Browse files Browse the repository at this point in the history
  • Loading branch information
bhirsz authored Dec 25, 2023
1 parent 83cd472 commit fc4ab6e
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 17 deletions.
12 changes: 12 additions & 0 deletions docs/releasenotes/unreleased/fix.4.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Set Local Variable not supported in RenameVariables (#593)
----------------------------------------------------------

``Set Local Variable`` keyword is now supported and scope of the variable is properly preserved:

```
${variable} Set Variable value
Set Global Variable ${VARIABLE}
Log ${VARIABLE}
Set Local Variable ${variable}
Log ${variable}
```
55 changes: 38 additions & 17 deletions robotidy/transformers/RenameVariables.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from robotidy.utils import misc, variable_matcher

SET_GLOBAL_VARIABLES = {"settestvariable", "settaskvariable", "setsuitevariable", "setglobalvariable"}
SET_LOCAL_VARIABLE = "setlocalvariable"


def is_set_global_variable(keyword: str) -> bool:
Expand All @@ -20,6 +21,11 @@ def is_set_global_variable(keyword: str) -> bool:
return normalized_name in SET_GLOBAL_VARIABLES


def is_set_local_variable(keyword: str) -> bool:
normalized_name = misc.normalize_name(misc.after_last_dot(keyword))
return normalized_name == SET_LOCAL_VARIABLE


def is_nested_variable(variable: str) -> bool:
"""Checks if variable name is nested.
Expand Down Expand Up @@ -50,35 +56,39 @@ def __init__(self):
self._local = set()
self._global = set()

def add_global(self, variable: str):
def _get_var_name(self, variable: str) -> "str|None":
if len(variable) > 1 and variable[0] in "$@&" and variable[1] != "{":
variable = f"{variable[0]}{{{variable[1:]}}}"
match = search_variable(variable, ignore_errors=True)
if not match.base:
return match.base

def add_global(self, variable: str):
var_name = self._get_var_name(variable)
if not var_name:
return
self._global.add(misc.normalize_name(match.base))
self._global.add(misc.normalize_name(var_name))

def add_local(self, variable: str, split_pattern: bool = False):
"""Add variable name to local cache.
If the variable is embedded argument, it can contain pattern we need to ignore (${var:[^pattern]})
"""
match = search_variable(variable, ignore_errors=True)
if not match.base:
var_name = self._get_var_name(variable)
if not var_name:
return
name = match.base
if split_pattern:
name = name.split(":", maxsplit=1)[0]
self._local.add(misc.normalize_name(name))
var_name = var_name.split(":", maxsplit=1)[0]
self._local.add(misc.normalize_name(var_name))

def change_scope_from_local_to_global(self, variable: str):
"""
Changes the variable scope from local to global by removing it from local cache and adding to global one.
"""
match = search_variable(variable, ignore_errors=True)
if not match.base:
var_name = self._get_var_name(variable)
if not var_name:
return
name = match.base
self._local.discard(misc.normalize_name(name))
self._global.add(misc.normalize_name(match.base))
self._local.discard(misc.normalize_name(var_name))
self._global.add(misc.normalize_name(var_name))

def is_local(self, variable: str):
return misc.normalize_name(variable) in self._local
Expand All @@ -98,8 +108,8 @@ class RenameVariables(Transformer):
- variable case depends on the variable scope (lowercase for local variables and uppercase for non-local variables)
- leading and trailing whitespace is stripped
- more than 2 consecutive whitespace in name is replaced by 1
- whitespace is replaced by _
- more than 2 consecutive whitespace in name replaced by 1
- whitespace replaced by _
- camelCase is converted to snake_case
Conventions can be configured or switched off using parameters - read more in the documentation.
Expand All @@ -115,7 +125,7 @@ class RenameVariables(Transformer):
*** Test Cases ***
Test
${local} Set Variable variable
${local} Set Variable value
Log ${local}
Log ${global}
Log ${local['item']}
Expand All @@ -141,7 +151,7 @@ class RenameVariables(Transformer):
*** Test Cases ***
Test
${local} Set Variable variable
${local} Set Variable value
Log ${local}
Log ${GLOBAL}
Log ${local['item']}
Expand Down Expand Up @@ -321,6 +331,7 @@ def visit_Keyword(self, node): # noqa
return self.generic_visit(node)

def visit_KeywordCall(self, node): # noqa
self.handle_set_local_variable(node)
if not self.disablers.is_node_disabled(node):
for token in node.data_tokens:
if token.type == Token.ASSIGN:
Expand All @@ -334,6 +345,16 @@ def visit_KeywordCall(self, node): # noqa
self.uppercase_global_name_in_set_variable(node)
return node

def handle_set_local_variable(self, node) -> None:
"""
Define local variable or reset scope of existing one to local.
"""
if not is_set_local_variable(node.keyword):
return
first_arg = node.get_token(Token.ARGUMENT)
if first_arg:
self.variables_scope.add_local(first_arg.value)

def uppercase_global_name_in_set_variable(self, node):
if not is_set_global_variable(node.keyword):
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ Set Variable
Set Global Variable &LOCAL_DICT
Set Global Variable ${LOCAL_${local4}}
Log Many ${LOCAL} ${LOCAL2} ${LOCAL3} ${local4} @{LOCAL_LIST} ${LOCAL_DICT}
Set Local Variable $local_list
Set Local Variable ${local2}
Log Many ${LOCAL} ${local2} ${LOCAL3} ${local4} @{local_list} ${LOCAL_DICT}
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,6 @@ Set Variable
Set Global Variable &local_dict
Set Global Variable ${local_${local4}}
Log Many ${local} ${local2} ${local3} ${local4} @{local_list} ${local_dict}
Set Local Variable $local_list
Set Local Variable ${local2}
Log Many ${local} ${local2} ${local3} ${local4} @{local_list} ${local_dict}

0 comments on commit fc4ab6e

Please sign in to comment.