Skip to content

Commit a7fd0e0

Browse files
committed
ICU-23265 Fixed parseMeasureUnitOption() so that it correctly handles measurement unit alises.
1 parent d5db803 commit a7fd0e0

File tree

8 files changed

+109
-21
lines changed

8 files changed

+109
-21
lines changed

icu4c/source/i18n/measunit.cpp

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2767,16 +2767,21 @@ StringEnumeration* MeasureUnit::getAvailableTypes(UErrorCode &errorCode) {
27672767
}
27682768

27692769
bool MeasureUnit::validateAndGet(StringPiece type, StringPiece subtype, MeasureUnit &result) {
2770-
// Find the type index using binary search
2770+
// Find the type and subtype indices using binary search
27712771
int32_t typeIdx = binarySearch(gTypes, 0, UPRV_LENGTHOF(gTypes), type);
2772-
if (typeIdx == -1) {
2773-
return false; // Type not found
2774-
}
2772+
int32_t subtypeIdx = typeIdx >= 0 ? binarySearch(typeIdx, subtype) : -1;
27752773

2776-
// Find the subtype within the type's range using binary search
2777-
int32_t subtypeIdx = binarySearch(typeIdx, subtype);
2778-
if (subtypeIdx == -1) {
2779-
return false; // Subtype not found
2774+
if (typeIdx >= 0 && subtypeIdx < 0) {
2775+
// if we did find the type, but didn't find the subtype, that might be because the sybtype name
2776+
// is an alias-- try using MeasureUnit::forIdentifier(), which will resolve aliases
2777+
UErrorCode localStatus = U_ZERO_ERROR;
2778+
MeasureUnit tempUnit = MeasureUnit::forIdentifier(subtype, localStatus);
2779+
if (U_SUCCESS(localStatus) && uprv_strcmp(type.data(), tempUnit.getType()) == 0) {
2780+
subtypeIdx = tempUnit.fSubTypeId + gOffsets[tempUnit.fTypeId];
2781+
}
2782+
}
2783+
if (typeIdx < 0 || subtypeIdx < 0) {
2784+
return false;
27802785
}
27812786

27822787
// Create the MeasureUnit and return it

icu4c/source/i18n/number_skeletons.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1076,7 +1076,7 @@ void blueprint_helpers::parseMeasureUnitOption(const StringSegment& segment, Mac
10761076
if (MeasureUnit::validateAndGet(type.toStringPiece(), subType.toStringPiece(), unit)) {
10771077
macros.unit = unit;
10781078
return;
1079-
}
1079+
}
10801080

10811081
status = U_NUMBER_SKELETON_SYNTAX_ERROR;
10821082
}

icu4c/source/test/depstest/dependencies.txt

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -892,7 +892,7 @@ library: i18n
892892
formatting formattable_cnv regex regex_cnv translit
893893
double_conversion number_representation number_output numberformatter
894894
number_skeletons number_usageprefs numberparser
895-
units_extra unitsformatter
895+
units unitsformatter
896896
universal_time_scale
897897
uclean_i18n
898898
display_options
@@ -1057,7 +1057,7 @@ group: number_skeletons
10571057
number_skeletons.o number_capi.o number_asformat.o numrange_capi.o
10581058
deps
10591059
numberformatter
1060-
units_extra
1060+
units
10611061

10621062
group: number_symbolswrapper
10631063
number_symbolswrapper.o
@@ -1128,21 +1128,16 @@ group: sharedbreakiterator
11281128
deps
11291129
breakiterator
11301130

1131-
group: units_extra
1132-
measunit_extra.o
1133-
deps
1134-
units bytestriebuilder bytestrie resourcebundle uclean_i18n
1135-
double_conversion
1136-
11371131
group: units
1138-
measunit.o currunit.o
1132+
measunit.o currunit.o measunit_extra.o
11391133
deps
1140-
stringenumeration errorcode
1134+
stringenumeration errorcode bytestriebuilder bytestrie resourcebundle uclean_i18n
1135+
double_conversion
11411136

11421137
group: unitsformatter
11431138
units_data.o units_converter.o units_complexconverter.o units_router.o
11441139
deps
1145-
resourcebundle units_extra double_conversion number_representation formattable sort
1140+
resourcebundle units double_conversion number_representation formattable sort
11461141
number_rounding
11471142

11481143
group: decnumber

icu4c/source/test/intltest/numbertest.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,6 +306,7 @@ class NumberSkeletonTest : public IntlTest {
306306
void perUnitToSkeleton();
307307
void measurementSystemOverride();
308308
void longSkeletonCrash();
309+
void unitAliases();
309310

310311
void runIndexedTest(int32_t index, UBool exec, const char*& name, char* par = nullptr) override;
311312

icu4c/source/test/intltest/numbertest_skeletons.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ void NumberSkeletonTest::runIndexedTest(int32_t index, UBool exec, const char*&
3636
TESTCASE_AUTO(perUnitToSkeleton);
3737
TESTCASE_AUTO(measurementSystemOverride);
3838
TESTCASE_AUTO(longSkeletonCrash);
39+
TESTCASE_AUTO(unitAliases);
3940
TESTCASE_AUTO_END;
4041
}
4142

@@ -581,4 +582,31 @@ void NumberSkeletonTest::longSkeletonCrash() {
581582
UnlocalizedNumberFormatter nf = NumberFormatter::forSkeleton(skeleton, err);
582583
}
583584

585+
void NumberSkeletonTest::unitAliases() {
586+
IcuTestErrorCode status(*this, "unitAliases");
587+
588+
struct TestCase {
589+
const char16_t* skeleton;
590+
const char16_t* expectedResult;
591+
} testCases[] = {
592+
{ u"measure-unit/concentr-part-per-1e6", u"3.14 ppm" },
593+
{ u"measure-unit/concentr-part-per-million", u"3.14 ppm" },
594+
{ u"measure-unit/concentr-permillion", u"3.14 ppm" },
595+
{ u"measure-unit/concentr-milligram-ofglucose-per-deciliter", u"3.14 mg/dL" },
596+
{ u"measure-unit/concentr-milligram-per-deciliter", u"3.14 mg/dL" },
597+
{ u"measure-unit/mass-tonne", u"3.14 t" },
598+
{ u"measure-unit/mass-metric-ton", u"3.14 t" },
599+
};
600+
601+
for (const auto& testCase : testCases) {
602+
LocalizedNumberFormatter nf = NumberFormatter::forSkeleton(testCase.skeleton, status).locale(Locale::getUS());
603+
UnicodeString actualResult = nf.formatDouble(3.14, status).toString(status);
604+
605+
status.setScope(testCase.skeleton);
606+
if (!status.errIfFailureAndReset()) {
607+
assertEquals(testCase.skeleton, testCase.expectedResult, actualResult);
608+
}
609+
}
610+
}
611+
584612
#endif /* #if !UCONFIG_NO_FORMATTING */

icu4j/main/core/src/main/java/com/ibm/icu/number/NumberSkeletonImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1094,11 +1094,12 @@ private static void parseMeasureUnitOption(StringSegment segment, MacroProps mac
10941094
}
10951095
String type = segment.subSequence(0, firstHyphen).toString();
10961096
String subType = segment.subSequence(firstHyphen + 1, segment.length()).toString();
1097-
MeasureUnit unit = MeasureUnit.getUnit(type, subType);
1097+
MeasureUnit unit = MeasureUnit.validateAndGet(type, subType);
10981098
if (unit != null) {
10991099
macros.unit = unit;
11001100
return;
11011101
}
1102+
11021103
throw new SkeletonSyntaxException("Unknown measure unit", segment);
11031104
}
11041105

icu4j/main/core/src/main/java/com/ibm/icu/util/MeasureUnit.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,23 @@ public static MeasureUnit getUnit(String type, String subtype) {
890890
return units != null ? units.get(subtype) : null;
891891
}
892892

893+
@Deprecated
894+
public static MeasureUnit validateAndGet(String type, String subtype) {
895+
MeasureUnit result = MeasureUnit.getUnit(type, subtype);
896+
897+
if (result == null) {
898+
try {
899+
result = MeasureUnit.forIdentifier(subtype);
900+
if (!result.getType().equals(type)) {
901+
result = null;
902+
}
903+
} catch (IllegalArgumentException e) {
904+
// leave result as null
905+
}
906+
}
907+
return result;
908+
}
909+
893910
static final UnicodeSet ASCII = new UnicodeSet('a', 'z').freeze();
894911
static final UnicodeSet ASCII_HYPHEN_DIGITS =
895912
new UnicodeSet('-', '-', '0', '9', 'a', 'z').freeze();

icu4j/main/core/src/test/java/com/ibm/icu/dev/test/number/NumberSkeletonTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,4 +549,45 @@ public void measurementSystemOverride() {
549549
"Wrong result: " + languageTag + ":" + skeleton, expectedResult, actualResult);
550550
}
551551
}
552+
553+
@Test
554+
public void unitAliases() {
555+
class TestCase {
556+
String skeleton;
557+
String expectedResult;
558+
559+
TestCase(String skeleton, String expectedResult) {
560+
this.skeleton = skeleton;
561+
this.expectedResult = expectedResult;
562+
}
563+
}
564+
565+
TestCase[] testCases =
566+
new TestCase[] {
567+
new TestCase("measure-unit/concentr-part-per-1e6", "3.14 ppm"),
568+
new TestCase("measure-unit/concentr-part-per-million", "3.14 ppm"),
569+
new TestCase("measure-unit/concentr-permillion", "3.14 ppm"),
570+
new TestCase(
571+
"measure-unit/concentr-milligram-ofglucose-per-deciliter",
572+
"3.14 mg/dL"),
573+
new TestCase("measure-unit/concentr-milligram-per-deciliter", "3.14 mg/dL"),
574+
new TestCase("measure-unit/mass-tonne", "3.14 t"),
575+
new TestCase("measure-unit/mass-metric-ton", "3.14 t"),
576+
};
577+
578+
for (TestCase testCase : testCases) {
579+
try {
580+
LocalizedNumberFormatter nf =
581+
NumberFormatter.forSkeleton(testCase.skeleton).locale(Locale.US);
582+
String actualResult = nf.format(3.14).toString();
583+
584+
assertEquals(
585+
"Wrong result for " + testCase.skeleton + ":",
586+
testCase.expectedResult,
587+
actualResult);
588+
} catch (SkeletonSyntaxException e) {
589+
fail(testCase.skeleton);
590+
}
591+
}
592+
}
552593
}

0 commit comments

Comments
 (0)