8333755: NumberFormat integer only parsing breaks when format has suffix

Reviewed-by: naoto

Author: Justin Lu

src/java.base/share/classes/java/text/CompactNumberFormat.java

@@ -1724,34 +1724,14 @@ public Number parse(String text, ParsePosition pos) {
         // Call DecimalFormat.subparseNumber() method to parse the
         // number part of the input text
         position = decimalFormat.subparseNumber(text, position,
-                digitList, false, false, status);
+                digitList, false, false, status).fullPos();
 
         if (position == -1) {
             // Unable to parse the number successfully
             pos.index = oldStart;
             pos.errorIndex = oldStart;
             return null;
         }
-
-        // If parse integer only is true and the parsing is broken at
-        // decimal point, then pass/ignore all digits and move pointer
-        // at the start of suffix, to process the suffix part
-        if (isParseIntegerOnly() && position < text.length()
-                && text.charAt(position) == symbols.getDecimalSeparator()) {
-            position++; // Pass decimal character
-            for (; position < text.length(); ++position) {
-                char ch = text.charAt(position);
-                int digit = ch - symbols.getZeroDigit();
-                if (digit < 0 || digit > 9) {
-                    digit = Character.digit(ch, 10);
-                    // Parse all digit characters
-                    if (!(digit >= 0 && digit <= 9)) {
-                        break;
-                    }
-                }
-            }
-        }
-
         // Number parsed successfully; match prefix and
         // suffix to obtain multiplier
         pos.index = position;
@@ -2372,6 +2352,8 @@ public void setGroupingUsed(boolean newValue) {
      * parsed as the value {@code 1234000} (1234 (integer part) * 1000
      * (thousand)) and the fractional part would be skipped.
      * The exact format accepted by the parse operation is locale dependent.
+     * @implSpec This implementation does not set the {@code ParsePosition} index
+     * to the position of the decimal symbol, but rather the end of the string.
      *
      * @return {@code true} if compact numbers should be parsed as integers
      *         only; {@code false} otherwise

src/java.base/share/classes/java/text/DecimalFormat.java

@@ -2150,10 +2150,7 @@ private void append(StringBuffer result, String string,
      *   #getGroupingSize()} is not adhered to
      *   <li> {@link #isGroupingUsed()} returns {@code false}, and the grouping
      *   symbol is found
-     *   <li> {@link #isParseIntegerOnly()} returns {@code true}, and the decimal
-     *   separator is found
-     *   <li> {@link #isGroupingUsed()} returns {@code true} and {@link
-     *   #isParseIntegerOnly()} returns {@code false}, and the grouping
+     *   <li> {@link #isGroupingUsed()} returns {@code true} and the grouping
      *   symbol occurs after the decimal separator
      *   <li> Any other characters are found, that are not the expected symbols,
      *   and are not digits that occur within the numerical portion
@@ -2379,7 +2376,8 @@ private final boolean subparse(String text, ParsePosition parsePosition,
 
         // position will serve as new index when success, otherwise it will
         // serve as errorIndex when failure
-        position = subparseNumber(text, position, digits, true, isExponent, status);
+        NumericPosition pos = subparseNumber(text, position, digits, true, isExponent, status);
+        position = pos.fullPos;
 
         // First character after the prefix was un-parseable, should
         // fail regardless if lenient or strict.
@@ -2422,9 +2420,15 @@ private final boolean subparse(String text, ParsePosition parsePosition,
                 return false;
             }
 
-            // No failures, thus increment the index by the suffix
-            parsePosition.index = position +
-                    (gotPositive ? positiveSuffix.length() : negativeSuffix.length());
+            // When parsing integer only, index should be int pos
+            // If intPos is 0, the entire value was integer
+            if (isParseIntegerOnly() && pos.intPos > 0) {
+                parsePosition.index = pos.intPos;
+            } else {
+                // increment the index by the suffix
+                parsePosition.index = position +
+                        (gotPositive ? positiveSuffix.length() : negativeSuffix.length());
+            }
         } else {
             parsePosition.index = position;
         }
@@ -2437,6 +2441,19 @@ private final boolean subparse(String text, ParsePosition parsePosition,
         return true;
     }
 
+    /**
+     * NumericPosition is a helper record class that stores two indices of interest.
+     * {@code fullPos} is either the first unparseable character or -1 in case
+     * of no valid number parsed. {@code intPos} reflects the position of
+     * a parsed decimal symbol, if one exists. When parsing with {@code isParseIntegerOnly()},
+     * {@code fullPos} is used to match the suffix, and reset the {@code ParsePosition}
+     * index to {@code intPos}.
+     *
+     * @param fullPos an index that reflects the full traversal of the numerical String
+     * @param intPos an index that reflects the position of a parsed decimal symbol.
+     */
+    record NumericPosition(int fullPos, int intPos) {}
+
     /**
      * Parses a number from the given {@code text}. The text is parsed
      * beginning at {@code position}, until an unparseable character is seen.
@@ -2449,14 +2466,15 @@ private final boolean subparse(String text, ParsePosition parsePosition,
      * @param status upon return contains boolean status flags indicating
      *               whether the value is infinite and whether it is
      *               positive
-     * @return returns the position of the first unparseable character or
-     *         -1 in case of no valid number parsed
+     * @return returns a {@code NumericPosition} that stores both a full
+     *         traversal index, and an int only index.
      */
-    int subparseNumber(String text, int position,
-                       DigitList digits, boolean checkExponent,
-                       boolean isExponent, boolean[] status) {
+    NumericPosition subparseNumber(String text, int position,
+                                   DigitList digits, boolean checkExponent,
+                                   boolean isExponent, boolean[] status) {
         // process digits or Inf, find decimal position
         status[STATUS_INFINITE] = false;
+        int intIndex = 0;
         if (!isExponent && text.regionMatches(position, symbols.getInfinity(), 0,
                 symbols.getInfinity().length())) {
             position += symbols.getInfinity().length();
@@ -2516,7 +2534,7 @@ int subparseNumber(String text, int position,
                 if (parseStrict && isGroupingUsed() && position == startPos + groupingSize
                         && prevSeparatorIndex == -groupingSize && !sawDecimal
                         && digit >= 0 && digit <= 9) {
-                    return position;
+                    return new NumericPosition(position, intIndex);
                 }
 
                 if (digit == 0) {
@@ -2538,37 +2556,44 @@ int subparseNumber(String text, int position,
                         --digits.decimalAt;
                     } else {
                         ++digitCount;
-                        digits.append((char)(digit + '0'));
+                        if (!sawDecimal || !isParseIntegerOnly()) {
+                            digits.append((char)(digit + '0'));
+                        }
                     }
                 } else if (digit > 0 && digit <= 9) { // [sic] digit==0 handled above
                     sawDigit = true;
                     ++digitCount;
-                    digits.append((char)(digit + '0'));
+                    if (!sawDecimal || !isParseIntegerOnly()) {
+                        digits.append((char) (digit + '0'));
+                    }
 
                     // Cancel out backup setting (see grouping handler below)
                     backup = -1;
                 } else if (!isExponent && ch == decimal) {
                     // Check grouping size on decimal separator
                     if (parseStrict && isGroupingViolation(position, prevSeparatorIndex)) {
-                        return groupingViolationIndex(position, prevSeparatorIndex);
+                        return new NumericPosition(
+                                groupingViolationIndex(position, prevSeparatorIndex), intIndex);
                     }
                     // If we're only parsing integers, or if we ALREADY saw the
                     // decimal, then don't parse this one.
-                    if (isParseIntegerOnly() || sawDecimal) {
+                    if (sawDecimal) {
                         break;
                     }
+                    intIndex = position;
                     digits.decimalAt = digitCount; // Not digits.count!
                     sawDecimal = true;
                 } else if (!isExponent && ch == grouping && isGroupingUsed()) {
                     if (parseStrict) {
                         // text should not start with grouping when strict
                         if (position == startPos) {
-                            return startPos;
+                            return new NumericPosition(startPos, intIndex);
                         }
                         // when strict, fail if grouping occurs after decimal OR
                         // current group violates grouping size
                         if (sawDecimal || (isGroupingViolation(position, prevSeparatorIndex))) {
-                            return groupingViolationIndex(position, prevSeparatorIndex);
+                            return new NumericPosition(
+                                    groupingViolationIndex(position, prevSeparatorIndex), intIndex);
                         }
                         prevSeparatorIndex = position; // track previous
                     } else {
@@ -2621,7 +2646,8 @@ int subparseNumber(String text, int position,
                     // "1,234%" and "1,234" both end with pos = 5, since '%' breaks
                     // the loop before incrementing position. In both cases, check
                     // should be done at pos = 4
-                    return groupingViolationIndex(position - 1, prevSeparatorIndex);
+                    return new NumericPosition(
+                            groupingViolationIndex(position - 1, prevSeparatorIndex), intIndex);
                 }
             }
 
@@ -2636,8 +2662,9 @@ int subparseNumber(String text, int position,
                 digits.decimalAt = digitCount; // Not digits.count!
             }
 
-            // Adjust for exponent, if any
-            if (exponent != 0) {
+            // If parsing integer only, adjust exponent if it occurs
+            // in integer portion, otherwise ignore it
+            if (!sawDecimal || !isParseIntegerOnly()) {
                 digits.decimalAt = shiftDecimalAt(digits.decimalAt, exponent);
             }
 
@@ -2646,10 +2673,10 @@ int subparseNumber(String text, int position,
             // parse "$" with pattern "$#0.00". (return index 0 and error
             // index 1).
             if (!sawDigit && digitCount == 0) {
-                return -1;
+                return new NumericPosition(-1, intIndex);
             }
         }
-        return position;
+        return new NumericPosition(position, intIndex);
     }
 
     // Calculate the final decimal position based off the exponent value
@@ -2917,7 +2944,8 @@ public int getMultiplier () {
      * have '{@code U+2030}'.
      *
      * <P>Example: with multiplier 100, 1.23 is formatted as "123", and
-     * "123" is parsed into 1.23.
+     * "123" is parsed into 1.23. If {@code isParseIntegerOnly()} returns {@code true},
+     * "123" is parsed into 1.
      *
      * @param newValue the new multiplier
      * @see #getMultiplier

src/java.base/share/classes/java/text/NumberFormat.java

@@ -468,12 +468,11 @@ public Number parse(String source) throws ParseException {
     }
 
     /**
-     * Returns true if this format will parse numbers as integers only.
+     * Returns {@code true} if this format will parse numbers as integers only.
+     * The {@code ParsePosition} index will be set to the position of the decimal
+     * symbol. The exact format accepted by the parse operation is locale dependent.
      * For example in the English locale, with ParseIntegerOnly true, the
-     * string "1234." would be parsed as the integer value 1234 and parsing
-     * would stop at the "." character.  Of course, the exact format accepted
-     * by the parse operation is locale dependent and determined by sub-classes
-     * of NumberFormat.
+     * string "123.45" would be parsed as the integer value 123.
      *
      * @return {@code true} if numbers should be parsed as integers only;
      *         {@code false} otherwise

test/jdk/java/text/Format/NumberFormat/BigDecimalParse.java

@@ -23,14 +23,18 @@
 
 /*
  * @test
- * @bug 4018937 8008577 8174269
+ * @bug 4018937 8008577 8174269 8333755
  * @summary Confirm that methods which are newly added to support BigDecimal and BigInteger work as expected.
  * @run junit/othervm BigDecimalParse
  */
 
 import java.math.BigDecimal;
-import java.text.*;
-import java.util.*;
+import java.text.DecimalFormat;
+import java.text.Format;
+import java.text.MessageFormat;
+import java.text.NumberFormat;
+import java.text.ParsePosition;
+import java.util.Locale;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.BeforeAll;
@@ -543,23 +547,23 @@ public void test_Parse_in_MessageFormat_NotParseIntegerOnly() {
     static final int[][] parsePosition2 = {     // {errorIndex, index}
         /*
          * Should keep in mind that the expected result is different from
-         * DecimalFormat.parse() for some cases.
+         * DecimalFormat.parse() for some cases. This is because parsing integer
+         * only will return a successful parse for the subformat, but since the index
+         * returned is not equal to the length, at the MessageFormat level, this
+         * will be interpreted as a failed parse, and so the DecimalFormat index
+         * should be reflected as the MessageFormat errorIndex.
          */
         {28, 0},        // parsing stopped at '.'
         {29, 0},        // parsing stopped at '.'
         {29, 0},        // parsing stopped at '.'
-        {2, 0},         // parsing stopped at '(' because cannot find ')'
-        {2, 0},         // parsing stopped at the first numeric
-                        // because cannot find '%'
-        {2, 0},         // parsing stopped at the first numeric
-                        // because cannot find '%'
+        {30, 0},        // parsing stopped at '.'
+        {31, 0},        // parsing stopped at '.'
+        {32, 0},        // parsing stopped at '.'
         {28, 0},        // parsing stopped at '.'
         {29, 0},        // parsing stopped at '.'
-
         {-1, 57}, {-1, 58}, {-1, 59}, {-1, 61},
         {56, 0},        // parsing stopped at '.'
-                        // because cannot find '%'
-        {2, 0},         // parsing stopped at '(' because cannot find ')'
+        {57, 0},        // parsing stopped at '.'
         {-1, 60}, {-1, 61},
         {28, 0},        // parsing stopped at '.'
         {-1, 88},