[MNG-8241] Handle non-BMP characters when comparing versions

[elharo]

This PR takes the path of treating all non-ASCII digits as non-numeric. Alternately we could keep the existing behavior for all BMP digits and also treat non-BMP digits as numeric. But we probably shouldn't treat BMP digits and non-BMP digits differently.

[gnodet]

It would make sense to also fix the behavior of the resolver related class:
https://github.com/apache/maven-resolver/blob/fa639fe1e76abc774d5ffd298c3dfa501cf305fc/maven-resolver-util/src/main/java/org/eclipse/aether/util/version/GenericVersion.java#L230-L258
A resolver release is planned very soon, and the whole Maven 4 API uses this class rather than the one in maven-artifact.

[gnodet]

The messages is wrong

[gnodet]

and that one too

[gnodet]

I'm not sure why not. It seems to me that later, the string will be parsed using Integer.parseInt, Long.parseLong, or new BigInteger(), and AFAIK, they all leverage the Character.digit(char, int) which should support everything that is considered a digit by Character.isDigit(char) (but not Character.isDigit(int)).
I think this has the same effect as we're only considering radix-10 when parsing.
An alternative to support supplemental characters would be to normalise those in a buffer while they are read in this loop and pass on the buffer rather than a substring.

[gnodet]

Something like the following maybe ?

@SuppressWarnings("checkstyle:innerassignment")
public final void parseVersion(String version) {
    this.value = version;

    items = new ListItem();

    version = version.toLowerCase(Locale.ENGLISH);

    ListItem list = items;

    Deque<Item> stack = new ArrayDeque<>();
    stack.push(list);

    boolean isDigit = false;

    boolean isCombination = false;

    int startIndex = 0;

    StringBuilder normalizedBuffer = new StringBuilder();

    for (int i = 0; i < version.length(); i++) {
        char character = version.charAt(i);
        int c = character;

        // Handle high surrogate
        if (Character.isHighSurrogate(character)) {
            try {
                char low = version.charAt(i + 1);
                char[] both = {character, low};
                c = Character.codePointAt(both, 0);
                i++;
            } catch (IndexOutOfBoundsException ex) {
                // Treat high surrogate without low surrogate as a regular character
            }
        }

        // Normalize the Unicode digits (including supplemental plane characters) using Character.digit()
        if (Character.isDigit(c)) {
            // Normalize Unicode digits to ASCII digits
            int normalizedDigit = Character.digit(c, 10);
            if (normalizedDigit != -1) {
                c = normalizedDigit + '0'; // Convert digit to its ASCII equivalent
            }
        }

        // Build the normalized substring on the fly
        if (c == '.' || c == '-' || i == version.length() - 1) {
            // If we've reached a separator or the end of the string, process the current segment
            if (i == startIndex && c != '-') {
                list.add(IntItem.ZERO);  // Handle empty sections
            } else {
                // Add the current segment to the list after normalizing it
                list.add(parseItem(isCombination, isDigit, normalizedBuffer.toString()));
            }
            isCombination = false;
            normalizedBuffer.setLength(0);  // Reset the buffer for the next segment
            startIndex = i + 1;

            // Handle separators
            if (c == '-' && !list.isEmpty()) {
                list.add(list = new ListItem());
                stack.push(list);
            }
        } else if (c >= '0' && c <= '9') {
            if (!isDigit && i > startIndex) {
                // Handle combination of a letter and number, like "X1"
                isCombination = true;
                if (!list.isEmpty()) {
                    list.add(list = new ListItem());
                    stack.push(list);
                }
            }
            isDigit = true;
        } else {
            if (isDigit && i > startIndex) {
                list.add(parseItem(isCombination, true, normalizedBuffer.toString()));
                normalizedBuffer.setLength(0);  // Reset buffer
                startIndex = i;

                list.add(list = new ListItem());
                stack.push(list);
                isCombination = false;
            }

            isDigit = false;
        }

        // Append the current character to the normalized buffer
        normalizedBuffer.append((char) c);
    }

    if (version.length() > startIndex) {
        // Final segment (if any)
        if (!isDigit && !list.isEmpty()) {
            list.add(list = new ListItem());
            stack.push(list);
        }

        list.add(parseItem(isCombination, isDigit, normalizedBuffer.toString()));
    }

    while (!stack.isEmpty()) {
        list = (ListItem) stack.pop();
        list.normalize();
    }
}

[elharo]

So, just to be clear you prefer the alternate approach of treating all Unicode digits as version numbers? As long as they're all base 10 this should work. Looking at Unicode details, there are some non-base 10 digits. They have a different Unicode character class, so I need to check what Character.isDigit is doing; i..e whether it's checking for decimal digits or all digits. If it is checking for all digits, I need to check the character class directly. To be specific we want De, possibly Di, but not Nu character classes.

[elharo]

OK, looks like Character.isDigit is checking specifically for decimal digits, not non-decimal digits

[elharo]

Ran into a problem with that approach. It treats different versions of digits as the same. E.g. 7 is the same as Arabic-Indic digit 7 even though they are different characters.

I can see an argument for normalizing all these digits to ASCII digits for Maven purposes, but that would touch a lot of different places in the code and ecosystem including things like file names and artifact URLs in Maven Central.
I'm going to think about this a little more, but for the moment it feels safer to treat all the non-ASCII digits as distinct non-numeric characters.

[gnodet]

Only supporting ASCII digits sounds fine to me too.

[elharo]

Probably worth filing a related bug for the resolver issues.