diff --git a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/BuiltInParameterTransformer.java b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/BuiltInParameterTransformer.java index c9fa432d95..2742506d87 100644 --- a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/BuiltInParameterTransformer.java +++ b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/BuiltInParameterTransformer.java @@ -3,7 +3,6 @@ import java.lang.reflect.Type; import java.math.BigDecimal; import java.math.BigInteger; -import java.text.NumberFormat; import java.util.Locale; import static java.util.Objects.requireNonNull; @@ -35,7 +34,7 @@ public Object transform(String fromValue, Type toValueType) { return new BigInteger(fromValue); } - if (BigDecimal.class.equals(toValueClass)) { + if (BigDecimal.class.equals(toValueClass) || Number.class.equals(toValueClass)) { return numberParser.parseBigDecimal(fromValue); } diff --git a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterType.java b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterType.java index efc2de167f..92755eeed0 100644 --- a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterType.java +++ b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterType.java @@ -22,6 +22,7 @@ public final class ParameterType implements Comparable> { private final boolean useForSnippets; private final CaptureGroupTransformer transformer; private final boolean anonymous; + private final boolean useRegexpMatchAsStrongTypeHint; static void checkParameterTypeName(String name) { String unescapedTypeName = UNESCAPE_PATTERN.matcher(name).replaceAll("$2"); @@ -37,9 +38,10 @@ static ParameterType createAnonymousParameterType(String regexp) { public Object transform(String[] arg) { throw new UnsupportedOperationException("Anonymous transform must be deanonymized before use"); } - }, false, true, true); + }, false, true, false, true); } + @SuppressWarnings("unchecked") public static ParameterType fromEnum(final Class enumClass) { Enum[] enumConstants = enumClass.getEnumConstants(); StringBuilder regexpBuilder = new StringBuilder(); @@ -51,16 +53,11 @@ public static ParameterType fromEnum(final Class enumClas enumClass.getSimpleName(), regexpBuilder.toString(), enumClass, - new Transformer() { - @Override - public E transform(String arg) { - return (E) Enum.valueOf(enumClass, arg); - } - } + (String arg) -> (E) Enum.valueOf(enumClass, arg) ); } - private ParameterType(String name, List regexps, Type type, CaptureGroupTransformer transformer, boolean useForSnippets, boolean preferForRegexpMatch, boolean anonymous) { + private ParameterType(String name, List regexps, Type type, CaptureGroupTransformer transformer, boolean useForSnippets, boolean preferForRegexpMatch, boolean useRegexpMatchAsStrongTypeHint, boolean anonymous) { if (regexps == null) throw new NullPointerException("regexps cannot be null"); if (type == null) throw new NullPointerException("type cannot be null"); if (transformer == null) throw new NullPointerException("transformer cannot be null"); @@ -72,10 +69,31 @@ private ParameterType(String name, List regexps, Type type, CaptureGroup this.useForSnippets = useForSnippets; this.preferForRegexpMatch = preferForRegexpMatch; this.anonymous = anonymous; + this.useRegexpMatchAsStrongTypeHint = useRegexpMatchAsStrongTypeHint; + } + + public ParameterType(String name, List regexps, Type type, CaptureGroupTransformer transformer, boolean useForSnippets, boolean preferForRegexpMatch, boolean useRegexpMatchAsStrongTypeHint) { + this(name, regexps, type, transformer, useForSnippets, preferForRegexpMatch, useRegexpMatchAsStrongTypeHint, false); } public ParameterType(String name, List regexps, Type type, CaptureGroupTransformer transformer, boolean useForSnippets, boolean preferForRegexpMatch) { - this(name, regexps, type, transformer, useForSnippets, preferForRegexpMatch, false); + // Unless explicitly set useRegexpMatchAsStrongTypeHint is true. + // + // Reasoning: + // 1. Pure cucumber expression users will not notice this in either scenario. + // 2. Pure regular expression users will benefit because BuiltInParameterTransformer can now seamlessly + // transform any captured values. (For all built in types useRegexpMatchAsStrongTypeHint is explicitly set to + // false.) + // 2. Regular expression users that define a default transformer have little need to define parameter types. The + // default transformer should be sufficiently powerful to meet their needs and will often allow users to add + // custom creation methods e.g. Jacksons @JsonFactory. + // 3. Users who mix regular and cucumber expressions may run into conflicts when a registered cucumber expression + // and unregistered happens to collide. However this was the situation before this flag was added. + // 4. Regular expression users who define custom parameter types do so with the expectation that the parameter + // will be matched. Subverting this expectation when the method signature does not match may result in a + // parameter transformer that is unable to convert to the desired type. Leaving the user puzzled as to why + // his transform was ignored. + this(name, regexps, type, transformer, useForSnippets, preferForRegexpMatch, true); } public ParameterType(String name, List regexps, Class type, CaptureGroupTransformer transformer, boolean useForSnippets, boolean preferForRegexpMatch) { @@ -94,14 +112,26 @@ public ParameterType(String name, String regexp, Class type, CaptureGroupTran this(name, singletonList(regexp), type, transformer, true, false); } + public ParameterType(String name, List regexps, Type type, Transformer transformer, boolean useForSnippets, boolean preferForRegexpMatch, boolean useRegexpMatchAsStrongTypeHint) { + this(name, regexps, type, new TransformerAdaptor<>(transformer), useForSnippets, preferForRegexpMatch, useRegexpMatchAsStrongTypeHint); + } + public ParameterType(String name, List regexps, Type type, Transformer transformer, boolean useForSnippets, boolean preferForRegexpMatch) { this(name, regexps, type, new TransformerAdaptor<>(transformer), useForSnippets, preferForRegexpMatch); } + public ParameterType(String name, List regexps, Class type, Transformer transformer, boolean useForSnippets, boolean preferForRegexpMatch, boolean useRegexpMatchAsStrongTypeHint) { + this(name, regexps, (Type) type, transformer, useForSnippets, preferForRegexpMatch, useRegexpMatchAsStrongTypeHint); + } + public ParameterType(String name, List regexps, Class type, Transformer transformer, boolean useForSnippets, boolean preferForRegexpMatch) { this(name, regexps, (Type) type, transformer, useForSnippets, preferForRegexpMatch); } + public ParameterType(String name, String regexp, Class type, Transformer transformer, boolean useForSnippets, boolean preferForRegexpMatch, boolean useRegexpMatchAsStrongTypeHint) { + this(name, singletonList(regexp), type, transformer, useForSnippets, preferForRegexpMatch, useRegexpMatchAsStrongTypeHint); + } + public ParameterType(String name, String regexp, Class type, Transformer transformer, boolean useForSnippets, boolean preferForRegexpMatch) { this(name, singletonList(regexp), type, transformer, useForSnippets, preferForRegexpMatch); } @@ -152,7 +182,7 @@ public boolean preferForRegexpMatch() { } /** - * Indicates whether or not this is a parameter type that should be used for generating + * Indicates whether or not this is a parameter type should be used for generating * {@link GeneratedExpression}s from text. Typically, parameter types with greedy regexps * should return false. * @@ -166,8 +196,22 @@ boolean isAnonymous() { return anonymous; } + /** + * Indicates whether or not this parameter provides a strong type hint when considering a + * regular expression match. If so, the type hint provided by the method arguments will be + * ignored. If not, when both type hints are in agreement, this parameter types transformer + * will be used. Otherwise parameter transformation for a regular expression match will be + * handled by {@link ParameterTypeRegistry#getDefaultParameterTransformer()}. + * + * @return true if this parameter type provides a type hint when considering a regular + * expression match + */ + public boolean useRegexpMatchAsStrongTypeHint() { + return useRegexpMatchAsStrongTypeHint; + } + ParameterType deAnonymize(Type type, Transformer transformer) { - return new ParameterType<>("anonymous", regexps, type, new TransformerAdaptor<>(transformer), useForSnippets, preferForRegexpMatch, anonymous); + return new ParameterType<>("anonymous", regexps, type, new TransformerAdaptor<>(transformer), useForSnippets, preferForRegexpMatch, useRegexpMatchAsStrongTypeHint, anonymous); } T transform(List groupValues) { @@ -204,7 +248,7 @@ public int compareTo(ParameterType o) { } public int weight() { - if(this.type.equals(Integer.class) || this.type.equals(Integer.TYPE)) { + if (this.type.equals(Integer.class) || this.type.equals(Integer.TYPE)) { return 1000; } return 0; diff --git a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeRegistry.java b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeRegistry.java index 8f74211625..fec4a0da36 100644 --- a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeRegistry.java +++ b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeRegistry.java @@ -48,55 +48,55 @@ private ParameterTypeRegistry(ParameterByTypeTransformer defaultParameterTransfo public BigInteger transform(String arg) throws Throwable { return (BigInteger) internalParameterTransformer.transform(arg, BigInteger.class); } - }, false, false)); + }, false, false, false)); defineParameterType(new ParameterType<>("bigdecimal", FLOAT_REGEXPS, BigDecimal.class, new Transformer() { @Override public BigDecimal transform(String arg) throws Throwable { return (BigDecimal) internalParameterTransformer.transform(arg, BigDecimal.class); } - }, false, false)); + }, false, false, false)); defineParameterType(new ParameterType<>("byte", INTEGER_REGEXPS, Byte.class, new Transformer() { @Override public Byte transform(String arg) throws Throwable { return (Byte) internalParameterTransformer.transform(arg, Byte.class); } - }, false, false)); + }, false, false, false)); defineParameterType(new ParameterType<>("short", INTEGER_REGEXPS, Short.class, new Transformer() { @Override public Short transform(String arg) throws Throwable { return (Short) internalParameterTransformer.transform(arg, Short.class); } - }, false, false)); + }, false, false, false)); defineParameterType(new ParameterType<>("int", INTEGER_REGEXPS, Integer.class, new Transformer() { @Override public Integer transform(String arg) throws Throwable { return (Integer) internalParameterTransformer.transform(arg, Integer.class); } - }, true, true)); + }, true, true, false)); defineParameterType(new ParameterType<>("long", INTEGER_REGEXPS, Long.class, new Transformer() { @Override public Long transform(String arg) throws Throwable { return (Long) internalParameterTransformer.transform(arg, Long.class); } - }, false, false)); + }, false, false, false)); defineParameterType(new ParameterType<>("float", FLOAT_REGEXPS, Float.class, new Transformer() { @Override public Float transform(String arg) throws Throwable { return (Float) internalParameterTransformer.transform(arg, Float.class); } - }, false, false)); + }, false, false, false)); defineParameterType(new ParameterType<>("double", FLOAT_REGEXPS, Double.class, new Transformer() { @Override public Double transform(String arg) throws Throwable { return (Double) internalParameterTransformer.transform(arg, Double.class); } - }, true, true)); + }, true, true, false)); defineParameterType(new ParameterType<>("word", WORD_REGEXPS, String.class, new Transformer() { @Override public String transform(String arg) throws Throwable { return (String) internalParameterTransformer.transform(arg, String.class); } - }, false, false)); + }, false, false, false)); defineParameterType(new ParameterType<>("string", STRING_REGEXPS, String.class, new Transformer() { @Override public String transform(String arg) throws Throwable { @@ -105,7 +105,7 @@ public String transform(String arg) throws Throwable { .replaceAll("\\\\'", "'"), String.class); } - }, true, false)); + }, true, false, false)); defineParameterType(createAnonymousParameterType(ANONYMOUS_REGEX)); } diff --git a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/RegularExpression.java b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/RegularExpression.java index 36eeaa13e4..18b15ea80b 100644 --- a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/RegularExpression.java +++ b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/RegularExpression.java @@ -36,10 +36,22 @@ public List> match(String text, Type... typeHints) { int typeHintIndex = 0; for (GroupBuilder groupBuilder : treeRegexp.getGroupBuilder().getChildren()) { final String parameterTypeRegexp = groupBuilder.getSource(); - final Type typeHint = typeHintIndex < typeHints.length ? typeHints[typeHintIndex++] : String.class; + boolean hasTypeHint = typeHintIndex < typeHints.length; + final Type typeHint = hasTypeHint ? typeHints[typeHintIndex++] : String.class; ParameterType parameterType = parameterTypeRegistry.lookupByRegexp(parameterTypeRegexp, expressionRegexp, text); + // When there is a conflict between the type hint from the regular expression and the method + // prefer the the parameter type associated with the regular expression. This ensures we will + // use the internal/user registered parameter transformer rather then the default. + // + // Unless the parameter type indicates it is the stronger type hint. + if (parameterType != null && hasTypeHint && !parameterType.useRegexpMatchAsStrongTypeHint()) { + if (!parameterType.getType().equals(typeHint)) { + parameterType = null; + } + } + if (parameterType == null) { parameterType = createAnonymousParameterType(parameterTypeRegexp); } diff --git a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/RegularExpressionTest.java b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/RegularExpressionTest.java index 2065e44cae..6ed3a6ef32 100644 --- a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/RegularExpressionTest.java +++ b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/RegularExpressionTest.java @@ -35,6 +35,19 @@ public void matches_positive_int() { assertEquals(singletonList(22), match); } + @Test + public void matches_positive_int_with_hint() { + List match = match(compile("(\\d+)"), "22", Integer.class); + assertEquals(singletonList(22), match); + } + + @Test + public void matches_positive_int_with_conflicting_type_hint() { + List match = match(compile("(\\d+)"), "22", String.class); + assertEquals(singletonList("22"), match); + } + + @Test public void matches_nested_capture_group_without_match() { List match = match(compile("^a user( named \"([^\"]*)\")?$"), "a user"); @@ -127,6 +140,39 @@ public String transform(String s) { assertEquals(asList("\" AND QUOTE \""), match); } + @Test + public void ignores_type_hint_when_parameter_type_has_strong_type_hint() { + parameterTypeRegistry.defineParameterType(new ParameterType<>( + "test", + "one|two|three", + Integer.class, + new Transformer() { + @Override + public Integer transform(String s) { + return 42; + } + }, false, false, true + )); + assertEquals(asList(42), match(compile("(one|two|three)"), "one", String.class)); + } + + + @Test + public void follows_type_hint_when_parameter_type_does_not_have_strong_type_hint() { + parameterTypeRegistry.defineParameterType(new ParameterType<>( + "test", + "one|two|three", + Integer.class, + new Transformer() { + @Override + public Integer transform(String s) { + return 42; + } + }, false, false, false + )); + assertEquals(asList("one"), match(compile("(one|two|three)"), "one", String.class)); + } + @Test public void matches_anonymous_parameter_type_with_hint() { assertEquals(singletonList(0.22f), match(compile("(.*)"), "0.22", Float.class));