Skip to content

Commit

Permalink
Ignore nested variables in rules 0309 and 0310 (#813)
Browse files Browse the repository at this point in the history 
* Update Bug Report template

* Small update to deprecation message

* Fix 0310 rule to ignore nested variables

* Update also W0309 section-variable-not-uppercase rule

* Fixes after review, add release notes

* Fix conditions

* Fixes

* Refactor

* Fix tests for RF3

* Update release notes, refactor

* Extend test code
  • Loading branch information
@mnojek
mnojek committed Apr 3, 2023
1 parent fa95e6d commit b5db600
Show file tree
Hide file tree
Showing 12 changed files with 211 additions and 27 deletions.
2 changes: 1 addition & 1 deletion .github/ISSUE_TEMPLATE/bug_report.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
60 changes: 60 additions & 0 deletions docs/releasenotes/3.1.0.rst
View file
Original file line number Diff line number Diff line change
@@ -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...
54 changes: 40 additions & 14 deletions robocop/checkers/naming.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 = {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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):
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion robocop/config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
10 changes: 10 additions & 0 deletions robocop/utils/misc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down
6 changes: 5 additions & 1 deletion tests/atest/rules/naming/non_local_variables_should_be_uppercase/expected_output.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
30 changes: 22 additions & 8 deletions tests/atest/rules/naming/non_local_variables_should_be_uppercase/test.robot
View file
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
15 changes: 14 additions & 1 deletion tests/atest/rules/naming/section_variable_not_uppercase/expected_output.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 9 additions & 0 deletions tests/atest/rules/naming/section_variable_not_uppercase/expected_output_rf3.txt
View file
Original file line number Diff line number Diff line change
@@ -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
23 changes: 23 additions & 0 deletions tests/atest/rules/naming/section_variable_not_uppercase/test.robot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ***
Expand Down
6 changes: 5 additions & 1 deletion tests/atest/rules/naming/section_variable_not_uppercase/test_rule.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.*")
21 changes: 21 additions & 0 deletions tests/utest/test_utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit b5db600

Please sign in to comment.