Skip to content

Commit 581ec82

Browse files
authored
feat: Improve performance of inconsistent-variable-name and possible-overwriting rules (#1646)
1 parent 4bb29c4 commit 581ec82

File tree

4 files changed

+20
-36
lines changed

4 files changed

+20
-36
lines changed

src/robocop/linter/rules/documentation.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,6 @@ def __init__(self) -> None:
145145
super().__init__()
146146

147147
def visit_Keyword(self, node: Keyword) -> None: # noqa: N802
148-
if node.name.lstrip().startswith("#"): # TODO: edge case for parsing with RF3?
149-
return
150148
self.check_if_docs_are_present(
151149
node, self.missing_doc_keyword, extend_disablers=True
152150
) # TODO: could be self.missing_doc_keyword.check_docs(node)

src/robocop/linter/rules/misc.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1209,6 +1209,7 @@ class UnusedVariablesChecker(VisitorChecker):
12091209
argument_overwritten_before_usage: arguments.ArgumentOverwrittenBeforeUsageRule
12101210
variable_overwritten_before_usage: variables.VariableOverwrittenBeforeUsageRule
12111211

1212+
_VARIABLE_START = set("$@&%")
12121213
_ESCAPED_VAR_PATTERN = re.compile(r"\$([A-Za-z_]\w*)")
12131214
_VARIABLE_NAME_PATTERN = re.compile(r"\w+")
12141215

@@ -1596,7 +1597,7 @@ def find_not_nested_variable(self, value: str, can_be_escaped: bool) -> None:
15961597
if pos == -1:
15971598
break
15981599
# must be preceded by an identifier char
1599-
if pos == 0 or value[pos - 1] not in identifiers:
1600+
if pos == 0 or value[pos - 1] not in self._VARIABLE_START:
16001601
i = pos + 1
16011602
continue
16021603
# found an identifier + '{' opening

src/robocop/linter/rules/naming.py

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
from robocop.linter.rules import Rule, RuleParam, RuleSeverity, VisitorChecker, deprecated, variables
1717
from robocop.linter.utils import misc as utils
1818
from robocop.parsing.run_keywords import iterate_keyword_names
19-
from robocop.parsing.variables import VariableMatches
2019
from robocop.version_handling import ROBOT_VERSION
2120

2221
if TYPE_CHECKING:
@@ -1181,6 +1180,8 @@ class SimilarVariableChecker(VisitorChecker):
11811180
possible_variable_overwriting: variables.PossibleVariableOverwritingRule
11821181
inconsistent_variable_name: variables.InconsistentVariableNameRule
11831182

1183+
_VAR_PATTERN = re.compile(r"[$@%&]\{([^{}]+)}")
1184+
11841185
def __init__(self) -> None:
11851186
self.assigned_variables = defaultdict(list)
11861187
self.parent_name = ""
@@ -1210,35 +1211,35 @@ def visit_KeywordCall(self, node) -> None: # noqa: N802
12101211
normalized = utils.normalize_robot_var_name(token.value)
12111212
assign_value = token.value # process assign last, cache for now
12121213
else:
1213-
self.find_not_nested_variable(token, token.value, is_var=False)
1214+
self.find_not_nested_variable(token)
12141215
if assign_value:
12151216
variable = search_variable(assign_value, ignore_errors=True)
12161217
self.assigned_variables[normalized].append(variable.base)
12171218
else:
12181219
for token in node.get_tokens(Token.ARGUMENT, Token.KEYWORD): # argument can be used in keyword name
1219-
self.find_not_nested_variable(token, token.value, is_var=False)
1220+
self.find_not_nested_variable(token)
12201221
tokens = node.get_tokens(Token.ASSIGN)
12211222
self.find_similar_variables(tokens, node)
12221223

12231224
def visit_Var(self, node) -> None: # noqa: N802
12241225
if node.errors: # for example invalid variable definition like $var}
12251226
return
12261227
for arg in node.get_tokens(Token.ARGUMENT):
1227-
self.find_not_nested_variable(arg, arg.value, is_var=False)
1228+
self.find_not_nested_variable(arg)
12281229
variable = node.get_token(Token.VARIABLE)
12291230
if variable:
12301231
self.find_similar_variables([variable], node, ignore_overwriting=not utils.is_var_scope_local(node))
12311232

12321233
def visit_If(self, node) -> None: # noqa: N802
12331234
for token in node.header.get_tokens(Token.ARGUMENT):
1234-
self.find_not_nested_variable(token, token.value, is_var=False)
1235+
self.find_not_nested_variable(token)
12351236
tokens = node.header.get_tokens(Token.ASSIGN)
12361237
self.find_similar_variables(tokens, node)
12371238
self.generic_visit(node)
12381239

12391240
def visit_While(self, node): # noqa: N802
12401241
for token in node.header.get_tokens(Token.ARGUMENT):
1241-
self.find_not_nested_variable(token, token.value, is_var=False)
1242+
self.find_not_nested_variable(token)
12421243
return self.generic_visit(node)
12431244

12441245
@staticmethod
@@ -1250,7 +1251,7 @@ def for_assign_vars(for_node) -> Iterable[str]:
12501251

12511252
def visit_For(self, node) -> None: # noqa: N802
12521253
for token in node.header.get_tokens(Token.ARGUMENT):
1253-
self.find_not_nested_variable(token, token.value, is_var=False)
1254+
self.find_not_nested_variable(token)
12541255
for var in self.for_assign_vars(node):
12551256
variable = search_variable(var, ignore_errors=True)
12561257
self.assigned_variables[utils.normalize_robot_var_name(var)].append(variable.base)
@@ -1260,12 +1261,14 @@ def visit_For(self, node) -> None: # noqa: N802
12601261

12611262
def visit_Return(self, node) -> None: # noqa: N802
12621263
for token in node.get_tokens(Token.ARGUMENT):
1263-
self.find_not_nested_variable(token, token.value, is_var=False)
1264+
self.find_not_nested_variable(token)
12641265

12651266
visit_ReturnStatement = visit_Teardown = visit_Timeout = visit_Return # noqa: N815
12661267

12671268
def parse_embedded_arguments(self, name_token) -> None:
12681269
"""Store embedded arguments from keyword name. Ignore embedded variables patterns (${var:pattern})."""
1270+
if "$" not in name_token.value:
1271+
return
12691272
try:
12701273
for token in name_token.tokenize_variables():
12711274
if token.type == Token.VARIABLE:
@@ -1301,37 +1304,15 @@ def check_inconsistent_naming(self, token, value: str, offset: int) -> None:
13011304
end_col=token.col_offset + offset + len(name) + 1,
13021305
)
13031306

1304-
def find_not_nested_variable(self, token, value, is_var: bool, offset: int = 0) -> None:
1307+
def find_not_nested_variable(self, token) -> None:
13051308
r"""
13061309
Find and process not nested variable.
13071310
13081311
Search `value` string until there is ${variable} without other variables inside.
13091312
Unescaped escaped syntax ($var or \\${var}) is ignored.
1310-
Offset is to determine position of the variable in the string.
1311-
For example 'This is ${var}' contains ${var} at 8th position.
13121313
"""
1313-
try:
1314-
variables = list(VariableMatches(value))
1315-
except VariableError: # for example ${variable which wasn't closed properly
1316-
return
1317-
if not variables:
1318-
if is_var:
1319-
self.check_inconsistent_naming(token, value, offset)
1320-
return
1321-
if is_var:
1322-
offset += 2 # handle ${var_${var}} case
1323-
for match in variables:
1324-
if match.before:
1325-
offset += len(match.before)
1326-
# match = search_variable(variable, ignore_errors=True)
1327-
if match.base and match.base.startswith("{") and match.base.endswith("}"): # inline val
1328-
self.find_not_nested_variable(token, match.base[1:-1].strip(), is_var=False, offset=offset + 1)
1329-
else:
1330-
self.find_not_nested_variable(token, match.base, is_var=True, offset=offset)
1331-
offset += len(match.match)
1332-
for item in match.items:
1333-
self.find_not_nested_variable(token, item, is_var=False, offset=offset)
1334-
offset += len(item)
1314+
for match in self._VAR_PATTERN.finditer(token.value):
1315+
self.check_inconsistent_naming(token, match.group(1), match.start(1) - 2)
13351316

13361317
def visit_vars_and_find_similar(self, node) -> None:
13371318
"""

tests/linter/rules/variables/unused_variable/test.robot

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,3 +278,7 @@ Not used mixed
278278
${var3} ${var4} Set Variable 2
279279
IF "${var}" == $var2 No Operation
280280
IF "${var4}" == $var3 No Operation
281+
282+
Nested used
283+
${var} Set Variable 1
284+
Log ${some_${var}} # even if variable unnesting fails, we use tokenizer

0 commit comments

Comments
 (0)