Skip to content

Commit ddaf044

Browse files
pekkaklarckpekkaklarck
committed
Fix handling invalid UK argspec with types
Also some refactoring: - Parse arguments to VariableMatch objects just once. - Reorder methods. Fixes #5443.
1 parent fd87e86 commit ddaf044

File tree

5 files changed

+156
-75
lines changed

5 files changed

+156
-75
lines changed

‎atest/robot/keywords/user_keyword_arguments.robot

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -85,22 +85,27 @@ Caller does not see modifications to varargs
8585

8686
Invalid Arguments Spec
8787
[Template] Verify Invalid Argument Spec
88-
0 338 Invalid argument syntax Invalid argument syntax 'no deco'.
89-
1 342 Non-default after defaults Non-default argument after default arguments.
90-
2 346 Default with varargs Only normal arguments accept default values, list arguments like '\@{varargs}' do not.
91-
3 350 Default with kwargs Only normal arguments accept default values, dictionary arguments like '\&{kwargs}' do not.
92-
4 354 Kwargs not last Only last argument can be kwargs.
93-
5 358 Multiple errors Multiple errors:
94-
... - Invalid argument syntax 'invalid'.
95-
... - Non-default argument after default arguments.
96-
... - Cannot have multiple varargs.
97-
... - Only last argument can be kwargs.
88+
0 Invalid argument syntax Invalid argument syntax 'no deco'.
89+
1 Non-default after default Non-default argument after default arguments.
90+
2 Non-default after default w/ types Non-default argument after default arguments.
91+
3 Default with varargs Only normal arguments accept default values, list arguments like '\@{varargs}' do not.
92+
4 Default with kwargs Only normal arguments accept default values, dictionary arguments like '\&{kwargs}' do not.
93+
5 Multiple varargs Cannot have multiple varargs.
94+
6 Multiple varargs w/ types Cannot have multiple varargs.
95+
7 Kwargs not last Only last argument can be kwargs.
96+
8 Kwargs not last w/ types Only last argument can be kwargs.
97+
9 Multiple errors Multiple errors:
98+
... - Invalid argument syntax 'invalid'.
99+
... - Non-default argument after default arguments.
100+
... - Cannot have multiple varargs.
101+
... - Only last argument can be kwargs.
98102

99103
*** Keywords ***
100104
Verify Invalid Argument Spec
101-
[Arguments] ${index} ${lineno} ${name} @{error}
105+
[Arguments] ${index} ${name} @{error}
102106
Check Test Case ${TEST NAME} - ${name}
103-
${error} = Catenate SEPARATOR=\n @{error}
107+
VAR ${error} @{error} separator=\n
108+
VAR ${lineno} ${{358 + ${index} * 4}}
104109
Error In File ${index} keywords/user_keyword_arguments.robot ${lineno}
105110
... Creating keyword '${name}' failed:
106111
... Invalid argument specification: ${error}

‎atest/testdata/keywords/user_keyword_arguments.robot

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,15 @@ Invalid Arguments Spec - Invalid argument syntax
199199
... Invalid argument specification: Invalid argument syntax 'no deco'.
200200
Invalid argument syntax
201201

202-
Invalid Arguments Spec - Non-default after defaults
202+
Invalid Arguments Spec - Non-default after default
203203
[Documentation] FAIL
204204
... Invalid argument specification: Non-default argument after default arguments.
205-
Non-default after defaults what ever args=accepted
205+
Non-default after default what ever args=accepted
206+
207+
Invalid Arguments Spec - Non-default after default w/ types
208+
[Documentation] FAIL
209+
... Invalid argument specification: Non-default argument after default arguments.
210+
Non-default after default w/ types
206211

207212
Invalid Arguments Spec - Default with varargs
208213
[Documentation] FAIL
@@ -214,11 +219,26 @@ Invalid Arguments Spec - Default with kwargs
214219
... Invalid argument specification: Only normal arguments accept default values, dictionary arguments like '&{kwargs}' do not.
215220
Default with kwargs
216221

222+
Invalid Arguments Spec - Multiple varargs
223+
[Documentation] FAIL
224+
... Invalid argument specification: Cannot have multiple varargs.
225+
Multiple varargs
226+
227+
Invalid Arguments Spec - Multiple varargs w/ types
228+
[Documentation] FAIL
229+
... Invalid argument specification: Cannot have multiple varargs.
230+
Multiple varargs w/ types
231+
217232
Invalid Arguments Spec - Kwargs not last
218233
[Documentation] FAIL
219234
... Invalid argument specification: Only last argument can be kwargs.
220235
Kwargs not last
221236

237+
Invalid Arguments Spec - Kwargs not last w/ types
238+
[Documentation] FAIL
239+
... Invalid argument specification: Only last argument can be kwargs.
240+
Kwargs not last w/ types
241+
222242
Invalid Arguments Spec - Multiple errors
223243
[Documentation] FAIL
224244
... Invalid argument specification: Multiple errors:
@@ -338,8 +358,12 @@ Invalid argument syntax
338358
[Arguments] no deco
339359
Fail Not executed
340360

341-
Non-default after defaults
342-
[Arguments] ${named}=value ${positional}
361+
Non-default after default
362+
[Arguments] ${with}=value ${without}
363+
Fail Not executed
364+
365+
Non-default after default w/ types
366+
[Arguments] ${with: str}=value ${without: int}
343367
Fail Not executed
344368

345369
Default with varargs
@@ -350,10 +374,22 @@ Default with kwargs
350374
[Arguments] &{kwargs}=invalid
351375
Fail Not executed
352376

377+
Multiple varargs
378+
[Arguments] @{v} @{w}
379+
Fail Not executed
380+
381+
Multiple varargs w/ types
382+
[Arguments] @{v: int} ${kwo} @{w: int}
383+
Fail Not executed
384+
353385
Kwargs not last
354386
[Arguments] &{kwargs} ${positional}
355387
Fail Not executed
356388

389+
Kwargs not last w/ types
390+
[Arguments] &{k1: int} ${k2: str}
391+
Fail Not executed
392+
357393
Multiple errors
358394
[Arguments] invalid ${optional}=default ${required} @{too} @{many} &{kwargs} ${x}
359395
Fail Not executed

‎src/robot/running/arguments/argumentparser.py

Lines changed: 60 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,6 @@ def parse(self, arguments, name=None):
135135
target = positional_or_named
136136
for arg in arguments:
137137
arg, default = self._validate_arg(arg)
138-
arg, type_ = self._split_type(arg)
139-
if type_:
140-
types[self._format_arg(arg)] = type_
141138
if var_named:
142139
self._report_error("Only last argument can be kwargs.")
143140
elif self._is_positional_only_separator(arg):
@@ -151,21 +148,25 @@ def parse(self, arguments, name=None):
151148
target = positional_or_named = []
152149
positional_only_separator_seen = True
153150
elif default is not NOT_SET:
151+
self._parse_type(arg, types)
154152
arg = self._format_arg(arg)
155153
target.append(arg)
156154
defaults[arg] = default
157155
elif self._is_var_named(arg):
156+
self._parse_type(arg, types)
158157
var_named = self._format_var_named(arg)
159158
elif self._is_var_positional(arg):
160159
if named_only_separator_seen:
161160
self._report_error("Cannot have multiple varargs.")
162-
if not self._is_named_only_separator(arg):
161+
elif not self._is_named_only_separator(arg):
162+
self._parse_type(arg, types)
163163
var_positional = self._format_var_positional(arg)
164164
named_only_separator_seen = True
165165
target = named_only
166-
elif defaults and not named_only_separator_seen:
167-
self._report_error("Non-default argument after default arguments.")
168166
else:
167+
if defaults and not named_only_separator_seen:
168+
self._report_error("Non-default argument after default arguments.")
169+
self._parse_type(arg, types)
169170
arg = self._format_arg(arg)
170171
target.append(arg)
171172
return ArgumentSpec(
@@ -185,11 +186,11 @@ def _validate_arg(self, arg):
185186
raise NotImplementedError
186187

187188
@abstractmethod
188-
def _is_var_named(self, arg):
189+
def _is_var_positional(self, arg):
189190
raise NotImplementedError
190191

191192
@abstractmethod
192-
def _format_var_named(self, kwargs):
193+
def _is_var_named(self, arg):
193194
raise NotImplementedError
194195

195196
@abstractmethod
@@ -201,24 +202,20 @@ def _is_named_only_separator(self, arg):
201202
raise NotImplementedError
202203

203204
@abstractmethod
204-
def _is_var_positional(self, arg):
205+
def _format_arg(self, arg):
205206
raise NotImplementedError
206207

207208
@abstractmethod
208-
def _format_var_positional(self, varargs):
209+
def _format_var_named(self, arg):
209210
raise NotImplementedError
210211

211-
def _format_arg(self, arg):
212-
return arg
213-
214-
def _add_arg(self, spec, arg, named_only=False):
215-
arg = self._format_arg(arg)
216-
target = spec.positional_or_named if not named_only else spec.named_only
217-
target.append(arg)
218-
return arg
212+
@abstractmethod
213+
def _format_var_positional(self, arg):
214+
raise NotImplementedError
219215

220-
def _split_type(self, arg):
221-
return arg, None
216+
@abstractmethod
217+
def _parse_type(self, arg, types):
218+
raise NotImplementedError
222219

223220

224221
class DynamicArgumentParser(ArgumentSpecParser):
@@ -242,68 +239,77 @@ def _is_valid_tuple(self, arg):
242239
and not (arg[0].startswith("*") and len(arg) == 2)
243240
)
244241

242+
def _is_var_positional(self, arg):
243+
return arg[:1] == "*"
244+
245245
def _is_var_named(self, arg):
246246
return arg[:2] == "**"
247247

248-
def _format_var_named(self, kwargs):
249-
return kwargs[2:]
250-
251-
def _is_var_positional(self, arg):
252-
return arg and arg[0] == "*"
253-
254248
def _is_positional_only_separator(self, arg):
255249
return arg == "/"
256250

257251
def _is_named_only_separator(self, arg):
258252
return arg == "*"
259253

260-
def _format_var_positional(self, varargs):
261-
return varargs[1:]
254+
def _format_arg(self, arg):
255+
return arg
256+
257+
def _format_var_positional(self, arg):
258+
return arg[1:]
259+
260+
def _format_var_named(self, arg):
261+
return arg[2:]
262+
263+
def _parse_type(self, arg, types):
264+
pass
262265

263266

264267
class UserKeywordArgumentParser(ArgumentSpecParser):
265268

266269
def _validate_arg(self, arg):
267270
arg, default = split_from_equals(arg)
268-
if not (is_assign(arg) or arg == "@{}"):
271+
match = search_variable(arg, parse_type=True, ignore_errors=True)
272+
if not (match.is_assign() or self._is_named_only_separator(match)):
269273
self._report_error(f"Invalid argument syntax '{arg}'.")
270-
return None, NOT_SET
271-
if default is None:
272-
return arg, NOT_SET
273-
if not is_scalar_assign(arg):
274-
typ = "list" if arg[0] == "@" else "dictionary"
274+
match = search_variable("")
275+
default = NOT_SET
276+
elif default is None:
277+
default = NOT_SET
278+
elif arg[0] != '$':
279+
kind = "list" if arg[0] == "@" else "dictionary"
275280
self._report_error(
276281
f"Only normal arguments accept default values, "
277-
f"{typ} arguments like '{arg}' do not."
282+
f"{kind} arguments like '{arg}' do not."
278283
)
279-
return arg, default
284+
default = NOT_SET
285+
return match, default
280286

281-
def _is_var_named(self, arg):
282-
return arg and arg[0] == "&"
283-
284-
def _format_var_named(self, kwargs):
285-
return kwargs[2:-1]
287+
def _is_var_positional(self, match):
288+
return match.identifier == "@"
286289

287-
def _is_var_positional(self, arg):
288-
return arg and arg[0] == "@"
290+
def _is_var_named(self, match):
291+
return match.identifier == "&"
289292

290293
def _is_positional_only_separator(self, arg):
291294
return False
292295

293-
def _is_named_only_separator(self, arg):
294-
return arg == "@{}"
296+
def _is_named_only_separator(self, match):
297+
return match.identifier == "@" and not match.base
295298

296-
def _format_var_positional(self, varargs):
297-
return varargs[2:-1]
299+
def _format_arg(self, match):
300+
return match.base
298301

299-
def _format_arg(self, arg):
300-
return arg[2:-1] if arg else ""
302+
def _format_var_named(self, match):
303+
return match.base
304+
305+
def _format_var_positional(self, match):
306+
return match.base
301307

302-
def _split_type(self, arg):
303-
match = search_variable(arg, parse_type=True)
308+
def _parse_type(self, match, types):
304309
try:
305310
info = TypeInfo.from_variable(match, handle_list_and_dict=False)
306311
except DataError as err:
307-
info = None
308-
self._report_error(f"Invalid argument '{arg}': {err}")
309-
return match.name, info
312+
self._report_error(f"Invalid argument '{match}': {err}")
313+
else:
314+
if info:
315+
types[match.base] = info

‎src/robot/running/builder/builders.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ def _build_suite_file(self, structure: SuiteFile):
276276
if not suite.tests:
277277
LOGGER.info(f"Data source '{source}' has no tests or tasks.")
278278
except DataError as err:
279-
raise DataError(f"Parsing '{source}' failed: {err.message}")
279+
raise DataError(f"Parsing '{source}' failed: {err.message}") from err
280280
return suite
281281

282282
def _build_suite_directory(self, structure: SuiteDirectory):

‎utest/parsing/test_model.py

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2018,7 +2018,7 @@ def test_invalid_arg_spec(self):
20182018
*** Keywords ***
20192019
Invalid
20202020
[Arguments] ooops ${optional}=default ${required}
2021-
... @{too} @{many} &{notlast} ${x}
2021+
... @{too} @{} @{many} &{notlast} ${x}
20222022
Keyword
20232023
"""
20242024
expected = Keyword(
@@ -2031,12 +2031,46 @@ def test_invalid_arg_spec(self):
20312031
Token(Token.ARGUMENT, "${optional}=default", 3, 28),
20322032
Token(Token.ARGUMENT, "${required}", 3, 51),
20332033
Token(Token.ARGUMENT, "@{too}", 4, 11),
2034-
Token(Token.ARGUMENT, "@{many}", 4, 21),
2035-
Token(Token.ARGUMENT, "&{notlast}", 4, 32),
2036-
Token(Token.ARGUMENT, "${x}", 4, 46),
2034+
Token(Token.ARGUMENT, "@{}", 4, 21),
2035+
Token(Token.ARGUMENT, "@{many}", 4, 28),
2036+
Token(Token.ARGUMENT, "&{notlast}", 4, 39),
2037+
Token(Token.ARGUMENT, "${x}", 4, 53),
20372038
],
20382039
errors=(
20392040
"Invalid argument syntax 'ooops'.",
2041+
"Non-default argument after default arguments.",
2042+
"Cannot have multiple varargs.",
2043+
"Cannot have multiple varargs.",
2044+
"Only last argument can be kwargs.",
2045+
),
2046+
),
2047+
KeywordCall(tokens=[Token(Token.KEYWORD, "Keyword", 5, 4)]),
2048+
],
2049+
)
2050+
get_and_assert_model(data, expected, depth=1)
2051+
2052+
def test_invalid_arg_spec_with_types(self):
2053+
data = """
2054+
*** Keywords ***
2055+
Invalid
2056+
[Arguments] ${optional: str}=default ${required: bool}
2057+
... @{too: int} @{many: float} &{not: bool} &{last: bool}
2058+
Keyword
2059+
"""
2060+
expected = Keyword(
2061+
header=KeywordName(tokens=[Token(Token.KEYWORD_NAME, "Invalid", 2, 0)]),
2062+
body=[
2063+
Arguments(
2064+
tokens=[
2065+
Token(Token.ARGUMENTS, "[Arguments]", 3, 4),
2066+
Token(Token.ARGUMENT, "${optional: str}=default", 3, 19),
2067+
Token(Token.ARGUMENT, "${required: bool}", 3, 47),
2068+
Token(Token.ARGUMENT, "@{too: int}", 4, 11),
2069+
Token(Token.ARGUMENT, "@{many: float}", 4, 26),
2070+
Token(Token.ARGUMENT, "&{not: bool}", 4, 44),
2071+
Token(Token.ARGUMENT, "&{last: bool}", 4, 60),
2072+
],
2073+
errors=(
20402074
"Non-default argument after default arguments.",
20412075
"Cannot have multiple varargs.",
20422076
"Only last argument can be kwargs.",

0 commit comments

Comments
 (0)