cucumber-expressons: Escape generated expressions. Fix for #345

cucumber-expressions/CHANGELOG.md

@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
    [#333](https://github.com/cucumber/cucumber/issues/333)
    [#334](https://github.com/cucumber/cucumber/pull/334)
    [jamis])
+* Matching a literal left curly brace [aslakhellesoy]
 
 ### Changed
 N/A
@@ -26,7 +27,9 @@ N/A
    [aslakhellesoy])
 
 ### Fixed
-N/A
+* Generated expressions escape `(` and `{` if they were present in the text.
+  ([#345](https://github.com/cucumber/cucumber/issues/345)
+  [aslakhellesoy])
 
 ## [5.0.13] - 2018-01-21
 

cucumber-expressions/README.md

@@ -128,16 +128,6 @@ It would also match this text:
 
     I have 42 cucumbers in my belly
 
-If you ever need to match those parentheses literally, you can escape the
-first opening parenthesis with a backslash:
-
-    I have {int} cucumber(s) in my belly \(amazing!)
-
-This expression would match the following examples:
-
-    I have 1 cucumber in my belly (amazing!)
-    I have 42 cucumbers in my belly (amazing!)
-
 ## Alternative text
 
 Sometimes you want to relax your language, to make it flow better. For example:
@@ -149,6 +139,20 @@ This would match either of those texts:
     I have 42 cucumbers in my belly
     I have 42 cucumbers in my stomach
 
+## Escaping
+
+If you ever need to match `()` or `{}` literally, you can escape the
+opening `(` or `{` with a backslash:
+
+    I have 42 \{what} cucumber(s) in my belly \(amazing!)
+
+This expression would match the following examples:
+
+    I have 42 {what} cucumber in my belly (amazing!)
+    I have 42 {what} cucumbers in my belly (amazing!)
+
+There is currently no way to escape the `/` character.
+
 ## Step Definition Snippets (Cucumber Expression generation)
 
 When Cucumber encounters a [Gherkin step](../docs/gherkin.md#steps) without a

cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/CucumberExpression.java

@@ -8,8 +8,7 @@
 public class CucumberExpression implements Expression {
     // Does not include (){} characters because they have special meaning
     private static final Pattern ESCAPE_PATTERN = Pattern.compile("([\\\\^\\[$.|?*+\\]])");
-    private static final Pattern PARAMETER_PATTERN = Pattern.compile("\\{([^}]+)}");
-    // Parentheses will be double-escaped due to ESCAPE_REGEXP
+    private static final Pattern PARAMETER_PATTERN = Pattern.compile("(\\\\\\\\)?\\{([^}]+)}");
     private static final Pattern OPTIONAL_PATTERN = Pattern.compile("(\\\\\\\\)?\\(([^)]+)\\)");
     private static final Pattern ALTERNATIVE_NON_WHITESPACE_TEXT_REGEXP = Pattern.compile("([^\\s^/]+)((/[^\\s^/]+)+)");
     private static final String DOUBLE_ESCAPE = "\\\\";
@@ -28,10 +27,12 @@ public CucumberExpression(String expression, ParameterTypeRegistry parameterType
         StringBuffer sb = new StringBuffer();
         while (matcher.find()) {
             // look for double-escaped parentheses
-            if (DOUBLE_ESCAPE.equals(matcher.group(1))) {
-                matcher.appendReplacement(sb, "\\\\(" + matcher.group(2) + "\\\\)");
+            String g1 = matcher.group(1);
+            String g2 = matcher.group(2);
+            if (DOUBLE_ESCAPE.equals(g1)) {
+                matcher.appendReplacement(sb, "\\\\(" + g2 + "\\\\)");
             } else {
-                matcher.appendReplacement(sb, "(?:" + matcher.group(2) + ")?");
+                matcher.appendReplacement(sb, "(?:" + g2 + ")?");
             }
         }
         matcher.appendTail(sb);
@@ -48,14 +49,18 @@ public CucumberExpression(String expression, ParameterTypeRegistry parameterType
         StringBuffer regexp = new StringBuffer();
         regexp.append("^");
         while (matcher.find()) {
-            String typeName = matcher.group(1);
-            ParameterType<?> parameterType = parameterTypeRegistry.lookupByTypeName(typeName);
-            if (parameterType == null) {
-                throw new UndefinedParameterTypeException(typeName);
+            String g1 = matcher.group(1);
+            if (DOUBLE_ESCAPE.equals(g1)) {
+                matcher.appendReplacement(regexp, "\\\\{" + matcher.group(2) + "\\\\}");
+            } else {
+                String typeName = matcher.group(2);
+                ParameterType<?> parameterType = parameterTypeRegistry.lookupByTypeName(typeName);
+                if (parameterType == null) {
+                    throw new UndefinedParameterTypeException(typeName);
+                }
+                parameterTypes.add(parameterType);
+                matcher.appendReplacement(regexp, Matcher.quoteReplacement(buildCaptureRegexp(parameterType.getRegexps())));
             }
-            parameterTypes.add(parameterType);
-
-            matcher.appendReplacement(regexp, Matcher.quoteReplacement(buildCaptureRegexp(parameterType.getRegexps())));
         }
         matcher.appendTail(regexp);
         regexp.append("$");

cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/CucumberExpressionGenerator.java

@@ -62,7 +62,7 @@ public List<GeneratedExpression> generateExpressions(String text) {
                 parameterTypeCombinations.add(new ArrayList<>(parameterTypes));
 
                 expressionTemplate
-                        .append(escapeForStringFormat(text.substring(pos, bestParameterTypeMatcher.start())))
+                        .append(escape(text.substring(pos, bestParameterTypeMatcher.start())))
                         .append("{%s}");
                 pos = bestParameterTypeMatcher.start() + bestParameterTypeMatcher.group().length();
             } else {
@@ -73,12 +73,14 @@ public List<GeneratedExpression> generateExpressions(String text) {
                 break;
             }
         }
-        expressionTemplate.append(escapeForStringFormat(text.substring(pos)));
+        expressionTemplate.append(escape(text.substring(pos)));
         return new CombinatorialGeneratedExpressionFactory(expressionTemplate.toString(), parameterTypeCombinations).generateExpressions();
     }
 
-    private String escapeForStringFormat(String s) {
-        return s.replaceAll("%", "%%");
+    private String escape(String s) {
+        return s.replaceAll("%", "%%") // Escape for String.format
+                .replaceAll("\\(", "\\\\(")
+                .replaceAll("\\{", "\\\\{");
     }
 
     /**

cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionGeneratorTest.java

@@ -34,6 +34,20 @@ public void generates_expression_for_no_args() {
         assertExpression("hello", Collections.<String>emptyList(), "hello");
     }
 
+    @Test
+    public void generates_expression_with_escaped_left_parenthesis() {
+        assertExpression(
+                "I have \\(a) {int} cukes", singletonList("int1"),
+                "I have (a) 2 cukes");
+    }
+
+    @Test
+    public void generates_expression_with_escaped_left_curly_brace() {
+        assertExpression(
+                "I have \\{a} {int} cukes", singletonList("int1"),
+                "I have {a} 2 cukes");
+    }
+
     @Test
     public void generates_expression_for_int_double_arg() {
         assertExpression(
@@ -240,7 +254,12 @@ public String apply(String s) {
 
     private void assertExpression(String expectedExpression, List<String> expectedArgumentNames, String text) {
         GeneratedExpression generatedExpression = generator.generateExpressions(text).get(0);
-        assertEquals(expectedArgumentNames, generatedExpression.getParameterNames());
         assertEquals(expectedExpression, generatedExpression.getSource());
+        assertEquals(expectedArgumentNames, generatedExpression.getParameterNames());
+
+        // Check that the generated expression matches the text
+        CucumberExpression cucumberExpression = new CucumberExpression(generatedExpression.getSource(), parameterTypeRegistry);
+        List<Argument<?>> match = cucumberExpression.match(text);
+        assertEquals(expectedArgumentNames.size(), match.size());
     }
 }