Fix unused-variable not handled in TRY branches (#1549)

Author: bhirsz

docs/releasenotes/changelog.md

@@ -1,5 +1,11 @@
 # Release notes
 
+## 7.0.0
+
+### Fixes
+
+- Fix ``unused-variable`` and ``variable-overwritten-before-usage`` rules not reporting violations in ``TRY`` blocks ([issue #1548](https://github.com/MarketSquare/robotframework-robocop/issues/1548))
+
 ## 6.13.0
 
 ### Features

src/robocop/linter/rules/misc.py

@@ -1140,6 +1140,7 @@ class CachedVariable:
     name: str
     token: Token
     is_used: bool
+    current_scopy_only: bool = False
 
 
 class SectionVariablesCollector(ast.NodeVisitor):
@@ -1306,8 +1307,8 @@ def visit_IfBranch(self, node) -> None:  # noqa: N802
 
     def add_variables_from_if_to_scope(self, if_variables: dict[str, CachedVariable]) -> None:
         """
-        Add all variables in given IF branch to common scope. If variable is used already in the branch, if it will
-        also be mark as used.
+        Add all variables in the given IF branch to a common scope. If a variable is used already in the branch, it
+        will also be marked as used.
         """
         if not self.variables:
             self.variables.append(if_variables)
@@ -1338,11 +1339,11 @@ def clear_variables_after_loop(self) -> None:
 
     def revisit_variables_used_in_loop(self) -> None:
         """
-        Due to recursive nature of the loops, we need to revisit variables used in the loop again in case
-        variable defined in the further part of the loop was used.
+        Due to the recursive nature of the loops, we need to revisit variables used in the loop again in case
+        the variable defined in the further part of the loop was used.
 
-        In case of nested FOR/WHILE loops we're storing variables in separate stacks, that are merged until we reach
-        outer END.
+        In case of nested FOR/WHILE loops, we're storing variables in separate stacks that are merged until we reach
+        the outer END.
 
         For example:
 
@@ -1400,26 +1401,50 @@ def try_assign(try_node) -> str:
     def visit_Try(self, node):  # noqa: N802
         if node.errors or node.header.errors:
             return
-        for token in node.header.get_tokens(Token.ARGUMENT, Token.OPTION):
-            self.find_not_nested_variable(token.value, is_var=False)
+        # first gather variables from the TRY node
         self.variables.append({})
-        if self.try_assign(node) is not None:
-            error_var = node.header.get_token(Token.VARIABLE)
-            if error_var is not None:
-                self.handle_assign_variable(error_var, ignore_var_conversion=False)
         for item in node.body:
             self.visit(item)
-        self.variables.pop()
-        if node.next:
-            self.visit_Try(node.next)
+        try_variables = self.variables.pop()
+        branch_variables = []
+        try_branch = node.next
+        while try_branch:
+            self.variables.append({})
+            # variables in EXCEPT  ${error_pattern}
+            for token in try_branch.header.get_tokens(Token.ARGUMENT, Token.OPTION):
+                self.find_not_nested_variable(token.value, is_var=False)
+            # except AS ${err}
+            if self.try_assign(try_branch) is not None:
+                error_var = try_branch.header.get_token(Token.VARIABLE)
+                if error_var is not None:
+                    self.handle_assign_variable(error_var, ignore_var_conversion=False)
+                    for variable in self.variables[-1].values():
+                        variable.current_scopy_only = True
+            # visit body of branch
+            for item in try_branch.body:
+                self.visit(item)
+            branch_variables.append(self.variables.pop())
+            try_branch = try_branch.next
+        for branch in branch_variables:
+            for name, variable in branch.items():
+                if variable.current_scopy_only:
+                    if not variable.is_used:
+                        self.report_arg_or_var_rule(self.unused_variable, variable.token, variable.name)
+                elif name not in try_variables:
+                    try_variables[name] = variable
+                else:
+                    try_variables[name].is_used = try_variables[name].is_used and variable.is_used
+                    if not variable.is_used:
+                        try_variables[name].token = variable.token
+        self.add_variables_from_if_to_scope(try_variables)
 
     def visit_Group(self, node):  # noqa: N802
         for token in node.header.get_tokens(Token.ARGUMENT):
             self.find_not_nested_variable(token.value, is_var=False)
         self.generic_visit(node)
 
     def visit_KeywordCall(self, node) -> None:  # noqa: N802
-        for token in node.get_tokens(Token.ARGUMENT, Token.KEYWORD):  # argument can be used in keyword name
+        for token in node.get_tokens(Token.ARGUMENT, Token.KEYWORD):  # argument can be used in the keyword name
             self.find_not_nested_variable(token.value, is_var=False)
         for token in node.get_tokens(Token.ASSIGN):  # we first check args, then assign for used and then overwritten
             self.handle_assign_variable(token)
@@ -1462,7 +1487,7 @@ def handle_assign_variable(self, token, ignore_var_conversion: bool = True) -> N
             variable_scope = self.variables[-1]
             if normalized in variable_scope:
                 is_used = variable_scope[normalized].is_used
-                if not variable_scope[normalized].is_used and not self.ignore_overwriting:
+                if not is_used and not self.ignore_overwriting:
                     self.report_arg_or_var_rule(
                         self.variable_overwritten_before_usage,
                         variable_scope[normalized].token,
@@ -1486,12 +1511,12 @@ def find_not_nested_variable(self, value, is_var) -> None:
         Find and process not nested variable.
 
         Search `value` string until there is ${variable} without other variables inside. Unescaped escaped syntax
-        ($var or \\${var}). If variable does exist in assign variables or arguments, it is removed to denote it was
+        ($var or \\${var}). If a variable does exist in assign variables or arguments, it is removed to denote it was
         used.
         """
         try:
             variables = list(VariableMatches(value))
-        except VariableError:  # for example ${variable which wasn't closed properly
+        except VariableError:  # for example, ${variable which wasn't closed properly
             return
         if not variables:
             if is_var:

tests/linter/rules/variables/unused_variable/expected_extended.txt

@@ -176,6 +176,46 @@ test.robot:193:9 VAR02 Variable '${var2}' is assigned but not used
  194 |     END
      |
 
+test.robot:244:9 VAR02 Variable '${var1}' is assigned but not used
+     |
+ 242 |     Log    ${var_used_only_before}    Keyword
+ 243 |     TRY
+ 244 |         ${var1}    Keyword
+     |         ^^^^^^^ VAR02
+ 245 |         ${var_used_outside1}    Keyword
+ 246 |         ${var_used_only_before}    Keyword
+     |
+
+test.robot:246:9 VAR02 Variable '${var_used_only_before}' is assigned but not used
+     |
+ 244 |         ${var1}    Keyword
+ 245 |         ${var_used_outside1}    Keyword
+ 246 |         ${var_used_only_before}    Keyword
+     |         ^^^^^^^^^^^^^^^^^^^^^^^ VAR02
+ 247 |     EXCEPT     Error message*  AS  ${var2}
+ 248 |         ${var3}    Keyword
+     |
+
+test.robot:247:36 VAR02 Variable '${var2}' is assigned but not used
+     |
+ 245 |         ${var_used_outside1}    Keyword
+ 246 |         ${var_used_only_before}    Keyword
+ 247 |     EXCEPT     Error message*  AS  ${var2}
+     |                                    ^^^^^^^ VAR02
+ 248 |         ${var3}    Keyword
+ 249 |         ${var_used_outside2}    Keyword
+     |
+
+test.robot:248:9 VAR02 Variable '${var3}' is assigned but not used
+     |
+ 246 |         ${var_used_only_before}    Keyword
+ 247 |     EXCEPT     Error message*  AS  ${var2}
+ 248 |         ${var3}    Keyword
+     |         ^^^^^^^ VAR02
+ 249 |         ${var_used_outside2}    Keyword
+ 250 |     END
+     |
+
 unused_section_vars.robot:21:1 VAR02 Variable '${GLOBAL_NOT_USED}' is assigned but not used
     |
  19 | *** Variables ***

tests/linter/rules/variables/unused_variable/expected_output_rf3.txt renamed to tests/linter/rules/variables/unused_variable/expected_output_after_rf4.txt

@@ -3,10 +3,20 @@ test.robot:19:15:19:22 [I] VAR02 Variable '${var2}' is assigned but not used
 test.robot:23:5:23:11 [I] VAR02 Variable '${var}' is assigned but not used
 test.robot:31:5:31:14 [I] VAR02 Variable '${assign}' is assigned but not used
 test.robot:39:9:39:20 [I] VAR02 Variable '${not_used}' is assigned but not used
+test.robot:42:9:42:26 [I] VAR02 Variable '${used_in_branch}' is assigned but not used
 test.robot:44:9:44:32 [I] VAR02 Variable '${not_used_from_branch}' is assigned but not used
 test.robot:56:13:56:30 [I] VAR02 Variable '${nested_define3}' is assigned but not used
 test.robot:66:12:66:18 [I] VAR02 Variable '${var}' is assigned but not used
 test.robot:92:12:92:23 [I] VAR02 Variable '${category}' is assigned but not used
 test.robot:140:5:140:11 [I] VAR02 Variable '${var}' is assigned but not used
 test.robot:143:5:143:14 [I] VAR02 Variable '${assign}' is assigned but not used
+test.robot:163:9:163:15 [I] VAR02 Variable '${var}' is assigned but not used
+test.robot:170:9:170:15 [I] VAR02 Variable '${var}' is assigned but not used
+test.robot:179:9:179:16 [I] VAR02 Variable '${var2}' is assigned but not used
+test.robot:184:9:184:15 [I] VAR02 Variable '${var}' is assigned but not used
+test.robot:193:9:193:16 [I] VAR02 Variable '${var2}' is assigned but not used
+test.robot:244:9:244:16 [I] VAR02 Variable '${var1}' is assigned but not used
+test.robot:246:9:246:32 [I] VAR02 Variable '${var_used_only_before}' is assigned but not used
+test.robot:247:36:247:43 [I] VAR02 Variable '${var2}' is assigned but not used
+test.robot:248:9:248:16 [I] VAR02 Variable '${var3}' is assigned but not used
 unused_section_vars.robot:21:1:21:19 [I] VAR02 Variable '${GLOBAL_NOT_USED}' is assigned but not used

tests/linter/rules/variables/unused_variable/expected_output_after_var.txt

@@ -17,4 +17,8 @@ test.robot:170:9:170:15 [I] VAR02 Variable '${var}' is assigned but not used
 test.robot:179:9:179:16 [I] VAR02 Variable '${var2}' is assigned but not used
 test.robot:184:9:184:15 [I] VAR02 Variable '${var}' is assigned but not used
 test.robot:193:9:193:16 [I] VAR02 Variable '${var2}' is assigned but not used
+test.robot:244:9:244:16 [I] VAR02 Variable '${var1}' is assigned but not used
+test.robot:246:9:246:32 [I] VAR02 Variable '${var_used_only_before}' is assigned but not used
+test.robot:247:36:247:43 [I] VAR02 Variable '${var2}' is assigned but not used
+test.robot:248:9:248:16 [I] VAR02 Variable '${var3}' is assigned but not used
 unused_section_vars.robot:21:1:21:19 [I] VAR02 Variable '${GLOBAL_NOT_USED}' is assigned but not used

tests/linter/rules/variables/unused_variable/expected_output_pre_var.txt renamed to tests/linter/rules/variables/unused_variable/expected_output_rf4.txt

@@ -15,4 +15,7 @@ test.robot:170:9:170:15 [I] VAR02 Variable '${var}' is assigned but not used
 test.robot:179:9:179:16 [I] VAR02 Variable '${var2}' is assigned but not used
 test.robot:184:9:184:15 [I] VAR02 Variable '${var}' is assigned but not used
 test.robot:193:9:193:16 [I] VAR02 Variable '${var2}' is assigned but not used
+test.robot:244:9:244:16 [I] VAR02 Variable '${var1}' is assigned but not used
+test.robot:246:9:246:32 [I] VAR02 Variable '${var_used_only_before}' is assigned but not used
+test.robot:248:9:248:16 [I] VAR02 Variable '${var3}' is assigned but not used
 unused_section_vars.robot:21:1:21:19 [I] VAR02 Variable '${GLOBAL_NOT_USED}' is assigned but not used

tests/linter/rules/variables/unused_variable/test.robot

@@ -237,3 +237,21 @@ Used in EXCEPT branch
     EXCEPT    ${var2}
         No Operation
     END
+
+Except with AS
+    Log    ${var_used_only_before}    Keyword
+    TRY
+        ${var1}    Keyword
+        ${var_used_outside1}    Keyword
+        ${var_used_only_before}    Keyword
+    EXCEPT     Error message*  AS  ${var2}
+        ${var3}    Keyword
+        ${var_used_outside2}    Keyword
+    END
+    Log    ${var_used_outside1}    Keyword
+    Log    ${var_used_outside2}    Keyword
+    TRY
+        No Operation
+    EXCEPT    Error message*  AS  ${var2}
+        Log    ${var2}
+    END

tests/linter/rules/variables/unused_variable/test_rule.py

@@ -21,9 +21,17 @@ def test_extended(self):
     def test_rule_pre_var(self):
         self.check_rule(
             src_files=["test.robot", "unused_section_vars.robot"],
-            expected_file="expected_output_pre_var.txt",
+            expected_file="expected_output_after_rf4.txt",
             issue_format="end_col",
-            test_on_version=">=4;<7",
+            test_on_version=">=5;<7",
+        )
+
+    def test_rule_pre_var_pre_try(self):
+        self.check_rule(
+            src_files=["test.robot", "unused_section_vars.robot"],
+            expected_file="expected_output_rf4.txt",
+            issue_format="end_col",
+            test_on_version="==4.*",
         )
 
     def test_rule_on_loops(self):

tests/linter/rules/variables/variable_overwritten_before_usage/test_rule.py

@@ -36,3 +36,6 @@ def test_variable_type_conversion(self):
             expected_file="variable_type_conversion_expected.txt",
             test_on_version=">=7.3",
         )
+
+    def test_try_except(self):
+        self.check_rule(src_files=["try_except.robot"], expected_file=None, test_on_version=">=5")

tests/linter/rules/variables/variable_overwritten_before_usage/try_except.robot

@@ -0,0 +1,22 @@
+*** Keywords ***
+TRY EXCEPT example
+    ${initial}    Keyword
+    TRY
+        ${initial}    Keyword
+        ${initial_from_try}    Keyword
+        ${initial_from_try2}    Keyword
+    EXCEPT     Error message*  AS  ${error}
+        ${initial_from_except}    Keyword
+        ${initial_from_except2}    Keyword
+    FINALLY
+        ${initial_from_finally}    Keyword
+        ${initial_from_finally2}    Keyword
+    ELSE
+        ${initial_from_else}    Keyword
+        ${initial_from_else2}    Keyword
+    END
+    ${initial_from_try}    Keyword
+    ${initial_from_except}    Keyword
+    ${initial_from_finally}    Keyword
+    ${initial_from_else}    Keyword
+    ${error}    Keyword