From e3bb5e5f0728bae0c941765f92175eb8c8cb2217 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Wed, 16 Sep 2020 19:39:13 +0200 Subject: [PATCH] ICU-20568 Add .unit().usage() support to ICU4J NumberFormatter (2/2) * Throw UnsupportedOperationException for "unsanctioned units" * Deal with the parent issue for LongNameMultiplexer * Fix NullPointerException: UnitConversionHandler.processQuantity must call fParent.processQuantity * toSkeleton not supported for not-built-in units * Add and use interface LongNameMultiplexer.ParentlessMicroPropsGenerator * Match up C++ and Java unit tests in NumberFormatterApiTest.java * Permit user-override of precision() for usage(), closes icu-units#95 * Use BogusRounder to propagate mathContext. * Port C++ change from PR #1322, commit c84ded050a, to Java. * Test the usage-without-unit error. Document it in NumberFormatterSettings.java * General review and corrections. --- icu4c/source/i18n/number_longnames.cpp | 6 +- icu4c/source/i18n/number_longnames.h | 13 +- icu4c/source/i18n/number_microprops.h | 10 +- icu4c/source/i18n/number_usageprefs.cpp | 6 +- icu4c/source/test/intltest/numbertest_api.cpp | 67 ++-- .../ibm/icu/impl/number/LongNameHandler.java | 43 ++- .../icu/impl/number/LongNameMultiplexer.java | 31 +- .../com/ibm/icu/impl/number/MicroProps.java | 24 +- .../icu/impl/number/MicroPropsGenerator.java | 1 + .../impl/number/MixedUnitLongNameHandler.java | 91 +++--- .../impl/number/UnitConversionHandler.java | 39 ++- .../icu/impl/number/UsagePrefsHandler.java | 31 +- .../ibm/icu/number/NumberFormatterImpl.java | 12 +- .../icu/number/NumberFormatterSettings.java | 31 +- .../ibm/icu/number/NumberSkeletonImpl.java | 5 +- .../src/com/ibm/icu/number/Precision.java | 42 ++- .../test/number/NumberFormatterApiTest.java | 286 ++++++++++-------- 17 files changed, 461 insertions(+), 277 deletions(-) diff --git a/icu4c/source/i18n/number_longnames.cpp b/icu4c/source/i18n/number_longnames.cpp index 953f12f215e..97a59730e9e 100644 --- a/icu4c/source/i18n/number_longnames.cpp +++ b/icu4c/source/i18n/number_longnames.cpp @@ -450,10 +450,8 @@ void MixedUnitLongNameHandler::processQuantity(DecimalQuantity &quantity, MicroP const Modifier *MixedUnitLongNameHandler::getMixedUnitModifier(DecimalQuantity &quantity, MicroProps µs, UErrorCode &status) const { - // TODO(icu-units#21): mixed units without usage() is not yet supported. - // That should be the only reason why this happens, so delete this whole if - // once fixed: if (micros.mixedMeasuresCount == 0) { + U_ASSERT(micros.mixedMeasuresCount > 0); // Mixed unit: we must have more than one unit value status = U_UNSUPPORTED_ERROR; return µs.helpers.emptyWeakModifier; } @@ -498,6 +496,7 @@ const Modifier *MixedUnitLongNameHandler::getMixedUnitModifier(DecimalQuantity & auto appendable = UnicodeStringAppendable(num); fIntegerFormatter.formatDecimalQuantity(fdec, status).appendTo(appendable, status); compiledFormatter.format(num, outputMeasuresList[i], status); + // TODO(icu-units#67): fix field positions } UnicodeString *finalSimpleFormats = &fMixedUnitData[(fMixedUnitCount - 1) * ARRAY_LENGTH]; @@ -515,6 +514,7 @@ const Modifier *MixedUnitLongNameHandler::getMixedUnitModifier(DecimalQuantity & return µs.helpers.emptyWeakModifier; } + // TODO(icu-units#67): fix field positions // Return a SimpleModifier for the "premixed" pattern micros.helpers.mixedUnitModifier = SimpleModifier(premixedCompiled, kUndefinedField, false, {this, SIGNUM_POS_ZERO, finalPlural}); diff --git a/icu4c/source/i18n/number_longnames.h b/icu4c/source/i18n/number_longnames.h index 4146f76c208..67f2316a9cd 100644 --- a/icu4c/source/i18n/number_longnames.h +++ b/icu4c/source/i18n/number_longnames.h @@ -159,14 +159,13 @@ class MixedUnitLongNameHandler : public MicroPropsGenerator, public ModifierStor // Not owned const MicroPropsGenerator *parent; - // Total number of units in the MeasureUnit this LongNameHandler was - // configured for: for "foot-and-inch", this will be 2. (If not a mixed unit, - // this will be 1.) + // Total number of units in the MeasureUnit this handler was configured for: + // for "foot-and-inch", this will be 2. int32_t fMixedUnitCount = 1; - // If this LongNameHandler is for a mixed unit, this stores unit data for - // each of the individual units. For each unit, it stores ARRAY_LENGTH - // strings, as returned by getMeasureData. (Each unit with index `i` has - // ARRAY_LENGTH strings starting at index `i*ARRAY_LENGTH` in this array.) + // Stores unit data for each of the individual units. For each unit, it + // stores ARRAY_LENGTH strings, as returned by getMeasureData. (Each unit + // with index `i` has ARRAY_LENGTH strings starting at index + // `i*ARRAY_LENGTH` in this array.) LocalArray fMixedUnitData; // A localized NumberFormatter used to format the integer-valued bigger // units of Mixed Unit measurements. diff --git a/icu4c/source/i18n/number_microprops.h b/icu4c/source/i18n/number_microprops.h index 7258cc2e640..058c5923b45 100644 --- a/icu4c/source/i18n/number_microprops.h +++ b/icu4c/source/i18n/number_microprops.h @@ -67,11 +67,11 @@ class IntMeasures : public MaybeStackArray { UErrorCode status = U_ZERO_ERROR; }; -// TODO(units): generated by MicroPropsGenerator, but inherits from it too. Do -// we want to better document why? There's an explanation for processQuantity: -// * As MicroProps is the "base instance", this implementation of -// * MicroPropsGenerator::processQuantity() just ensures that the output -// * `micros` is correctly initialized. +/** + * MicroProps is the first MicroPropsGenerator that should be should be called, + * producing an initialized MicroProps instance that will be passed on and + * modified throughout the rest of the chain of MicroPropsGenerator instances. + */ struct MicroProps : public MicroPropsGenerator { // NOTE: All of these fields are properly initialized in NumberFormatterImpl. diff --git a/icu4c/source/i18n/number_usageprefs.cpp b/icu4c/source/i18n/number_usageprefs.cpp index a1347e672a8..494b9687b28 100644 --- a/icu4c/source/i18n/number_usageprefs.cpp +++ b/icu4c/source/i18n/number_usageprefs.cpp @@ -103,6 +103,8 @@ void Usage::set(StringPiece value) { fUsage[fLength] = 0; } +// Populates micros.mixedMeasures and modifies quantity, based on the values in +// measures. void mixedMeasuresToMicros(const MaybeStackVector &measures, DecimalQuantity *quantity, MicroProps *micros, UErrorCode status) { micros->mixedMeasuresCount = measures.length() - 1; @@ -159,13 +161,13 @@ void UsagePrefsHandler::processQuantity(DecimalQuantity &quantity, MicroProps &m if (U_FAILURE(status)) { return; } - const MaybeStackVector& routedUnits = routed.measures; + const MaybeStackVector& routedMeasures = routed.measures; micros.outputUnit = routed.outputUnit.copy(status).build(status); if (U_FAILURE(status)) { return; } - mixedMeasuresToMicros(routedUnits, &quantity, µs, status); + mixedMeasuresToMicros(routedMeasures, &quantity, µs, status); UnicodeString precisionSkeleton = routed.precision; if (micros.rounder.fPrecision.isBogus()) { diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp index cce8ddfac3f..ecc0dcab189 100644 --- a/icu4c/source/test/intltest/numbertest_api.cpp +++ b/icu4c/source/test/intltest/numbertest_api.cpp @@ -906,6 +906,11 @@ void NumberFormatterApiTest::unitUsage() { FormattedNumber formattedNum; UnicodeString uTestCase; + status.assertSuccess(); + formattedNum = + NumberFormatter::with().usage("road").locale(Locale::getEnglish()).formatInt(1, status); + status.expectErrorAndReset(U_ILLEGAL_ARGUMENT_ERROR); + unloc_formatter = NumberFormatter::with().usage("road").unit(MeasureUnit::getMeter()); uTestCase = u"unitUsage() en-ZA road"; @@ -1113,25 +1118,49 @@ void NumberFormatterApiTest::unitUsage() { u"0 pounds, 0 ounces"); assertFormatDescendingBig( - u"Scientific notation with Usage: possible when using a reasonable Precision", - u"scientific @### usage/default measure-unit/area-square-meter unit-width-full-name", - u"scientific @### usage/default unit/square-meter unit-width-full-name", - NumberFormatter::with() - .unit(SQUARE_METER) - .usage("default") - .notation(Notation::scientific()) - .precision(Precision::minMaxSignificantDigits(1, 4)) - .unitWidth(UNumberUnitWidth::UNUM_UNIT_WIDTH_FULL_NAME), - Locale("en-ZA"), - u"8,765E1 square kilometres", - u"8,765E0 square kilometres", - u"8,765E1 hectares", - u"8,765E0 hectares", - u"8,765E3 square metres", - u"8,765E2 square metres", - u"8,765E1 square metres", - u"8,765E0 square metres", - u"0E0 square centimetres"); + u"Scientific notation with Usage: possible when using a reasonable Precision", + u"scientific @### usage/default measure-unit/area-square-meter unit-width-full-name", + u"scientific @### usage/default unit/square-meter unit-width-full-name", + NumberFormatter::with() + .unit(SQUARE_METER) + .usage("default") + .notation(Notation::scientific()) + .precision(Precision::minMaxSignificantDigits(1, 4)) + .unitWidth(UNumberUnitWidth::UNUM_UNIT_WIDTH_FULL_NAME), + Locale("en-ZA"), + u"8,765E1 square kilometres", + u"8,765E0 square kilometres", + u"8,765E1 hectares", + u"8,765E0 hectares", + u"8,765E3 square metres", + u"8,765E2 square metres", + u"8,765E1 square metres", + u"8,765E0 square metres", + u"0E0 square centimetres"); + + assertFormatSingle( + u"Rounding Mode propagates: rounding down", + u"usage/road measure-unit/length-centimeter rounding-mode-floor", + u"usage/road unit/centimeter rounding-mode-floor", + NumberFormatter::with() + .unit(MeasureUnit::forIdentifier("centimeter", status)) + .usage("road") + .roundingMode(UNUM_ROUND_FLOOR), + Locale("en-ZA"), + 34500, + u"300 m"); + + assertFormatSingle( + u"Rounding Mode propagates: rounding up", + u"usage/road measure-unit/length-centimeter rounding-mode-ceiling", + u"usage/road unit/centimeter rounding-mode-ceiling", + NumberFormatter::with() + .unit(MeasureUnit::forIdentifier("centimeter", status)) + .usage("road") + .roundingMode(UNUM_ROUND_CEILING), + Locale("en-ZA"), + 30500, + u"350 m"); } void NumberFormatterApiTest::unitUsageErrorCodes() { diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameHandler.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameHandler.java index 695fc6abef7..e320575ff1a 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameHandler.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameHandler.java @@ -22,11 +22,12 @@ import com.ibm.icu.util.MeasureUnit; import com.ibm.icu.util.ULocale; import com.ibm.icu.util.UResourceBundle; -public class LongNameHandler implements MicroPropsGenerator, ModifierStore { +public class LongNameHandler + implements MicroPropsGenerator, ModifierStore, LongNameMultiplexer.ParentlessMicroPropsGenerator { private static final int DNAM_INDEX = StandardPlural.COUNT; private static final int PER_INDEX = StandardPlural.COUNT + 1; - protected static final int ARRAY_LENGTH = StandardPlural.COUNT + 2; + static final int ARRAY_LENGTH = StandardPlural.COUNT + 2; private static int getIndex(String pluralKeyword) { // pluralKeyword can also be "dnam" or "per" @@ -39,7 +40,7 @@ public class LongNameHandler implements MicroPropsGenerator, ModifierStore { } } - protected static String getWithPlural(String[] strings, StandardPlural plural) { + static String getWithPlural(String[] strings, StandardPlural plural) { String result = strings[plural.ordinal()]; if (result == null) { result = strings[StandardPlural.OTHER.ordinal()]; @@ -79,7 +80,7 @@ public class LongNameHandler implements MicroPropsGenerator, ModifierStore { // NOTE: outArray MUST have at least ARRAY_LENGTH entries. No bounds checking is performed. - protected static void getMeasureData( + static void getMeasureData( ULocale locale, MeasureUnit unit, UnitWidth width, @@ -199,13 +200,13 @@ public class LongNameHandler implements MicroPropsGenerator, ModifierStore { *

* Mixed units are not supported, use MixedUnitLongNameHandler.forMeasureUnit. * - * @param locale The desired locale. - * @param unit The measure unit to construct a LongNameHandler for. If - * `perUnit` is also defined, `unit` must not be a mixed unit. - * @param perUnit If `unit` is a mixed unit, `perUnit` must be "none". - * @param width Specifies the desired unit rendering. - * @param rules Does not take ownership. - * @param parent Does not take ownership. + * @param locale The desired locale. + * @param unit The measure unit to construct a LongNameHandler for. If + * `perUnit` is also defined, `unit` must not be a mixed unit. + * @param perUnit If `unit` is a mixed unit, `perUnit` must be null. + * @param width Specifies the desired unit rendering. + * @param rules Plural rules. + * @param parent Plural rules. */ public static LongNameHandler forMeasureUnit( ULocale locale, @@ -214,6 +215,12 @@ public class LongNameHandler implements MicroPropsGenerator, ModifierStore { UnitWidth width, PluralRules rules, MicroPropsGenerator parent) { + if (unit.getType() == null || (perUnit != null && perUnit.getType() == null)) { + // TODO(ICU-20941): Unsanctioned unit. Not yet fully supported. Set an + // error code. Once we support not-built-in units here, unitRef may be + // anything, but if not built-in, perUnit has to be "none". + throw new UnsupportedOperationException("Unsanctioned units, not yet supported"); + } if (perUnit != null) { // Compound unit: first try to simplify (e.g., meters per second is its own unit). MeasureUnit simplified = MeasureUnit.resolveUnitPerUnit(unit, perUnit); @@ -314,6 +321,20 @@ public class LongNameHandler implements MicroPropsGenerator, ModifierStore { return micros; } + /** + * Produces a plural-appropriate Modifier for a unit: `quantity` is taken as + * the final smallest unit, while the larger unit values must be provided + * via `micros.mixedMeasures`. + * + * Does not call parent.processQuantity, so cannot get a MicroProps instance + * that way. Instead, the instance is passed in as a parameter. + */ + public MicroProps processQuantityWithMicros(DecimalQuantity quantity, MicroProps micros) { + StandardPlural pluralForm = RoundingUtils.getPluralSafe(micros.rounder, rules, quantity); + micros.modOuter = modifiers.get(pluralForm); + return micros; + } + @Override public Modifier getModifier(Signum signum, StandardPlural plural) { // Signum ignored diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameMultiplexer.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameMultiplexer.java index 9f21d009b57..a3f03c96aba 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameMultiplexer.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/LongNameMultiplexer.java @@ -1,7 +1,5 @@ // © 2020 and later: Unicode, Inc. and others. // License & terms of use: http://www.unicode.org/copyright.html - - package com.ibm.icu.impl.number; import com.ibm.icu.number.NumberFormatter; @@ -13,10 +11,27 @@ import com.ibm.icu.util.ULocale; import java.util.ArrayList; import java.util.List; +/** + * A MicroPropsGenerator that multiplexes between different LongNameHandlers, + * depending on the outputUnit. + * + * See processQuantity() for the input requirements. + */ public class LongNameMultiplexer implements MicroPropsGenerator { + /** + * LongNameMultiplexer calls the parent MicroPropsGenerator itself, + * receiving the MicroProps instance in use for this formatting pipeline. + * Next it multiplexes between name handlers (fHandlers) which are not given + * access to a parent. Consequently LongNameMultiplexer must give these + * handlers the MicroProps instance. + */ + public static interface ParentlessMicroPropsGenerator { + public MicroProps processQuantityWithMicros(DecimalQuantity quantity, MicroProps micros); + } + private final MicroPropsGenerator fParent; - private List fHandlers; + private List fHandlers; // Each MeasureUnit corresponds to the same-index MicroPropsGenerator // pointed to in fHandlers. @@ -50,7 +65,7 @@ public class LongNameMultiplexer implements MicroPropsGenerator { result.fHandlers.add(mlnh); } else { LongNameHandler lnh = LongNameHandler - .forMeasureUnit(locale, unit, NoUnit.BASE, width, rules, null ); + .forMeasureUnit(locale, unit, NoUnit.BASE, width, rules, null); result.fHandlers.add(lnh); } } @@ -62,7 +77,6 @@ public class LongNameMultiplexer implements MicroPropsGenerator { // one of the units provided to the factory function. @Override public MicroProps processQuantity(DecimalQuantity quantity) { - // We call parent->processQuantity() from the Multiplexer, instead of // letting LongNameHandler handle it: we don't know which LongNameHandler to // call until we've called the parent! @@ -70,12 +84,11 @@ public class LongNameMultiplexer implements MicroPropsGenerator { // Call the correct LongNameHandler based on outputUnit for (int i = 0; i < this.fHandlers.size(); i++) { - if (fMeasureUnits.get(i).equals( micros.outputUnit)) { - return fHandlers.get(i).processQuantity(quantity); + if (fMeasureUnits.get(i).equals(micros.outputUnit)) { + ParentlessMicroPropsGenerator handler = fHandlers.get(i); + return handler.processQuantityWithMicros(quantity, micros); } - } - throw new AssertionError (" We shouldn't receive any outputUnit for which we haven't already got a LongNameHandler"); } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroProps.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroProps.java index 3179e6c4a75..dfb5ffdec48 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroProps.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroProps.java @@ -12,11 +12,11 @@ import com.ibm.icu.util.MeasureUnit; import java.util.List; -// TODO(units): generated by MicroPropsGenerator, but inherits from it too. Do we want to better document why? -// There's an explanation for processQuantity: -// - As MicroProps is the "base instance", this implementation of -// - MicoPropsGenerator::processQuantity() just ensures that the output -// - `micros` is correctly initialized. +/** + * MicroProps is the first MicroPropsGenerator that should be should be called, + * producing an initialized MicroProps instance that will be passed on and + * modified throughout the rest of the chain of MicroPropsGenerator instances. + */ public class MicroProps implements Cloneable, MicroPropsGenerator { // Populated globally: public SignDisplay sign; @@ -55,7 +55,7 @@ public class MicroProps implements Cloneable, MicroPropsGenerator { // In the case of mixed units, this is the set of integer-only units // *preceding* the final unit. - public List mixedMeasures ; + public List mixedMeasures; private volatile boolean exhausted; @@ -79,17 +79,19 @@ public class MicroProps implements Cloneable, MicroPropsGenerator { * will be modified and thus not be available for re-use. * * @param quantity The quantity for consideration and optional mutation. - * @return a MicroProps instance to populate. + * @return an initialized MicroProps instance. */ @Override public MicroProps processQuantity(DecimalQuantity quantity) { if (immutable) { return (MicroProps) this.clone(); + } else if (exhausted) { + // Safety check + throw new AssertionError("Cannot re-use a mutable MicroProps in the quantity chain"); + } else { + exhausted = true; + return this; } - - assert !exhausted : "Cannot re-use a mutable MicroProps in the quantity chain"; - exhausted = true; - return this; } @Override diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroPropsGenerator.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroPropsGenerator.java index 1ed61199b45..8580c54ca86 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroPropsGenerator.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MicroPropsGenerator.java @@ -19,6 +19,7 @@ package com.ibm.icu.impl.number; * {@link MicroProps} with properties that are not quantity-dependent. Each element in the linked list * calls {@link #processQuantity} on its "parent", then does its work, and then returns the result. * + *

* This chain of MicroPropsGenerators is typically constructed by NumberFormatterImpl::macrosToMicroGenerator() when * constructing a NumberFormatter. * diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MixedUnitLongNameHandler.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MixedUnitLongNameHandler.java index 7ad64bb82f3..1c01c358748 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MixedUnitLongNameHandler.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/MixedUnitLongNameHandler.java @@ -1,10 +1,9 @@ // © 2020 and later: Unicode, Inc. and others. // License & terms of use: http://www.unicode.org/copyright.html - - package com.ibm.icu.impl.number; import com.ibm.icu.impl.FormattedStringBuilder; +import com.ibm.icu.impl.SimpleFormatterImpl; import com.ibm.icu.impl.StandardPlural; import com.ibm.icu.number.LocalizedNumberFormatter; import com.ibm.icu.number.NumberFormatter; @@ -13,26 +12,28 @@ import com.ibm.icu.text.PluralRules; import com.ibm.icu.text.SimpleFormatter; import com.ibm.icu.util.MeasureUnit; import com.ibm.icu.util.ULocale; - import java.util.ArrayList; import java.util.List; -public class MixedUnitLongNameHandler implements MicroPropsGenerator, ModifierStore { - // Not owned +/** Similar to LongNameHandler, but only for MIXED units. */ +public class MixedUnitLongNameHandler + implements MicroPropsGenerator, ModifierStore, LongNameMultiplexer.ParentlessMicroPropsGenerator { private final PluralRules rules; - // Not owned private final MicroPropsGenerator parent; - // If this LongNameHandler is for a mixed unit, this stores unit data for - // each of the individual units. For each unit, it stores ARRAY_LENGTH - // strings, as returned by getMeasureData. + /** + * Stores unit data for each of the individual units. For each unit, it + * stores ARRAY_LENGTH strings, as returned by getMeasureData. + */ private List fMixedUnitData; - // A localized NumberFormatter used to format the integer-valued bigger - // units of Mixed Unit measurements. + /** + * A localized NumberFormatter used to format the integer-valued bigger + * units of Mixed Unit measurements. + */ private LocalizedNumberFormatter fIntegerFormatter; - // A localised list formatter for joining mixed units together. + /** A localised list formatter for joining mixed units together. */ private ListFormatter fListFormatter; private MixedUnitLongNameHandler(PluralRules rules, MicroPropsGenerator parent) { @@ -49,8 +50,8 @@ public class MixedUnitLongNameHandler implements MicroPropsGenerator, ModifierSt * @param mixedUnit The mixed measure unit to construct a * MixedUnitLongNameHandler for. * @param width Specifies the desired unit rendering. - * @param rules Does not take ownership. - * @param parent Does not take ownership. + * @param rules PluralRules instance. + * @param parent MicroPropsGenerator instance. */ public static MixedUnitLongNameHandler forMeasureUnit(ULocale locale, MeasureUnit mixedUnit, NumberFormatter.UnitWidth width, PluralRules rules, @@ -89,43 +90,61 @@ public class MixedUnitLongNameHandler implements MicroPropsGenerator, ModifierSt /** * Produces a plural-appropriate Modifier for a mixed unit: `quantity` is * taken as the final smallest unit, while the larger unit values must be - * provided via `micros.mixedMeasures`. + * provided by `micros.mixedMeasures`, micros being the MicroProps instance + * returned by the parent. + * + * This function must not be called if this instance has no parent: call + * processQuantityWithMicros() instead. */ @Override public MicroProps processQuantity(DecimalQuantity quantity) { assert (fMixedUnitData.size() > 1); MicroProps micros; - // if (parent != null) micros = parent.processQuantity(quantity); micros.modOuter = getMixedUnitModifier(quantity, micros); return micros; } - // Required for ModifierStore. And ModifierStore is required by - // SimpleModifier constructor's last parameter. We assert his will never get - // called though. + /** + * Produces a plural-appropriate Modifier for a mixed unit: `quantity` is + * taken as the final smallest unit, while the larger unit values must be + * provided via `micros.mixedMeasures`. + * + * Does not call parent.processQuantity, so cannot get a MicroProps instance + * that way. Instead, the instance is passed in as a parameter. + */ + public MicroProps processQuantityWithMicros(DecimalQuantity quantity, MicroProps micros) { + assert (fMixedUnitData.size() > 1); + micros.modOuter = getMixedUnitModifier(quantity, micros); + return micros; + } + + /** + * Required for ModifierStore. And ModifierStore is required by + * SimpleModifier constructor's last parameter. We assert his will never get + * called though. + */ @Override public Modifier getModifier(Modifier.Signum signum, StandardPlural plural) { // TODO(units): investigate this method while investigating where // LongNameHandler.getModifier() gets used. To be sure it remains // unreachable: - + assert false : "should be unreachable"; return null; } - // For a mixed unit, returns a Modifier that takes only one parameter: the - // smallest and final unit of the set. The bigger units' values and labels - // get baked into this Modifier, together with the unit label of the final - // unit. + /** + * For a mixed unit, returns a Modifier that takes only one parameter: the + * smallest and final unit of the set. The bigger units' values and labels + * get baked into this Modifier, together with the unit label of the final + * unit. + */ private Modifier getMixedUnitModifier(DecimalQuantity quantity, MicroProps micros) { - // TODO(icu-units#21): mixed units without usage() is not yet supported. - // That should be the only reason why this happens, so delete this whole if - // once fixed: if (micros.mixedMeasures.size() == 0) { + assert false : "Mixed unit: we must have more than one unit value"; throw new UnsupportedOperationException(); } - // Algorithm: // // For the mixed-units measurement of: "3 yard, 1 foot, 2.6 inch", we should @@ -153,30 +172,30 @@ public class MixedUnitLongNameHandler implements MicroPropsGenerator, ModifierSt String simpleFormat = LongNameHandler.getWithPlural(this.fMixedUnitData.get(i), pluralForm); SimpleFormatter compiledFormatter = SimpleFormatter.compileMinMaxArguments(simpleFormat, 0, 1); - FormattedStringBuilder appendable = new FormattedStringBuilder(); this.fIntegerFormatter.formatImpl(fdec, appendable); outputMeasuresList.add(compiledFormatter.format(appendable.toString())); - // TODO: fix this issue https://github.com/icu-units/icu/issues/67 + // TODO(icu-units#67): fix field positions } String[] finalSimpleFormats = this.fMixedUnitData.get(this.fMixedUnitData.size() - 1); StandardPlural finalPlural = RoundingUtils.getPluralSafe(micros.rounder, rules, quantity); String finalSimpleFormat = LongNameHandler.getWithPlural(finalSimpleFormats, finalPlural); SimpleFormatter finalFormatter = SimpleFormatter.compileMinMaxArguments(finalSimpleFormat, 0, 1); - finalFormatter.format("{0}", outputMeasuresList.get(outputMeasuresList.size() -1)); + outputMeasuresList.add(finalFormatter.format("{0}")); // Combine list into a "premixed" pattern String premixedFormatPattern = this.fListFormatter.format(outputMeasuresList); - SimpleFormatter premixedCompiled = SimpleFormatter.compileMinMaxArguments(premixedFormatPattern, 0, 1); + StringBuilder sb = new StringBuilder(); + String premixedCompiled = + SimpleFormatterImpl.compileToStringMinMaxArguments(premixedFormatPattern, sb, 0, 1); - // Return a SimpleModifier for the "premixed" pattern + // TODO(icu-units#67): fix field positions Modifier.Parameters params = new Modifier.Parameters(); params.obj = this; params.signum = Modifier.Signum.POS_ZERO; params.plural = finalPlural; - - return new SimpleModifier(premixedCompiled.getTextWithNoArguments(), null, false, params); - /*TODO: it was SimpleModifier(premixedCompiled, kUndefinedField, false, {this, SIGNUM_POS_ZERO, finalPlural});*/ + // Return a SimpleModifier for the "premixed" pattern + return new SimpleModifier(premixedCompiled, null, false, params); } } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UnitConversionHandler.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UnitConversionHandler.java index cf3fd236a90..db270596c57 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UnitConversionHandler.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UnitConversionHandler.java @@ -1,6 +1,5 @@ // © 2020 and later: Unicode, Inc. and others. // License & terms of use: http://www.unicode.org/copyright.html - package com.ibm.icu.impl.number; import com.ibm.icu.impl.units.ComplexUnitsConverter; @@ -13,8 +12,9 @@ import java.util.ArrayList; import java.util.List; /** - * A MicroPropsGenerator which converts a measurement from a simple MeasureUnit - * to a Mixed MeasureUnit. + * A MicroPropsGenerator which converts a measurement from one MeasureUnit to + * another. In particular, the output MeasureUnit may be a mixed unit. (The + * input unit may not be a mixed unit.) */ public class UnitConversionHandler implements MicroPropsGenerator { @@ -22,22 +22,23 @@ public class UnitConversionHandler implements MicroPropsGenerator { private MeasureUnit fOutputUnit; private ComplexUnitsConverter fComplexUnitConverter; - public UnitConversionHandler(MeasureUnit outputUnit, MicroPropsGenerator parent) { + /** + * Constructor. + * + * @param inputUnit Specifies the input MeasureUnit. Mixed units are not + * supported as input (because input is just a single decimal quantity). + * @param outputUnit Specifies the output MeasureUnit. + * @param parent The parent MicroPropsGenerator. + */ + public UnitConversionHandler(MeasureUnit inputUnit, + MeasureUnit outputUnit, + MicroPropsGenerator parent) { this.fOutputUnit = outputUnit; this.fParent = parent; - - List singleUnits = outputUnit.splitToSingleUnits(); - - assert outputUnit.getComplexity() == MeasureUnit.Complexity.MIXED; - assert singleUnits.size() > 1; - + MeasureUnitImpl inputUnitImpl = MeasureUnitImpl.forIdentifier(inputUnit.getIdentifier()); MeasureUnitImpl outputUnitImpl = MeasureUnitImpl.forIdentifier(outputUnit.getIdentifier()); - // TODO(icu-units#97): The input unit should be the largest unit, not the first unit, in the identifier. - this.fComplexUnitConverter = - new ComplexUnitsConverter( - new MeasureUnitImpl(outputUnitImpl.getSingleUnits().get(0)), - outputUnitImpl, - new UnitsData().getConversionRates()); + this.fComplexUnitConverter = new ComplexUnitsConverter(inputUnitImpl, outputUnitImpl, + new UnitsData().getConversionRates()); } /** @@ -45,16 +46,12 @@ public class UnitConversionHandler implements MicroPropsGenerator { */ @Override public MicroProps processQuantity(DecimalQuantity quantity) { - /*TODO: Questions : shall we check the parent if it is equals null */ - MicroProps result = this.fParent == null? - this.fParent.processQuantity(quantity): - new MicroProps(false); + MicroProps result = this.fParent.processQuantity(quantity); quantity.roundToInfinity(); // Enables toDouble List measures = this.fComplexUnitConverter.convert(quantity.toBigDecimal()); result.outputUnit = this.fOutputUnit; - result.mixedMeasures = new ArrayList<>(); UsagePrefsHandler.mixedMeasuresToMicros(measures, quantity, result); return result; diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UsagePrefsHandler.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UsagePrefsHandler.java index ee012d297d3..74c13b660bc 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UsagePrefsHandler.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/UsagePrefsHandler.java @@ -1,6 +1,5 @@ // © 2020 and later: Unicode, Inc. and others. // License & terms of use: http://www.unicode.org/copyright.html - package com.ibm.icu.impl.number; import com.ibm.icu.impl.IllegalIcuArgumentException; @@ -47,15 +46,18 @@ public class UsagePrefsHandler implements MicroPropsGenerator { return Precision.increment(num.divide(den, MathContext.DECIMAL128)); } + /** + * Populates micros.mixedMeasures and modifies quantity, based on the values + * in measures. + */ protected static void mixedMeasuresToMicros(List measures, DecimalQuantity quantity, MicroProps micros) { + micros.mixedMeasures = new ArrayList<>(); if (measures.size() > 1) { // For debugging assert (micros.outputUnit.getComplexity() == MeasureUnit.Complexity.MIXED); - // Check that we received measurements with the expected MeasureUnits: - List singleUnits = micros.outputUnit.splitToSingleUnits(); - - assert measures.size() == singleUnits.size(); + // Check that we received the expected number of measurements: + assert measures.size() == micros.outputUnit.splitToSingleUnits().size(); // Mixed units: except for the last value, we pass all values to the // LongNameHandler via micros->mixedMeasures. @@ -95,7 +97,6 @@ public class UsagePrefsHandler implements MicroPropsGenerator { final List routedMeasures = routed.measures; micros.outputUnit = routed.outputUnit.build(); - micros.mixedMeasures = new ArrayList<>(); UsagePrefsHandler.mixedMeasuresToMicros(routedMeasures, quantity, micros); @@ -103,14 +104,16 @@ public class UsagePrefsHandler implements MicroPropsGenerator { assert micros.rounder != null; - // TODO: use the user precision if the user already set precision. - if (precisionSkeleton != null && precisionSkeleton.length() > 0) { - micros.rounder = parseSkeletonToPrecision(precisionSkeleton); - } else { - // We use the same rounding mode as COMPACT notation: known to be a - // human-friendly rounding mode: integers, but add a decimal digit - // as needed to ensure we have at least 2 significant digits. - micros.rounder = Precision.integer().withMinDigits(2); + if (micros.rounder instanceof Precision.BogusRounder) { + Precision.BogusRounder rounder = (Precision.BogusRounder)micros.rounder; + if (precisionSkeleton != null && precisionSkeleton.length() > 0) { + micros.rounder = rounder.into(parseSkeletonToPrecision(precisionSkeleton)); + } else { + // We use the same rounding mode as COMPACT notation: known to be a + // human-friendly rounding mode: integers, but add a decimal digit + // as needed to ensure we have at least 2 significant digits. + micros.rounder = rounder.into(Precision.integer().withMinDigits(2)); + } } return micros; diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java index 1b91cbb0b6b..b639c8f71f0 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterImpl.java @@ -210,10 +210,9 @@ class NumberFormatterImpl { || !(isPercent || isPermille) || isCompactNotation ); - - // TODO(icu-units#95): Add the logic in this file that sets the rounder to bogus/pass-through if isMixedUnit is true. boolean isMixedUnit = isCldrUnit && macros.unit.getType() == null && - macros.unit.getComplexity() == MeasureUnit.Complexity.MIXED; + macros.unit.getComplexity() == MeasureUnit.Complexity.MIXED; + PluralRules rules = macros.rules; // Select the numbering system. @@ -275,7 +274,9 @@ class NumberFormatterImpl { } chain = usagePrefsHandler = new UsagePrefsHandler(macros.loc, macros.unit, macros.usage, chain); } else if (isMixedUnit) { - chain = new UnitConversionHandler(macros.unit, chain); + // TODO(icu-units#97): The input unit should be the largest unit, not the first unit, in the identifier. + MeasureUnit inputUnit = macros.unit.splitToSingleUnits().get(0); + chain = new UnitConversionHandler(inputUnit, macros.unit, chain); } // Multiplier @@ -290,6 +291,9 @@ class NumberFormatterImpl { micros.rounder = Precision.COMPACT_STRATEGY; } else if (isCurrency) { micros.rounder = Precision.MONETARY_STANDARD; + } else if (macros.usage != null) { + // Bogus Precision - it will get set in the UsagePrefsHandler instead + micros.rounder = Precision.BOGUS_PRECISION; } else { micros.rounder = Precision.DEFAULT_MAX_FRAC_6; } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterSettings.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterSettings.java index fb1072bea24..6d223bfdb37 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterSettings.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberFormatterSettings.java @@ -44,8 +44,8 @@ public abstract class NumberFormatterSettings parent; private final int key; @@ -134,7 +134,7 @@ public abstract class NumberFormatterSettings * The default is to render without units (equivalent to {@link NoUnit#BASE}). * - *

+ *

* If the input usage is correctly set the output unit will change * according to `usage`, `locale` and `unit` value. *

@@ -492,44 +492,57 @@ public abstract class NumberFormatterSettings * When a `usage` is specified, the output unit will change depending on the * `Locale` and the unit quantity. For example, formatting length * measurements specified in meters: * - * `NumberFormatter.with().usage("person").unit(MeasureUnit.METER).locale(new ULocale("en-US"))` - * * When formatting 0.25, the output will be "10 inches". - * * When formatting 1.50, the output will be "4 feet and 11 inches". + *

+ * 

+     * NumberFormatter.with().usage("person").unit(MeasureUnit.METER).locale(new ULocale("en-US"))
+     *
+ *
    + *
  • When formatting 0.25, the output will be "10 inches". + *
  • When formatting 1.50, the output will be "4 feet and 11 inches". + *
    • * + *

    * The input unit specified via unit() determines the type of measurement * being formatted (e.g. "length" when the unit is "foot"). The usage * requested will be looked for only within this category of measurement * units. * + *

    * The output unit can be found via FormattedNumber.getOutputUnit(). * + *

    * If the usage has multiple parts (e.g. "land-agriculture-grain") and does * not match a known usage preference, the last part will be dropped * repeatedly until a match is found (e.g. trying "land-agriculture", then * "land"). If a match is still not found, usage will fall back to * "default". * + *

    * Setting usage to an empty string clears the usage (disables usage-based * localized formatting). * + *

    + * Setting a usage string but not a correct input unit will result in an + * U_ILLEGAL_ARGUMENT_ERROR. * + *

    * When using usage, specifying rounding or precision is unnecessary. * Specifying a precision in some manner will override the default * formatting. * - * * @param usage A usage parameter from the units resource. * @return The fluent chain * @throws IllegalArgumentException in case of Setting a usage string but not a correct input unit. - * @draft ICU 67 + * @draft ICU 68 * @provisional This API might change or be removed in a future release. */ public T usage(String usage) { diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberSkeletonImpl.java b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberSkeletonImpl.java index 02565f040e4..07e81c45ec9 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/NumberSkeletonImpl.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/NumberSkeletonImpl.java @@ -1468,10 +1468,13 @@ class NumberSkeletonImpl { } else if (macros.unit == MeasureUnit.PERMILLE) { sb.append("permille"); return true; - } else { + } else if (macros.unit.getType() != null) { sb.append("measure-unit/"); BlueprintHelpers.generateMeasureUnitOption(macros.unit, sb); return true; + } else { + // TODO(icu-units#35): add support for not-built-in units. + throw new UnsupportedOperationException(); } } diff --git a/icu4j/main/classes/core/src/com/ibm/icu/number/Precision.java b/icu4j/main/classes/core/src/com/ibm/icu/number/Precision.java index d2dfb6bb15b..a984ae06502 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/number/Precision.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/number/Precision.java @@ -20,7 +20,6 @@ import com.ibm.icu.util.Currency.CurrencyUsage; * * @stable ICU 62 * @see NumberFormatter - * @internal */ public abstract class Precision { @@ -366,6 +365,13 @@ public abstract class Precision { // PACKAGE-PRIVATE APIS // ////////////////////////// + /** + * @internal + * @deprecated ICU internal only. + */ + @Deprecated + public static final BogusRounder BOGUS_PRECISION = new BogusRounder(); + static final InfiniteRounderImpl NONE = new InfiniteRounderImpl(); static final FractionRounderImpl FIXED_FRAC_0 = new FractionRounderImpl(0, 0); @@ -545,6 +551,40 @@ public abstract class Precision { // INTERNALS // /////////////// + /** + * An BogusRounder's MathContext into precision. + * + * @internal + * @deprecated This API is ICU internal only. + */ + @Deprecated + public static class BogusRounder extends Precision { + @Override + public void apply(DecimalQuantity value) { + throw new AssertionError("BogusRounder must not be applied"); + } + + @Override + BogusRounder createCopy() { + BogusRounder copy = new BogusRounder(); + copy.mathContext = mathContext; + return copy; + } + + /** + * Copies the BogusRounder's MathContext into precision. + * + * @internal + * @deprecated This API is ICU internal only. + */ + @Deprecated + public Precision into(Precision precision) { + Precision copy = precision.createCopy(); + copy.mathContext = mathContext; + return copy; + } + } + static class InfiniteRounderImpl extends Precision { public InfiniteRounderImpl() { diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java index b10b4143054..3bed7ba80de 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/number/NumberFormatterApiTest.java @@ -21,6 +21,7 @@ import org.junit.Test; import com.ibm.icu.dev.test.TestFmwk; import com.ibm.icu.dev.test.format.FormattedValueTest; import com.ibm.icu.dev.test.serializable.SerializableTestUtility; +import com.ibm.icu.impl.IllegalIcuArgumentException; import com.ibm.icu.impl.number.Grouper; import com.ibm.icu.impl.number.LocalizedNumberFormatterAsFormat; import com.ibm.icu.impl.number.MacroProps; @@ -644,6 +645,17 @@ public class NumberFormatterApiTest extends TestFmwk { 1, "kelvin"); + assertFormatSingle( + "Person unit not in short form", + "measure-unit/duration-year-person unit-width-full-name", + "unit/year-person unit-width-full-name", + NumberFormatter.with() + .unit(MeasureUnit.YEAR_PERSON) + .unitWidth(UnitWidth.FULL_NAME), + ULocale.forLanguageTag("es-MX"), + 5, + "5 a\u00F1os"); + // TODO(icu-units#35): skeleton generation. assertFormatSingle( "Mixed unit", @@ -738,6 +750,17 @@ public class NumberFormatterApiTest extends TestFmwk { "0.008765 m/s", "0 m/s"); + // TODO(icu-units#35): does not normalize as desired: while "unit/*" does + // get split into unit/perUnit, ".unit(*)" and "measure-unit/*" don't: + assertFormatSingle( + "Built-in unit, meter-per-second", + "measure-unit/speed-meter-per-second", + "~unit/meter-per-second", + NumberFormatter.with().unit(MeasureUnit.METER_PER_SECOND), + new ULocale("en-GB"), + 2.4, + "2.4 m/s"); + assertFormatDescending( "Pounds Per Square Mile Short (secondary unit has per-format)", "measure-unit/mass-pound per-measure-unit/area-square-mile", @@ -770,16 +793,22 @@ public class NumberFormatterApiTest extends TestFmwk { "0.008765 J/fur", "0 J/fur"); - // TODO(icu-units#35): does not normalize as desired: while "unit/*" does - // get split into unit/perUnit, ".unit(*)" and "measure-unit/*" don't: - assertFormatSingle( - "Built-in unit, meter-per-second", - "measure-unit/speed-meter-per-second", - "~unit/meter-per-second", - NumberFormatter.with().unit(MeasureUnit.METER_PER_SECOND), - new ULocale("en-GB"), - 2.4, - "2.4 m/s"); + // // TODO(ICU-20941): Support constructions such as this one. + // assertFormatDescending( + // "Joules Per Furlong Short with unit identifier via API", + // "measure-unit/energy-joule per-measure-unit/length-furlong", + // "unit/joule-per-furlong", + // NumberFormatter.with().unit(MeasureUnit.forIdentifier("joule-per-furlong")), + // ULocale.ENGLISH, + // "87,650 J/fur", + // "8,765 J/fur", + // "876.5 J/fur", + // "87.65 J/fur", + // "8.765 J/fur", + // "0.8765 J/fur", + // "0.08765 J/fur", + // "0.008765 J/fur", + // "0 J/fur"); // TODO(icu-units#59): THIS UNIT TEST DEMONSTRATES UNDESIRABLE BEHAVIOUR! // When specifying built-in types, one can give both a unit and a perUnit. @@ -839,6 +868,13 @@ public class NumberFormatterApiTest extends TestFmwk { FormattedNumber formattedNum; String uTestCase; + try { + NumberFormatter.with().usage("road").locale(ULocale.ENGLISH).format(1); + fail("should give an error, usage() without unit() is invalid"); + } catch (IllegalIcuArgumentException e) { + // Pass + } + unloc_formatter = NumberFormatter.with().usage("road").unit(MeasureUnit.METER); uTestCase = "unitUsage() en-ZA road"; @@ -881,15 +917,10 @@ public class NumberFormatterApiTest extends TestFmwk { uTestCase = "unitUsage() en-GB road"; formatter = unloc_formatter.locale(new ULocale("en-GB")); formattedNum = formatter.format(321d); - - // status.errIfFailureAndReset("unitUsage() en-GB road, formatDouble(...)"); - assertTrue( uTestCase + ", got outputUnit: \"" + formattedNum.getOutputUnit().getIdentifier() + "\"", MeasureUnit.YARD.equals(formattedNum.getOutputUnit())); - // status.errIfFailureAndReset("unitUsage() en-GB road, getOutputUnit(...)"); assertEquals(uTestCase, "350 yd", formattedNum.toString()); - //status.errIfFailureAndReset("unitUsage() en-GB road, toString(...)"); { final Object[][] expectedFieldPositions = { {NumberFormat.Field.INTEGER, 0, 3}, @@ -919,15 +950,10 @@ public class NumberFormatterApiTest extends TestFmwk { uTestCase = "unitUsage() en-US road"; formatter = unloc_formatter.locale(new ULocale("en-US")); formattedNum = formatter.format(321d); - // status.errIfFailureAndReset("unitUsage() en-US road, formatDouble(...)"); - assertTrue( uTestCase + ", got outputUnit: \"" + formattedNum.getOutputUnit().getIdentifier() + "\"", MeasureUnit.FOOT == formattedNum.getOutputUnit()); - // status.errIfFailureAndReset("unitUsage() en-US road, getOutputUnit(...)"); - assertEquals(uTestCase, "1,050 ft", formattedNum.toString()); - // status.errIfFailureAndReset("unitUsage() en-US road, toString(...)"); { final Object[][] expectedFieldPositions = { {NumberFormat.Field.GROUPING_SEPARATOR, 1, 2}, @@ -958,15 +984,10 @@ public class NumberFormatterApiTest extends TestFmwk { uTestCase = "unitUsage() en-GB person"; formatter = unloc_formatter.locale(new ULocale("en-GB")); formattedNum = formatter.format(80d); - // status.errIfFailureAndReset("unitUsage() en-GB person formatDouble"); - assertTrue( uTestCase + ", got outputUnit: \"" + formattedNum.getOutputUnit().getIdentifier() + "\"", MeasureUnit.forIdentifier("stone-and-pound").equals(formattedNum.getOutputUnit())); - // status.errIfFailureAndReset("unitUsage() en-GB person - formattedNum.getOutputUnit(status)"); - assertEquals(uTestCase, "12 st, 8.4 lb", formattedNum.toString()); - //status.errIfFailureAndReset("unitUsage() en-GB person, toString(...)"); { final Object[][] expectedFieldPositions = { // // Desired output: TODO(icu-units#67) @@ -1053,29 +1074,51 @@ public class NumberFormatterApiTest extends TestFmwk { "0 pounds, 0.31 ounces", "0 pounds, 0 ounces"); - // TODO: this is about the user overriding the usage precision. - // TODO: should be done! -// assertFormatDescendingBig( -// "Scientific notation with Usage: possible when using a reasonable Precision", -// "scientific @### usage/default measure-unit/area-square-meter unit-width-full-name", -// "scientific @### usage/default unit/square-meter unit-width-full-name", -// NumberFormatter.with() -// .unit(MeasureUnit.SQUARE_METER) -// .usage("default") -// .notation(Notation.scientific()) -// .precision(Precision.minMaxSignificantDigits(1, 4)) -// .unitWidth(UnitWidth.FULL_NAME), -// new ULocale("en-ZA"), -// "8,765E1 square kilometres", -// "8,765E0 square kilometres", -// "8,765E1 hectares", -// "8,765E0 hectares", -// "8,765E3 square metres", -// "8,765E2 square metres", -// "8,765E1 square metres", -// "8,765E0 square metres", -// "0E0 square centimetres"); - } + assertFormatDescendingBig( + "Scientific notation with Usage: possible when using a reasonable Precision", + "scientific @### usage/default measure-unit/area-square-meter unit-width-full-name", + "scientific @### usage/default unit/square-meter unit-width-full-name", + NumberFormatter.with() + .unit(MeasureUnit.SQUARE_METER) + .usage("default") + .notation(Notation.scientific()) + .precision(Precision.minMaxSignificantDigits(1, 4)) + .unitWidth(UnitWidth.FULL_NAME), + new ULocale("en-ZA"), + "8,765E1 square kilometres", + "8,765E0 square kilometres", + "8,765E1 hectares", + "8,765E0 hectares", + "8,765E3 square metres", + "8,765E2 square metres", + "8,765E1 square metres", + "8,765E0 square metres", + "0E0 square centimetres"); + + assertFormatSingle( + "Rounding Mode propagates: rounding down", + "usage/road measure-unit/length-centimeter rounding-mode-floor", + "usage/road unit/centimeter rounding-mode-floor", + NumberFormatter.with() + .unit(MeasureUnit.forIdentifier("centimeter")) + .usage("road") + .roundingMode(RoundingMode.FLOOR), + new ULocale("en-ZA"), + 34500, + "300 m"); + + assertFormatSingle( + "Rounding Mode propagates: rounding up", + "usage/road measure-unit/length-centimeter rounding-mode-ceiling", + "usage/road unit/centimeter rounding-mode-ceiling", + NumberFormatter.with() + .unit(MeasureUnit.forIdentifier("centimeter")) + .usage("road") + .roundingMode(RoundingMode.CEILING), + new ULocale("en-ZA"), + 30500, + "350 m"); +} @Test @@ -1122,18 +1165,17 @@ public class NumberFormatterApiTest extends TestFmwk { 321, "300 m"); - // TODO(younies): enable this test case -// assertFormatSingle( -// "Precision can be overridden: override takes precedence", -// "usage/road measure-unit/length-meter @#", -// "usage/road unit/meter @#", -// NumberFormatter.with() -// .unit(MeasureUnit.METER) -// .usage("road") -// .precision(Precision.maxSignificantDigits(2)), -// new ULocale("en-ZA"), -// 321, -// "320 m"); + assertFormatSingle( + "Precision can be overridden: override takes precedence", + "usage/road measure-unit/length-meter @#", + "usage/road unit/meter @#", + NumberFormatter.with() + .unit(MeasureUnit.METER) + .usage("road") + .precision(Precision.maxSignificantDigits(2)), + new ULocale("en-ZA"), + 321, + "320 m"); assertFormatSingle( "Compact notation with Usage: bizarre, but possible (short)", @@ -1147,74 +1189,70 @@ public class NumberFormatterApiTest extends TestFmwk { 987654321L, "988K km"); + assertFormatSingle( + "Compact notation with Usage: bizarre, but possible (short, precision override)", + "compact-short usage/road measure-unit/length-meter @#", + "compact-short usage/road unit/meter @#", + NumberFormatter.with() + .unit(MeasureUnit.METER) + .usage("road") + .notation(Notation.compactShort()) + .precision(Precision.maxSignificantDigits(2)), + new ULocale("en-ZA"), + 987654321L, + "990K km"); + assertFormatSingle( + "Compact notation with Usage: unusual but possible (long)", + "compact-long usage/road measure-unit/length-meter @#", + "compact-long usage/road unit/meter @#", + NumberFormatter.with() + .unit(MeasureUnit.METER) + .usage("road") + .notation(Notation.compactLong()) + .precision(Precision.maxSignificantDigits(2)), + new ULocale("en-ZA"), + 987654321, + "990 thousand km"); - // TODO(younies): enable override precision test cases. -// assertFormatSingle( -// "Compact notation with Usage: bizarre, but possible (short, precision override)", -// "compact-short usage/road measure-unit/length-meter @#", -// "compact-short usage/road unit/meter @#", -// NumberFormatter.with() -// .unit(MeasureUnit.METER) -// .usage("road") -// .notation(Notation.compactShort()) -// .precision(Precision.maxSignificantDigits(2)), -// new ULocale("en-ZA"), -// 987654321L, -// "990K km"); + assertFormatSingle( + "Compact notation with Usage: unusual but possible (long, precision override)", + "compact-long usage/road measure-unit/length-meter @#", + "compact-long usage/road unit/meter @#", + NumberFormatter.with() + .unit(MeasureUnit.METER) + .usage("road") + .notation(Notation.compactLong()) + .precision(Precision.maxSignificantDigits(2)), + new ULocale("en-ZA"), + 987654321, + "990 thousand km"); - // TODO(younies): enable override precision test cases. -// assertFormatSingle( -// "Compact notation with Usage: unusual but possible (long)", -// "compact-long usage/road measure-unit/length-meter @#", -// "compact-long usage/road unit/meter @#", -// NumberFormatter.with() -// .unit(MeasureUnit.METER) -// .usage("road") -// .notation(Notation.compactLong()) -// .precision(Precision.maxSignificantDigits(2)), -// new ULocale("en-ZA"), -// 987654321, -// "990 thousand km"); + assertFormatSingle( + "Scientific notation, not recommended, requires precision override for road", + "scientific usage/road measure-unit/length-meter", + "scientific usage/road unit/meter", + NumberFormatter.with() + .unit(MeasureUnit.METER) + .usage("road") + .notation(Notation.scientific()), + new ULocale("en-ZA"), + 321.45, + // Rounding to the nearest "50" is not exponent-adjusted in scientific notation: + "0E2 m"); - // TODO(younies): enable override precision test cases. -// assertFormatSingle( -// "Compact notation with Usage: unusual but possible (long, precision override)", -// "compact-long usage/road measure-unit/length-meter @#", -// "compact-long usage/road unit/meter @#", -// NumberFormatter.with() -// .unit(MeasureUnit.METER) -// .usage("road") -// .notation(Notation.compactLong()) -// .precision(Precision.maxSignificantDigits(2)), -// new ULocale("en-ZA"), -// 987654321, -// "990 thousand km"); - - // TODO(younies): enable override precision test cases. -// assertFormatSingle( -// "Scientific notation, not recommended, requires precision override for road", -// "scientific usage/road measure-unit/length-meter", -// "scientific usage/road unit/meter", -// NumberFormatter.with().unit(MeasureUnit.METER).usage("road").notation(Notation.scientific()), -// new ULocale("en-ZA"), -// 321.45, -// // Rounding to the nearest "50" is not exponent-adjusted in scientific notation: -// "0E2 m"); - - // TODO(younies): enable override precision test cases. -// assertFormatSingle( -// "Scientific notation with Usage: possible when using a reasonable Precision", -// "scientific usage/road measure-unit/length-meter @###", -// "scientific usage/road unit/meter @###", -// NumberFormatter.with() -// .unit(MeasureUnit.METER) -// .usage("road") -// .notation(Notation.scientific()) -// .precision(Precision.maxSignificantDigits(4)), -// new ULocale("en-ZA"), -// 321.45, // 0.45 rounds down, 0.55 rounds up. -// "3,214E2 m"); + assertFormatSingle( + "Scientific notation with Usage: possible when using a reasonable Precision", + "scientific usage/road measure-unit/length-meter @###", + "scientific usage/road unit/meter @###", + NumberFormatter.with() + .unit(MeasureUnit.METER) + .usage("road") + .notation(Notation.scientific()) + .precision(Precision.maxSignificantDigits(4)), + new ULocale("en-ZA"), + 321.45, // 0.45 rounds down, 0.55 rounds up. + "3,214E2 m"); assertFormatSingle( "Scientific notation with Usage: possible when using a reasonable Precision",