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.

Sign up for GitHub

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

tag-expressions: support escape backslashes in tag expressions #1778

Merged
merged 13 commits into from
Oct 8, 2021
Merged

tag-expressions: support escape backslashes in tag expressions #1778

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
cbf95f9
tag-expressions: javascript: support escaped baclslashes
yusuke-noda Oct 5, 2021
191698a
tag-expressions: java: support escaped backslashes
yusuke-noda Oct 5, 2021
6e0c036
tag-expressions: go: support escaped backslashes
yusuke-noda Oct 5, 2021
b460667
tag-expressions: ruby: support escaped backslashes
yusuke-noda Oct 5, 2021
74089c5
Merge branch 'main' into tag-expressions-escape-backslash
mpkorstanje Oct 5, 2021
6e24638
Merge branch 'main' into tag-expressions-escape-backslash
aurelien-reeves Oct 8, 2021
0572ead
Merge branch 'main' into tag-expressions-escape-backslash
aslakhellesoy Oct 8, 2021
6f5f827
Merge branch 'main' into tag-expressions-escape-backslash
aurelien-reeves Oct 8, 2021
c2b41e2
Merge remote-tracking branch 'origin/main' into tag-expressions-escap…
ehuelsmann Oct 8, 2021
3286f3b
[tag-expressions] Update CHANGELOG.md
ehuelsmann Oct 8, 2021
5ec82ba
Merge branch 'tag-expressions-escape-backslash' of github.com:yusuke-…
ehuelsmann Oct 8, 2021
8d74abe
Update changelog
aurelien-reeves Oct 8, 2021
d2c01a7
Remove double entry in changelog
aurelien-reeves Oct 8, 2021
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
16 changes: 14 additions & 2 deletions tag-expressions/go/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,20 @@ func tokenize(expr string) []string {
for _, c := range expr {
if unicode.IsSpace(c) {
collectToken()
escaped = false
continue
}

ch := string(c)

switch ch {
case "\\":
escaped = true
if escaped {
token.WriteString(ch)
escaped = false
} else {
escaped = true
}
Copy link
Contributor

@mpkorstanje mpkorstanje Oct 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Structurally the go implementation differs quite a bit from the other implementations. So I would not be opposed to lifting this case out into a separate if-block so that the escaped = false in the unicode.IsSpace(c) { block is not needed and so that the different implementations resemble each other again.

case "(", ")":
if escaped {
token.WriteString(ch)
Expand All @@ -130,6 +136,7 @@ func tokenize(expr string) []string {
}
default:
token.WriteString(ch)
escaped = false
}
}

Expand Down Expand Up @@ -192,7 +199,12 @@ func (l *literalExpr) Evaluate(variables []string) bool {

func (l *literalExpr) ToString() string {
return strings.Replace(
strings.Replace(l.value, "(", "\\(", -1),
strings.Replace(
strings.Replace(l.value, "\\", "\\\\", -1),
"(",
"\\(",
-1,
),
")",
"\\)",
-1,
Expand Down
36 changes: 36 additions & 0 deletions tag-expressions/go/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,16 @@ func TestParseForValidCases(t *testing.T) {
given: "not a\\(\\) or b and not c or not d or e and f",
expected: "( ( ( not ( a\\(\\) ) or ( b and not ( c ) ) ) or not ( d ) ) or ( e and f ) )",
},
{
name: "test escaping backslash",
given: "(a\\\\ and b\\\\\\( and c\\\\\\) and d\\\\)",
expected: "( ( ( a\\\\ and b\\\\\\( ) and c\\\\\\) ) and d\\\\ )",
},
{
name: "test backslash",
given: "(a\\ and \\b) and c\\",
expected: "( ( a and b ) and c )",
},
}

for _, tc := range cases {
Expand Down Expand Up @@ -176,6 +186,32 @@ func TestParseForEvaluationErrors(t *testing.T) {
require.True(t, expr.Evaluate([]string{"x"}))
},
},
{
name: "evaluate expressions with escaped backslash",
given: "x\\\\ or(y\\\\\\)) or(z\\\\)",
expectations: func(t *testing.T, expr Evaluatable) {
require.False(t, expr.Evaluate([]string{}))
require.True(t, expr.Evaluate([]string{"x\\"}))
require.True(t, expr.Evaluate([]string{"y\\)"}))
require.True(t, expr.Evaluate([]string{"z\\"}))
require.False(t, expr.Evaluate([]string{"x"}))
require.False(t, expr.Evaluate([]string{"y)"}))
require.False(t, expr.Evaluate([]string{"z"}))
},
},
{
name: "evaluate expressions with backslash",
given: "\\x or y\\ or z\\",
expectations: func(t *testing.T, expr Evaluatable) {
require.False(t, expr.Evaluate([]string{}))
require.True(t, expr.Evaluate([]string{"x"}))
require.True(t, expr.Evaluate([]string{"y"}))
require.True(t, expr.Evaluate([]string{"z"}))
require.False(t, expr.Evaluate([]string{"\\x"}))
require.False(t, expr.Evaluate([]string{"y\\"}))
require.False(t, expr.Evaluate([]string{"z\\"}))
},
},
}

for _, tc := range cases {
Expand Down
4 changes: 2 additions & 2 deletions tag-expressions/java/src/main/java/io/cucumber/tagexpressions/TagExpressionParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ private static List<String> tokenize(String expr) {
StringBuilder token = null;
for (int i = 0; i < expr.length(); i++) {
char c = expr.charAt(i);
if (ESCAPING_CHAR == c) {
if (ESCAPING_CHAR == c && !isEscaped) {
isEscaped = true;
} else {
if (Character.isWhitespace(c)) { // skip
Expand Down Expand Up @@ -197,7 +197,7 @@ public boolean evaluate(List<String> variables) {

@Override
public String toString() {
return value.replaceAll("\\(", "\\\\(").replaceAll("\\)", "\\\\)");
return value.replaceAll("\\\\", "\\\\\\\\").replaceAll("\\(", "\\\\(").replaceAll("\\)", "\\\\)");
}
}

Expand Down
6 changes: 5 additions & 1 deletion ...s/java/src/test/java/io/cucumber/tagexpressions/TagExpressionParserParameterizedTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ static Stream<Arguments> data() {
arguments("not a", "not ( a )"),
arguments("( a and b ) or ( c and d )", "( ( a and b ) or ( c and d ) )"),
arguments("not a or b and not c or not d or e and f", "( ( ( not ( a ) or ( b and not ( c ) ) ) or not ( d ) ) or ( e and f ) )"),
arguments("not a\\(1\\) or b and not c or not d or e and f", "( ( ( not ( a\\(1\\) ) or ( b and not ( c ) ) ) or not ( d ) ) or ( e and f ) )")
arguments("not a\\(1\\) or b and not c or not d or e and f", "( ( ( not ( a\\(1\\) ) or ( b and not ( c ) ) ) or not ( d ) ) or ( e and f ) )"),
arguments("a\\\\ and b", "( a\\\\ and b )"),
arguments("\\a and b\\ and c\\", "( ( a and b ) and c )"),
arguments("a\\\\\\( and b\\\\\\)", "( a\\\\\\( and b\\\\\\) )"),
arguments("(a and \\b)", "( a and b )")
);
}

Expand Down
18 changes: 18 additions & 0 deletions tag-expressions/java/src/test/java/io/cucumber/tagexpressions/TagExpressionParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,22 @@ void errors_when_there_are_only_operators() {
assertThat("Unexpected message", thrownException.getMessage(), is(equalTo("Tag expression 'or or' could not be parsed because of syntax error: expected operand")));
}

@Test
void evaluates_expr_with_escaped_backslash() {
Expression expr = TagExpressionParser.parse("@a\\\\ or (@b\\\\\\)) or (@c\\\\)");
assertFalse(expr.evaluate(asList("@a @b) @c".split(" "))));
assertTrue(expr.evaluate(asList("@a\\".split(" "))));
assertTrue(expr.evaluate(asList("@b\\)".split(" "))));
assertTrue(expr.evaluate(asList("@c\\".split(" "))));
}

@Test
void evaluates_expr_with_backslash() {
Expression expr = TagExpressionParser.parse("@\\a or @b\\ or @c\\");
assertFalse(expr.evaluate(asList("@\\a @b\\ @c\\".split(" "))));
assertTrue(expr.evaluate(asList("@a".split(" "))));
assertTrue(expr.evaluate(asList("@b".split(" "))));
assertTrue(expr.evaluate(asList("@c".split(" "))));
}

}
4 changes: 2 additions & 2 deletions tag-expressions/javascript/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ function tokenize(expr: string): string[] {
let token
for (let i = 0; i < expr.length; i++) {
const c = expr.charAt(i)
if ('\\' === c) {
if ('\\' === c && !isEscaped) {
isEscaped = true
} else {
if (/\s/.test(c)) {
Expand Down Expand Up @@ -171,7 +171,7 @@ class Literal implements Node {
}

public toString() {
return this.value.replace(/\(/g, '\\(').replace(/\)/g, '\\)')
return this.value.replace(/\\/g, '\\\\').replace(/\(/g, '\\(').replace(/\)/g, '\\)')
}
}

Expand Down
29 changes: 29 additions & 0 deletions tag-expressions/javascript/test/tag_expression_parser_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ describe('TagExpressionParser', () => {
'not a\\(\\) or b and not c or not d or e and f',
'( ( ( not ( a\\(\\) ) or ( b and not ( c ) ) ) or not ( d ) ) or ( e and f ) )',
],
['a\\\\ and b', '( a\\\\ and b )'],
['\\a and b', '( a and b )'],
['a\\ and b', '( a and b )'],
['a and b\\', '( a and b )'],
['( a and b\\\\)', '( a and b\\\\ )'],
['a\\\\\\( and b\\\\\\)', '( a\\\\\\( and b\\\\\\) )'],
['(a and \\b)', '( a and b )'],
// a or not b
]
tests.forEach(function (inOut) {
Expand Down Expand Up @@ -89,5 +96,27 @@ describe('TagExpressionParser', () => {
assert.strictEqual(expr.evaluate(['y']), true)
assert.strictEqual(expr.evaluate(['x']), true)
})

it('evaluates expressions with escaped backslash', function () {
const expr = parse('x\\\\ or(y\\\\\\)) or(z\\\\)')
assert.strictEqual(expr.evaluate([]), false)
assert.strictEqual(expr.evaluate(['x\\']), true)
assert.strictEqual(expr.evaluate(['y\\)']), true)
assert.strictEqual(expr.evaluate(['z\\']), true)
assert.strictEqual(expr.evaluate(['x']), false)
assert.strictEqual(expr.evaluate(['y)']), false)
assert.strictEqual(expr.evaluate(['z']), false)
})

it('evaluates expressions with backslash', function () {
const expr = parse('\\x or y\\ or z\\')
assert.strictEqual(expr.evaluate([]), false)
assert.strictEqual(expr.evaluate(['x']), true)
assert.strictEqual(expr.evaluate(['y']), true)
assert.strictEqual(expr.evaluate(['z']), true)
assert.strictEqual(expr.evaluate(['\\x']), false)
assert.strictEqual(expr.evaluate(['y\\']), false)
assert.strictEqual(expr.evaluate(['z\\']), false)
})
})
})
4 changes: 2 additions & 2 deletions tag-expressions/ruby/lib/cucumber/tag_expressions/expressions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ module TagExpressions
# Literal expression node
class Literal
def initialize(value)
@value = value.gsub(/\\\(/, '(').gsub(/\\\)/, ')')
@value = value
end

def evaluate(variables)
variables.include?(@value)
end

def to_s
@value.gsub(/\(/, '\\(').gsub(/\)/, '\\)')
@value.gsub(/\\/, "\\\\\\\\").gsub(/\(/, "\\(").gsub(/\)/, "\\)")
end
end

Expand Down
31 changes: 30 additions & 1 deletion tag-expressions/ruby/lib/cucumber/tag_expressions/parser.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,36 @@ def precedence(token)
end

def tokens(infix_expression)
infix_expression.gsub(/(?<!\\)\(/, ' ( ').gsub(/(?<!\\)\)/, ' ) ').strip.split(/\s+/)
escaped = false
token = ""
result = []
infix_expression.chars.each do | ch |
if ch == '\\' && !escaped
escaped = true
else
if ch.match(/\s/)
if token.length > 0
result.push(token)
token = ""
end
else
if (ch == '(' || ch == ')') && !escaped
if token.length > 0
result.push(token)
token = ""
end
result.push(ch)
else
token = token + ch
end
end
escaped = false
end
end
if token.length > 0
result.push(token)
end
result
end

def process_tokens!(infix_expression)
Expand Down
29 changes: 29 additions & 0 deletions tag-expressions/ruby/spec/expressions_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,33 @@
include_examples 'expression node', infix_expression, data
end
end

describe Cucumber::TagExpressions::Or do
context '#evaluate' do
infix_expression = 'x\\\\ or (y\\\\\\)) or (z\\\\)'
data = [[%w(), false],
[%w(x), false],
[%w(y\)), false],
[%w(z), false],
[%w(x\\), true],
[%w(y\\\)), true],
[%w(z\\), true]]
include_examples 'expression node', infix_expression, data
end
end

describe Cucumber::TagExpressions::Or do
context '#evaluate' do
infix_expression = '\\x or y\\ or z\\'
data = [[%w(), false],
[%w(\\x), false],
[%w(y\\), false],
[%w(z\\), false],
[%w(x), true],
[%w(y), true],
[%w(z), true]]
include_examples 'expression node', infix_expression, data
end
end

end
6 changes: 5 additions & 1 deletion tag-expressions/ruby/spec/parser_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
['not a or b and not c or not d or e and f',
'( ( ( not ( a ) or ( b and not ( c ) ) ) or not ( d ) ) or ( e and f ) )'],
['not a\\(\\) or b and not c or not d or e and f',
'( ( ( not ( a\\(\\) ) or ( b and not ( c ) ) ) or not ( d ) ) or ( e and f ) )']
'( ( ( not ( a\\(\\) ) or ( b and not ( c ) ) ) or not ( d ) ) or ( e and f ) )'],
['a\\\\ and b', '( a\\\\ and b )'],
['\\a and b\\ and c\\', '( ( a and b ) and c )'],
['(a and b\\\\)', '( a and b\\\\ )'],
['a\\\\\\( and b\\\\\\)', '( a\\\\\\( and b\\\\\\) )']
]

error_test_data = [
Expand Down