Repository: MarketSquare/robotframework-robocop
Pull request #1022 (2023-12-26 17:22:53)

Count VAR as keyword call in length rules
Relates #973 

Also introduced new way of handling ``RETURN`` and ``[Return]`` mess (why, just why change class name to one that was assigned for different model just for sake changing it.. ;))

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1022?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`873237f`)](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/commit/873237f98b8f74f078d89b783c71a8cd49db3758?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) 96.39% compared to head [(`460dde5`)](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1022?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) 96.40%.


<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##           master    #1022   +/-   ##
=======================================
  Coverage   96.39%   96.40%           
=======================================
  Files          37       37           
  Lines        4553     4565   +12     
=======================================
+ Hits         4389     4401   +12     
  Misses        164      164           
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1022?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare).


Patch:
diff --git a/docs/releasenotes/unreleased/rules.1.rst b/docs/releasenotes/unreleased/rules.1.rst
index a39beab..be14dbf 100644
--- a/docs/releasenotes/unreleased/rules.1.rst
+++ b/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``
diff --git a/robocop/checkers/lengths.py b/robocop/checkers/lengths.py
index 832c131..34fe32b 100644
--- a/robocop/checkers/lengths.py
+++ b/robocop/checkers/lengths.py
@@ -16,13 +16,18 @@ from robot.parsing.model.statements import (
 )
 
 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 @@ class LengthChecker(VisitorChecker):
                 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 @@ class LengthChecker(VisitorChecker):
         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 @@ class LengthChecker(VisitorChecker):
                 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
 
 
diff --git a/robocop/checkers/misc.py b/robocop/checkers/misc.py
index 58d6e2d..031891b 100644
--- a/robocop/checkers/misc.py
+++ b/robocop/checkers/misc.py
@@ -9,7 +9,7 @@ from robot.api import Token
 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 @@ try:
 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 @@ from robocop.utils import (
     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 @@ class IfChecker(VisitorChecker):
             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
diff --git a/robocop/utils/misc.py b/robocop/utils/misc.py
index 9d43176..c7cb93a 100644
--- a/robocop/utils/misc.py
+++ b/robocop/utils/misc.py
@@ -3,7 +3,7 @@ import difflib
 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 @@ ROBOT_WITH_LANG = Version("6.0")
 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
 


Test patch:
diff --git a/tests/atest/rules/lengths/too_few_calls_in_keyword/expected_output.txt b/tests/atest/rules/lengths/too_few_calls_in_keyword/expected_output.txt
index b1a373f..c6672ee 100644
--- a/tests/atest/rules/lengths/too_few_calls_in_keyword/expected_output.txt
+++ b/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)
diff --git a/tests/atest/rules/lengths/too_few_calls_in_keyword/expected_output_severity.txt b/tests/atest/rules/lengths/too_few_calls_in_keyword/expected_output_severity.txt
index 45a3e78..daaf728 100644
--- a/tests/atest/rules/lengths/too_few_calls_in_keyword/expected_output_severity.txt
+++ b/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)
\ No newline at end of file
+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)
\ No newline at end of file
diff --git a/tests/atest/rules/lengths/too_few_calls_in_keyword/test.robot b/tests/atest/rules/lengths/too_few_calls_in_keyword/test.robot
index 0340415..ad5eae5 100644
--- a/tests/atest/rules/lengths/too_few_calls_in_keyword/test.robot
+++ b/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
 
diff --git a/tests/atest/rules/lengths/too_few_calls_in_test_case/test.robot b/tests/atest/rules/lengths/too_few_calls_in_test_case/test.robot
index fe7c82c..1b9ba5e 100644
--- a/tests/atest/rules/lengths/too_few_calls_in_test_case/test.robot
+++ b/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
diff --git a/tests/atest/rules/lengths/too_long_keyword/expected_output.txt b/tests/atest/rules/lengths/too_long_keyword/expected_output.txt
index ab06f15..8ec6c7a 100644
--- a/tests/atest/rules/lengths/too_long_keyword/expected_output.txt
+++ b/tests/atest/rules/lengths/too_long_keyword/expected_output.txt
@@ -1,1 +1,1 @@
-test.robot:15:1 [W] 0501 Keyword 'Some Keyword' is too long (45/40)
\ No newline at end of file
+test.robot:15:1 [W] 0501 Keyword 'Some Keyword' is too long (46/40)
\ No newline at end of file
diff --git a/tests/atest/rules/lengths/too_long_keyword/test.robot b/tests/atest/rules/lengths/too_long_keyword/test.robot
index 2b4af79..c6b33c8 100644
--- a/tests/atest/rules/lengths/too_long_keyword/test.robot
+++ b/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
diff --git a/tests/atest/rules/lengths/too_long_test_case/test.robot b/tests/atest/rules/lengths/too_long_test_case/test.robot
index 3c49f82..b1eb81b 100644
--- a/tests/atest/rules/lengths/too_long_test_case/test.robot
+++ b/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
diff --git a/tests/atest/rules/lengths/too_many_calls_in_keyword/test.robot b/tests/atest/rules/lengths/too_many_calls_in_keyword/test.robot
index 29c3216..2fc77e8 100644
--- a/tests/atest/rules/lengths/too_many_calls_in_keyword/test.robot
+++ b/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
diff --git a/tests/atest/rules/lengths/too_many_calls_in_test_case/test.robot b/tests/atest/rules/lengths/too_many_calls_in_test_case/test.robot
index ff30d63..d1e2719 100644
--- a/tests/atest/rules/lengths/too_many_calls_in_test_case/test.robot
+++ b/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