Add rules checking that builtin and custom libraries are imported in …

…alphabetical order. (#1088)

* Add rules checking that builtin and custom libraries are imported in
alphabetical order.

* Move rule checking order of non builtin libraries imports to community rules.

* Add community rule checking order of Resource imports

* Reformat Rules definitions to be consistence with existing

robocop/checkers/community_rules/misc.py

@@ -0,0 +1,100 @@
+from robot.api import Token
+from robot.libraries import STDLIBS
+
+from robocop.checkers import VisitorChecker
+from robocop.rules import Rule, RuleSeverity
+
+RULE_CATEGORY_ID = "01"
+
+
+rules = {
+    "10101": Rule(
+        rule_id="10101",
+        name="non-builtin-imports-not-sorted",
+        msg="Non builtin library import '{{ custom_import }}' should be placed before '{{ previous_custom_import }}'",
+        severity=RuleSeverity.WARNING,
+        added_in_version="5.2.0",
+        enabled=False,
+        docs="""
+        Example of rule violation:
+
+            *** Settings ***
+            Library    Collections
+            Library    CustomLibrary
+            Library    AnotherCustomLibrary  # AnotherCustomLibrary library defined after custom CustomLibrary
+
+        """,
+    ),
+    "10102": Rule(
+        rule_id="10102",
+        name="resources-imports-not-sorted",
+        msg="Resource import '{{ resource_import }}' should be placed before '{{ previous_resource_import }}'",
+        severity=RuleSeverity.WARNING,
+        added_in_version="5.2.0",
+        enabled=False,
+        docs="""
+        Example of rule violation:
+
+            *** Settings ***
+            Resource   CustomResource.resource
+            Resource   AnotherFile.resource
+
+        """,
+    ),
+}
+
+
+class NonBuiltinLibrariesImportOrderChecker(VisitorChecker):
+    """
+    Find and report Non Builtin Libraries or Resources imported not in alphabetical order.
+    """
+
+    reports = (
+        "non-builtin-imports-not-sorted",
+        "resources-imports-not-sorted",
+    )
+
+    def __init__(self):
+        self.non_builtin_libraries = []
+        self.resources = []
+        super().__init__()
+
+    def visit_File(self, node):  # noqa
+        self.non_builtin_libraries = []
+        self.resources = []
+        self.generic_visit(node)
+        previous = None
+        for library in self.non_builtin_libraries:
+            if previous is not None and library.name < previous.name:
+                lib_name = library.get_token(Token.NAME)
+                self.report(
+                    "non-builtin-imports-not-sorted",
+                    custom_import=library.name,
+                    previous_custom_import=previous.name,
+                    node=library,
+                    col=lib_name.col_offset + 1,
+                    end_col=lib_name.end_col_offset + 1,
+                )
+            previous = library
+        previous = None
+        for resource in self.resources:
+            if previous is not None and resource.name < previous.name:
+                resource_name = resource.get_token(Token.NAME)
+                self.report(
+                    "resources-imports-not-sorted",
+                    resource_import=resource.name,
+                    previous_resource_import=previous.name,
+                    node=resource,
+                    col=resource_name.col_offset + 1,
+                    end_col=resource_name.end_col_offset + 1,
+                )
+            previous = resource
+
+    def visit_LibraryImport(self, node):  # noqa
+        if node.name and node.name not in STDLIBS:
+            self.non_builtin_libraries.append(node)
+
+    def visit_ResourceImport(self, node):  # noqa
+        if not node.name:
+            return
+        self.resources.append(node)

robocop/checkers/misc.py

@@ -586,6 +586,21 @@
         """,
         added_in_version="4.0.0",
     ),
+    "0926": Rule(
+        rule_id="0926",
+        name="builtin-imports-not-sorted",
+        msg="BuiltIn library import '{{ builtin_import }}' should be placed before '{{ previous_builtin_import }}'",
+        severity=RuleSeverity.WARNING,
+        added_in_version="5.2.0",
+        docs="""
+        Example of rule violation::
+
+            *** Settings ***
+            Library    OperatingSystem
+            Library    Collections  # BuiltIn libraries imported not in alphabetical order
+
+        """,
+    ),
 }
 
 
@@ -796,7 +811,10 @@ class SettingsOrderChecker(VisitorChecker):
     BuiltIn libraries imports should always be placed before other libraries imports.
     """
 
-    reports = ("wrong-import-order",)
+    reports = (
+        "wrong-import-order",
+        "builtin-imports-not-sorted",
+    )
 
     def __init__(self):
         self.libraries = []
@@ -806,6 +824,7 @@ def visit_File(self, node):  # noqa
         self.libraries = []
         self.generic_visit(node)
         first_non_builtin = None
+        previous_builtin = None
         for library in self.libraries:
             if first_non_builtin is None:
                 if library.name not in STDLIBS:
@@ -821,6 +840,18 @@ def visit_File(self, node):  # noqa
                         col=lib_name.col_offset + 1,
                         end_col=lib_name.end_col_offset + 1,
                     )
+            if library.name in STDLIBS:
+                if previous_builtin is not None and library.name < previous_builtin.name:
+                    lib_name = library.get_token(Token.NAME)
+                    self.report(
+                        "builtin-imports-not-sorted",
+                        builtin_import=library.name,
+                        previous_builtin_import=previous_builtin.name,
+                        node=library,
+                        col=lib_name.col_offset + 1,
+                        end_col=lib_name.end_col_offset + 1,
+                    )
+                previous_builtin = library
 
     def visit_LibraryImport(self, node):  # noqa
         if not node.name:

tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/expected_output.txt

@@ -0,0 +1,2 @@
+no_builtins.robot:4:12 [W] 10101 Non builtin library import 'OwnLib.py' should be placed before 'RequestsLibrary'
+test.robot:9:12 [W] 10101 Non builtin library import 'AnotherLibrary' should be placed before 'RequestsLibrary'

tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/no_builtins.robot

@@ -0,0 +1,11 @@
+*** Settings ***
+Library    RequestsLibrary
+Resource   ..${/}Libs${/}MyResource.robot
+Library    OwnLib.py
+Variables    variables.py
+Test Template    Keyword
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/no_imports.robot

@@ -0,0 +1,9 @@
+*** Settings ***
+Resource   ..${/}Libs${/}MyResource.robot
+Variables    variables.py
+Test Template    Keyword
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/only_builtins.robot

@@ -0,0 +1,12 @@
+*** Settings ***
+Library    Collections
+Library    String
+Resource   ..${/}Libs${/}MyResource.robot
+Variables    variables.py
+Test Template    Keyword
+Library    BuiltIn
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/test.robot

@@ -0,0 +1,18 @@
+*** Settings ***
+Library    DateTime
+Library    Collections
+Library    OperatingSystem
+Library    XML
+Library    String
+Library    OwnLib.py
+Library    RequestsLibrary
+Library    AnotherLibrary
+Resource   ..${/}Libs${/}MyResource.robot
+
+Variables    variables.py
+Test Template    Keyword
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/test_rule.py

@@ -0,0 +1,6 @@
+from tests.atest.utils import RuleAcceptance
+
+
+class TestRuleAcceptance(RuleAcceptance):
+    def test_rule(self):
+        self.check_rule(expected_file="expected_output.txt")

tests/atest/rules/community_rules/misc/resources_imports_not_sorted/expected_output.txt

@@ -0,0 +1,2 @@
+test.robot:5:12 [W] 10102 Resource import 'AnotherFile.resource' should be placed before 'CustomResource.resource'
+test.robot:6:12 [W] 10102 Resource import '..\${/}Libs\${/}MyResource.robot' should be placed before 'AnotherFile.resource'

tests/atest/rules/community_rules/misc/resources_imports_not_sorted/no_resources.robot

@@ -0,0 +1,9 @@
+*** Settings ***
+Library    RequestsLibrary
+Variables    variables.py
+Test Template    Keyword
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/community_rules/misc/resources_imports_not_sorted/test.robot

@@ -0,0 +1,14 @@
+*** Settings ***
+Library    DateTime
+Library    RequestsLibrary
+Resource   CustomResource.resource
+Resource   AnotherFile.resource
+Resource   ..${/}Libs${/}MyResource.robot
+
+Variables    variables.py
+Test Template    Keyword
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/community_rules/misc/resources_imports_not_sorted/test_rule.py

@@ -0,0 +1,6 @@
+from tests.atest.utils import RuleAcceptance
+
+
+class TestRuleAcceptance(RuleAcceptance):
+    def test_rule(self):
+        self.check_rule(expected_file="expected_output.txt")

tests/atest/rules/misc/builtin_imports_not_sorted/expected_output.txt

@@ -0,0 +1,4 @@
+only_builtins.robot:4:12 [W] 0926 BuiltIn library import 'String' should be placed before 'XML'
+only_builtins.robot:8:12 [W] 0926 BuiltIn library import 'BuiltIn' should be placed before 'String'
+test.robot:3:12 [W] 0926 BuiltIn library import 'Collections' should be placed before 'DateTime'
+test.robot:6:12 [W] 0926 BuiltIn library import 'String' should be placed before 'XML'

tests/atest/rules/misc/builtin_imports_not_sorted/no_builtins.robot

@@ -0,0 +1,11 @@
+*** Settings ***
+Library    RequestsLibrary
+Resource   ..${/}Libs${/}MyResource.robot
+Library    OwnLib.py
+Variables    variables.py
+Test Template    Keyword
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/misc/builtin_imports_not_sorted/no_imports.robot

@@ -0,0 +1,9 @@
+*** Settings ***
+Resource   ..${/}Libs${/}MyResource.robot
+Variables    variables.py
+Test Template    Keyword
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/misc/builtin_imports_not_sorted/only_builtins.robot

@@ -0,0 +1,13 @@
+*** Settings ***
+Library    Collections
+Library    XML
+Library    String
+Resource   ..${/}Libs${/}MyResource.robot
+Variables    variables.py
+Test Template    Keyword
+Library    BuiltIn
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/misc/builtin_imports_not_sorted/test.robot

@@ -0,0 +1,16 @@
+*** Settings ***
+Library    DateTime
+Library    Collections
+Library    OperatingSystem
+Library    XML
+Library    String
+Library    RequestsLibrary
+Resource   ..${/}Libs${/}MyResource.robot
+Library    OwnLib.py
+Variables    variables.py
+Test Template    Keyword
+
+
+*** Keywords ***
+Keyword
+    No Operation

tests/atest/rules/misc/builtin_imports_not_sorted/test_rule.py

@@ -0,0 +1,6 @@
+from tests.atest.utils import RuleAcceptance
+
+
+class TestRuleAcceptance(RuleAcceptance):
+    def test_rule(self):
+        self.check_rule(expected_file="expected_output.txt")

tests/atest/utils/__init__.py

@@ -46,7 +46,14 @@ def load_expected_file(test_data, expected_file):
         return []
     expected = test_data / expected_file
     with open(expected, encoding="utf-8") as f:
-        return sorted([line.rstrip("\n").replace(r"${/}", os.path.sep) for line in f])
+        return sorted(
+            [
+                re.sub(r"(?<!\\)(?:\\\\)*(\${/})", os.path.sep.replace("\\", "\\\\"), line.rstrip("\n")).replace(
+                    "\\${/}", "${/}"
+                )
+                for line in f
+            ]
+        )
 
 
 def configure_robocop_with_rule(args, runner, rule, path, src_files: Optional[List], format):