Implement bidi default strategy and update tests from unicode-org/message-format-wg#917

Author: catamorphism

icu4c/source/i18n/messageformat2.cpp

@@ -197,11 +197,7 @@ FunctionOptions MessageFormatter::resolveOptions(Environment& env,
     return FunctionOptions(std::move(*optionsVector), status);
 }
 
-static UBiDiDirection getBiDiDirection(const Locale& locale,
-                                       const UnicodeString& s) {
-    if (s.isEmpty()) {
-        return locale.isRightToLeft() ? UBIDI_RTL : UBIDI_LTR;
-    }
+static UBiDiDirection getBiDiDirection(const UnicodeString& s) {
     if (s == u"ltr") {
         return UBIDI_LTR;
     }
@@ -211,7 +207,7 @@ static UBiDiDirection getBiDiDirection(const Locale& locale,
     if (s == u"auto") {
         return UBIDI_MIXED;
     }
-    return UBIDI_NEUTRAL;
+    return UBIDI_MIXED; // stands in for "unknown"
 }
 
 FunctionContext MessageFormatter::makeFunctionContext(const FunctionOptions& options) const {
@@ -235,8 +231,7 @@ FunctionContext MessageFormatter::makeFunctionContext(const FunctionOptions& opt
             localeToUse = locale;
         }
     }
-    UBiDiDirection dir = getBiDiDirection(localeToUse,
-                                          options.getStringFunctionOption(UnicodeString("u:dir")));
+    UBiDiDirection dir = getBiDiDirection(options.getStringFunctionOption(UnicodeString("u:dir")));
     UnicodeString id = options.getStringFunctionOption(UnicodeString("u:id"));
 
     return FunctionContext(localeToUse, dir, id);
@@ -360,6 +355,48 @@ FunctionContext MessageFormatter::makeFunctionContext(const FunctionOptions& opt
     }
 }
 
+UnicodeString& MessageFormatter::bidiIsolate(UBiDiDirection dir, UnicodeString& fmt) const {
+    // If strategy is 'none', just return the string
+    if (bidiIsolationStrategy == U_MF_BIDI_NONE) {
+        return fmt;
+    }
+
+    /* 1. Let msgdir be the directionality of the whole message, one of « 'LTR', 'RTL', 'unknown' ». These correspond to the message having left-to-right directionality, right-to-left directionality, and to the message's directionality not being known. */
+    bool isLtr = !locale.isRightToLeft();
+
+    // 2i Let fmt be the formatted string representation of the resolved value of exp.
+    // (Passed as argument)
+
+    // 2ii Let dir be the directionality of fmt, one of « 'LTR', 'RTL', 'unknown' », with the same meanings as for msgdir
+    // (Passed as argument)
+
+    // 2iii. If dir is 'LTR'
+    switch (dir) {
+        case UBIDI_LTR:
+            if (isLtr) {
+                // 2iii(a). If msgdir is 'LTR' in the formatted output, let fmt be itself
+                return fmt;
+            }
+            // 2iii(b) Else, in the formatted output, prefix fmt with U+2066 LEFT-TO-RIGHT ISOLATE and postfix it with U+2069 POP DIRECTIONAL ISOLATE.
+            fmt.insert(0, LRI_CHAR);
+            fmt.insert(fmt.length(), PDI_CHAR);
+            break;
+        // 2iv. Else, if dir is 'RTL': 
+        case UBIDI_RTL:
+            // 2iv(a). In the formatted output, prefix fmt with U+2067 RIGHT-TO-LEFT ISOLATE and postfix it with U+2069 POP DIRECTIONAL ISOLATE.
+            fmt.insert(0, RLI_CHAR);
+            fmt.insert(fmt.length(), PDI_CHAR);
+            break;
+        // 2v. Else: 
+        default:
+            // 2v(a). In the formatted output, prefix fmt with U+2068 FIRST STRONG ISOLATE and postfix it with U+2069 POP DIRECTIONAL ISOLATE.
+            fmt.insert(0, FSI_CHAR);
+            fmt.insert(fmt.length(), PDI_CHAR);
+            break;
+    }
+    return fmt;
+}
+
 // Formats each text and expression part of a pattern, appending the results to `result`
 void MessageFormatter::formatPattern(MessageContext& context,
                                      Environment& globalEnv,
@@ -385,7 +422,8 @@ void MessageFormatter::formatPattern(MessageContext& context,
                   const FunctionValue* val = partVal.getValue(status);
                   // Shouldn't be null or a fallback
                   U_ASSERT(U_SUCCESS(status));
-                  result += val->formatToString(status);
+                  UnicodeString fmt = val->formatToString(status);
+                  result += bidiIsolate(val->getDirection(), fmt);
                   // Handle formatting errors. `formatToString()` can't take a context and thus can't
                   // register an error directly
                   if (status == U_MF_FORMATTING_ERROR) {
@@ -415,7 +453,7 @@ void MessageFormatter::resolveSelectors(MessageContext& context, Environment& en
     // 2. For each expression exp of the message's selectors
     for (int32_t i = 0; i < dataModel.numSelectors(); i++) {
         // 2i. Let rv be the resolved value of exp.
-        InternalValue& rv = evalExpression(env, selectors[i], context, status);
+        InternalValue& rv = evalOperand(env, Operand(selectors[i]), context, status);
         if (rv.isSelectable()) {
             // 2ii. If selection is supported for rv:
             // (True if this code has been reached)

icu4c/source/i18n/messageformat2_evaluation.cpp

@@ -27,6 +27,7 @@ using namespace data_model;
 BaseValue::BaseValue(const Locale& loc, const Formattable& source)
     : locale(loc) {
     operand = source;
+    dir = UBIDI_MIXED; // represents 'unknown'
 }
 
 /* static */ BaseValue* BaseValue::create(const Locale& locale,
@@ -35,18 +36,16 @@ BaseValue::BaseValue(const Locale& loc, const Formattable& source)
     return message2::create<BaseValue>(BaseValue(locale, source), errorCode);
 }
 
-extern UnicodeString formattableToString(const Locale&, const UBiDiDirection, const Formattable&, UErrorCode&);
+extern UnicodeString formattableToString(const Locale&, const Formattable&, UErrorCode&);
 
 UnicodeString BaseValue::formatToString(UErrorCode& errorCode) const {
-    return formattableToString(locale,
-                               UBIDI_NEUTRAL,
-                               operand,
-                               errorCode);
+    return formattableToString(locale, operand, errorCode);
 }
 
 BaseValue& BaseValue::operator=(BaseValue&& other) noexcept {
     operand = std::move(other.operand);
     opts = std::move(other.opts);
+    dir = other.dir;
     locale = other.locale;
 
     return *this;

icu4c/source/i18n/messageformat2_formattable.cpp

@@ -8,6 +8,7 @@
 #if !UCONFIG_NO_MF2
 
 #include "unicode/messageformat2_formattable.h"
+#include "unicode/messageformat2.h"
 #include "unicode/smpdtfmt.h"
 #include "unicode/ubidi.h"
 #include "messageformat2_allocation.h"
@@ -186,35 +187,39 @@ namespace message2 {
         df->format(date, result, 0, errorCode);
     }
 
-    static UnicodeString& handleBiDi(const Locale& locale,
+/*
+    static UnicodeString& handleBiDi(MessageFormatter::UMFBidiIsolationStrategy strategy,
+                                     const Locale& locale,
                                      UBiDiDirection dir,
                                      UnicodeString& result) {
-        switch (dir) {
-        case UBIDI_LTR:
-            if (locale.isRightToLeft()) {
-                result.insert(0, LRI_CHAR);
+        if (strategy == MessageFormatter::U_MF_BIDI_DEFAULT) {
+            switch (dir) {
+            case UBIDI_LTR:
+                if (locale.isRightToLeft()) {
+                    result.insert(0, LRI_CHAR);
+                    result.insert(result.length(), PDI_CHAR);
+                }
+                break;
+            case UBIDI_RTL:
+                result.insert(0, RLI_CHAR);
                 result.insert(result.length(), PDI_CHAR);
+                break;
+            case UBIDI_NEUTRAL:
+                // Do nothing
+                break;
+            case UBIDI_MIXED:
+                // mixed = auto
+                result.insert(0, FSI_CHAR);
+                result.insert(result.length(), PDI_CHAR);
+                break;
             }
-            break;
-        case UBIDI_RTL:
-            result.insert(0, RLI_CHAR);
-            result.insert(result.length(), PDI_CHAR);
-            break;
-        case UBIDI_NEUTRAL:
-            // Do nothing
-            break;
-        case UBIDI_MIXED:
-            // mixed = auto
-            result.insert(0, FSI_CHAR);
-            result.insert(result.length(), PDI_CHAR);
-            break;
         }
 
         return result;
     }
+*/
 
     UnicodeString formattableToString(const Locale& locale,
-                                      UBiDiDirection dir,
                                       const Formattable& toFormat,
                                       UErrorCode& status) {
         EMPTY_ON_ERROR(status);
@@ -277,7 +282,7 @@ namespace message2 {
         }
         }
 
-        return handleBiDi(locale, dir, result);
+        return result;
     }
 
 } // namespace message2

icu4c/source/i18n/messageformat2_formatter.cpp

@@ -90,6 +90,13 @@ namespace message2 {
                return *this;
     }
 
+    MessageFormatter::Builder&
+    MessageFormatter::Builder::setBidiIsolationStrategy(
+        MessageFormatter::UMFBidiIsolationStrategy strategy) {
+            bidiIsolationStrategy = strategy;
+            return *this;
+    }
+
     /*
       This build() method is non-destructive, which entails the risk that
       its borrowed MFFunctionRegistry and (if the setDataModel() method was called)
@@ -147,6 +154,7 @@ namespace message2 {
 
         normalizedInput = builder.normalizedInput;
         signalErrors = builder.signalErrors;
+        bidiIsolationStrategy = builder.bidiIsolationStrategy;
 
         // Build data model
         // First, check that there is a data model
@@ -194,6 +202,7 @@ namespace message2 {
         dataModel = std::move(other.dataModel);
         normalizedInput = std::move(other.normalizedInput);
         signalErrors = other.signalErrors;
+        bidiIsolationStrategy = other.bidiIsolationStrategy;
         errors = other.errors;
         other.errors = nullptr;
         return *this;

icu4c/source/i18n/messageformat2_function_registry.cpp

@@ -540,6 +540,7 @@ StandardFunctions::NumberValue::NumberValue(const Number& parent,
     locale = context.getLocale();
     opts = options.mergeOptions(arg.getResolvedOptions(), errorCode);
     operand = arg.getOperand();
+    dir = locale.isRightToLeft() ? UBIDI_RTL : UBIDI_LTR;
 
     number::LocalizedNumberFormatter realFormatter;
     realFormatter = formatterForOptions(parent, locale, opts, errorCode);
@@ -785,6 +786,7 @@ StandardFunctions::DateTimeValue::DateTimeValue(DateTime::DateTimeType type,
     const Locale& locale = context.getLocale();
     operand = val.getOperand();
     opts = options.mergeOptions(val.getResolvedOptions(), errorCode);
+    dir = locale.isRightToLeft() ? UBIDI_RTL : UBIDI_LTR;
 
     const Formattable* source = &operand;
 
@@ -1039,7 +1041,6 @@ StandardFunctions::String::string(UErrorCode& success) {
 }
 
 extern UnicodeString formattableToString(const Locale&,
-                                         const UBiDiDirection,
                                          const Formattable&,
                                          UErrorCode&);
 
@@ -1071,9 +1072,10 @@ StandardFunctions::StringValue::StringValue(const FunctionContext& context,
                                             UErrorCode& status) {
     CHECK_ERROR(status);
     operand = val.getOperand();
+    dir = context.getDirection();
     // No options
     // Convert to string
-    formattedString = formattableToString(context.getLocale(), context.getDirection(), operand, status);
+    formattedString = formattableToString(context.getLocale(), operand, status);
 }
 
 void StandardFunctions::StringValue::selectKeys(const UnicodeString* keys,

icu4c/source/i18n/messageformat2_parser.cpp

@@ -1972,7 +1972,7 @@ void Parser::parseSelectors(UErrorCode& status) {
     // Parse selectors
     // "Backtracking" is required here. It's not clear if whitespace is
     // (`[s]` selector) or (`[s]` variant)
-    while (isWhitespace(peek()) || peek() == DOLLAR) {
+    while (isWhitespace(peek()) || isBidiControl(peek()) || peek() == DOLLAR) {
         int32_t whitespaceStart = index;
         parseRequiredWhitespace(status);
         // Restore precondition

icu4c/source/i18n/unicode/messageformat2.h

@@ -165,6 +165,31 @@ namespace message2 {
             U_MF_STRICT
         } UMFErrorHandlingBehavior;
 
+        /**
+         * Used in conjunction with the
+         * MessageFormatter::Builder::setBidiIsolationStrategy() method.
+         *
+         * @internal ICU 77 technology preview
+         * @deprecated This API is for technology preview only.
+         */
+        typedef enum UMFBidiIsolationStrategy {
+            /**
+             * Do not perform bidi isolation (default)
+             *
+             * @internal ICU 77 technology preview
+             * @deprecated This API is for technology preview only.
+             */
+            U_MF_BIDI_NONE = 0,
+            /**
+             * Perform bidi isolation using the "default" strategy
+             * described in the MF2 specification
+             * https://github.com/unicode-org/message-format-wg/blob/main/spec/formatting.md#handling-bidirectional-text
+             *
+             * @internal ICU 77 technology preview
+             * @deprecated This API is for technology preview only.
+             */
+            U_MF_BIDI_DEFAULT
+        } UMFBidiIsolationStrategy;
         /**
          * The mutable Builder class allows each part of the MessageFormatter to be initialized
          * separately; calling its `build()` method yields an immutable MessageFormatter.
@@ -194,6 +219,9 @@ namespace message2 {
             const MFFunctionRegistry* customMFFunctionRegistry;
             // Error behavior; see comment in `MessageFormatter` class
             bool signalErrors = false;
+            // Bidi isolation strategy
+            MessageFormatter::UMFBidiIsolationStrategy
+                bidiIsolationStrategy = U_MF_BIDI_NONE;
 
             void clearState();
         public:
@@ -277,6 +305,27 @@ namespace message2 {
              * @deprecated This API is for technology preview only.
              */
             Builder& setErrorHandlingBehavior(UMFErrorHandlingBehavior type);
+            /**
+             * Set the bidi isolation behavior for this formatter.
+             *
+             * "None" means that no bidi isolation will be performed.
+             * "Default" means that the default bidi isolation strategy
+             * as described in the MF2 specification
+             * ( https://github.com/unicode-org/message-format-wg/blob/main/spec/formatting.md#handling-bidirectional-text )
+             * will be applied.
+             *
+             * @param strategy An enum with type UMFBidiIsolationStrategy;
+             *             if strategy == U_MF_BIDI_NONE, then the behavior is "None".
+             *             If strategy == U_MF_BIDI_DEFAULT, then the behavior is "Default".
+             *
+             * The default is "None".
+             *
+             * @return       A reference to the builder.
+             *
+             * @internal ICU 76 technology preview
+             * @deprecated This API is for technology preview only.
+             */
+            Builder& setBidiIsolationStrategy(UMFBidiIsolationStrategy strategy);
             /**
              * Constructs a new immutable MessageFormatter using the pattern or data model
              * that was previously set, and the locale (if it was previously set)
@@ -351,6 +400,7 @@ namespace message2 {
 
         // Formatting methods
         [[nodiscard]] InternalValue evalLiteral(const data_model::Literal&, UErrorCode&) const;
+        [[nodiscard]] UnicodeString& bidiIsolate(UBiDiDirection dir, UnicodeString&) const;
         void formatPattern(MessageContext&, Environment&, const data_model::Pattern&, UErrorCode&, UnicodeString&) const;
         FunctionContext makeFunctionContext(const FunctionOptions&) const;
         [[nodiscard]] InternalValue& apply(Environment&, const FunctionName&, InternalValue&, FunctionOptions&&,
@@ -426,6 +476,9 @@ namespace message2 {
         // formatting methods return best-effort output.
         // The default is false.
         bool signalErrors = false;
+
+        // Bidi isolation strategy.
+        UMFBidiIsolationStrategy bidiIsolationStrategy = U_MF_BIDI_NONE;
     }; // class MessageFormatter
 
 } // namespace message2