Handle VAR syntax in variable usage rules (#1023)

MarketSquare · Dec 27, 2023 · b4b6419 · b4b6419
1 parent c4cde29
commit b4b6419
Show file tree

Hide file tree

Showing 16 changed files with 118 additions and 8 deletions.
diff --git a/docs/releasenotes/unreleased/rules.1.rst b/docs/releasenotes/unreleased/rules.1.rst
@@ -11,3 +11,7 @@ Following rules now support ``VAR`` syntax:
 - W0503 ``too-many-calls-in-keyword``
 - W0504 ``too-long-test-case``
 - W0505 ``too-many-calls-in-test-case``
+- W0919 ``unused-argument``
+- I0920 ``unused-variable``
+- W0921 ``argument-overwritten-before-usage``
+- W0922 ``variable-overwritten-before-usage``
diff --git a/robocop/checkers/misc.py b/robocop/checkers/misc.py
@@ -1207,6 +1207,15 @@ def visit_KeywordCall(self, node):  # noqa
         for token in node.get_tokens(Token.ASSIGN):  # we first check args, then assign for used and then overwritten
             self.handle_assign_variable(token)
 
+    def visit_Var(self, node):  # noqa
+        if node.errors:  # for example invalid variable definition like $var}
+            return
+        for arg in node.get_tokens(Token.ARGUMENT):
+            self.find_not_nested_variable(arg.value, is_var=False)
+        variable = node.get_token(Token.VARIABLE)
+        if variable:
+            self.handle_assign_variable(variable)
+
     def visit_Return(self, node):  # noqa
         for token in node.get_tokens(Token.ARGUMENT):
             self.find_not_nested_variable(token.value, is_var=False)

diff --git a/tests/atest/rules/misc/argument_overwritten_before_usage/expected_output_after_var.txt b/tests/atest/rules/misc/argument_overwritten_before_usage/expected_output_after_var.txt
@@ -0,0 +1,6 @@
+test.robot:22:31:22:46 [W] 0921 Keyword argument '${overwritten1}' is overwritten before usage
+test.robot:22:50:22:65 [W] 0921 Keyword argument '${overwritten2}' is overwritten before usage
+test.robot:22:69:22:84 [W] 0921 Keyword argument '${overwritten3}' is overwritten before usage
+test.robot:39:20:39:28 [W] 0921 Keyword argument '${error}' is overwritten before usage
+test.robot:70:31:70:46 [W] 0921 Keyword argument '${overwritten1}' is overwritten before usage
+test.robot:70:50:70:65 [W] 0921 Keyword argument '${overwritten2}' is overwritten before usage
diff --git a/...rwritten_before_usage/expected_output.txt → ..._before_usage/expected_output_pre_var.txt b/...rwritten_before_usage/expected_output.txt → ..._before_usage/expected_output_pre_var.txt
diff --git a/tests/atest/rules/misc/argument_overwritten_before_usage/test.robot b/tests/atest/rules/misc/argument_overwritten_before_usage/test.robot
@@ -65,3 +65,11 @@ Overwritten In Inline IF - Just Variable
     [Documentation]    Should not raise anything - it is not argument
     ${variable}    Set Variable    default value
     ${variable}    IF  ${CONDITION}  Replace String  ${variable}  TAG  ${CONDITION_TAG}
+
+Overwritten in VAR
+    [Arguments]    ${arg1}    ${overwritten1}    ${overwritten2}    ${overwritten_but_used}
+    VAR    ${overwritten1}    ${arg1}
+    VAR    ${overwritten2}    ${arg1}
+    VAR    ${overwritten_but_used}    String with ${overwritten_but_used}
+    Keyword Call    ${overwritten1}  # used, but overwritten before
+    Keyword Call    ${overwritten2}  # used, but overwritten before
diff --git a/tests/atest/rules/misc/argument_overwritten_before_usage/test_rule.py b/tests/atest/rules/misc/argument_overwritten_before_usage/test_rule.py
@@ -2,9 +2,20 @@
 
 
 class TestRuleAcceptance(RuleAcceptance):
-    def test_rule(self):
+    def test_rule_after_var(self):
         self.check_rule(
-            src_files=["test.robot"], expected_file="expected_output.txt", issue_format="end_col", target_version=">=5"
+            src_files=["test.robot"],
+            expected_file="expected_output_after_var.txt",
+            issue_format="end_col",
+            target_version=">=7",
+        )
+
+    def test_rule_pre_var(self):
+        self.check_rule(
+            src_files=["test.robot"],
+            expected_file="expected_output_pre_var.txt",
+            issue_format="end_col",
+            target_version=">=5;<7",
         )
 
     def test_rule_rf3_rf4(self):

diff --git a/tests/atest/rules/misc/unused_argument/test.robot b/tests/atest/rules/misc/unused_argument/test.robot
@@ -200,3 +200,11 @@ Loop Header From Arguments And Global Count
         Set To Dictionary    ${item}    displayOrder=${count}
         ${count}    Evaluate    ${count} + 1
     END
+
+Overwritten in VAR
+    [Arguments]    ${arg1}    ${overwritten1}    ${overwritten2}    ${overwritten_but_used}
+    VAR    ${overwritten1}    ${arg1}
+    VAR    ${overwritten2}    ${arg1}
+    VAR    ${overwritten_but_used}    String with ${overwritten_but_used}
+    Keyword Call    ${overwritten1}  # used, but overwritten before
+    Keyword Call    ${overwritten2}  # used, but overwritten before
diff --git a/tests/atest/rules/misc/unused_variable/expected_output_after_var.txt b/tests/atest/rules/misc/unused_variable/expected_output_after_var.txt
@@ -0,0 +1,12 @@
+test.robot:14:5:14:11 [I] 0920 Variable '${var}' is assigned but not used
+test.robot:18:15:18:22 [I] 0920 Variable '${var2}' is assigned but not used
+test.robot:22:5:22:11 [I] 0920 Variable '${var}' is assigned but not used
+test.robot:30:5:30:14 [I] 0920 Variable '${assign}' is assigned but not used
+test.robot:41:12:41:18 [I] 0920 Variable '${var}' is assigned but not used
+test.robot:67:12:67:23 [I] 0920 Variable '${category}' is assigned but not used
+test.robot:115:5:115:11 [I] 0920 Variable '${var}' is assigned but not used
+test.robot:118:5:118:14 [I] 0920 Variable '${assign}' is assigned but not used
+test.robot:121:12:121:23 [I] 0920 Variable '${not_used}' is assigned but not used
+test.robot:122:12:122:30 [I] 0920 Variable '${not_used_global}' is assigned but not used
+test.robot:126:12:126:26 [I] 0920 Variable '${used_in_var}' is assigned but not used
+test.robot:129:12:129:23 [I] 0920 Variable '${variable}' is assigned but not used
diff --git a/.../misc/unused_variable/expected_output.txt → ...used_variable/expected_output_pre_var.txt b/.../misc/unused_variable/expected_output.txt → ...used_variable/expected_output_pre_var.txt
diff --git a/tests/atest/rules/misc/unused_variable/test.robot b/tests/atest/rules/misc/unused_variable/test.robot
@@ -116,3 +116,15 @@ Inline If - Overwritten Variable
 
 InlineIf - Assign With The Same Name As Arg
     ${assign}    IF    condition    Do Nothing    ELSE    Use    ${assign}
+
+Unused With VAR
+    VAR    ${not_used}    value
+    VAR    ${not_used_global}    value    scope=TEST
+    VAR    ${used_in_kw}    value
+    Keyword Call    ${used_in_kw}
+    VAR    ${used_in_var}    value
+    VAR    ${used_in_var}    ${used_in_var}    scope=SUITE
+    VAR    ${used_without_sign}=    value
+    Keyword Call    ${used_without_sign}
+    VAR    ${variable}  # missing value
+    VAR    $variable  # ignored for invalid variable name
diff --git a/tests/atest/rules/misc/unused_variable/test_rule.py b/tests/atest/rules/misc/unused_variable/test_rule.py
@@ -2,8 +2,21 @@
 
 
 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_rule_after_var(self):
+        self.check_rule(
+            src_files=["test.robot"],
+            expected_file="expected_output_after_var.txt",
+            issue_format="end_col",
+            target_version=">=7",
+        )
+
+    def test_rule_pre_var(self):
+        self.check_rule(
+            src_files=["test.robot"],
+            expected_file="expected_output_pre_var.txt",
+            issue_format="end_col",
+            target_version="<7",
+        )
 
     def test_rule_on_loops(self):
         self.check_rule(

diff --git a/tests/atest/rules/misc/variable_overwritten_before_usage/expected_output_after_var.txt b/tests/atest/rules/misc/variable_overwritten_before_usage/expected_output_after_var.txt
@@ -0,0 +1,6 @@
+test.robot:7:5 [W] 0922 Local variable '${value}' is overwritten before usage
+test.robot:15:5 [W] 0922 Local variable '${val_ue}' is overwritten before usage
+test.robot:121:12 [W] 0922 Local variable '${variable}' is overwritten before usage
+test.robot:123:12 [W] 0922 Local variable '${variable2}' is overwritten before usage
+test.robot:127:5 [W] 0922 Local variable '${from_keyword3}' is overwritten before usage
+test.robot:131:12 [W] 0922 Local variable '${variable3}' is overwritten before usage
diff --git a/...rwritten_before_usage/expected_output.txt → ..._before_usage/expected_output_pre_var.txt b/...rwritten_before_usage/expected_output.txt → ..._before_usage/expected_output_pre_var.txt
diff --git a/tests/atest/rules/misc/variable_overwritten_before_usage/test.robot b/tests/atest/rules/misc/variable_overwritten_before_usage/test.robot
@@ -116,3 +116,18 @@ Overwritten In IF
     ELSE
         ${variable}    Set Variable    special value
     END
+
+Overwritten With VAR
+    VAR    ${variable}    value
+    VAR    ${variable}    value
+    VAR    ${variable2}    value
+    VAR    ${variable2} =    value
+    ${from_keyword}    Keyword Call
+    ${from_keyword2}    Keyword Call
+    ${from_keyword3}    Keyword Call
+    VAR    ${from_keyword}    ${from_keyword}    scope=GLOBAL  # fine
+    VAR    ${from_keyword2}    ${from_keyword2}val  # fine
+    VAR    ${from_keyword3}    value  # not fine
+    VAR    ${variable3}    value
+    VAR    ${variable3}  # missing value
+    VAR    $variable  # ignored for invalid variable name
diff --git a/tests/atest/rules/misc/variable_overwritten_before_usage/test_rule.py b/tests/atest/rules/misc/variable_overwritten_before_usage/test_rule.py
@@ -2,8 +2,11 @@
 
 
 class TestRuleAcceptance(RuleAcceptance):
-    def test_rule(self):
-        self.check_rule(src_files=["test.robot"], expected_file="expected_output.txt", target_version=">=4")
+    def test_rule_after_var(self):
+        self.check_rule(src_files=["test.robot"], expected_file="expected_output_after_var.txt", target_version=">=7")
+
+    def test_rule_pre_var(self):
+        self.check_rule(src_files=["test.robot"], expected_file="expected_output_pre_var.txt", target_version=">=4;<7")
 
     def test_rule_rf3(self):
         self.check_rule(src_files=["test.robot"], expected_file="expected_output_rf3.txt", target_version="==3.*")

diff --git a/tests/atest/utils/__init__.py b/tests/atest/utils/__init__.py
@@ -4,7 +4,7 @@
 import re
 import sys
 from pathlib import Path
-from typing import List, Optional
+from typing import List, Optional, Union
 
 import pytest
 
@@ -151,9 +151,12 @@ def rule_is_enabled(self, robocop_rules):
         return robocop_rules[self.rule_name].enabled_in_version
 
     @staticmethod
-    def enabled_in_version(target_version):
+    def enabled_in_version(target_version: Optional[Union["list", str]]):
         if target_version is None:
             return True
         if isinstance(target_version, list):
             return any(ROBOT_VERSION in VersionSpecifier(version) for version in target_version)
+        if ";" in target_version:
+            must_match_versions = target_version.split(";")
+            return all(ROBOT_VERSION in VersionSpecifier(version) for version in must_match_versions)
         return ROBOT_VERSION in VersionSpecifier(target_version)