From 6d7ee3b5567edd876787c8ead5188045afbd88f6 Mon Sep 17 00:00:00 2001 From: szymonslodkowski Date: Thu, 6 Jun 2024 13:13:50 +0200 Subject: [PATCH] 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 | 100 ++++++++++++++++++ robocop/checkers/misc.py | 33 +++++- .../__init__.py | 0 .../expected_output.txt | 2 + .../no_builtins.robot | 11 ++ .../no_imports.robot | 9 ++ .../only_builtins.robot | 12 +++ .../non_builtin_imports_not_sorted/test.robot | 18 ++++ .../test_rule.py | 6 ++ .../resources_imports_not_sorted/__init__.py | 0 .../expected_output.txt | 2 + .../no_resources.robot | 9 ++ .../resources_imports_not_sorted/test.robot | 14 +++ .../resources_imports_not_sorted/test_rule.py | 6 ++ .../builtin_imports_not_sorted/__init__.py | 0 .../expected_output.txt | 4 + .../no_builtins.robot | 11 ++ .../no_imports.robot | 9 ++ .../only_builtins.robot | 13 +++ .../builtin_imports_not_sorted/test.robot | 16 +++ .../builtin_imports_not_sorted/test_rule.py | 6 ++ tests/atest/utils/__init__.py | 9 +- 22 files changed, 288 insertions(+), 2 deletions(-) create mode 100644 robocop/checkers/community_rules/misc.py create mode 100644 tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/__init__.py create mode 100644 tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/expected_output.txt create mode 100644 tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/no_builtins.robot create mode 100644 tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/no_imports.robot create mode 100644 tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/only_builtins.robot create mode 100644 tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/test.robot create mode 100644 tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/test_rule.py create mode 100644 tests/atest/rules/community_rules/misc/resources_imports_not_sorted/__init__.py create mode 100644 tests/atest/rules/community_rules/misc/resources_imports_not_sorted/expected_output.txt create mode 100644 tests/atest/rules/community_rules/misc/resources_imports_not_sorted/no_resources.robot create mode 100644 tests/atest/rules/community_rules/misc/resources_imports_not_sorted/test.robot create mode 100644 tests/atest/rules/community_rules/misc/resources_imports_not_sorted/test_rule.py create mode 100644 tests/atest/rules/misc/builtin_imports_not_sorted/__init__.py create mode 100644 tests/atest/rules/misc/builtin_imports_not_sorted/expected_output.txt create mode 100644 tests/atest/rules/misc/builtin_imports_not_sorted/no_builtins.robot create mode 100644 tests/atest/rules/misc/builtin_imports_not_sorted/no_imports.robot create mode 100644 tests/atest/rules/misc/builtin_imports_not_sorted/only_builtins.robot create mode 100644 tests/atest/rules/misc/builtin_imports_not_sorted/test.robot create mode 100644 tests/atest/rules/misc/builtin_imports_not_sorted/test_rule.py diff --git a/robocop/checkers/community_rules/misc.py b/robocop/checkers/community_rules/misc.py new file mode 100644 index 000000000..bb3314bb8 --- /dev/null +++ b/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) diff --git a/robocop/checkers/misc.py b/robocop/checkers/misc.py index e2da669a2..16b5e2587 100644 --- a/robocop/checkers/misc.py +++ b/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: diff --git a/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/__init__.py b/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/expected_output.txt b/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/expected_output.txt new file mode 100644 index 000000000..6b2ff6656 --- /dev/null +++ b/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' \ No newline at end of file diff --git a/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/no_builtins.robot b/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/no_builtins.robot new file mode 100644 index 000000000..a4d1cacf9 --- /dev/null +++ b/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 diff --git a/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/no_imports.robot b/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/no_imports.robot new file mode 100644 index 000000000..8b3fbab29 --- /dev/null +++ b/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 diff --git a/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/only_builtins.robot b/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/only_builtins.robot new file mode 100644 index 000000000..464a2bb4d --- /dev/null +++ b/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 diff --git a/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/test.robot b/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/test.robot new file mode 100644 index 000000000..cef9b9d9a --- /dev/null +++ b/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 diff --git a/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/test_rule.py b/tests/atest/rules/community_rules/misc/non_builtin_imports_not_sorted/test_rule.py new file mode 100644 index 000000000..88377fa01 --- /dev/null +++ b/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") diff --git a/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/__init__.py b/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/expected_output.txt b/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/expected_output.txt new file mode 100644 index 000000000..e8005bcba --- /dev/null +++ b/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' \ No newline at end of file diff --git a/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/no_resources.robot b/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/no_resources.robot new file mode 100644 index 000000000..f95b100af --- /dev/null +++ b/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 diff --git a/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/test.robot b/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/test.robot new file mode 100644 index 000000000..56846f078 --- /dev/null +++ b/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 diff --git a/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/test_rule.py b/tests/atest/rules/community_rules/misc/resources_imports_not_sorted/test_rule.py new file mode 100644 index 000000000..88377fa01 --- /dev/null +++ b/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") diff --git a/tests/atest/rules/misc/builtin_imports_not_sorted/__init__.py b/tests/atest/rules/misc/builtin_imports_not_sorted/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/atest/rules/misc/builtin_imports_not_sorted/expected_output.txt b/tests/atest/rules/misc/builtin_imports_not_sorted/expected_output.txt new file mode 100644 index 000000000..02290520f --- /dev/null +++ b/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' \ No newline at end of file diff --git a/tests/atest/rules/misc/builtin_imports_not_sorted/no_builtins.robot b/tests/atest/rules/misc/builtin_imports_not_sorted/no_builtins.robot new file mode 100644 index 000000000..a4d1cacf9 --- /dev/null +++ b/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 diff --git a/tests/atest/rules/misc/builtin_imports_not_sorted/no_imports.robot b/tests/atest/rules/misc/builtin_imports_not_sorted/no_imports.robot new file mode 100644 index 000000000..8b3fbab29 --- /dev/null +++ b/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 diff --git a/tests/atest/rules/misc/builtin_imports_not_sorted/only_builtins.robot b/tests/atest/rules/misc/builtin_imports_not_sorted/only_builtins.robot new file mode 100644 index 000000000..7f14f7d45 --- /dev/null +++ b/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 diff --git a/tests/atest/rules/misc/builtin_imports_not_sorted/test.robot b/tests/atest/rules/misc/builtin_imports_not_sorted/test.robot new file mode 100644 index 000000000..e8b8f139f --- /dev/null +++ b/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 diff --git a/tests/atest/rules/misc/builtin_imports_not_sorted/test_rule.py b/tests/atest/rules/misc/builtin_imports_not_sorted/test_rule.py new file mode 100644 index 000000000..88377fa01 --- /dev/null +++ b/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") diff --git a/tests/atest/utils/__init__.py b/tests/atest/utils/__init__.py index a42289089..f9c5510e0 100644 --- a/tests/atest/utils/__init__.py +++ b/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"(?