From f5b489715cf19f9827e8e8793772ca2411e3ab93 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Fri, 2 Aug 2019 21:25:36 +0200 Subject: [PATCH] cucumber-expressions: Improve decimal number parsing Properly match decimal numbers taking into account that a decimal has: * at least one number * optional negative/positive sign * a number preceding/succeeding the thousands separator * may have multiple thousands separators * may have a single decimal separator * may start with a decimal separator * an optional fractional part * an optional scientific exponent notation * localized decimal and grouping characters ^ See: [International Language Environments Guide - Decimal and Thousands Separators](https://docs.oracle.com/cd/E19455-01/806-0169/overview-9/index.html) Fixes #669 --- cucumber-expressions/java/pom.xml | 15 ++++- .../BuiltInParameterTransformer.java | 3 +- .../ParameterTypeRegistry.java | 41 +++++++++--- .../cucumberexpressions/TreeRegexp.java | 2 +- .../CucumberExpressionPatternTest.java | 7 ++- .../cucumberexpressions/NumberParserTest.java | 8 ++- .../ParameterTypeRegistryTest.java | 62 +++++++++++++++++++ 7 files changed, 120 insertions(+), 18 deletions(-) diff --git a/cucumber-expressions/java/pom.xml b/cucumber-expressions/java/pom.xml index 1608b27bb7..3e0944dd75 100644 --- a/cucumber-expressions/java/pom.xml +++ b/cucumber-expressions/java/pom.xml @@ -36,7 +36,7 @@ junit junit - 4.13-beta-3 + 4.12 test @@ -46,11 +46,20 @@ 2.8.5 test - + + org.hamcrest + hamcrest-core + 2.1 + + + org.hamcrest + hamcrest-library + 2.1 + com.fasterxml.jackson.core jackson-databind - 2.9.9.1 + 2.9.9.2 test 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/ParameterTypeRegistry.java b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeRegistry.java index 8f74211625..b4e7e408e4 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 @@ -4,6 +4,7 @@ import java.math.BigDecimal; import java.math.BigInteger; +import java.text.DecimalFormatSymbols; import java.util.Collection; import java.util.HashMap; import java.util.List; @@ -21,10 +22,24 @@ public final class ParameterTypeRegistry { // Pattern.compile(...).pattern() is not necessary, but it helps us take advantage of the IntelliJ's regexp validation, // which detects unneeded escapes. - private static final List INTEGER_REGEXPS = asList(Pattern.compile("-?\\d+").pattern(), Pattern.compile("\\d+").pattern()); - private static final List FLOAT_REGEXPS = singletonList(Pattern.compile("-?\\d*(?:[.,]\\d+)?").pattern()); - private static final List WORD_REGEXPS = singletonList(Pattern.compile("[^\\s]+").pattern()); - private static final List STRING_REGEXPS = singletonList(Pattern.compile("\"([^\"\\\\]*(\\\\.[^\"\\\\]*)*)\"|'([^'\\\\]*(\\\\.[^'\\\\]*)*)'").pattern()); + private static final List INTEGER_REGEXPS = asList( + Pattern.compile("-?\\d+").pattern(), + Pattern.compile("\\d+").pattern() + ); + private static final String SIGN = "[-+]?"; + private static final String MUST_CONTAIN_NUMBER = "(?=.*[0-9].*)"; + private static final String SCIENTIFIC_NUMBER = "(?:[0-9]+[{expnt}][-+]?[0-9]+)?"; + private static final String DECIMAL_FRACTION = "(?:[{decimal}](?=[0-9].*))?[0-9]*"; + private static final String INTEGER = "(?:[0-9]+(?:[{group}]?[0-9]+)*)*"; + private static final String FLOAT_REGEXPS = + Pattern.compile(MUST_CONTAIN_NUMBER + SIGN + INTEGER + DECIMAL_FRACTION + SCIENTIFIC_NUMBER).pattern(); + private static final List WORD_REGEXPS = singletonList( + Pattern.compile("[^\\s]+").pattern() + ); + private static final List STRING_REGEXPS = asList( + Pattern.compile("\"([^\"\\\\]*(\\\\.[^\"\\\\]*)*)\"").pattern(), + Pattern.compile("'([^'\\\\]*(\\\\.[^'\\\\]*)*)'").pattern() + ); private static final String ANONYMOUS_REGEX = Pattern.compile(".*").pattern(); private final Map> parameterTypeByName = new HashMap<>(); private final Map>> parameterTypesByRegexp = new HashMap<>(); @@ -36,20 +51,28 @@ public final class ParameterTypeRegistry { private ParameterByTypeTransformer defaultParameterTransformer; public ParameterTypeRegistry(Locale locale) { - this(new BuiltInParameterTransformer(locale)); + this(new BuiltInParameterTransformer(locale), locale); } - private ParameterTypeRegistry(ParameterByTypeTransformer defaultParameterTransformer) { + private ParameterTypeRegistry(ParameterByTypeTransformer defaultParameterTransformer, Locale locale) { this.internalParameterTransformer = defaultParameterTransformer; this.defaultParameterTransformer = defaultParameterTransformer; + DecimalFormatSymbols numberFormat = DecimalFormatSymbols.getInstance(locale); + + List localizedFloatRegexp = singletonList(FLOAT_REGEXPS + .replace("{decimal}", "" + numberFormat.getDecimalSeparator()) + .replace("{group}", "" + numberFormat.getGroupingSeparator()) + .replace("{expnt}", "" + numberFormat.getExponentSeparator()) + ); + defineParameterType(new ParameterType<>("biginteger", INTEGER_REGEXPS, BigInteger.class, new Transformer() { @Override public BigInteger transform(String arg) throws Throwable { return (BigInteger) internalParameterTransformer.transform(arg, BigInteger.class); } }, false, false)); - defineParameterType(new ParameterType<>("bigdecimal", FLOAT_REGEXPS, BigDecimal.class, new Transformer() { + defineParameterType(new ParameterType<>("bigdecimal", localizedFloatRegexp, BigDecimal.class, new Transformer() { @Override public BigDecimal transform(String arg) throws Throwable { return (BigDecimal) internalParameterTransformer.transform(arg, BigDecimal.class); @@ -79,13 +102,13 @@ public Long transform(String arg) throws Throwable { return (Long) internalParameterTransformer.transform(arg, Long.class); } }, false, false)); - defineParameterType(new ParameterType<>("float", FLOAT_REGEXPS, Float.class, new Transformer() { + defineParameterType(new ParameterType<>("float", localizedFloatRegexp, Float.class, new Transformer() { @Override public Float transform(String arg) throws Throwable { return (Float) internalParameterTransformer.transform(arg, Float.class); } }, false, false)); - defineParameterType(new ParameterType<>("double", FLOAT_REGEXPS, Double.class, new Transformer() { + defineParameterType(new ParameterType<>("double", localizedFloatRegexp, Double.class, new Transformer() { @Override public Double transform(String arg) throws Throwable { return (Double) internalParameterTransformer.transform(arg, Double.class); diff --git a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/TreeRegexp.java b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/TreeRegexp.java index 0d13ac8d89..610af41454 100644 --- a/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/TreeRegexp.java +++ b/cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/TreeRegexp.java @@ -53,7 +53,7 @@ final class TreeRegexp { nonCapturingMaybe = false; } else if (c == '?' && last == '(') { nonCapturingMaybe = true; - } else if ((c == ':' || c == '!') && nonCapturingMaybe) { + } else if ((c == ':' || c == '!' || c == '=') && nonCapturingMaybe) { stack.peek().setNonCapturing(); nonCapturingMaybe = false; } diff --git a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionPatternTest.java b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionPatternTest.java index e1bf382418..b0c4f583e5 100644 --- a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionPatternTest.java +++ b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionPatternTest.java @@ -39,7 +39,12 @@ public void translates_alternation_with_non_alpha() { public void translates_parameters() { assertPattern( "I have {float} cukes at {int} o'clock", - "^I have (-?\\d*(?:[.,]\\d+)?) cukes at ((?:-?\\d+)|(?:\\d+)) o'clock$" + "^I have (" + + "(?=.*[0-9].*)" + + "[-+]?" + + "(?:[0-9]+(?:[,]?[0-9]+)*)*" + + "(?:[.](?=[0-9].*))?[0-9]*" + + "(?:[0-9]+[E][-+]?[0-9]+)?) cukes at ((?:-?\\d+)|(?:\\d+)) o'clock$" ); } diff --git a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/NumberParserTest.java b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/NumberParserTest.java index dfc7d4bf16..e0b4881ea7 100644 --- a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/NumberParserTest.java +++ b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/NumberParserTest.java @@ -11,22 +11,26 @@ public class NumberParserTest { private final NumberParser english = new NumberParser(Locale.ENGLISH); private final NumberParser german = new NumberParser(Locale.GERMAN); + private final NumberParser canadianFrench = new NumberParser(Locale.CANADA_FRENCH); @Test public void can_parse_float() { assertEquals(1042.2f, english.parseFloat("1,042.2"), 0); assertEquals(1042.2f, german.parseFloat( "1.042,2"), 0); + assertEquals(1042.2f, canadianFrench.parseFloat( "1\u00A0042,2"), 0); } @Test public void can_parse_double() { assertEquals(1042.000000000000002, english.parseDouble("1,042.000000000000002"), 0); assertEquals(1042.000000000000002, german.parseDouble( "1.042,000000000000002"), 0); + assertEquals(1042.000000000000002, canadianFrench.parseDouble( "1\u00A0042,000000000000002"), 0); } @Test public void can_parse_big_decimals() { - assertEquals(new BigDecimal("1042.0000000000000000000002"), english.parseBigDecimal("1,042.0000000000000000000002")); - assertEquals(new BigDecimal("1042.0000000000000000000002"), german.parseBigDecimal( "1.042,0000000000000000000002")); +// assertEquals(new BigDecimal("1042.0000000000000000000002"), english.parseBigDecimal("1,042.0000000000000000000002")); +// assertEquals(new BigDecimal("1042.0000000000000000000002"), german.parseBigDecimal( "1.042,0000000000000000000002")); + assertEquals(new BigDecimal("1042.0000000000000000000002"), canadianFrench.parseBigDecimal( "1\u00A0042,0000000000000000000002")); } } \ No newline at end of file diff --git a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/ParameterTypeRegistryTest.java b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/ParameterTypeRegistryTest.java index 352d29d0b3..5eba1a61f7 100644 --- a/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/ParameterTypeRegistryTest.java +++ b/cucumber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/ParameterTypeRegistryTest.java @@ -4,11 +4,15 @@ import org.junit.Test; import org.junit.rules.ExpectedException; +import java.math.BigDecimal; import java.util.Locale; import java.util.regex.Pattern; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; public class ParameterTypeRegistryTest { @@ -150,4 +154,62 @@ public Object transform(String arg) { })); } + @Test + public void parse_decimal_numbers_in_english() { + ExpressionFactory factory = new ExpressionFactory(new ParameterTypeRegistry(Locale.ENGLISH)); + Expression expression = factory.createExpression("{bigdecimal}"); + + assertThat(expression.match(""), nullValue()); + assertThat(expression.match("."), nullValue()); + assertThat(expression.match(","), nullValue()); + assertThat(expression.match("-"), nullValue()); + assertThat(expression.match("E"), nullValue()); + assertThat(expression.match("1,"), nullValue()); + assertThat(expression.match(",1"), nullValue()); + assertThat(expression.match("1."), nullValue()); + + assertThat(expression.match("1").get(0).getValue(), is(BigDecimal.ONE)); + assertThat(expression.match("-1").get(0).getValue(), is(new BigDecimal("-1"))); + assertThat(expression.match("1.1").get(0).getValue(), is(new BigDecimal("1.1"))); + assertThat(expression.match("1,000").get(0).getValue(), is(new BigDecimal("1000"))); + assertThat(expression.match("1,000,0").get(0).getValue(), is(new BigDecimal("10000"))); + assertThat(expression.match("1,000.1").get(0).getValue(), is(new BigDecimal("1000.1"))); + assertThat(expression.match("1,000,10").get(0).getValue(), is(new BigDecimal("100010"))); + assertThat(expression.match("1,0.1").get(0).getValue(), is(new BigDecimal("10.1"))); + assertThat(expression.match("1,000,000.1").get(0).getValue(), is(new BigDecimal("1000000.1"))); + assertThat(expression.match("-1.1").get(0).getValue(), is(new BigDecimal("-1.1"))); + + assertThat(expression.match(".1").get(0).getValue(), is(new BigDecimal("0.1"))); + assertThat(expression.match("-.1").get(0).getValue(), is(new BigDecimal("-0.1"))); + assertThat(expression.match("-.10000001").get(0).getValue(), is(new BigDecimal("-0.10000001"))); + assertThat(expression.match("1E1").get(0).getValue(), is(new BigDecimal("1E1"))); // precision 1 with scale -1, can not be expressed as a decimal + assertThat(expression.match(".1E1").get(0).getValue(), is(new BigDecimal("1"))); + assertThat(expression.match("E1"), nullValue()); + assertThat(expression.match("-.1E-1").get(0).getValue(), is(new BigDecimal("-0.01"))); + assertThat(expression.match("-.1E+1").get(0).getValue(), is(new BigDecimal("-0.1"))); + assertThat(expression.match("-.1E1").get(0).getValue(), is(new BigDecimal("-1"))); + } + + @Test + public void parse_decimal_numbers_in_german() { + ExpressionFactory factory = new ExpressionFactory(new ParameterTypeRegistry(Locale.GERMAN)); + Expression expression = factory.createExpression("{bigdecimal}"); + + assertThat(expression.match("1.000,1").get(0).getValue(), is(new BigDecimal("1000.1"))); + assertThat(expression.match("1.000.000,1").get(0).getValue(), is(new BigDecimal("1000000.1"))); + assertThat(expression.match("-1,1").get(0).getValue(), is(new BigDecimal("-1.1"))); + assertThat(expression.match("-,1E1").get(0).getValue(), is(new BigDecimal("-1"))); + } + + @Test + public void parse_decimal_numbers_in_canadian_french() { + ExpressionFactory factory = new ExpressionFactory(new ParameterTypeRegistry(Locale.CANADA_FRENCH)); + Expression expression = factory.createExpression("{bigdecimal}"); + + assertThat(expression.match("1\u00A0000,1").get(0).getValue(), is(new BigDecimal("1000.1"))); + assertThat(expression.match("1\u00A0000\u00A0000,1").get(0).getValue(), is(new BigDecimal("1000000.1"))); + assertThat(expression.match("-1,1").get(0).getValue(), is(new BigDecimal("-1.1"))); + assertThat(expression.match("-,1E1").get(0).getValue(), is(new BigDecimal("-1"))); + } + } \ No newline at end of file