Count VAR as keyword call in length rules (#1022)

docs/releasenotes/unreleased/rules.1.rst

@@ -6,3 +6,8 @@ Following rules now support ``VAR`` syntax:
 - W0310 ``non-local-variables-should-be-uppercase``
 - I0317 ``hyphen-in-variable-name``
 - W0324 ``overwriting-reserved-variable``
+- W0501 ``too-long-keyword``
+- W0502 ``too-few-calls-in-keyword``
+- W0503 ``too-many-calls-in-keyword``
+- W0504 ``too-long-test-case``
+- W0505 ``too-many-calls-in-test-case``

robocop/checkers/lengths.py

@@ -16,13 +16,18 @@
 )
 
 try:
-    from robot.api.parsing import Break, Continue, ReturnStatement
+    from robot.api.parsing import Break, Continue
 except ImportError:
-    ReturnStatement, Break, Continue = None, None, None
+    Break, Continue = None, None
+try:  # RF7+
+    from robot.api.parsing import Var
+except ImportError:
+    Var = None
 
 from robocop.checkers import RawFileChecker, VisitorChecker
 from robocop.rules import Rule, RuleParam, RuleSeverity, SeverityThreshold
 from robocop.utils import get_section_name, normalize_robot_name, pattern_type, str2bool
+from robocop.utils.misc import RETURN_CLASSES
 
 RULE_CATEGORY_ID = "05"
 
@@ -378,6 +383,22 @@ class LengthChecker(VisitorChecker):
         "too-many-arguments",
     )
 
+    def __init__(self):
+        self.keyword_call_alike = tuple(
+            klass
+            for klass in (
+                KeywordCall,
+                TemplateArguments,
+                RETURN_CLASSES.return_class,
+                RETURN_CLASSES.return_setting_class,
+                Break,
+                Continue,
+                Var,
+            )
+            if klass
+        )
+        super().__init__()
+
     def visit_File(self, node):
         if node.end_lineno > self.param("file-too-long", "max_lines"):
             self.report(
@@ -422,7 +443,7 @@ def visit_Keyword(self, node):  # noqa
                 sev_threshold_value=length,
             )
             return
-        key_calls = LengthChecker.count_keyword_calls(node)
+        key_calls = self.count_keyword_calls(node)
         if key_calls < self.param("too-few-calls-in-keyword", "min_calls"):
             self.report(
                 "too-few-calls-in-keyword",
@@ -474,7 +495,7 @@ def visit_TestCase(self, node):  # noqa
         skip_too_few = test_is_templated and self.param("too-few-calls-in-test-case", "ignore_templated")
         if skip_too_few and skip_too_many:
             return
-        key_calls = LengthChecker.count_keyword_calls(node)
+        key_calls = self.count_keyword_calls(node)
         if not skip_too_many and (key_calls > self.param("too-many-calls-in-test-case", "max_calls")):
             self.report(
                 "too-many-calls-in-test-case",
@@ -498,24 +519,18 @@ def visit_TestCase(self, node):  # noqa
                 end_col=node.col_offset + len(node.name) + 1,
             )
 
-    @staticmethod
-    def count_keyword_calls(node):
-        # ReturnStatement is imported and evaluates to true in RF 5.0+, we don't need to also check Break/Continue
-        if (
-            isinstance(node, (KeywordCall, TemplateArguments))
-            or ReturnStatement
-            and isinstance(node, (Break, Continue, ReturnStatement))
-        ):
+    def count_keyword_calls(self, node):
+        if isinstance(node, self.keyword_call_alike):
             return 1
         if not hasattr(node, "body"):
             return 0
-        calls = sum(LengthChecker.count_keyword_calls(child) for child in node.body)
+        calls = sum(self.count_keyword_calls(child) for child in node.body)
         while node and getattr(node, "orelse", None):
             node = node.orelse
-            calls += sum(LengthChecker.count_keyword_calls(child) for child in node.body)
+            calls += sum(self.count_keyword_calls(child) for child in node.body)
         while node and getattr(node, "next", None):
             node = node.next
-            calls += sum(LengthChecker.count_keyword_calls(child) for child in node.body)
+            calls += sum(self.count_keyword_calls(child) for child in node.body)
         return calls
 
 

robocop/checkers/misc.py

@@ -9,7 +9,7 @@
 from robot.errors import VariableError
 from robot.libraries import STDLIBS
 from robot.parsing.model.blocks import TestCaseSection
-from robot.parsing.model.statements import Arguments, KeywordCall, Return, Teardown
+from robot.parsing.model.statements import Arguments, KeywordCall, Teardown
 from robot.utils import unescape
 from robot.variables.search import search_variable
 
@@ -18,13 +18,9 @@
 except ImportError:
     from robot.parsing.model.statements import Comment, EmptyLine, Variable
 try:
-    from robot.api.parsing import Break, Continue, InlineIfHeader, ReturnStatement
+    from robot.api.parsing import Break, Continue, InlineIfHeader
 except ImportError:
-    ReturnStatement, InlineIfHeader, Break, Continue = None, None, None, None
-try:
-    from robot.api.parsing import ReturnSetting  # RF 7.0 [Return]
-except ImportError:
-    ReturnSetting = None
+    InlineIfHeader, Break, Continue = None, None, None
 
 from robocop.checkers import VisitorChecker
 from robocop.rules import Rule, RuleParam, RuleSeverity, SeverityThreshold
@@ -38,7 +34,7 @@
     parse_assignment_sign_type,
     token_col,
 )
-from robocop.utils.misc import find_escaped_variables
+from robocop.utils.misc import RETURN_CLASSES, find_escaped_variables
 from robocop.utils.variable_matcher import VariableMatches
 
 RULE_CATEGORY_ID = "09"
@@ -599,17 +595,13 @@ class ReturnChecker(VisitorChecker):
         "empty-return",
     )
 
-    def __init__(self):
-        self.return_class = Return if ROBOT_VERSION.major < 7 else ReturnSetting
-        super().__init__()
-
     def visit_Keyword(self, node):  # noqa
         return_setting_node = None
         keyword_after_return = False
         return_from = False
         error = ""
         for child in node.body:
-            if isinstance(child, self.return_class):
+            if isinstance(child, RETURN_CLASSES.return_setting_class):
                 return_setting_node = child
                 error = (
                     "[Return] is not defined at the end of keyword. "
@@ -653,15 +645,11 @@ class UnreachableCodeChecker(VisitorChecker):
 
     reports = ("unreachable-code",)
 
-    def __init__(self):
-        self.return_class = ReturnStatement if ROBOT_VERSION.major < 7 else Return
-        super().__init__()
-
     def visit_Keyword(self, node):  # noqa
         statement_node = None
 
         for child in node.body:
-            if isinstance(child, (self.return_class, Break, Continue)):
+            if isinstance(child, (RETURN_CLASSES.return_class, Break, Continue)):
                 statement_node = child
             elif not isinstance(child, (EmptyLine, Comment, Teardown)):
                 if statement_node is not None:
@@ -963,7 +951,7 @@ def check_whether_if_should_be_inline(self, node):
             or node.orelse  # TODO it could still report with orelse? if short enough
             # IF with one branch and assign require ELSE to be valid, better to ignore it
             or getattr(node.body[0], "assign", None)
-            or not isinstance(node.body[0], (KeywordCall, ReturnStatement, Break, Continue))  # type: ignore[arg-type]
+            or not isinstance(node.body[0], (KeywordCall, RETURN_CLASSES.return_class, Break, Continue))  # type: ignore[arg-type]
         ):
             return
         min_possible = self.tokens_length(node.header.tokens) + self.tokens_length(node.body[0].tokens[1:]) + 2

robocop/utils/misc.py

@@ -3,7 +3,7 @@
 import re
 import token as python_token
 import tokenize
-from collections import Counter, defaultdict
+from collections import Counter, defaultdict, namedtuple
 from io import StringIO
 from pathlib import Path
 from tokenize import generate_tokens
@@ -29,6 +29,35 @@
 ROBOCOP_RULES_URL = "https://robocop.readthedocs.io/en/{version}/rules_list.html"
 
 
+ReturnClasses = namedtuple("ReturnClasses", ["return_class", "return_setting_class"])
+
+
+def get_return_classes() -> ReturnClasses:
+    """
+    Robot Framework change model names for [Return] and RETURN depending on the RF version. To achieve backward
+    compatibility we need to define mapping.
+    """
+    from robot.parsing.model.statements import Return
+
+    if ROBOT_VERSION.major < 5:
+        return_class = Return  # it does not exist, but we define it for backward compatibility
+        return_setting_class = Return
+    elif ROBOT_VERSION.major < 7:
+        from robot.api.parsing import ReturnStatement
+
+        return_class = ReturnStatement
+        return_setting_class = Return
+    else:
+        from robot.api.parsing import ReturnSetting
+
+        return_class = Return
+        return_setting_class = ReturnSetting
+    return ReturnClasses(return_class, return_setting_class)
+
+
+RETURN_CLASSES = get_return_classes()
+
+
 def rf_supports_lang():
     return ROBOT_VERSION >= ROBOT_WITH_LANG
 

tests/atest/rules/lengths/too_few_calls_in_keyword/expected_output.txt

@@ -1,2 +1,2 @@
-test.robot:25:1 [W] 0502 Keyword 'Empty Keyword' has too few keywords inside (0/1)
-test.robot:27:1 [W] 0502 Keyword 'Keyword' has too few keywords inside (0/1)
+test.robot:30:1 [W] 0502 Keyword 'Empty Keyword' has too few keywords inside (0/1)
+test.robot:32:1 [W] 0502 Keyword 'Keyword' has too few keywords inside (0/1)

tests/atest/rules/lengths/too_few_calls_in_keyword/expected_output_severity.txt

@@ -1,3 +1,3 @@
-test.robot:22:1 [W] 0502 Keyword 'Short Keyword' has too few keywords inside (1/1)
-test.robot:25:1 [E] 0502 Keyword 'Empty Keyword' has too few keywords inside (0/0)
-test.robot:27:1 [E] 0502 Keyword 'Keyword' has too few keywords inside (0/0)
+test.robot:27:1 [W] 0502 Keyword 'Short Keyword' has too few keywords inside (1/1)
+test.robot:30:1 [E] 0502 Keyword 'Empty Keyword' has too few keywords inside (0/0)
+test.robot:32:1 [E] 0502 Keyword 'Keyword' has too few keywords inside (0/0)

tests/atest/rules/lengths/too_few_calls_in_keyword/test.robot

@@ -19,6 +19,11 @@ Keyword
     No Operation
     Fail
 
+Only VAR
+    VAR    ${variable}
+    VAR    ${variable}
+    VAR    ${variable}
+
 Short Keyword
     Keyword
 

tests/atest/rules/lengths/too_few_calls_in_test_case/test.robot

@@ -32,6 +32,11 @@ Test case with settings and no keyword calls
     [Timeout]    1 min
     [Documentation]    doc
 
+Test only with VAR
+    VAR    ${variable}
+    VAR    ${variable}
+    VAR    ${variable}
+
 
 *** Keywords ***
 Keyword

tests/atest/rules/lengths/too_long_keyword/expected_output.txt

@@ -1 +1 @@
-test.robot:15:1 [W] 0501 Keyword 'Some Keyword' is too long (45/40)
+test.robot:15:1 [W] 0501 Keyword 'Some Keyword' is too long (46/40)

tests/atest/rules/lengths/too_long_keyword/test.robot

@@ -58,6 +58,7 @@ Some Keyword
     ...   ${var}
     ...   ${var}
     ...   ${var}
+    VAR    ${variable}
 
 
 Keyword with comments at the end

tests/atest/rules/lengths/too_long_test_case/test.robot

@@ -24,7 +24,7 @@ Test
     ...  ${var}
     ...  ${var}
     ...  ${var}
-    One More
+    VAR    ${variable}    value
 
 Test with comments at the end
     Pass

tests/atest/rules/lengths/too_many_calls_in_keyword/test.robot

@@ -19,7 +19,7 @@ Some Keyword
     No Operation
     Fail
     No Operation
-    No Operation
+    VAR    ${variable}    value
     No Operation
     No Operation
     No Operation

tests/atest/rules/lengths/too_many_calls_in_test_case/test.robot

@@ -17,7 +17,7 @@ Test
     Keyword
     Keyword
     Keyword
-    Keyword
+    VAR    ${variable}    value    scope=GLOBAL
     One More