Just say no to strings

Author: eggrobin

UnicodeJsps/src/main/java/org/unicode/jsp/UnicodeSetUtilities.java

@@ -7,6 +7,7 @@
 import com.ibm.icu.text.UTF16.StringComparator;
 import com.ibm.icu.text.UnicodeSet;
 import com.ibm.icu.util.ULocale;
+import com.ibm.icu.util.VersionInfo;
 import java.text.ParsePosition;
 import java.util.Comparator;
 import java.util.List;
@@ -294,11 +295,15 @@ private boolean applyPropertyAlias0(
                     if (isAge) {
                         set =
                                 prop.getSet(
-                                        new UnicodePropertySymbolTable.ComparisonMatcher(
-                                                propertyValue,
+                                        new UnicodePropertySymbolTable.ComparisonMatcher<
+                                                VersionInfo>(
+                                                UnicodePropertySymbolTable.parseVersionInfoOrMax(
+                                                        propertyValue),
                                                 UnicodePropertySymbolTable.Relation.geq,
-                                                UnicodePropertySymbolTable
-                                                        .VERSION_STRING_COMPARATOR));
+                                                Comparator.nullsFirst(Comparator.naturalOrder()),
+                                                (s) ->
+                                                        UnicodePropertySymbolTable
+                                                                .parseVersionInfoOrMax(s)));
                     } else {
                         if (prop.getName().equals("General_Category")) {
                             for (String[] coarseValue : COARSE_GENERAL_CATEGORIES) {

unicodetools/src/main/java/org/unicode/props/UnicodePropertySymbolTable.java

@@ -12,6 +12,7 @@
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
+import java.util.function.Function;
 import org.unicode.props.UnicodeProperty.PatternMatcher;
 
 /**
@@ -201,10 +202,14 @@ public boolean applyPropertyAlias0(
                 if (isAge) {
                     set =
                             prop.getSet(
-                                    new ComparisonMatcher(
-                                            propertyValue,
+                                    new ComparisonMatcher<VersionInfo>(
+                                            UnicodePropertySymbolTable.parseVersionInfoOrMax(
+                                                    propertyValue),
                                             Relation.geq,
-                                            VERSION_STRING_COMPARATOR));
+                                            Comparator.nullsFirst(Comparator.naturalOrder()),
+                                            (s) ->
+                                                    UnicodePropertySymbolTable
+                                                            .parseVersionInfoOrMax(s)));
                 } else {
                     set = prop.getSet(propertyValue);
                 }
@@ -244,20 +249,26 @@ public enum Relation {
         greater
     }
 
-    public static class ComparisonMatcher implements PatternMatcher {
+    public static class ComparisonMatcher<T> implements PatternMatcher {
         final Relation relation;
-        final Comparator<String> comparator;
-        String pattern;
+        final Comparator<T> comparator;
+        final Function<String, T> parser;
+        T expected;
 
-        public ComparisonMatcher(String pattern, Relation relation, Comparator<String> comparator) {
+        public ComparisonMatcher(
+                T expected,
+                Relation relation,
+                Comparator<T> comparator,
+                Function<String, T> parser) {
             this.relation = relation;
-            this.pattern = pattern;
+            this.expected = expected;
             this.comparator = comparator;
+            this.parser = parser;
         }
 
         @Override
         public boolean test(String value) {
-            int comp = comparator.compare(pattern, value);
+            int comp = comparator.compare(expected, parser.apply(value));
             switch (relation) {
                 case less:
                     return comp < 0;
@@ -274,47 +285,19 @@ public boolean test(String value) {
 
         @Override
         public PatternMatcher set(String pattern) {
-            this.pattern = pattern;
+            this.expected = parser.apply(pattern);
             return this;
         }
     }
 
-    /**
-     * Special parser for version strings. Anything not parsable is higher than everything
-     * parseable.
-     */
-    public static final Comparator<String> VERSION_STRING_COMPARATOR =
-            new Comparator<String>() {
-
-                @Override
-                public int compare(String o1, String o2) {
-                    if (o1 == o2) {
-                        return 0;
-                    } else if (o1 == null) {
-                        return -1;
-                    } else if (o2 == null) {
-                        return 1;
-                    } else {
-                        boolean o1Valid = true;
-                        boolean o2Valid = true;
-                        VersionInfo v1 = null;
-                        VersionInfo v2 = null;
-                        try {
-                            v1 = VersionInfo.getInstance(o1);
-                        } catch (IllegalArgumentException e) {
-                            o1Valid = false;
-                        }
-                        try {
-                            v2 = VersionInfo.getInstance(o2);
-                        } catch (IllegalArgumentException e) {
-                            o2Valid = false;
-                        }
-                        if (o1Valid && o2Valid) {
-                            return v1.compareTo(v2);
-                        } else {
-                            return o2Valid ? 1 : o1Valid ? -1 : o1.compareTo(o2);
-                        }
-                    }
-                }
-            };
+    public static VersionInfo parseVersionInfoOrMax(String s) {
+        if (s == null) {
+            return null;
+        }
+        try {
+            return VersionInfo.getInstance(s);
+        } catch (IllegalArgumentException e) {
+            return VersionInfo.getInstance(255, 255, 255, 255);
+        }
+    }
 }

unicodetools/src/test/java/org/unicode/unittest/TestUnicodeSet.java

@@ -2,6 +2,8 @@
 
 import com.ibm.icu.impl.UnicodeRegex;
 import com.ibm.icu.text.UnicodeSet;
+import com.ibm.icu.util.VersionInfo;
+import java.util.Comparator;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import org.junit.jupiter.api.Test;
@@ -48,12 +50,18 @@ private void checkOrder(String d1, String d2, Comparison expected) {
                 d1 + " ?< " + d2,
                 expected,
                 Comparison.fromCompareResult(
-                        UnicodePropertySymbolTable.VERSION_STRING_COMPARATOR.compare(d1, d2)));
+                        Comparator.nullsFirst(Comparator.<VersionInfo>naturalOrder())
+                                .compare(
+                                        UnicodePropertySymbolTable.parseVersionInfoOrMax(d1),
+                                        UnicodePropertySymbolTable.parseVersionInfoOrMax(d2))));
         assertEquals(
                 d2 + " ?< " + d1,
                 expected.opposite(),
                 Comparison.fromCompareResult(
-                        UnicodePropertySymbolTable.VERSION_STRING_COMPARATOR.compare(d2, d1)));
+                        Comparator.nullsFirst(Comparator.<VersionInfo>naturalOrder())
+                                .compare(
+                                        UnicodePropertySymbolTable.parseVersionInfoOrMax(d2),
+                                        UnicodePropertySymbolTable.parseVersionInfoOrMax(d1))));
     }
 
     @Test