diff --git a/robocop/checkers/community_rules/keywords.py b/robocop/checkers/community_rules/keywords.py index 8c04bb1c..d3550e17 100644 --- a/robocop/checkers/community_rules/keywords.py +++ b/robocop/checkers/community_rules/keywords.py @@ -1,6 +1,7 @@ from typing import Set from robot.api import Token +from robot.model import Keyword from robot.utils.robottime import timestr_to_secs from robocop.checkers import VisitorChecker @@ -88,6 +89,54 @@ def comma_separated_list(value: str) -> Set[str]: """, ), + "10003": Rule( + rule_id="10003", + name="no-embedded-keyword-arguments", + msg="Not allowed embedded arguments {{ arguments }} found in keyword '{{ keyword }}'", + severity=RuleSeverity.WARNING, + added_in_version="5.5.0", + enabled=False, + docs=""" + Avoid using embedded arguments in keywords. + + When using embedded keyword arguments, you mix what you do (the keyword name) with the data + related to the action (the arguments). Mixing these two concepts can create + hard-to-understand code, which can result in mistakes in your test code. + + Embedded keyword arguments can also make it hard to understand which keyword you're using. + Sometimes even Robotframework gets confused when naming conflicts occur. There are ways to + fix naming conflicts, but this adds unnecessary complexity to your keyword. + + + To prevent these issues, use normal arguments instead. + + Example: + + Using a keyword with one embedded argument. Buying the drink and the size of the drink are + jumbled together. + + *** Test Cases *** + Prepare for an amazing movie + Buy a large soda + + *** Keywords *** + Buy a ${size} soda + # Do something wonderful + + Change the embedded argument to a normal argument. Now buying the drink is separate from the + size of the drink. In this approach, it's easier to see that you can change the size of your + drink. + + *** Test Cases *** + Prepare for an amazing movie + Buy a soda size=large + + *** Keywords *** + Buy a soda + [Arguments] ${size} + # Do something wonderful + """, + ), } @@ -178,3 +227,22 @@ def visit_Template(self, node): # noqa def visit_KeywordCall(self, node): # noqa self.check_keyword_naming_with_subkeywords(node, Token.KEYWORD) + + +class NoEmbeddedKeywordArgumentsChecker(VisitorChecker): + reports = ("no-embedded-keyword-arguments",) + + def visit_Keyword(self, node: Keyword): # noqa + name_token: Token = node.header.get_token(Token.KEYWORD_NAME) + variable_tokens = [t for t in name_token.tokenize_variables() if t.type == Token.VARIABLE] + + if len(variable_tokens) == 0: + return + + self.report( + "no-embedded-keyword-arguments", + node=name_token, + end_col=name_token.end_col_offset + 1, + arguments=", ".join([t.value for t in variable_tokens]), + keyword=name_token, + ) diff --git a/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/__init__.py b/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/expected_output.txt b/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/expected_output.txt new file mode 100644 index 00000000..c5854788 --- /dev/null +++ b/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/expected_output.txt @@ -0,0 +1,5 @@ +test.robot:12:1 [W] 10003 Not allowed embedded arguments ${count}, ${arguments} found in keyword 'Keyword with ${count} embedded ${arguments}' +test.robot:15:1 [W] 10003 Not allowed embedded arguments ${keyword}, ${with}, ${only}, ${embedded}, ${arguments} found in keyword '${keyword} ${with} ${only} ${embedded} ${arguments}' +test.robot:18:1 [W] 10003 Not allowed embedded arguments ${embedded} found in keyword 'Keyword with both ${embedded} and normal arguments' +test.robot:22:1 [W] 10003 Not allowed embedded arguments ${regex:(\d{4}-\d{2}-\d{2}|today)} found in keyword 'Keyword with embedded ${regex:(\d{4}-\d{2}-\d{2}|today)} argument' +test.robot:9:1 [W] 10003 Not allowed embedded arguments ${one} found in keyword 'Keyword with ${one} embedded argument' diff --git a/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/test.robot b/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/test.robot new file mode 100644 index 00000000..c6ea1399 --- /dev/null +++ b/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/test.robot @@ -0,0 +1,23 @@ +*** Keywords *** +Keyword without any arguments + No Operation + +Keyword with normal arguments + [Arguments] ${foo} ${bar} ${baz}=123 + No Operation + +Keyword with ${one} embedded argument + No Operation + +Keyword with ${count} embedded ${arguments} + No Operation + +${keyword} ${with} ${only} ${embedded} ${arguments} + No Operation + +Keyword with both ${embedded} and normal arguments + [Arguments] ${foo} + No Operation + +Keyword with embedded ${regex:(\d{4}-\d{2}-\d{2}|today)} argument + No Operation diff --git a/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/test_rule.py b/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/test_rule.py new file mode 100644 index 00000000..9f603a5a --- /dev/null +++ b/tests/atest/rules/community_rules/keywords/no_embedded_keyword_arguments/test_rule.py @@ -0,0 +1,9 @@ +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", + )