Skip to content

Commit 0daf9df

Browse files
committed
[bugfix] Correct the determination of regular vs non-regular integer-part-grouping-positions.
All W3C QT3 format-number tests are now passing except: 1. `numberformat121` and `numberformat122` which raise a java.lang.StackOverflowError due to a lack of tail-call-optimisation 2. `numberformat127` and `numberformat128` which appear to be incorrect, see: w3c/qt3tests#73
1 parent 7bcff61 commit 0daf9df

File tree

1 file changed

+28
-16
lines changed

1 file changed

+28
-16
lines changed

exist-core/src/main/java/org/exist/xquery/functions/fn/FnFormatNumbers.java

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,10 @@ private Tuple2<SubPicture, Optional<SubPicture>> analyzePictureString(final Deci
331331
analyzePassiveChar(decimalFormat, c, capturePrefix, subPicture);
332332
}
333333

334+
if (state == AnalyzeState.INTEGER_PART) {
335+
subPicture.incrementIntegerPartExtent();
336+
}
337+
334338
break; // end of INTEGER_PART
335339

336340

@@ -796,6 +800,8 @@ private String format(final NumericValue number, final DecimalFormat decimalForm
796800
* See https://www.w3.org/TR/xpath-functions-31/#analyzing-picture-string
797801
*/
798802
private static class SubPicture {
803+
private int integerPartStartIdx = 0;
804+
private int integerPartLength = 0;
799805
private int[] integerPartGroupingPositions;
800806
private int minimumIntegerPartSize;
801807
private int scalingFactor;
@@ -815,6 +821,8 @@ private static class SubPicture {
815821
public SubPicture copy() {
816822
final SubPicture copy = new SubPicture();
817823

824+
copy.integerPartStartIdx = integerPartStartIdx;
825+
copy.integerPartLength = integerPartLength;
818826
copy.integerPartGroupingPositions = integerPartGroupingPositions == null ? null : Arrays.copyOf(integerPartGroupingPositions, integerPartGroupingPositions.length);
819827
copy.minimumIntegerPartSize = minimumIntegerPartSize;
820828
copy.scalingFactor = scalingFactor;
@@ -865,7 +873,7 @@ public void incrementIntegerPartGroupingPosition() {
865873
* @return the value of G if regular, or -1 if irregular
866874
*/
867875
public int integerPartGroupingPositionsAreRegular() {
868-
// There is an least one grouping-separator in the integer part of the sub-picture.
876+
// There is at least one grouping-separator in the integer part of the sub-picture.
869877
if (integerPartGroupingPositions.length > 0) {
870878

871879
// There is a positive integer G (the grouping size) such that the position of every grouping-separator
@@ -891,23 +899,23 @@ public int integerPartGroupingPositionsAreRegular() {
891899
return -1;
892900
}
893901

894-
// Every position in the integer part of the sub-picture that is a positive integer multiple of G is
895-
// occupied by a grouping-separator.
896-
final int largestGroupPosition = integerPartGroupingPositions[integerPartGroupingPositions.length - 1];
897-
int m = 2;
898-
for (int p = g; p <= largestGroupPosition; p = g * m++) {
902+
// Check that every position in the integer part of the sub-picture that is a positive integer multiple
903+
// of G is occupied by a grouping-separator.
904+
// We can test this by determining if the leftmost group (the group to the left of the leftmost
905+
// separator) is not larger than G.
899906

900-
boolean isGroupSeparator = false;
901-
for (final int integerPartGroupingPosition : integerPartGroupingPositions) {
902-
if (integerPartGroupingPosition == p) {
903-
isGroupSeparator = true;
904-
break;
905-
}
906-
}
907+
// Calculate total active characters: integerPartLength includes all characters (digits + separators),
908+
// so we subtract the number of separators to get just the active characters.
909+
final int totalActiveCharacters = integerPartLength - integerPartGroupingPositions.length;
907910

908-
if (!isGroupSeparator) {
909-
return -1;
910-
}
911+
// The leftmost separator is always at index 0, and its numberOfCharacters tells us how many active
912+
// characters are to the right of it. Therefore, the leftmost group size is the remaining characters.
913+
final int leftmostGroupSize = totalActiveCharacters - integerPartGroupingPositions[0];
914+
915+
// If the leftmost group is larger than G, it means that there should have been another separator
916+
// within it (at position G from the right of the leftmost group), but there isn't... so it's irregular!
917+
if (leftmostGroupSize > g) {
918+
return -1;
911919
}
912920

913921
return g;
@@ -916,6 +924,10 @@ public int integerPartGroupingPositionsAreRegular() {
916924
return -1;
917925
}
918926

927+
public void incrementIntegerPartExtent() {
928+
integerPartLength++;
929+
}
930+
919931
public void incrementMinimumIntegerPartSize() {
920932
minimumIntegerPartSize++;
921933
}

0 commit comments

Comments
 (0)