Ignore nested variables in rules 0309 and 0310

.github/ISSUE_TEMPLATE/bug_report.yml

@@ -15,7 +15,7 @@ body:
     id: run-command
     attributes:
       label: What command/code did you try to run?
-      description: Please include as much details as possible
+      description: Please include as much details as possible. Consider using \`\`\` for formatting.
       placeholder: Paste the command and/or the code snippet
     validations:
       required: true

docs/releasenotes/3.1.0.rst

@@ -0,0 +1,60 @@
+:orphan:
+
+=============
+Robocop 3.1.0
+=============
+
+<Release description>
+
+You can install the latest available version by running
+
+::
+
+    pip install --upgrade robotframework-robocop
+
+or to install exactly this version
+
+::
+
+    pip install robotframework-robocop==3.1.0
+
+.. contents::
+   :depth: 2
+   :local:
+
+Most important changes
+======================
+
+Change title (#0)
+-----------------------------------------------
+
+Description
+
+Rule changes
+============
+
+Rules W0310 and W0309 now handle nested variables (#678)
+------------------------------------------------------------------------------------------
+
+Rules W0310 ``non-local-variables-should-be-uppercase`` and W0309 ``section-variable-not-uppercase``
+were previously reporting when the variable had another nested variable with lowercase name,
+e.g. `${EXAMPLE_${lowercase}}`.
+Now, the nested variable names passed as an argument to one of the keywords `Set Test Variable`,
+`Set Suite Variable` or `Set Global Variable` are ignored and if the rest of the name is uppercase, the rules
+will not report the issue anymore.
+For variables in Variables section, the name still needs to be all uppercase (including
+nested variable), because all nested variables in this section need to be global anyway.
+
+
+Other features
+==============
+
+Feature title (#0)
+--------------------------------
+
+Description
+
+Acknowledgements
+================
+
+Thanks to...

robocop/checkers/naming.py

@@ -9,6 +9,7 @@
 from robot.api import Token
 from robot.parsing.model.blocks import Keyword
 from robot.parsing.model.statements import Arguments, KeywordCall
+from robot.variables.search import search_variable
 
 from robocop.checkers import VisitorChecker
 from robocop.rules import Rule, RuleParam, RuleSeverity
@@ -22,6 +23,7 @@
     remove_robot_vars,
     token_col,
 )
+from robocop.utils.misc import remove_nested_variables
 from robocop.utils.run_keywords import iterate_keyword_names
 
 rules = {
@@ -165,6 +167,22 @@
         name="non-local-variables-should-be-uppercase",
         msg="Test, suite and global variables should be uppercase",
         severity=RuleSeverity.WARNING,
+        docs="""
+        Good::
+
+            Set Task Variable    ${MY_VAR}           1
+            Set Suite Variable   ${MY VAR}           1
+            Set Test Variable    ${MY_VAR}           1
+            Set Global Variable  ${MY VAR${nested}}  1
+
+        Bad::
+
+            Set Task Variable    ${my_var}           1
+            Set Suite Variable   ${My Var}           1
+            Set Test Variable    ${myvar}            1
+            Set Global Variable  ${my_var${NESTED}}  1
+
+        """,
     ),
     "0311": Rule(
         rule_id="0311",
@@ -648,19 +666,21 @@ def __init__(self):
         }
         super().__init__()
 
-    def visit_VariableSection(self, node):  # noqa
-        for child in node.body:
-            if not child.data_tokens:
-                continue
-            token = child.data_tokens[0]
-            if token.type == Token.VARIABLE and token.value and not token.value.isupper():
-                self.report(
-                    "section-variable-not-uppercase",
-                    variable_name=token.value,
-                    lineno=token.lineno,
-                    col=token.col_offset + 1,
-                    end_col=token.col_offset + len(token.value) + 1,
-                )
+    def visit_Variable(self, node):  # noqa
+        token = node.data_tokens[0]
+        var_name = search_variable(token.value).base
+        if var_name is None:
+            return  # in RF<=5, a continuation mark ` ...` is wrongly considered a variable
+        # in Variables section, everything needs to be in uppercase
+        # because even when the variable is nested, it needs to be global
+        if not var_name.isupper():
+            self.report(
+                "section-variable-not-uppercase",
+                variable_name=token.value.strip(),
+                lineno=token.lineno,
+                col=token.col_offset + 1,
+                end_col=token.col_offset + len(token.value) + 1,
+            )
 
     def visit_KeywordCall(self, node):  # noqa
         for token in node.get_tokens(Token.ASSIGN):
@@ -679,7 +699,13 @@ def visit_KeywordCall(self, node):  # noqa
             if len(node.data_tokens) < 2:
                 return
             token = node.data_tokens[1]
-            if token.type == Token.ARGUMENT and not token.value.isupper():
+            if not token.value:
+                return
+            var_name = search_variable(token.value).base
+            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,

robocop/config.py

@@ -504,7 +504,7 @@ def check_deprecations(self, rules):
         }
         deprecated = {
             # "rule-name": "deprecation message"
-            "bad-indent": "`strict` and `ignore_uneven` parameters are no longer available for this rule. Take a look at new E1017 bad-block-indent rule that replaces them."  # warning added in v.3.0.0
+            "bad-indent": "'strict' and 'ignore_uneven' parameters are no longer available for this rule. Take a look at new E1017 bad-block-indent rule that replaces them."  # warning added in v.3.0.0
         }
         deprecation_header = "### DEPRECATION WARNING ###"
         deprecation_footer = "This information will disappear in the next version.\n\n"

robocop/utils/misc.py

@@ -14,6 +14,7 @@
 except ImportError:
     from robot.parsing.model.statements import Variable
 
+from robot.variables.search import VariableIterator
 from robot.version import VERSION as RF_VERSION
 
 from robocop.exceptions import InvalidExternalCheckerError
@@ -84,6 +85,15 @@ def normalize_robot_var_name(name: str) -> str:
     return normalize_robot_name(name)[2:-1] if name else ""
 
 
+def remove_nested_variables(var_name):
+    for prefix, match, suffix in VariableIterator(var_name, ignore_errors=True):
+        if match:  # if nested variable exists
+            # take what surrounds it and run the check again
+            var_name = remove_nested_variables(prefix + suffix)
+            break
+    return var_name.strip()
+
+
 def keyword_col(node) -> int:
     return token_col(node, Token.KEYWORD)
 

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

@@ -5,4 +5,8 @@ test.robot:6:26 [W] 0310 Test, suite and global variables should be uppercase
 test.robot:15:24 [W] 0310 Test, suite and global variables should be uppercase
 test.robot:16:25 [W] 0310 Test, suite and global variables should be uppercase
 test.robot:17:24 [W] 0310 Test, suite and global variables should be uppercase
-test.robot:18:26 [W] 0310 Test, suite and global variables should be uppercase
+test.robot:18:26 [W] 0310 Test, suite and global variables should be uppercase
+test.robot:28:24 [W] 0310 Test, suite and global variables should be uppercase
+test.robot:29:25 [W] 0310 Test, suite and global variables should be uppercase
+test.robot:30:24 [W] 0310 Test, suite and global variables should be uppercase
+test.robot:31:26 [W] 0310 Test, suite and global variables should be uppercase

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

@@ -1,9 +1,9 @@
 *** Test Cases ***
 Test
-    Set Task Variable  ${my_var}  1
-    Set Suite Variable  ${my var}  1
-    Set Test Variable  ${My Var}  1
-    Set Global Variable  ${My_Var}  1
+    Set Task Variable  ${my_var}  0
+    Set Suite Variable  ${my var}  0
+    Set Test Variable  ${My Var}  0
+    Set Global Variable  ${My_Var}  0
     Set Task Variable  ${MY_VAR}  1
     Set Suite Variable  ${MY VAR}  1
     Set Test Variable  ${MY_VAR}  1
@@ -12,11 +12,25 @@ Test
 
 *** Keywords ***
 Some Keyword
-    Set Task Variable  ${my_var}  1
-    Set Suite Variable  ${my var}  1
-    Set Test Variable  ${My Var}  1
-    Set Global Variable  ${My_Var}  1
+    Set Task Variable  ${my_var}  0
+    Set Suite Variable  ${my var}  0
+    Set Test Variable  ${My Var}  0
+    Set Global Variable  ${My_Var}  0
     Set Task Variable  ${MY_VAR}  1
     Set Suite Variable  ${MY VAR}  1
     Set Test Variable  ${MY_VAR}  1
     Set Global Variable  ${MY VAR}  1
+    Set Global Variable
+    ...
+    ...    previous arg is empty now
+
+Keyword With Nested Variables
+    Set Task Variable  ${${var@{var}}multiple_nestings&{var}}  0
+    Set Suite Variable  ${nesting at the end${VAR}}  0
+    Set Test Variable  ${${var}Front Nesting}  0
+    Set Global Variable  ${Middle_${var}_Nesting}  0
+    Set Task Variable  ${CAPITAL_${var}}  1
+    Set Suite Variable  ${CAPITAL SPACE @{var}}  1
+    Set Test Variable  ${CAPITAL_&{VAR}}  1
+    Set Global Variable  ${${Var} MY VAR}  1
+    Set Global Variable  ${${Var} TWO VARS${var}}  1

tests/atest/rules/naming/section_variable_not_uppercase/expected_output.txt

@@ -4,4 +4,17 @@ test.robot:7:1 [W] 0309 Section variable '${my var}' name should be uppercase
 test.robot:8:1 [W] 0309 Section variable '${MyVar}' name should be uppercase
 test.robot:9:1 [W] 0309 Section variable '${My_Var}' name should be uppercase
 test.robot:10:1 [W] 0309 Section variable '${myVar}' name should be uppercase
-test.robot:11:1 [W] 0309 Section variable '${MY_var}' name should be uppercase
+test.robot:11:1 [W] 0309 Section variable '${MY_var}' name should be uppercase
+test.robot:14:1 [W] 0309 Section variable '${my_var}' name should be uppercase
+test.robot:16:1 [W] 0309 Section variable '${var}' name should be uppercase
+test.robot:17:1 [W] 0309 Section variable '@{var3}' name should be uppercase
+test.robot:21:1 [W] 0309 Section variable '@{var4}' name should be uppercase
+test.robot:25:1 [W] 0309 Section variable '${MY_VAR${var}}' name should be uppercase
+test.robot:27:1 [W] 0309 Section variable '${${var}MY VAR${VAR}}' name should be uppercase
+test.robot:28:1 [W] 0309 Section variable '${${var${VAR}}MY VAR${VAR}}' name should be uppercase
+test.robot:29:1 [W] 0309 Section variable '${@{VAR}[1]MY_VAR&{var.param}}' name should be uppercase
+test.robot:30:1 [W] 0309 Section variable '${${var${VAR}}my_var}' name should be uppercase
+test.robot:31:1 [W] 0309 Section variable '${${VAR}my_var}' name should be uppercase
+test.robot:32:1 [W] 0309 Section variable '${${VAR}my_var${var}}' name should be uppercase
+test.robot:33:1 [W] 0309 Section variable '${@{VAR}[1]my_var}' name should be uppercase
+test.robot:34:1 [W] 0309 Section variable '${@{VAR}[1]my_var&{var.param}}' name should be uppercase

tests/atest/rules/naming/section_variable_not_uppercase/expected_output_rf3.txt

@@ -0,0 +1,9 @@
+test.robot:5:1 [W] 0309 Section variable '${my_var}' name should be uppercase
+test.robot:6:1 [W] 0309 Section variable '${My Var}' name should be uppercase
+test.robot:7:1 [W] 0309 Section variable '${my var}' name should be uppercase
+test.robot:8:1 [W] 0309 Section variable '${MyVar}' name should be uppercase
+test.robot:9:1 [W] 0309 Section variable '${My_Var}' name should be uppercase
+test.robot:10:1 [W] 0309 Section variable '${myVar}' name should be uppercase
+test.robot:11:1 [W] 0309 Section variable '${MY_var}' name should be uppercase
+test.robot:14:1 [W] 0309 Section variable '${my_var}' name should be uppercase
+test.robot:21:1 [W] 0309 Section variable '@{var4}' name should be uppercase

tests/atest/rules/naming/section_variable_not_uppercase/test.robot

@@ -9,6 +9,29 @@ ${MyVar}   7
 ${My_Var}  8
 ${myVar}   9
 ${MY_var}  10
+${MY_VAR}
+...         11
+${my_var}
+...         12
+ ${var}  1
+ @{var3}  a
+ ...      b
+ ...      c
+...      d
+@{var4}  a
+...      1
+ ...     2
+
+${MY_VAR${var}}  11
+${MY VAR${VAR}}  11
+${${var}MY VAR${VAR}}  11
+${${var${VAR}}MY VAR${VAR}}  11
+${@{VAR}[1]MY_VAR&{var.param}}  11
+${${var${VAR}}my_var}  11
+${${VAR}my_var}  11
+${${VAR}my_var${var}}  11
+${@{VAR}[1]my_var}  11
+${@{VAR}[1]my_var&{var.param}}  11
 
 
 *** Test Cases ***

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

@@ -3,4 +3,8 @@
 
 class TestRuleAcceptance(RuleAcceptance):
     def test_rule(self):
-        self.check_rule(src_files=["test.robot"], expected_file="expected_output.txt")
+        self.check_rule(src_files=["test.robot"], expected_file="expected_output.txt", target_version=">=4")
+
+    def test_rule_rf3(self):
+        # RF3 doesn't recognize nested variables in Variables section
+        self.check_rule(src_files=["test.robot"], expected_file="expected_output_rf3.txt", target_version="==3.*")

tests/utest/test_utils.py

@@ -9,6 +9,7 @@
     pattern_type,
     remove_robot_vars,
 )
+from robocop.utils.misc import remove_nested_variables
 from robocop.utils.version_matching import Version, VersionSpecifier
 
 
@@ -192,6 +193,26 @@ def test_invalid_pattern_type(self):
             pattern_type(r"[\911]")
         assert r"Invalid regex pattern: bad escape \\9 at position 1" in str(error)
 
+    @pytest.mark.parametrize(
+        "var_name, normalized_var_name",
+        [
+            ("var", "var"),
+            ("my_var", "my_var"),
+            ("my var", "my var"),
+            ("my_var${var}", "my_var"),
+            ("my_var${MY_VAR}", "my_var"),
+            ("my_var${my var}", "my_var"),
+            ("${VAR}var${var}", "var"),
+            ("${VAR${var}}var${var}", "var"),
+            ("@{VAR${var}}var&{var}", "var"),
+            ("@{VAR}[1]var&{var}", "var"),
+            ("@{VAR}[1]var&{var.param}", "var"),
+            ("my ${VAR} var", "my  var"),
+        ],
+    )
+    def test_remove_nested_variables(self, var_name, normalized_var_name):
+        assert remove_nested_variables(var_name) == normalized_var_name
+
 
 @pytest.mark.parametrize(
     "robot_version, rule_version, should_pass",