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

Rule/no embedded keyword arguments #1120

Merged
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
68 changes: 68 additions & 0 deletions robocop/checkers/community_rules/keywords.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
""",
),
}


Expand Down Expand Up @@ -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,
)
Original file line number Diff line number Diff line change
@@ -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'
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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",
)
Loading