Do not rename embedded variables in RenameKeywords (#428)

Author: bhirsz

docs/releasenotes/3.3.1.rst

@@ -0,0 +1,30 @@
+Robotidy 3.3.1
+=========================================
+Fix release for RenameKeyword variable matching improvements.
+
+You can install the latest available version by running::
+
+    pip install --upgrade robotframework-tidy
+
+or to install exactly this version::
+
+    pip install robotframework-tidy==3.3.1
+
+
+Do not rename variables in RenameKeyword (#417)
+------------------------------------------------
+Variables were also renamed in ``RenameKeyword`` transformer.
+Following code:
+
+```
+Login With '${user.id}'
+```
+
+was transformed to:
+
+```
+Login With '${user.Id}'
+```
+
+To fix this issue we redesigned keyword name matching in the transformer.
+Now it should properly escape all variables (and also possible indices).

robotidy/transformers/RenameKeywords.py

@@ -3,6 +3,7 @@
 from typing import Optional
 
 from robot.api.parsing import ModelTransformer, Token
+from robot.variables.search import VariableIterator
 
 from robotidy.disablers import skip_if_disabled, skip_section_if_disabled
 from robotidy.exceptions import InvalidParameterValueError
@@ -86,21 +87,68 @@ def visit_Section(self, node):  # noqa
         return self.generic_visit(node)
 
     def rename_node(self, token, is_keyword_call):
+        if self.replace_pattern is not None:
+            new_value = self.rename_with_pattern(token.value, is_keyword_call=is_keyword_call)
+        else:
+            new_value = self.normalize_name(token.value, is_keyword_call=is_keyword_call)
+        if not new_value.strip():  # do not allow renames that removes keywords altogether
+            return
+        token.value = new_value
+
+    def normalize_name(self, value, is_keyword_call):
+        var_found = False
+        parts = []
+        new_name, remaining = "", ""
+        for prefix, match, remaining in VariableIterator(value, ignore_errors=True):
+            var_found = True
+            # rename strips whitespace, so we need to preserve it if needed
+            trailing_space = " " if prefix.endswith(" ") else ""
+            remaining_space = " " if remaining.startswith(" ") else ""
+            parts.extend([self.rename_part(prefix, is_keyword_call), trailing_space, match, remaining_space])
+        if var_found:
+            parts.append(self.rename_part(remaining, is_keyword_call))
+            new_name = "".join(parts)
+        else:
+            new_name = self.rename_part(value, is_keyword_call)
+        return new_name
+
+    def rename_part(self, part: str, is_keyword_call: bool):
         values = []
-        split_names = token.value.split(".")
+        split_names = part.split(".")
         for index, value in enumerate(split_names, start=1):
             if is_keyword_call and self.ignore_library and index != len(split_names):
                 values.append(value)
                 continue
-            if self.replace_pattern is not None:
-                value = self.replace_pattern.sub(repl=self.replace_to, string=value)
-            if self.remove_underscores and set(value) != {"_"}:
-                value = re.sub("_+", " ", value)  # replace one or more _ with one space
-            value = value.strip()
-            # capitalize first letter of every word, leave rest untouched
-            value = "".join([a if a.isupper() else b for a, b in zip(value, string.capwords(value))])
+            value = self.remove_underscores_and_capitalize(value)
             values.append(value)
-        token.value = ".".join(values)
+        return ".".join(values)
+
+    def remove_underscores_and_capitalize(self, value: str):
+        if self.remove_underscores:
+            value = re.sub("_+", " ", value)  # replace one or more _ with one space
+        value = value.strip()
+        # capitalize first letter of every word, leave rest untouched
+        return "".join([a if a.isupper() else b for a, b in zip(value, string.capwords(value))])
+
+    def rename_with_pattern(self, value: str, is_keyword_call: bool):
+        lib_name = ""
+        if is_keyword_call and "." in value:
+            # rename only non lib part
+            found_lib = -1
+            for prefix, _, _ in VariableIterator(value):
+                found_lib = prefix.find(".")
+                break
+            if found_lib != -1:
+                lib_name = value[: found_lib + 1]
+                value = value[found_lib + 1 :]
+            else:
+                lib_name, value = value.split(".", maxsplit=1)
+                lib_name += "."
+        if lib_name and not self.ignore_library:
+            lib_name = self.remove_underscores_and_capitalize(lib_name)
+        return lib_name + self.remove_underscores_and_capitalize(
+            self.replace_pattern.sub(repl=self.replace_to, string=value)
+        )
 
     @skip_if_disabled
     def visit_KeywordName(self, node):  # noqa

tests/atest/transformers/RenameKeywords/expected/embedded_variables.robot

@@ -0,0 +1,10 @@
+*** Keywords ***
+Embedded ${variables} That Should Be ${ignored.and.dots}
+    Login With '{user.Uid}' And '${user.password}' To Check Validation
+
+Variable With Square Brackets
+    Normalize This${variable['test']}
+    Normalize This${variable}['test']
+
+Invalid Syntax
+    Not Closed ${var

tests/atest/transformers/RenameKeywords/expected/library_embedded_var_pattern.robot

@@ -0,0 +1,3 @@
+*** Test Cases ***
+Library And Embedded Variable And Pattern
+    LibraryName.With.Dots.New Name ${keyword} And ${var}

tests/atest/transformers/RenameKeywords/expected/rename_pattern_partial.robot

@@ -47,6 +47,6 @@ All Upper Case
     Connect To System ABC
     Create Product DFG
 
-Underscores And.Dots
+Underscores And. Dots
     Foo.BAR Baz A B C
     Foo Bar BAZ

tests/atest/transformers/RenameKeywords/expected/rename_pattern_whole.robot

@@ -47,6 +47,6 @@ All Upper Case
     Connect To System ABC
     Create Product DFG
 
-Underscores And.Dots
+Underscores And. Dots
     Foo.BAR Baz A B C
     Foo Bar BAZ

tests/atest/transformers/RenameKeywords/source/embedded_variables.robot

@@ -0,0 +1,10 @@
+*** Keywords ***
+Embedded ${variables} That should Be ${ignored.and.dots}
+    Login With '{user.uid}'_and '${user.password}' to Check_Validation
+
+Variable With Square Brackets
+    normalize_this${variable['test']}
+    normalize_this${variable}['test']
+
+Invalid Syntax
+    Not Closed ${var

tests/atest/transformers/RenameKeywords/source/library_embedded_var_pattern.robot

@@ -0,0 +1,3 @@
+*** Test Cases ***
+Library And Embedded Variable And Pattern
+    LibraryName.With.Dots.Rename with ${keyword} Variable

tests/atest/transformers/RenameKeywords/test_transformer.py

@@ -58,3 +58,12 @@ def test_disablers(self):
 
     def test_run_keywords(self):
         self.compare(source="run_keywords.robot")
+
+    def test_embedded_variables(self):
+        self.compare(source="embedded_variables.robot")
+
+    def test_embedded_with_pattern(self):
+        self.compare(
+            config=":replace_pattern=(?i)rename\s?with\s.+variable$:replace_to=New_Name_${keyword}_And_${var}",
+            source="library_embedded_var_pattern.robot",
+        )