Skip to content

Commit

Permalink
cucumber-expressions: Improve decimal number parsing
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@mpkorstanje
mpkorstanje committed Aug 3, 2019
1 parent 0e63c88 commit f5b4897
Show file tree
Hide file tree
Showing 7 changed files with 120 additions and 18 deletions.
15 changes: 12 additions & 3 deletions cucumber-expressions/java/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>4.13-beta-3</version>
<version>4.12</version>
<scope>test</scope>
</dependency>

Expand All @@ -46,11 +46,20 @@
<version>2.8.5</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-core</artifactId>
<version>2.1</version>
</dependency>
<dependency>
<groupId>org.hamcrest</groupId>
<artifactId>hamcrest-library</artifactId>
<version>2.1</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.9.9.1</version>
<version>2.9.9.2</version>
<scope>test</scope>
</dependency>

Expand Down
3 changes: 1 addition & 2 deletions ...sions/java/src/main/java/io/cucumber/cucumberexpressions/BuiltInParameterTransformer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
41 changes: 32 additions & 9 deletions ...expressions/java/src/main/java/io/cucumber/cucumberexpressions/ParameterTypeRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<String> INTEGER_REGEXPS = asList(Pattern.compile("-?\\d+").pattern(), Pattern.compile("\\d+").pattern());
private static final List<String> FLOAT_REGEXPS = singletonList(Pattern.compile("-?\\d*(?:[.,]\\d+)?").pattern());
private static final List<String> WORD_REGEXPS = singletonList(Pattern.compile("[^\\s]+").pattern());
private static final List<String> STRING_REGEXPS = singletonList(Pattern.compile("\"([^\"\\\\]*(\\\\.[^\"\\\\]*)*)\"|'([^'\\\\]*(\\\\.[^'\\\\]*)*)'").pattern());
private static final List<String> 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<String> WORD_REGEXPS = singletonList(
Pattern.compile("[^\\s]+").pattern()
);
private static final List<String> STRING_REGEXPS = asList(
Pattern.compile("\"([^\"\\\\]*(\\\\.[^\"\\\\]*)*)\"").pattern(),
Pattern.compile("'([^'\\\\]*(\\\\.[^'\\\\]*)*)'").pattern()
);
private static final String ANONYMOUS_REGEX = Pattern.compile(".*").pattern();
private final Map<String, ParameterType<?>> parameterTypeByName = new HashMap<>();
private final Map<String, SortedSet<ParameterType<?>>> parameterTypesByRegexp = new HashMap<>();
Expand All @@ -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<String> 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<BigInteger>() {
@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<BigDecimal>() {
defineParameterType(new ParameterType<>("bigdecimal", localizedFloatRegexp, BigDecimal.class, new Transformer<BigDecimal>() {
@Override
public BigDecimal transform(String arg) throws Throwable {
return (BigDecimal) internalParameterTransformer.transform(arg, BigDecimal.class);
Expand Down Expand Up @@ -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<Float>() {
defineParameterType(new ParameterType<>("float", localizedFloatRegexp, Float.class, new Transformer<Float>() {
@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<Double>() {
defineParameterType(new ParameterType<>("double", localizedFloatRegexp, Double.class, new Transformer<Double>() {
@Override
public Double transform(String arg) throws Throwable {
return (Double) internalParameterTransformer.transform(arg, Double.class);
Expand Down
2 changes: 1 addition & 1 deletion cucumber-expressions/java/src/main/java/io/cucumber/cucumberexpressions/TreeRegexp.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
7 changes: 6 additions & 1 deletion ...ons/java/src/test/java/io/cucumber/cucumberexpressions/CucumberExpressionPatternTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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$"
);
}

Expand Down
8 changes: 6 additions & 2 deletions ...mber-expressions/java/src/test/java/io/cucumber/cucumberexpressions/NumberParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
}
}
62 changes: 62 additions & 0 deletions ...essions/java/src/test/java/io/cucumber/cucumberexpressions/ParameterTypeRegistryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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")));
}

}

0 comments on commit f5b4897

Please sign in to comment.