Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Jun 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions robocop/checkers/community_rules/misc.py
Original file line number Diff line number Diff line change
@@ -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

Check warning on line 99 in robocop/checkers/community_rules/misc.py

View check run for this annotation

Codecov / codecov/patch

robocop/checkers/community_rules/misc.py#L99

Added line #L99 was not covered by tests
self.resources.append(node)
33 changes: 32 additions & 1 deletion robocop/checkers/misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
bhirsz marked this conversation as resolved.
Show resolved Hide resolved
added_in_version="5.2.0",
docs="""
Example of rule violation::

*** Settings ***
Library OperatingSystem
Library Collections # BuiltIn libraries imported not in alphabetical order

""",
),
}


Expand Down Expand Up @@ -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 = []
Expand All @@ -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:
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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'
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
*** Settings ***
Library RequestsLibrary
Resource ..${/}Libs${/}MyResource.robot
Library OwnLib.py
Variables variables.py
Test Template Keyword


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


*** Keywords ***
Keyword
No Operation
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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")
Original file line number Diff line number Diff line change
@@ -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'
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
*** Settings ***
Library RequestsLibrary
Variables variables.py
Test Template Keyword


*** Keywords ***
Keyword
No Operation
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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")
Empty file.
Original file line number Diff line number Diff line change
@@ -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'
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
*** Settings ***
Library RequestsLibrary
Resource ..${/}Libs${/}MyResource.robot
Library OwnLib.py
Variables variables.py
Test Template Keyword


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


*** Keywords ***
Keyword
No Operation
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions tests/atest/rules/misc/builtin_imports_not_sorted/test.robot
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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")
9 changes: 8 additions & 1 deletion tests/atest/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
Loading