Repository: MarketSquare/robotframework-robocop
Pull request #1023 (2023-12-26 18:09:51)

Handle VAR syntax in variable usage rules
Relates #973

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1023?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) Report
Attention: `7 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`873237f`)](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/commit/873237f98b8f74f078d89b783c71a8cd49db3758?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) 96.39% compared to head [(`fabbfb2`)](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1023?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) 94.97%.

| [Files](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1023?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) | Patch % | Lines |
|---|---|---|
| [robocop/checkers/misc.py](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1023?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare#diff-cm9ib2NvcC9jaGVja2Vycy9taXNjLnB5) | 12.50% | [7 Missing :warning: ](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1023?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) |

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##           master    #1023      +/-   ##
==========================================
- Coverage   96.39%   94.97%   -1.42%     
==========================================
  Files          37       37              
  Lines        4553     4561       +8     
==========================================
- Hits         4389     4332      -57     
- Misses        164      229      +65     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1023?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare).


Patch:
diff --git a/docs/releasenotes/unreleased/rules.1.rst b/docs/releasenotes/unreleased/rules.1.rst
index be14dbf..5adb996 100644
--- 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
index 031891b..4f9dfeb 100644
--- a/robocop/checkers/misc.py
+++ b/robocop/checkers/misc.py
@@ -1207,6 +1207,15 @@ class UnusedVariablesChecker(VisitorChecker):
         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)


Test patch:
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
new file mode 100644
index 0000000..10c4a81
--- /dev/null
+++ 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
\ No newline at end of file
diff --git a/tests/atest/rules/misc/argument_overwritten_before_usage/expected_output.txt b/tests/atest/rules/misc/argument_overwritten_before_usage/expected_output_pre_var.txt
similarity index 100%
rename from tests/atest/rules/misc/argument_overwritten_before_usage/expected_output.txt
rename to tests/atest/rules/misc/argument_overwritten_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
index 88ef73b..289d74c 100644
--- 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
index 70c2301..d2f86c4 100644
--- 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 @@ from tests.atest.utils import RuleAcceptance
 
 
 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
index 304dac5..8af07e2 100644
--- 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
new file mode 100644
index 0000000..29cd701
--- /dev/null
+++ 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
\ No newline at end of file
diff --git a/tests/atest/rules/misc/unused_variable/expected_output.txt b/tests/atest/rules/misc/unused_variable/expected_output_pre_var.txt
similarity index 100%
rename from tests/atest/rules/misc/unused_variable/expected_output.txt
rename to tests/atest/rules/misc/unused_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
index 221164f..28cd3a2 100644
--- 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
index f685af8..3a863c2 100644
--- a/tests/atest/rules/misc/unused_variable/test_rule.py
+++ b/tests/atest/rules/misc/unused_variable/test_rule.py
@@ -2,8 +2,21 @@ from tests.atest.utils import RuleAcceptance
 
 
 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
new file mode 100644
index 0000000..830e378
--- /dev/null
+++ 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
\ No newline at end of file
diff --git a/tests/atest/rules/misc/variable_overwritten_before_usage/expected_output.txt b/tests/atest/rules/misc/variable_overwritten_before_usage/expected_output_pre_var.txt
similarity index 100%
rename from tests/atest/rules/misc/variable_overwritten_before_usage/expected_output.txt
rename to tests/atest/rules/misc/variable_overwritten_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
index 4c07e15..1029f25 100644
--- 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
index 6224edf..b6c2320 100644
--- 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 @@ from tests.atest.utils import RuleAcceptance
 
 
 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
index b72039a..31618cf 100644
--- a/tests/atest/utils/__init__.py
+++ b/tests/atest/utils/__init__.py
@@ -4,7 +4,7 @@ import os
 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 @@ class RuleAcceptance:
         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)