Handle VAR syntax in variable naming rules (#1018)

docs/releasenotes/unreleased/rules.1.rst

@@ -0,0 +1,8 @@
+Updated existing rules with VAR support (#973)
+-----------------------------------------------
+
+Following rules now support ``VAR`` syntax:
+
+- W0310 ``non-local-variables-should-be-uppercase``
+- I0317 ``hyphen-in-variable-name``
+- W0324 ``overwriting-reserved-variable``

robocop/checkers/naming.py

@@ -596,7 +596,7 @@ def check_if_pattern_in_node_name(self, node, name_of_node, is_keyword=False):
             # Loop and skip variables:
             # Search pattern from start_pos to variable starting position
             # example `Keyword With ${em.bedded} Two ${second.Argument} Argument``
-            # is splitted to:
+            # is split to:
             #   1. `Keyword With `
             #   2. ` Two `
             #   3. ` Argument` - last part is searched in finditer part after this loop
@@ -981,7 +981,6 @@ def visit_Variable(self, node):  # noqa
     def visit_KeywordCall(self, node):  # noqa
         for token in node.get_tokens(Token.ASSIGN):
             self.check_for_reserved_naming_or_hyphen(token, "Variable", is_assign=True)
-
         if not node.keyword:
             return
         if normalize_robot_name(node.keyword, remove_prefix="builtin.") in SET_VARIABLE_VARIANTS:
@@ -996,16 +995,38 @@ def visit_KeywordCall(self, node):  # noqa
                 return  # TODO: Ignore for now, for example ${not  closed in variables will throw it
             if var_name is None:  # possibly $escaped or \${escaped}, or invalid variable name
                 return
-            normalized_var_name = remove_nested_variables(var_name)
-            # a variable as a keyword argument can contain lowercase nested variable
-            # because the actual value of it may be uppercase
-            if not normalized_var_name.isupper():
-                self.report(
-                    "non-local-variables-should-be-uppercase",
-                    node=node,
-                    col=token.col_offset + 1,
-                    end_col=token.end_col_offset + 1,
-                )
+            self.check_non_local_variable(var_name, node, token)
+
+    def check_non_local_variable(self, variable_name: str, node, token):
+        normalized_var_name = remove_nested_variables(variable_name)
+        # a variable as a keyword argument can contain lowercase nested variable
+        # because the actual value of it may be uppercase
+        if not normalized_var_name.isupper():
+            self.report(
+                "non-local-variables-should-be-uppercase",
+                node=node,
+                col=token.col_offset + 1,
+                end_col=token.end_col_offset + 1,
+            )
+
+    @staticmethod
+    def _is_var_scope_local(node):
+        is_local = True
+        for option in node.get_tokens(Token.OPTION):
+            if "scope=" in option.value:
+                is_local = option.value.lower() == "scope=local"
+        return is_local
+
+    def visit_Var(self, node):  # noqa
+        if node.errors:  # for example invalid variable definition like $var}
+            return
+        variable = node.get_token(Token.VARIABLE)
+        if not variable:
+            return
+        self.check_for_reserved_naming_or_hyphen(variable, "Variable", is_assign=True)
+        # TODO Check supported syntax for variable, ie ${{var}}?
+        if not self._is_var_scope_local(node):
+            self.check_non_local_variable(variable.value, node, variable)
 
     def visit_If(self, node):  # noqa
         for token in node.header.get_tokens(Token.ASSIGN):

tests/atest/rules/naming/hyphen_in_variable_name/VAR_syntax.robot

@@ -0,0 +1,8 @@
+*** Test Cases ***
+My Test Case
+    VAR    ${r}   ${2-1}  # this is fine
+    VAR    ${a-b}    1  # this will warn - because if it's later used as ${a-b} it can lead to ambiguous results
+    VAR    ${a\-b}  1  # this will warn
+    VAR    ${-}    1   scope=GLOBAL  # this will warn
+    VAR    ${a-}   1   # this will warn
+    VAR    ${-b}   1   # this will warn

tests/atest/rules/naming/hyphen_in_variable_name/expected_output_var.txt

@@ -0,0 +1,5 @@
+VAR_syntax.robot:4:12 [I] 0317 Use underscore in variable name '${a-b}' instead of hyphens to avoid treating them like minus sign
+VAR_syntax.robot:5:12 [I] 0317 Use underscore in variable name '${a\-b}' instead of hyphens to avoid treating them like minus sign
+VAR_syntax.robot:6:12 [I] 0317 Use underscore in variable name '${-}' instead of hyphens to avoid treating them like minus sign
+VAR_syntax.robot:7:12 [I] 0317 Use underscore in variable name '${a-}' instead of hyphens to avoid treating them like minus sign
+VAR_syntax.robot:8:12 [I] 0317 Use underscore in variable name '${-b}' instead of hyphens to avoid treating them like minus sign

tests/atest/rules/naming/hyphen_in_variable_name/test_rule.py

@@ -4,3 +4,6 @@
 class TestRuleAcceptance(RuleAcceptance):
     def test_rule(self):
         self.check_rule(src_files=["test.robot"], expected_file="expected_output.txt")
+
+    def test_var_syntax(self):
+        self.check_rule(src_files=["VAR_syntax.robot"], expected_file="expected_output_var.txt", target_version=">=7")

tests/atest/rules/naming/non_local_variables_should_be_uppercase/VAR_syntax.robot

@@ -0,0 +1,10 @@
+*** Keywords ***
+VAR syntax
+    VAR    ${suite}    value    scope=SUITE
+    VAR    ${global}    value    scope=GLOBAL
+    VAR    ${test}    value    scope=TEST
+    VAR    ${task}    value    scope=TASK
+    VAR    ${local_default}    value    scope=local
+    VAR    ${local_default}    value
+    VAR    ${invalid
+

tests/atest/rules/naming/non_local_variables_should_be_uppercase/expected_output_var.txt

@@ -0,0 +1,4 @@
+VAR_syntax.robot:3:12 [W] 0310 Test, suite and global variables should be uppercase
+VAR_syntax.robot:4:12 [W] 0310 Test, suite and global variables should be uppercase
+VAR_syntax.robot:5:12 [W] 0310 Test, suite and global variables should be uppercase
+VAR_syntax.robot:6:12 [W] 0310 Test, suite and global variables should be uppercase

tests/atest/rules/naming/non_local_variables_should_be_uppercase/test_rule.py

@@ -4,3 +4,6 @@
 class TestRuleAcceptance(RuleAcceptance):
     def test_rule(self):
         self.check_rule(src_files=["test.robot"], expected_file="expected_output.txt")
+
+    def test_var(self):
+        self.check_rule(src_files=["VAR_syntax.robot"], expected_file="expected_output_var.txt", target_version=">=7")

tests/atest/rules/naming/overwriting_reserved_variable/VAR_syntax.robot

@@ -0,0 +1,12 @@
+*** Test Cases ***
+Overwrite reserved with VAR
+    VAR     ${TEST_NAME}    new_value
+    VAR    ${TEST DOCUMENTATION}    new value    scope=GLOBAL
+    VAR    ${LOG LEVEL}    ${OPTIONS}    scope=LOCAL
+
+
+*** Keywords ***
+Overwrite reserved with VAR
+    VAR     ${TEST_NAME}    new_value
+    VAR    ${TEST DOCUMENTATION}    new value    scope=GLOBAL
+    VAR    ${LOG LEVEL}    ${OPTIONS}    scope=LOCAL

tests/atest/rules/naming/overwriting_reserved_variable/expected_output_var.txt

@@ -0,0 +1,6 @@
+VAR_syntax.robot:3:13 [W] 0324 Variable '${TEST_NAME}' overwrites reserved variable '${TEST_NAME}'
+VAR_syntax.robot:4:12 [W] 0324 Variable '${TEST DOCUMENTATION}' overwrites reserved variable '${TEST_DOCUMENTATION}'
+VAR_syntax.robot:5:12 [W] 0324 Variable '${LOG LEVEL}' overwrites reserved variable '${LOG_LEVEL}'
+VAR_syntax.robot:10:13 [W] 0324 Variable '${TEST_NAME}' overwrites reserved variable '${TEST_NAME}'
+VAR_syntax.robot:11:12 [W] 0324 Variable '${TEST DOCUMENTATION}' overwrites reserved variable '${TEST_DOCUMENTATION}'
+VAR_syntax.robot:12:12 [W] 0324 Variable '${LOG LEVEL}' overwrites reserved variable '${LOG_LEVEL}'

tests/atest/rules/naming/overwriting_reserved_variable/test_rule.py

@@ -4,3 +4,6 @@
 class TestRuleAcceptance(RuleAcceptance):
     def test_rule(self):
         self.check_rule(src_files=["test.robot"], expected_file="expected_output.txt", issue_format="end_col")
+
+    def test_var(self):
+        self.check_rule(src_files=["VAR_syntax.robot"], expected_file="expected_output_var.txt", target_version=">=7")