Repository: MarketSquare/robotframework-robocop
Pull request #1090 (2024-06-11 11:50:51)

Implement rules checking order of elements in keyword and test case
Implement rules verifying order of elements in keyword and test case.

szymonslodkowski: I believe in this  PR I implemented what in scope of #997 
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1090?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) Report
Attention: Patch coverage is `95.45455%` with `2 lines` in your changes missing coverage. Please review.
> Project coverage is 94.91%. Comparing base [(`f62508c`)](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/commit/f62508cff6bcaf0f82139e831e40c601aba5c584?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) to head [(`3c1f8fd`)](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/commit/3c1f8fd134e812a0d620f52d89af4ce423a68524?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare).
> Report is 20 commits behind head on master.

| [Files](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1090?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) | Patch % | Lines |
|---|---|---|
| [robocop/checkers/misc.py](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1090?src=pr&el=tree&filepath=robocop%2Fcheckers%2Fmisc.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare#diff-cm9ib2NvcC9jaGVja2Vycy9taXNjLnB5) | 96.66% | [1 Missing :warning: ](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1090?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) |
| [robocop/utils/misc.py](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1090?src=pr&el=tree&filepath=robocop%2Futils%2Fmisc.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare#diff-cm9ib2NvcC91dGlscy9taXNjLnB5) | 92.85% | [1 Missing :warning: ](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1090?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=MarketSquare) |

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


```diff
@@            Coverage Diff             @@
##           master    #1090      +/-   ##
==========================================
- Coverage   97.02%   94.91%   -2.11%     
==========================================
  Files          37       38       +1     
  Lines        4670     4817     +147     
==========================================
+ Hits         4531     4572      +41     
- Misses        139      245     +106     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/MarketSquare/robotframework-robocop/pull/1090?dropdown=coverage&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/robocop/checkers/misc.py b/robocop/checkers/misc.py
index 16b5e25..18ddef8 100644
--- a/robocop/checkers/misc.py
+++ b/robocop/checkers/misc.py
@@ -9,7 +9,7 @@ from typing import Dict, List, Optional
 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.blocks import Keyword, TestCase, TestCaseSection
 from robot.parsing.model.statements import Arguments, KeywordCall, Teardown
 from robot.utils import unescape
 from robot.variables.search import search_variable
@@ -35,7 +35,13 @@ from robocop.utils import (
     parse_assignment_sign_type,
     token_col,
 )
-from robocop.utils.misc import RETURN_CLASSES, _is_var_scope_local, find_escaped_variables
+from robocop.utils.misc import (
+    RETURN_CLASSES,
+    _is_var_scope_local,
+    find_escaped_variables,
+    parse_keyword_order_param,
+    parse_test_case_order_param,
+)
 from robocop.utils.variable_matcher import VariableMatches
 
 RULE_CATEGORY_ID = "09"
@@ -601,6 +607,78 @@ rules = {
 
         """,
     ),
+    "0927": Rule(
+        RuleParam(
+            name="sections_order",
+            default="documentation,tags,timeout,setup,template,keyword,teardown",
+            converter=parse_test_case_order_param,
+            show_type="str",
+            desc="order of sections in comma-separated list",
+        ),
+        rule_id="0927",
+        name="test-case-section-out-of-order",
+        msg="'{{ section_name }}' is in wrong place of Test Case. "
+        "Recommended order of elements in Test Cases: {{ recommended_order }}",
+        severity=RuleSeverity.WARNING,
+        added_in_version="5.3.0",
+        docs="""
+        Sections should be defined in order set by ``sections_order``
+        parameter (default: ``documentation,tags,timeout,setup,template,keyword,teardown``).
+
+        To change the default order use following option::
+
+            robocop --configure test-case-section-out-of-order:sections_order:comma,separated,list,of,sections
+
+        where section should be case-insensitive name from the list: 
+        documentation, tags, timeout, setup, template, keywords, teardown. 
+        Order of not configured sections is ignored.
+    
+        Example of rule violation::
+
+            *** Test Cases ***
+            Keyword After Teardown
+                [Documentation]    This is test Documentation
+                [Tags]    tag1    tag2
+                [Teardown]    Log    abc
+                Keyword1
+        """,
+    ),
+    "0928": Rule(
+        RuleParam(
+            name="sections_order",
+            default="documentation,tags,arguments,timeout,setup,keyword,teardown",
+            converter=parse_keyword_order_param,
+            show_type="str",
+            desc="order of sections in comma-separated list",
+        ),
+        rule_id="0928",
+        name="keyword-section-out-of-order",
+        msg="'{{ section_name }}' is in wrong place of Keyword. "
+        "Recommended order of elements in Keyword: {{ recommended_order }}",
+        severity=RuleSeverity.WARNING,
+        added_in_version="5.3.0",
+        docs="""
+        Sections should be defined in order set by ``sections_order``
+        parameter (default: ``documentation,tags,arguments,timeout,setup,keyword,teardown``).
+
+        To change the default order use following option::
+
+            robocop --configure keyword-section-out-of-order:sections_order:comma,separated,list,of,sections
+
+        where section should be case-insensitive name from the list: 
+        documentation, tags, arguments, timeout, setup, keyword, teardown. 
+        Order of not configured sections is ignored.
+
+        Example of rule violation::
+
+            *** Keywords ***
+            Keyword After Teardown
+                [Documentation]    This is keyword Documentation
+                [Tags]    tag1    tag2
+                [Teardown]    Log    abc
+                Keyword1
+        """,
+    ),
 }
 
 
@@ -1566,3 +1644,44 @@ class ExpressionsChecker(VisitorChecker):
                 col=position,
                 end_col=position + len(variable) + len(right_side),
             )
+
+
+class TestAndKeywordOrderChecker(VisitorChecker):
+    reports = ("test-case-section-out-of-order", "keyword-section-out-of-order")
+
+    def __init__(self):
+        self.rules_by_node_type = {}
+        self.expected_order = {}
+        super().__init__()
+
+    def visit_File(self, node):  # noqa
+        self.rules_by_node_type = {Keyword: "keyword-section-out-of-order", TestCase: "test-case-section-out-of-order"}
+        self.expected_order = {
+            Keyword: self.param("keyword-section-out-of-order", "sections_order"),
+            TestCase: self.param("test-case-section-out-of-order", "sections_order"),
+        }
+        self.generic_visit(node)
+
+    def check_order(self, node):
+        max_order_indicator = -1
+        for subnode in node.body:
+            try:
+                subnode_type = subnode.type
+            except AttributeError:
+                continue
+            if subnode_type not in self.expected_order[type(node)]:
+                continue
+            this_node_expected_order = self.expected_order[type(node)].index(subnode.type)
+            if this_node_expected_order < max_order_indicator:
+                self.report(
+                    self.rules_by_node_type[type(node)],
+                    section_name=subnode_type,
+                    recommended_order=", ".join(self.expected_order[type(node)]),
+                    node=subnode,
+                    col=subnode.col_offset + 1,
+                    end_col=subnode.end_col_offset + 1,
+                )
+            else:
+                max_order_indicator = this_node_expected_order
+
+    visit_Keyword = visit_TestCase = check_order
diff --git a/robocop/utils/misc.py b/robocop/utils/misc.py
index a01ee95..9ba8fef 100644
--- a/robocop/utils/misc.py
+++ b/robocop/utils/misc.py
@@ -378,3 +378,39 @@ def _is_var_scope_local(node) -> bool:
         if "scope=" in option.value:
             is_local = option.value.lower() == "scope=local"
     return is_local
+
+
+def parse_order_comma_sep_list(value: str, mapping: dict) -> list:
+    ordered = []
+    for item in value.split(","):
+        item_lower = item.lower()
+        if item_lower not in mapping:
+            raise ValueError(f"Invalid value: {item}. Supported values: {', '.join(mapping.keys())}")
+        ordered.append(mapping[item_lower])
+    return ordered
+
+
+def parse_keyword_order_param(value: str) -> list:
+    mapping = {
+        "documentation": Token.DOCUMENTATION,
+        "tags": Token.TAGS,
+        "arguments": Token.ARGUMENTS,
+        "timeout": Token.TIMEOUT,
+        "setup": Token.SETUP,
+        "keyword": Token.KEYWORD,
+        "teardown": Token.TEARDOWN,
+    }
+    return parse_order_comma_sep_list(value, mapping)
+
+
+def parse_test_case_order_param(value: str) -> list:
+    mapping = {
+        "documentation": Token.DOCUMENTATION,
+        "tags": Token.TAGS,
+        "timeout": Token.TIMEOUT,
+        "setup": Token.SETUP,
+        "template": Token.TEMPLATE,
+        "keyword": Token.KEYWORD,
+        "teardown": Token.TEARDOWN,
+    }
+    return parse_order_comma_sep_list(value, mapping)


Test patch:
diff --git a/tests/atest/rules/misc/keyword_section_out_of_order/__init__.py b/tests/atest/rules/misc/keyword_section_out_of_order/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/atest/rules/misc/keyword_section_out_of_order/expected_output.txt b/tests/atest/rules/misc/keyword_section_out_of_order/expected_output.txt
new file mode 100644
index 0000000..bccc915
--- /dev/null
+++ b/tests/atest/rules/misc/keyword_section_out_of_order/expected_output.txt
@@ -0,0 +1,4 @@
+test.robot:25:1 [W] 0928 'DOCUMENTATION' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
+test.robot:31:1 [W] 0928 'TAGS' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
+test.robot:37:1 [W] 0928 'ARGUMENTS' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
+test.robot:44:1 [W] 0928 'KEYWORD' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
diff --git a/tests/atest/rules/misc/keyword_section_out_of_order/expected_output_rf7.txt b/tests/atest/rules/misc/keyword_section_out_of_order/expected_output_rf7.txt
new file mode 100644
index 0000000..ab0978a
--- /dev/null
+++ b/tests/atest/rules/misc/keyword_section_out_of_order/expected_output_rf7.txt
@@ -0,0 +1,6 @@
+test.robot:25:1 [W] 0928 'DOCUMENTATION' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
+test.robot:31:1 [W] 0928 'TAGS' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
+test.robot:37:1 [W] 0928 'ARGUMENTS' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
+test.robot:44:1 [W] 0928 'KEYWORD' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
+test_kw_setup_added_in_rf7.robot:27:1 [W] 0928 'TIMEOUT' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
+test_kw_setup_added_in_rf7.robot:33:1 [W] 0928 'SETUP' is in wrong place of Keyword. Recommended order of elements in Keyword: DOCUMENTATION, TAGS, ARGUMENTS, TIMEOUT, SETUP, KEYWORD, TEARDOWN
diff --git a/tests/atest/rules/misc/keyword_section_out_of_order/expected_output_teardown_keyword.txt b/tests/atest/rules/misc/keyword_section_out_of_order/expected_output_teardown_keyword.txt
new file mode 100644
index 0000000..aa2bb2c
--- /dev/null
+++ b/tests/atest/rules/misc/keyword_section_out_of_order/expected_output_teardown_keyword.txt
@@ -0,0 +1,2 @@
+test.robot:21:1 [W] 0928 'TEARDOWN' is in wrong place of Keyword. Recommended order of elements in Keyword: TEARDOWN, KEYWORD
+test.robot:43:1 [W] 0928 'TEARDOWN' is in wrong place of Keyword. Recommended order of elements in Keyword: TEARDOWN, KEYWORD
diff --git a/tests/atest/rules/misc/keyword_section_out_of_order/test.robot b/tests/atest/rules/misc/keyword_section_out_of_order/test.robot
new file mode 100644
index 0000000..556135f
--- /dev/null
+++ b/tests/atest/rules/misc/keyword_section_out_of_order/test.robot
@@ -0,0 +1,44 @@
+*** Settings ***
+Documentation  doc
+
+
+*** Test Cases ***
+Test
+    [Documentation]  doc
+    [Tags]  sometag
+    Correct Keyword    arg
+
+
+*** Keywords ***
+Correct Keyword
+    [Documentation]  this is doc
+    [Tags]    xd
+    [Arguments]    ${argument1}
+    [Timeout]    30
+    No Operation
+    No Operation
+    Fail
+    [Teardown]    Log    Cleaning
+
+Documentation After Tags
+    [Tags]    xd
+    [Documentation]  this is doc
+    No Operation
+
+Tags After Arguments
+    [Documentation]  this is doc
+    [Arguments]    ${argument1}
+    [Tags]    xd
+    Fail
+
+Arguments After Timeout
+    [Documentation]  this is doc
+    [Timeout]    30
+    [Arguments]    ${argument1}
+    No Operation
+
+Teardown After Teardown
+    [Documentation]  this is doc
+    No Operation
+    [Teardown]    Log    Cleaning
+    No Operation
diff --git a/tests/atest/rules/misc/keyword_section_out_of_order/test_kw_setup_added_in_rf7.robot b/tests/atest/rules/misc/keyword_section_out_of_order/test_kw_setup_added_in_rf7.robot
new file mode 100644
index 0000000..5ad3459
--- /dev/null
+++ b/tests/atest/rules/misc/keyword_section_out_of_order/test_kw_setup_added_in_rf7.robot
@@ -0,0 +1,35 @@
+*** Settings ***
+Documentation  doc
+
+
+*** Test Cases ***
+Test
+    [Documentation]  doc
+    [Tags]  sometag
+    Correct Keyword    arg
+
+
+*** Keywords ***
+Correct Keyword
+    [Documentation]  this is doc
+    [Tags]    xd
+    [Arguments]    ${argument1}
+    [Timeout]    30
+    [Setup]    Log    Preparing
+    No Operation
+    No Operation
+    Fail
+    [Teardown]    Log    Cleaning
+
+Timeout After Setup
+    [Documentation]  this is doc
+    [Setup]    Log    Preparing
+    [Timeout]    30
+    No Operation
+
+Setup After Keyword
+    [Documentation]  this is doc
+    No Operation
+    [Setup]    Log    Preparing
+    Log    Test
+
diff --git a/tests/atest/rules/misc/keyword_section_out_of_order/test_rule.py b/tests/atest/rules/misc/keyword_section_out_of_order/test_rule.py
new file mode 100644
index 0000000..3c66d1a
--- /dev/null
+++ b/tests/atest/rules/misc/keyword_section_out_of_order/test_rule.py
@@ -0,0 +1,20 @@
+from tests.atest.utils import RuleAcceptance
+
+
+class TestRuleAcceptance(RuleAcceptance):
+    def test_rule(self):
+        self.check_rule(src_files=["test.robot"], expected_file="expected_output.txt")
+
+    def test_rule_setup(self):
+        self.check_rule(
+            src_files=["test.robot", "test_kw_setup_added_in_rf7.robot"],
+            expected_file="expected_output_rf7.txt",
+            target_version=">=7",
+        )
+
+    def test_rule_teardown_keyword(self):
+        self.check_rule(
+            src_files=["test.robot"],
+            config="-c keyword-section-out-of-order:sections_order:teardown,keyword",
+            expected_file="expected_output_teardown_keyword.txt",
+        )
diff --git a/tests/atest/rules/misc/test_case_section_out_of_order/__init__.py b/tests/atest/rules/misc/test_case_section_out_of_order/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/atest/rules/misc/test_case_section_out_of_order/expected_output.txt b/tests/atest/rules/misc/test_case_section_out_of_order/expected_output.txt
new file mode 100644
index 0000000..f9d342c
--- /dev/null
+++ b/tests/atest/rules/misc/test_case_section_out_of_order/expected_output.txt
@@ -0,0 +1,6 @@
+test.robot:15:1 [W] 0927 'KEYWORD' is in wrong place of Test Case. Recommended order of elements in Test Cases: DOCUMENTATION, TAGS, TIMEOUT, SETUP, TEMPLATE, KEYWORD, TEARDOWN
+test.robot:19:1 [W] 0927 'DOCUMENTATION' is in wrong place of Test Case. Recommended order of elements in Test Cases: DOCUMENTATION, TAGS, TIMEOUT, SETUP, TEMPLATE, KEYWORD, TEARDOWN
+test.robot:25:1 [W] 0927 'TAGS' is in wrong place of Test Case. Recommended order of elements in Test Cases: DOCUMENTATION, TAGS, TIMEOUT, SETUP, TEMPLATE, KEYWORD, TEARDOWN
+test.robot:31:1 [W] 0927 'TIMEOUT' is in wrong place of Test Case. Recommended order of elements in Test Cases: DOCUMENTATION, TAGS, TIMEOUT, SETUP, TEMPLATE, KEYWORD, TEARDOWN
+test.robot:37:1 [W] 0927 'SETUP' is in wrong place of Test Case. Recommended order of elements in Test Cases: DOCUMENTATION, TAGS, TIMEOUT, SETUP, TEMPLATE, KEYWORD, TEARDOWN
+test.robot:43:1 [W] 0927 'TEMPLATE' is in wrong place of Test Case. Recommended order of elements in Test Cases: DOCUMENTATION, TAGS, TIMEOUT, SETUP, TEMPLATE, KEYWORD, TEARDOWN
diff --git a/tests/atest/rules/misc/test_case_section_out_of_order/expected_output_teardown_keyword.txt b/tests/atest/rules/misc/test_case_section_out_of_order/expected_output_teardown_keyword.txt
new file mode 100644
index 0000000..f0020aa
--- /dev/null
+++ b/tests/atest/rules/misc/test_case_section_out_of_order/expected_output_teardown_keyword.txt
@@ -0,0 +1,1 @@
+test.robot:9:1 [W] 0927 'TEARDOWN' is in wrong place of Test Case. Recommended order of elements in Test Cases: TEARDOWN, KEYWORD
diff --git a/tests/atest/rules/misc/test_case_section_out_of_order/test.robot b/tests/atest/rules/misc/test_case_section_out_of_order/test.robot
new file mode 100644
index 0000000..f75b202
--- /dev/null
+++ b/tests/atest/rules/misc/test_case_section_out_of_order/test.robot
@@ -0,0 +1,57 @@
+*** Settings ***
+Documentation    Test Suite
+
+*** Test Cases ***
+Correct
+    [Documentation]    This is test Documentation
+    [Tags]    tag1    tag2
+    Keyword1
+    [Teardown]    Teardown
+
+Keyword After Teardown
+    [Documentation]    This is test Documentation
+    [Tags]    tag1    tag2
+    [Teardown]    Log    abc
+    Keyword1
+
+Documentation After Tags
+    [Tags]    tag1    tag2
+    [Documentation]    This is test Documentation
+    Keyword1
+
+Tags After Timeout
+    [Documentation]    This is test Documentation
+    [Timeout]    30
+    [Tags]    MyTag
+    Keyword1
+
+Timeout After Setup
+    [Documentation]    This is test Documentation
+    [Setup]    Log    Setting up
+    [Timeout]    30
+    Keyword1
+
+Setup After Template
+    [Documentation]    This is test Documentation
+    [Template]    Do Stuff
+    [Setup]    Log    Setting up
+    a    b
+
+Template After Teardown
+    [Documentation]    This is test Documentation
+    [Teardown]    Teardown
+    [Template]    Do Stuff
+    a    b
+
+
+*** Keywords ***
+Keyword1
+    No Operation
+
+Teardown
+    No Operation
+
+Do Stuff
+    [Arguments]    ${arg1}    ${arg2}
+    Log    ${arg1}
+    Log    ${arg2}
diff --git a/tests/atest/rules/misc/test_case_section_out_of_order/test_rule.py b/tests/atest/rules/misc/test_case_section_out_of_order/test_rule.py
new file mode 100644
index 0000000..7667789
--- /dev/null
+++ b/tests/atest/rules/misc/test_case_section_out_of_order/test_rule.py
@@ -0,0 +1,12 @@
+from tests.atest.utils import RuleAcceptance
+
+
+class TestRuleAcceptance(RuleAcceptance):
+    def test_rule(self):
+        self.check_rule(expected_file="expected_output.txt")
+
+    def test_rule_teardown_keyword(self):
+        self.check_rule(
+            config="-c test-case-section-out-of-order:sections_order:teardown,keyword",
+            expected_file="expected_output_teardown_keyword.txt",
+        )