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

catamorphism · catamorphism · commit dfffa44456b5 · 2025-02-25T18:25:07.000-08:00
diff --git a/icu4c/source/i18n/messageformat2.cpp b/icu4c/source/i18n/messageformat2.cpp
@@ -251,11 +251,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;
     }
@@ -265,7 +261,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 {
@@ -289,8 +285,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);
@@ -415,6 +410,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,
@@ -440,7 +477,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) {
diff --git a/icu4c/source/i18n/messageformat2_evaluation.cpp b/icu4c/source/i18n/messageformat2_evaluation.cpp
@@ -32,6 +32,7 @@ BaseValue::BaseValue(const Locale& loc, const UnicodeString& fb, const Formattab
     fallback += LEFT_CURLY_BRACE;
     fallback += fb;
     fallback += RIGHT_CURLY_BRACE;
+    dir = UBIDI_MIXED; // represents 'unknown'
 }
 
 /* static */ BaseValue* BaseValue::create(const Locale& locale,
@@ -41,18 +42,16 @@ BaseValue::BaseValue(const Locale& loc, const UnicodeString& fb, const Formattab
     return message2::create<BaseValue>(BaseValue(locale, fallback, 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;
     fallback = other.fallback;
 
diff --git a/icu4c/source/i18n/messageformat2_formattable.cpp b/icu4c/source/i18n/messageformat2_formattable.cpp
@@ -10,6 +10,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"
@@ -188,35 +189,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);
@@ -279,7 +284,7 @@ namespace message2 {
         }
         }
 
-        return handleBiDi(locale, dir, result);
+        return result;
     }
 
 } // namespace message2
diff --git a/icu4c/source/i18n/messageformat2_formatter.cpp b/icu4c/source/i18n/messageformat2_formatter.cpp
@@ -92,6 +92,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)
@@ -158,6 +165,7 @@ namespace message2 {
 
         normalizedInput = builder.normalizedInput;
         signalErrors = builder.signalErrors;
+        bidiIsolationStrategy = builder.bidiIsolationStrategy;
 
         // Build data model
         // First, check that there is a data model
@@ -205,6 +213,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;
diff --git a/icu4c/source/i18n/messageformat2_function_registry.cpp b/icu4c/source/i18n/messageformat2_function_registry.cpp
@@ -589,6 +589,7 @@ StandardFunctions::NumberValue::NumberValue(const Number& parent,
     opts = options.mergeOptions(arg.getResolvedOptions(), errorCode);
     operand = arg.getOperand();
     functionName = UnicodeString(parent.isInteger ? "integer" : "number");
+    dir = locale.isRightToLeft() ? UBIDI_RTL : UBIDI_LTR;
 
     number::LocalizedNumberFormatter realFormatter;
     realFormatter = formatterForOptions(parent, locale, opts, errorCode);
@@ -860,6 +861,7 @@ StandardFunctions::DateTimeValue::DateTimeValue(DateTime::DateTimeType type,
             functionName = UnicodeString("time");
             break;
     }
+    dir = locale.isRightToLeft() ? UBIDI_RTL : UBIDI_LTR;
 
     const Formattable* source = &operand;
 
@@ -1112,7 +1114,6 @@ StandardFunctions::String::string(UErrorCode& success) {
 }
 
 extern UnicodeString formattableToString(const Locale&,
-                                         const UBiDiDirection,
                                          const Formattable&,
                                          UErrorCode&);
 
@@ -1145,9 +1146,10 @@ StandardFunctions::StringValue::StringValue(const FunctionContext& context,
     CHECK_ERROR(status);
     operand = val.getOperand();
     functionName = UnicodeString("string");
+    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,
diff --git a/icu4c/source/i18n/messageformat2_parser.cpp b/icu4c/source/i18n/messageformat2_parser.cpp
@@ -2021,7 +2021,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
diff --git a/icu4c/source/i18n/unicode/messageformat2.h b/icu4c/source/i18n/unicode/messageformat2.h
@@ -168,6 +168,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.
@@ -197,6 +222,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:
@@ -280,6 +308,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)
@@ -356,6 +405,7 @@ namespace message2 {
 
         // Formatting methods
         [[nodiscard]] InternalValue evalLiteral(const UnicodeString&, 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&&,
@@ -433,6 +483,8 @@ namespace message2 {
         // The default is false.
         bool signalErrors = false;
 
+        // Bidi isolation strategy.
+        UMFBidiIsolationStrategy bidiIsolationStrategy = U_MF_BIDI_NONE;
     }; // class MessageFormatter
 
 } // namespace message2
diff --git a/icu4c/source/i18n/unicode/messageformat2_function_registry.h b/icu4c/source/i18n/unicode/messageformat2_function_registry.h
@@ -369,6 +369,17 @@ namespace message2 {
              * @deprecated This API is for technology preview only.
              */
             virtual const FunctionOptions& getResolvedOptions() const { return opts; }
+            /*
+             * Returns the directionality of this value, i.e. the directionality
+             * that its formatted result should have.
+             *
+             * @return A UBiDiDirection indicating the directionality that
+             *         the formatted result of this value should have.
+             *
+             * @internal ICU 77 technology preview
+             * @deprecated This API is for technology preview only.
+             */
+            virtual UBiDiDirection getDirection() const { return dir; }
             /**
              * Returns true if this value supports selection. The default method
              * returns false. The method must be overridden for values that support
@@ -486,6 +497,14 @@ namespace message2 {
              * @deprecated This API is for technology preview only.
              */
             UnicodeString fallback;
+         private:
+            /*
+             * Directionality that this value should be formatted with.
+             *
+             * @internal ICU 77 technology preview
+             * @deprecated This API is for technology preview only.
+             */
+            UBiDiDirection dir;
     }; // class FunctionValue
 
 } // namespace message2
diff --git a/icu4c/source/test/intltest/messageformat2test_read_json.cpp b/icu4c/source/test/intltest/messageformat2test_read_json.cpp
diff --git a/icu4c/source/test/intltest/messageformat2test_utils.h b/icu4c/source/test/intltest/messageformat2test_utils.h
diff --git a/testdata/message2/bidi.json b/testdata/message2/bidi.json
diff --git a/testdata/message2/u-options.json b/testdata/message2/u-options.json
