Skip to content

Commit 0ced63a

Browse files
pekkaklarckpekkaklarck
committed
Require variables to have matching curly braces
This implementation is pretty ugly and VariableSplitter in general needs refactoring. That can be done separately. Fixes robotframework#3288.
1 parent 970e56a commit 0ced63a

File tree

7 files changed

+107
-61
lines changed

7 files changed

+107
-61
lines changed

‎atest/testdata/keywords/embedded_arguments.robot‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,8 @@ Custom Regexp With Curly Braces
6767
Custom Regexp With Escape Chars
6868
Custom Regexp With Escape Chars e.g. \\, \\\\ and c:\\temp\\test.txt
6969
Custom Regexp With \\}
70+
Custom Regexp With \\{
71+
Custom Regexp With \\{}
7072

7173
Grouping Custom Regexp
7274
${matches} = Grouping Custom Regexp(erts)
@@ -228,14 +230,14 @@ I want ${integer:whatever} and ${string:everwhat} as variables
228230
Should Be Equal ${integer} ${42}
229231
Should Be Equal ${string} 42
230232

231-
Today is ${date:\d{4\}-\d{2\}-\d{2\}}
233+
Today is ${date:\d{4}-\d{2}-\d{2}}
232234
Should Be Equal ${date} 2011-06-21
233235

234-
Today is ${day1:\w{6,9\}} and tomorrow is ${day2:\w{6,9\}}
236+
Today is ${day1:\w{6,9}} and tomorrow is ${day2:\w{6,9}}
235237
Should Be Equal ${day1} Tuesday
236238
Should Be Equal ${day2} Wednesday
237239

238-
Literal ${Curly:{} Brace
240+
Literal ${Curly:\{} Brace
239241
Should Be Equal ${Curly} {
240242

241243
Literal ${Curly:\}} Brace
@@ -249,6 +251,12 @@ Custom Regexp With Escape Chars e.g. ${1E:\\\\}, ${2E:\\\\\\\\} and ${PATH:c:\\\
249251
Custom Regexp With ${pattern:\\\\\}}
250252
Should Be Equal ${pattern} \\}
251253

254+
Custom Regexp With ${pattern:\\\\\{}
255+
Should Be Equal ${pattern} \\{
256+
257+
Custom Regexp With ${pattern:\\\\{}}
258+
Should Be Equal ${pattern} \\{}
259+
252260
Grouping ${x:Cu(st|ts)(om)?} ${y:Regexp\(?erts\)?}
253261
[Return] ${x}-${y}
254262

‎atest/testdata/keywords/embedded_arguments_library_keywords.robot‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ Custom Regexp With Curly Braces
5959
Custom Regexp With Escape Chars
6060
Custom Regexp With Escape Chars e.g. \\, \\\\ and c:\\temp\\test.txt
6161
Custom Regexp With \\}
62+
Custom Regexp With \\{
63+
Custom Regexp With \\{}
6264

6365
Grouping Custom Regexp
6466
${matches} = Grouping Custom Regexp(erts)

‎atest/testdata/keywords/resources/embedded_args_in_lk_1.py‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,24 +65,24 @@ def i_want_as_variables(integer, string):
6565
should_be_equal(string, "42")
6666

6767

68-
@keyword(name=r"Today is ${date:\d{4\}-\d{2\}-\d{2\}}")
68+
@keyword(name=r"Today is ${date:\d{4}-\d{2}-\d{2}}")
6969
def today_is(date):
7070
should_be_equal(date, "2011-06-21")
7171

7272

73-
@keyword(name=r"Today is ${day1:\w{6,9\}} and tomorrow is ${day2:\w{6,9\}}")
73+
@keyword(name=r"Today is ${day1:\w\{6,9\}} and tomorrow is ${day2:\w{6,9}}")
7474
def today_is_and_tomorrow_is(day1, day2):
7575
should_be_equal(day1, "Tuesday")
7676
should_be_equal(day2, "Wednesday")
7777

7878

79-
@keyword(name="Literal ${Curly:{} Brace")
80-
def literal_brace(curly):
79+
@keyword(name=r"Literal ${Curly:\{} Brace")
80+
def literal_opening_curly_brace(curly):
8181
should_be_equal(curly, "{")
8282

8383

8484
@keyword(name="Literal ${Curly:\}} Brace")
85-
def literal_escaped_brace(curly):
85+
def literal_closing_curly_brace(curly):
8686
should_be_equal(curly, "}")
8787

8888

@@ -94,9 +94,19 @@ def custom_regexp_with_escape_chars(e1, e2, path):
9494
should_be_equal(path, "c:\\temp\\test.txt")
9595

9696

97-
@keyword(name=r"Custom Regexp With ${pattern:\\\\\}}")
98-
def custom_regexp_with(pattern):
99-
should_be_equal(pattern, "\\}")
97+
@keyword(name=r"Custom Regexp With ${escapes:\\\\\}}")
98+
def custom_regexp_with_escapes_1(escapes):
99+
should_be_equal(escapes, r'\}')
100+
101+
102+
@keyword(name=r"Custom Regexp With ${escapes:\\\\\{}")
103+
def custom_regexp_with_escapes_2(escapes):
104+
should_be_equal(escapes, r'\{')
105+
106+
107+
@keyword(name=r"Custom Regexp With ${escapes:\\\\{}}")
108+
def custom_regexp_with_escapes_3(escapes):
109+
should_be_equal(escapes, r'\{}')
100110

101111

102112
@keyword(name=r"Grouping ${x:Cu(st|ts)(om)?} ${y:Regexp\(?erts\)?}")

‎doc/userguide/src/CreatingTestData/CreatingUserKeywords.rst‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,7 @@ __ `Ignoring Given/When/Then/And/But prefixes`_
528528

529529
Using custom regular expressions
530530
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
531+
531532
When keywords with embedded arguments are called, the values are
532533
matched internally using `regular expressions`__
533534
(regexps for short). The default logic goes so that every argument in
@@ -582,10 +583,10 @@ regular expressions is illustrated by the examples below.
582583
I execute "${cmd}" with "${opts}"
583584
Run Process ${cmd} ${opts} shell=True
584585

585-
I type ${a:\d+} ${operator:[+-]} ${b:\d+}
586-
Calculate ${a} ${operator} ${b}
586+
I type ${num1:\d+} ${operator:[+-]} ${num2:\d+}
587+
Calculate ${num1} ${operator} ${num2}
587588

588-
Today is ${date:\d{4\}-\d{2\}-\d{2\}}
589+
Today is ${date:\d{4}-\d{2}-\d{2}}
589590
Log ${date}
590591

591592
In the above example keyword :name:`I execute "ls" with "-lh"` matches
@@ -614,23 +615,22 @@ errors`__.
614615
Escaping special characters
615616
'''''''''''''''''''''''''''
616617

617-
There are some special characters that need to be escaped when used in
618-
the custom embedded arguments regexp. First of all, possible closing
619-
curly braces (`}`) in the pattern need to be escaped with a single backslash
620-
(`\}`) because otherwise the argument would end already there. This is
621-
illustrated in the previous example with keyword
622-
:name:`Today is ${date:\\d{4\\}-\\d{2\\}-\\d{2\\}}`.
623-
624-
Backslash (:codesc:`\\`) is a special character in Python regular
625-
expression syntax and thus needs to be escaped if you want to have a
626-
literal backslash character. The safest escape sequence in this case
627-
is four backslashes (`\\\\`) but, depending on the next
628-
character, also two backslashes may be enough.
629-
630-
Notice also that keyword names and possible embedded arguments in them
631-
should *not* be escaped using the normal `test data escaping
632-
rules`__. This means that, for example, backslashes in expressions
633-
like `${name:\w+}` should not be escaped.
618+
Regular expressions use the backslash character (:codesc:`\\`) heavily both
619+
to escape characters that have a special meaning in regexps (e.g. `\$`) and
620+
to form special sequences (e.g. `\d`). Typically in Robot Framework data
621+
backslash characters `need to be escaped`__ with another backslash, but
622+
that is not required in this context. If there is a need to have a literal
623+
backslash in the pattern, then the backslash must be escaped.
624+
625+
Possible lone opening and closing curly braces in the pattern must be escaped
626+
like `${open:\}}` and `${close:\{}`. If there are matching braces like
627+
`${two digits:\d{2}}`, escaping is not needed. Escaping only opening or
628+
closing brace is not allowed.
629+
630+
.. warning:: Prior to Robot Framework 3.2 it was mandatory to escape all
631+
closing curly braces in the pattern like `${two digits:\d{2\}}`.
632+
This syntax is unfortunately not supported by Robot Framework 3.2
633+
or newer and keywords using it must be updated when upgrading.
634634

635635
Using variables with custom embedded argument regular expressions
636636
'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ def __nonzero__(self):
3636
class EmbeddedArgumentParser(object):
3737
_regexp_extension = re.compile(r'(?<!\\)\(\?.+\)')
3838
_regexp_group_start = re.compile(r'(?<!\\)\((.*?)\)')
39+
_escaped_curly = re.compile(r'(\\+)([{}])')
3940
_regexp_group_escape = r'(?:\1)'
4041
_default_pattern = '.*?'
4142
_variable_pattern = r'\$\{[^\}]+\}'
@@ -60,7 +61,7 @@ def _get_name_and_pattern(self, name):
6061
def _format_custom_regexp(self, pattern):
6162
for formatter in (self._regexp_extensions_are_not_allowed,
6263
self._make_groups_non_capturing,
63-
self._unescape_closing_curly,
64+
self._unescape_curly_braces,
6465
self._add_automatic_variable_pattern):
6566
pattern = formatter(pattern)
6667
return pattern
@@ -74,8 +75,11 @@ def _regexp_extensions_are_not_allowed(self, pattern):
7475
def _make_groups_non_capturing(self, pattern):
7576
return self._regexp_group_start.sub(self._regexp_group_escape, pattern)
7677

77-
def _unescape_closing_curly(self, pattern):
78-
return pattern.replace('\\}', '}')
78+
def _unescape_curly_braces(self, pattern):
79+
def unescaper(match):
80+
backslashes = len(match.group(1))
81+
return '\\' * (backslashes // 2 * 2) + match.group(2)
82+
return self._escaped_curly.sub(unescaper, pattern)
7983

8084
def _add_automatic_variable_pattern(self, pattern):
8185
return '%s|%s' % (pattern, self._variable_pattern)

‎src/robot/variables/splitter.py‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ def _finalize(self):
6262
if self.items:
6363
self.end += len(''.join(self.items)) + 2 * len(self.items)
6464

65-
def _split(self, string):
65+
def _split(self, string, start=0):
6666
start_index, max_index = self._find_variable(string)
6767
if start_index == -1:
6868
return False
69-
self.start = start_index
69+
self.start = start_index + start
7070
self._open_curly = 1
7171
self._variable_chars = [string[start_index], '{']
7272
self._item_chars = []
@@ -77,6 +77,14 @@ def _split(self, string):
7777
state = state(char, index)
7878
if state is None or (index == max_index and not self._scanning_item(state)):
7979
break
80+
if self._open_curly != 0:
81+
self.identifier = None
82+
self.base = None
83+
self.items = []
84+
self.start = -1
85+
self.end = -1
86+
self._may_have_internal_variables = False
87+
return self._split(string[start_index:], start_index)
8088
return True
8189

8290
def _scanning_item(self, state):
@@ -121,6 +129,8 @@ def _variable_state(self, char, index):
121129
if not self._can_have_item():
122130
return None
123131
return self._waiting_item_state
132+
elif char == '{' and not self._is_escaped(self._string, index):
133+
self._open_curly += 1
124134
elif char in self._identifiers:
125135
return self._internal_variable_start_state
126136
return self._variable_state

‎utest/variables/test_variablesplitter.py‎

Lines changed: 37 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -25,33 +25,45 @@ def test_backslashes(self):
2525
self._test(inp)
2626

2727
def test_one_var(self):
28-
self._test('${hello}', '${hello}', 0)
29-
self._test('1 @{hello} more', '@{hello}', 2)
30-
self._test('*{hi}}', '*{hi}', 0)
31-
self._test('{%{{hi}}', '%{{hi}', 1)
32-
self._test('-= ${} =-', '${}', 3)
33-
# In this case splitter thinks there are internal but there aren't.
34-
# Better check would probably spent more time than that is saved when
35-
# variable base is processed again in this special case.
36-
self._test('%{hi%{u}', '%{hi%{u}', 0, internal=True)
37-
38-
def test_escape_internal_closing_curly(self):
39-
self._test(r'${embed:\d{2\}}', '${embed:\d{2\}}')
40-
self._test(r'{}{${e:\d\{4\}-\d{2\}-\d{2\}}}}',
41-
'${e:\d\{4\}-\d{2\}-\d{2\}}', start=3)
42-
self._test(r'$&{\{\}{\}\\}{}', r'&{\{\}{\}\\}', start=1)
43-
self._test(r'${&{\}{\\\\}\\}}{}', r'${&{\}{\\\\}\\}', internal=True)
44-
45-
def test_no_unescaped_internal_closing_curly(self):
28+
self._test('${hello}', '${hello}')
29+
self._test('1 @{hello} more', '@{hello}', start=2)
30+
self._test('*{hi}}', '*{hi}')
31+
self._test('{%{{hi}}', '%{{hi}}', start=1)
32+
self._test('-= ${} =-', '${}', start=3)
33+
self._test('%{hi%{u}', '%{u}', start=4)
34+
35+
def test_escape_internal_curlys(self):
36+
self._test(r'${embed:\d\{2\}}', '${embed:\d\{2\}}')
37+
self._test(r'{}{${e:\d\{4\}-\d\{2\}-\d\{2\}}}}',
38+
'${e:\d\{4\}-\d\{2\}-\d\{2\}}', start=3)
39+
self._test(r'$&{\{\}\{\}\\}{}', r'&{\{\}\{\}\\}', start=1)
40+
self._test(r'${&{\}\{\\\\}\\}}{}', r'${&{\}\{\\\\}\\}', internal=True)
41+
42+
def test_matching_internal_curlys_dont_need_to_be_escaped(self):
43+
self._test(r'${embed:\d{2}}', r'${embed:\d{2}}')
44+
self._test(r'{}{${e:\d{4}-\d{2}-\d{2}}}}',
45+
r'${e:\d{4}-\d{2}-\d{2}}', start=3)
46+
self._test(r'$&{{}{}\\}{}', r'&{{}{}\\}', start=1)
47+
self._test(r'${&{{\\\\}\\}}{}}', r'${&{{\\\\}\\}}', internal=True)
48+
49+
def test_no_unescaped_closing_curly(self):
4650
self._test(r'${x\}')
4751
self._test(r'${x\\\}')
4852
self._test(r'${x\\\\\\\}')
4953

5054
def test_uneven_curlys(self):
51-
self._test('${x:{}', '${x:{}')
52-
self._test('${x:{{}}', '${x:{{}')
53-
self._test('xx${x:{}xx', '${x:{}', start=2)
54-
self._test('{%{{}{{}}{{', '%{{}', start=1)
55+
self._test('${x:{}')
56+
self._test('${y:{{}}')
57+
self._test('xx${z:{}xx')
58+
self._test('{%{{}{{}}{{')
59+
self._test('${xx:{}}}}}', '${xx:{}}')
60+
61+
def test_escaped_uneven_curlys(self):
62+
self._test(r'${x:\{}', r'${x:\{}')
63+
self._test(r'${y:{\{}}', r'${y:{\{}}')
64+
self._test(r'xx${z:\{}xx', r'${z:\{}', start=2)
65+
self._test(r'{%{{}\{{}}{{', r'%{{}\{{}}', start=1)
66+
self._test(r'${xx:{}\}\}\}}', r'${xx:{}\}\}\}}')
5567

5668
def test_multiple_vars(self):
5769
self._test('${hello} ${world}', '${hello}', 0)
@@ -102,7 +114,7 @@ def test_list_item(self):
102114
def test_list_item_with_internal_vars(self):
103115
self._test('${x}[${i}]', '${x}', items='${i}')
104116
self._test('xx ${x}[${i}] ${xyz}', '${x}', start=3, items='${i}')
105-
self._test('$$$$${X{X}[${${i}-${${${i}}}}]', '${X{X}', start=4,
117+
self._test('$$$$${XX}[${${i}-${${${i}}}}]', '${XX}', start=4,
106118
items='${${i}-${${${i}}}}')
107119
self._test('${${i}}[${j{}]', '${${i}}', items='${j{}', internal=True)
108120

@@ -116,7 +128,7 @@ def test_dict_item(self):
116128
def test_dict_item_with_internal_vars(self):
117129
self._test('${x}[${i}]', '${x}', items='${i}')
118130
self._test('xx ${x}[${i}] ${xyz}', '${x}', start=3, items='${i}')
119-
self._test('$$$$${X{X}[${${i}-${${${i}}}}]', '${X{X}', start=4,
131+
self._test('$$$$${XX}[${${i}-${${${i}}}}]', '${XX}', start=4,
120132
items='${${i}-${${${i}}}}')
121133
self._test('${${i}}[${j{}]', '${${i}}', items='${j{}', internal=True)
122134

@@ -204,7 +216,7 @@ def test_is_variable(self):
204216
for no in ['', 'xxx', '${var} not alone', '\\${notvat}', '\\\\${var}',
205217
'${var}xx}', '${x}${y}']:
206218
assert_false(VariableSplitter(no).is_variable(), no)
207-
for yes in ['${var}', '${var${}', '${var${internal}}', '@{var}',
219+
for yes in ['${var}', r'${var$\{}', '${var${internal}}', '@{var}',
208220
'@{var}[0]']:
209221
assert_true(VariableSplitter(yes).is_variable(), yes)
210222

0 commit comments

Comments
 (0)