Skip to content

Commit

Permalink
Fix Inline If with assign variables raising argument/variable overwri…
Browse files Browse the repository at this point in the history
…tten before usage
  • Loading branch information
bhirsz committed Oct 29, 2023
1 parent ddb1bdc commit fae9633
Show file tree
Hide file tree
Showing 8 changed files with 85 additions and 11 deletions.
28 changes: 28 additions & 0 deletions docs/releasenotes/unreleased/fixes.1.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
Inline If with assign variables handling variables in invalid order (#987)
---------------------------------------------------------------------------

Inline If did not properly recognized that variables were used inside if loop before assigning return value.
Following code::

*** Keywords ***
Keyword With Argument
[Arguments] ${arg}
${arg} IF ${VALUE} Use ${arg}

Keyword With Local Variable
${var} Set Variable default value
${var} IF ${VALUE} Use ${var}

Will no longer raise W0921 ``argument-overwritten-before-usage`` and W0922 ``variable-overwritten-before-usage``.

And following code::

*** Keywords ***
Inline If - Overwritten Variable
${var} Set Variable default
${var} IF condition Use ${var}

InlineIf - Assign With The Same Name As Arg
${assign} IF condition Do Nothing ELSE Use ${assign}

Should now raise I0920 ``unused-variable`` for ``${var}`` and ``${assign}`` variables.
4 changes: 2 additions & 2 deletions robocop/checkers/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1117,14 +1117,14 @@ def visit_If(self, node): # noqa
return node
for token in node.header.get_tokens(Token.ARGUMENT):
self.find_not_nested_variable(token.value, is_var=False)
for token in node.header.get_tokens(Token.ASSIGN):
self.handle_assign_variable(token)
self.variables.append({})
for item in node.body:
self.visit(item)
self.variables.pop()
if node.orelse:
self.visit(node.orelse)
for token in node.header.get_tokens(Token.ASSIGN):
self.handle_assign_variable(token)

def clear_variables_after_loop(self):
"""Remove used variables after loop finishes."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,20 @@ Overwritten In Try As
Errors In Arguments
[Arguments] ${argument} = value
Log ${argument}

Overwritten In Inline IF
[Arguments] ${arg}
${arg} IF ${CONDITION} Replace String ${arg} TAG ${CONDITION_TAG}

Overwritten In Inline IF ELSE
[Arguments] ${arg}
${arg} IF ${CONDITION} Do Nothing ELSE Replace String ${arg} TAG ${CONDITION_TAG}

Overwritten In Inline IF ELSE IF
[Arguments] ${arg}
${arg} IF ${CONDITION} Do Nothing ELSE IF ${OTHER} Replace String ${arg} TAG ${CONDITION_TAG}

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}
4 changes: 3 additions & 1 deletion tests/atest/rules/misc/unused_variable/expected_output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,6 @@ 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: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
7 changes: 7 additions & 0 deletions tests/atest/rules/misc/unused_variable/test.robot
Original file line number Diff line number Diff line change
Expand Up @@ -109,3 +109,10 @@ Use Item With Method
${string} Set Variable string
${lower_string} Set Variable ${string.lower()}
Log ${lower_string}
Inline If - Overwritten Variable
${var} Set Variable default
${var} IF condition Use ${var}
InlineIf - Assign With The Same Name As Arg
${assign} IF condition Do Nothing ELSE Use ${assign}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
*** Keywords ***
Overwritten In Inline IF
${variable} Set Variable default value
${variable} IF ${CONDITION} Replace String ${variable} TAG ${CONDITION_TAG}

Overwritten In Inline IF ELSE
${variable} Set Variable default value
${variable} IF ${CONDITION} Do Nothing ELSE Replace String ${variable} TAG ${CONDITION_TAG}

Overwritten In Inline IF ELSE IF
${variable} Set Variable default value
${variable} IF ${CONDITION} Do Nothing ELSE IF ${OTHER} Replace String ${variable} TAG ${CONDITION_TAG}

Overwritten In Inline IF - Arguments
[Documentation] Should not raise anything - it is argument.
[Arguments] ${arg}
${arg} IF ${CONDITION} Replace String ${arg} TAG ${CONDITION_TAG}
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ def test_rule_on_loops(self):
issue_format="end_col",
target_version=">=5",
)

def test_rule_inline_if(self):
self.check_rule(src_files=["inline_if.robot"], expected_file=None, target_version=">=4")
16 changes: 8 additions & 8 deletions tests/atest/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,14 @@ def check_rule(
if actual != expected:
missing_expected = sorted(set(actual) - set(expected))
missing_actual = sorted(set(expected) - set(actual))
present_in_actual = "\n ".join(missing_expected)
present_in_expected = "\n ".join(missing_actual)
raise AssertionError(
"Actual issues are different than expected.\n"
f"Actual issues not found in expected:\n {present_in_actual}"
f"\n\nExpected issues not found in actual:\n {present_in_expected}"
)
assert actual == expected, f"{actual} != {expected}"
error = "Actual issues are different than expected.\n"
if missing_expected:
present_in_actual = "\n ".join(missing_expected)
error += f"Actual issues not found in expected:\n {present_in_actual}\n\n"
if missing_actual:
present_in_expected = "\n ".join(missing_actual)
error += f"Expected issues not found in actual:\n {present_in_expected}"
pytest.fail(error, pytrace=False)

def get_issue_format(self, issue_format):
if issue_format == "default":
Expand Down

0 comments on commit fae9633

Please sign in to comment.