Repository: MarketSquare/robotframework-robocop (python)

Issue:
Option to disable empty-variable rule for VAR and leave it for *** Variables *** section
Since new `VAR` syntax has by default the same settings as creating empty variables in `*** Variables ***` section, it is inconsistent with old `Create Dictionary` / `Create List` / `Set Variable` keywords that allowed creating empty variables with no need to use placeholders.

For example, this:
```robotframework
&{some_dict}    Create Dictionary  # no issues
```
with VAR and default for `empty-variable` rule is suggested to be refactored into this:
```robotframework
VAR    &{some_dict}    # now Robocop will complain
```

It seems inconsistent with previous way of creating empty variables inside of keywords. Maybe that suggestion is not necessarily bad but it would be nice-to-have feature to separate the setting for Variables section and for inline-created VARs. 

Pull request:
Make empty-variable configurable
Closes #1056 

Patch:
diff --git a/docs/releasenotes/unreleased/rules.1.rst b/docs/releasenotes/unreleased/rules.1.rst
new file mode 100644
index 0000000..ae75c73
--- /dev/null
+++ b/docs/releasenotes/unreleased/rules.1.rst
@@ -0,0 +1,5 @@
+empty-variable can be now disabled for VAR (#1056)
+--------------------------------------------------
+
+I0912 ``empty-variable`` received new parameter ``variable_source`` which allows to enable the rule either only for
+variables from ```*** Variables ***``` section or only ``VAR`` statements. By default it works on both.
diff --git a/robocop/checkers/misc.py b/robocop/checkers/misc.py
index 18ddef8..cdf58d6 100644
--- a/robocop/checkers/misc.py
+++ b/robocop/checkers/misc.py
@@ -46,6 +46,11 @@ from robocop.utils.variable_matcher import VariableMatches
 
 RULE_CATEGORY_ID = "09"
 
+
+def comma_separated_list(value: str) -> List[str]:
+    return value.split(",")
+
+
 rules = {
     "0901": Rule(
         rule_id="0901",
@@ -207,11 +212,20 @@ rules = {
         added_in_version="1.7.0",
     ),
     "0912": Rule(
+        RuleParam(
+            name="variable_source",
+            default="section,var",
+            converter=comma_separated_list,
+            show_type="comma separated list",
+            desc="Variable sources that will be checked",
+        ),
         rule_id="0912",
         name="empty-variable",
         msg="Use built-in variable {{ var_type }}{EMPTY} instead of leaving variable without value or using backslash",
         severity=RuleSeverity.INFO,
         docs="""
+        Variables with placeholder ${EMPTY} values are more explicit.
+        
         Example of rule violation::
 
             *** Variables ***
@@ -222,6 +236,13 @@ rules = {
             ...  value
             ${EMPTY_WITH_BACKSLASH}  \\       # used backslash
 
+            *** Keywords ***
+            Create Variables
+                VAR    ${var_no_value}  # missing value
+                VAR    ${var_with_empty}    ${EMPTY}
+
+        You can configure ``empty-variable`` rule to run only in ```*** Variables ***``` section or on
+        ``VAR`` statements using ``variable_source`` parameter.
         """,
         added_in_version="1.10.0",
     ),
@@ -942,6 +963,27 @@ class EmptyVariableChecker(VisitorChecker):
 
     reports = ("empty-variable",)
 
+    def __init__(self):
+        self.visit_var_section = False
+        self.visit_var = False
+        super().__init__()
+
+    def visit_File(self, node):  # noqa
+        variable_source = self.param("empty-variable", "variable_source")
+        self.visit_var_section = "section" in variable_source
+        self.visit_var = "var" in variable_source
+        self.generic_visit(node)
+
+    def visit_VariableSection(self, node):  # noqa
+        if self.visit_var_section:
+            self.generic_visit(node)
+
+    def visit_KeywordSection(self, node):  # noqa
+        if self.visit_var:
+            self.generic_visit(node)
+
+    visit_TestCaseSection = visit_KeywordSection
+
     def visit_Variable(self, node):  # noqa
         if get_errors(node):
             return


Test patch:
diff --git a/tests/atest/rules/misc/empty_variable/expected_output_var.txt b/tests/atest/rules/misc/empty_variable/expected_output_var.txt
new file mode 100644
index 0000000..ad87cce
--- /dev/null
+++ b/tests/atest/rules/misc/empty_variable/expected_output_var.txt
@@ -0,0 +1,4 @@
+test.robot:51:5 [I] 0912 Use built-in variable ${EMPTY} instead of leaving variable without value or using backslash
+test.robot:53:9 [I] 0912 Use built-in variable @{EMPTY} instead of leaving variable without value or using backslash
+test.robot:54:9 [I] 0912 Use built-in variable &{EMPTY} instead of leaving variable without value or using backslash
+test.robot:62:8 [I] 0912 Use built-in variable ${EMPTY} instead of leaving variable without value or using backslash
\ No newline at end of file
diff --git a/tests/atest/rules/misc/empty_variable/test_rule.py b/tests/atest/rules/misc/empty_variable/test_rule.py
index d08cc26..70a7627 100644
--- a/tests/atest/rules/misc/empty_variable/test_rule.py
+++ b/tests/atest/rules/misc/empty_variable/test_rule.py
@@ -5,5 +5,21 @@ class TestRuleAcceptance(RuleAcceptance):
     def test_rule(self):
         self.check_rule(src_files=["test.robot"], expected_file="expected_output.txt", target_version=">=7")
 
+    def test_rule_only_var(self):
+        self.check_rule(
+            src_files=["test.robot"],
+            expected_file="expected_output_var.txt",
+            config="-c empty-variable:variable_source:var",
+            target_version=">=7",
+        )
+
+    def test_rule_only_var_section(self):
+        self.check_rule(
+            src_files=["test.robot"],
+            expected_file="expected_output_pre_var.txt",
+            config="-c empty-variable:variable_source:section",
+            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="<7")