Skip to content

Commit

Permalink
Add Intl.Collator implementation using ICU4C (#1413) (#1413)
Browse files Browse the repository at this point in the history
Summary:
Original Author: [email protected]
Original Git: a84b2f9
Original Reviewed By: avp
Original Revision: D64839005

Add Intl.Collator implementation using ICU4C for non-Android / non-Apple platforms.

Classes without ICU / Platform API dependency:
* Constants - String constants for options bag’s names and values, valid option values, other constants.
* IntlUtils - Conversion between UTF-8 and UTF-16 ASCII strings, convert string to bool, i.e. case- insensitive match against “true”, lowercase ASCII strings.
* LocaleBCP47Object - Wrapper over a BCP47Parser::ParsedLocaleIdentifier, provides factory method to create instances from a BCP47 locale string, methods to get canonicalized locale string and locale string without extensions, and implements the CanonicalizeLocaleList() spec operation.
* OptionHelpers - Gets string, bool, and number option from options bag.

ICU dependent classes:
* Collator - Intl.Collator implementation using ICU collator API.
* Locale - Conversion between BCP47 and ICU locale strings.
* LocaleResolver - Implements ResolveLocale() and SupportedLocales() spec operations, supports both “lookup” and “best fit” locale matching. “best fit” locale matching uses ICU acceptLanguage API.

Fix memory leaks in existing DateTimeFormat implementation in PlatformIntlICU.cpp by wrapping created ICU object pointers in unique_ptr.

Migrate ResolveLocale, SupportedLocalesOf, GetCanonicalLocales API implementation to use the corresponding newly added supporting classes.

Add test cases to verify invalid locales and options input would result in throwing JS exception in collation.js. Add tests cases to verify resolution of locale extensions and options in a new file collation-resolved-options.js.

Pull Request resolved: #1413

Pulled By: neildhar

Reviewed By: avp

Differential Revision: D68920589

fbshipit-source-id: 84e18ebb06aed81bd8a2ba8b3a5c6bdcf3637df7
  • Loading branch information
lavenzg authored and facebook-github-bot committed Feb 14, 2025
1 parent c605bcb commit 58aa95b
Show file tree
Hide file tree
Showing 24 changed files with 2,564 additions and 685 deletions.
113 changes: 113 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,116 @@ jobs:
run: |-
cmake -S hermes -B build -DCMAKE_BUILD_TYPE=Debug
cmake --build build --target check-hermes all -j 4
test-windows:
runs-on: windows-2019
steps:
- uses: actions/[email protected]
with:
path: hermes
- name: Run Hermes regression tests
run: |-
cmake -S hermes -B build -G 'Visual Studio 16 2019'
cmake --build build --target check-hermes -- -m /p:UseMultiToolTask=true -m /p:EnforceProcessCountAcrossBuilds=true
test-e2e:
runs-on: ubuntu-22.04
env:
ANDROID_NDK: /usr/local/lib/android/sdk/ndk/27.1.12297006
HERMES_WS_DIR: /home/runner/work/hermes
REACT_NATIVE_OVERRIDE_HERMES_DIR: /home/runner/work/hermes/hermes
steps:
- name: Install Node
uses: actions/[email protected]
- name: Install JDK
uses: actions/setup-java@v3
with:
distribution: "temurin"
java-version: "17"
- name: Checkout Hermes
uses: actions/[email protected]
- name: Checkout React Native
run: |-
cd "$HERMES_WS_DIR"
git clone --depth=1 https://github.com/facebook/react-native
cd react-native
yarn install
echo "console.log('Using Hermes: ' + (global.HermesInternal != null));" >> packages/rn-tester/js/RNTesterApp.android.js
- name: Run RNTester
uses: ReactiveCircus/[email protected]
with:
api-level: 29
ndk: 27.1.12297006
cmake: 3.22.1
script: |
cd ../react-native && ./gradlew -PreactNativeArchitectures=x86 :packages:rn-tester:android:app:installHermesRelease
adb shell am start com.facebook.react.uiapp/.RNTesterActivity
timeout 30s adb logcat -e "Using Hermes: true" -m 1
test-e2e-intl:
runs-on: ubuntu-22.04
env:
HERMES_WS_DIR: /home/runner/work/hermes
ANDROID_NDK: /usr/local/lib/android/sdk/ndk/27.1.12297006
steps:
- name: Checkout Hermes
uses: actions/[email protected]
- name: Checkout Test262
run: |-
cd "$HERMES_WS_DIR"
git clone https://github.com/tc39/test262
cd test262
git checkout 62626e083bd506124aac6c799464d76c2c42851b
- name: Build Hermes Compiler
run: |-
cd "$HERMES_WS_DIR"
cmake -S hermes -B ./build -DCMAKE_BUILD_TYPE=Release
cmake --build ./build -j 4 --target hermesc
- name: Run android tests
uses: ReactiveCircus/[email protected]
with:
api-level: 29
emulator-options: -timezone Europe/Paris -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
script: cd android && ./gradlew :intltest:prepareTests && ./gradlew -Pabis=x86 :intltest:connectedAndroidTest
test-macos-test262:
runs-on: macos-latest
steps:
- uses: maxim-lobanov/setup-xcode@v1
with:
xcode-version: 15.4
- uses: actions/[email protected]
with:
path: hermes
- name: Setup dependencies
run: |-
brew install cmake ninja
# Check out test262 at a pinned revision to reduce flakiness
git clone https://github.com/tc39/test262
cd test262
git checkout 62626e083bd506124aac6c799464d76c2c42851b
- name: Run Hermes tests and test262 with Intl
run: |-
cmake -S hermes -B build -GNinja -DHERMES_ENABLE_INTL=ON
cmake --build ./build
cmake --build ./build --target check-hermes
python3 hermes/utils/testsuite/run_testsuite.py --test-intl test262/test -b build/bin
test-linux-test262:
runs-on: ubuntu-20.04
steps:
- uses: actions/[email protected]
with:
path: hermes
- name: Setup dependencies
run: |-
sudo apt update
sudo apt install -y git openssh-client cmake build-essential \
libreadline-dev libicu-dev zip python3
# Check out test262 at a pinned revision to reduce flakiness
git clone https://github.com/tc39/test262
cd test262
git checkout 62626e083bd506124aac6c799464d76c2c42851b
- name: Run test262 with Intl
run: |-
cmake -S hermes -B build -DHERMES_ENABLE_INTL=ON -DCMAKE_CXX_FLAGS=-O2 -DCMAKE_C_FLAGS=-O2
cmake --build ./build -j 4
# Not running Hermes test until more of Intl is built out:
# toLocaleLowerCase and toLocaleUpperCase are the two main ones.
# cmake --build ./build --target check-hermes -j 4
python3 hermes/utils/testsuite/run_testsuite.py --test-intl test262/test -b build/bin
68 changes: 0 additions & 68 deletions include/hermes/Platform/Intl/PlatformIntlShared.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,57 +14,6 @@
namespace hermes {
namespace platform_intl {

struct LocaleMatch {
std::u16string locale;
std::map<std::u16string, std::u16string> extensions;
};

struct ResolvedLocale {
std::u16string locale;
std::u16string dataLocale;
std::unordered_map<std::u16string, std::u16string> extensions;
};

/// https://402.ecma-international.org/8.0/#sec-canonicalizelocalelist
vm::CallResult<std::vector<std::u16string>> canonicalizeLocaleList(
vm::Runtime &runtime,
const std::vector<std::u16string> &locales);

/// https://402.ecma-international.org/8.0/#sec-intl.getcanonicallocales
vm::CallResult<std::vector<std::u16string>> getCanonicalLocales(
vm::Runtime &runtime,
const std::vector<std::u16string> &locales);

/// https://402.ecma-international.org/8.0/#sec-bestavailablelocale
std::optional<std::u16string> bestAvailableLocale(
const std::vector<std::u16string> &availableLocales,
const std::u16string &locale);

/// https://402.ecma-international.org/8.0/#sec-lookupsupportedlocales
std::vector<std::u16string> lookupSupportedLocales(
const std::vector<std::u16string> &availableLocales,
const std::vector<std::u16string> &requestedLocales);

/// https://402.ecma-international.org/8.0/#sec-supportedlocales
std::vector<std::u16string> supportedLocales(
const std::vector<std::u16string> &availableLocales,
const std::vector<std::u16string> &requestedLocales);

/// https://402.ecma-international.org/8.0/#sec-getoption
vm::CallResult<std::optional<std::u16string>> getOptionString(
vm::Runtime &runtime,
const Options &options,
const std::u16string &property,
llvh::ArrayRef<std::u16string_view> values,
std::optional<std::u16string_view> fallback);

/// https://402.ecma-international.org/8.0/#sec-getoption
std::optional<bool> getOptionBool(
vm::Runtime &runtime,
const Options &options,
const std::u16string &property,
std::optional<bool> fallback);

/// https://402.ecma-international.org/8.0/#sec-todatetimeoptions
vm::CallResult<Options> toDateTimeOptions(
vm::Runtime &runtime,
Expand All @@ -75,23 +24,6 @@ vm::CallResult<Options> toDateTimeOptions(
/// https://402.ecma-international.org/8.0/#sec-case-sensitivity-and-case-mapping
std::u16string toASCIIUppercase(std::u16string_view tz);

/// https://402.ecma-international.org/8.0/#sec-defaultnumberoption
vm::CallResult<std::optional<uint8_t>> defaultNumberOption(
vm::Runtime &runtime,
const std::u16string &property,
std::optional<Option> value,
const std::uint8_t minimum,
const std::uint8_t maximum,
std::optional<uint8_t> fallback);

/// https://402.ecma-international.org/8.0/#sec-getoption
vm::CallResult<std::optional<uint8_t>> getNumberOption(
vm::Runtime &runtime,
const Options &options,
const std::u16string &property,
const std::uint8_t minimum,
const std::uint8_t maximum,
std::optional<uint8_t> fallback);
} // namespace platform_intl
} // namespace hermes

Expand Down
10 changes: 9 additions & 1 deletion lib/Platform/Intl/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,15 @@ if(HERMES_ENABLE_INTL)
set_target_properties(hermesPlatformIntl_obj PROPERTIES UNITY_BUILD false)
target_compile_options(hermesPlatformIntl_obj PRIVATE -fobjc-arc)
else()
add_hermes_library(hermesPlatformIntl PlatformIntlICU.cpp PlatformIntlShared.cpp
add_hermes_library(hermesPlatformIntl
PlatformIntlICU.cpp
PlatformIntlShared.cpp
impl_icu/Collator.cpp
impl_icu/IntlUtils.cpp
impl_icu/LocaleConverter.cpp
impl_icu/LocaleBCP47Object.cpp
impl_icu/LocaleResolver.cpp
impl_icu/OptionHelpers.cpp
LINK_OBJLIBS
hermesBCP47Parser
hermesPublic
Expand Down
Loading

0 comments on commit 58aa95b

Please sign in to comment.