Skip to content

Commit

Permalink
Simplify Icu API (#3503)
Browse files Browse the repository at this point in the history
* Simplify `Icu` API

* Fix docs

* Document internals

* Simplify inner API
  • Loading branch information
jedel1043 committed Dec 7, 2023
1 parent 279caab commit 52b39fa
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 174 deletions.
13 changes: 8 additions & 5 deletions core/engine/src/builtins/intl/collator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ use crate::{
OrdinaryObject,
},
context::{
icu::IntlProvider,
intrinsics::{Intrinsics, StandardConstructor, StandardConstructors},
BoaProvider,
},
js_string,
native_function::NativeFunction,
Expand Down Expand Up @@ -79,7 +79,7 @@ impl Service for Collator {

type LocaleOptions = CollatorLocaleOptions;

fn resolve(locale: &mut Locale, options: &mut Self::LocaleOptions, provider: &BoaProvider) {
fn resolve(locale: &mut Locale, options: &mut Self::LocaleOptions, provider: &IntlProvider) {
let collation = options
.collation
.take()
Expand Down Expand Up @@ -274,8 +274,11 @@ impl BuiltInConstructor for Collator {

// 18. Let relevantExtensionKeys be %Collator%.[[RelevantExtensionKeys]].
// 19. Let r be ResolveLocale(%Collator%.[[AvailableLocales]], requestedLocales, opt, relevantExtensionKeys, localeData).
let mut locale =
resolve_locale::<Self>(&requested_locales, &mut intl_options, context.icu());
let mut locale = resolve_locale::<Self>(
&requested_locales,
&mut intl_options,
context.intl_provider(),
);

let collator_locale = {
// `collator_locale` needs to be different from the resolved locale because ECMA402 doesn't
Expand Down Expand Up @@ -332,7 +335,7 @@ impl BuiltInConstructor for Collator {
.unzip();

let collator =
NativeCollator::try_new_unstable(context.icu().provider(), &collator_locale, {
NativeCollator::try_new_unstable(context.intl_provider(), &collator_locale, {
let mut options = icu_collator::CollatorOptions::new();
options.strength = strength;
options.case_level = case_level;
Expand Down
8 changes: 4 additions & 4 deletions core/engine/src/builtins/intl/list_format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ impl BuiltInConstructor for ListFormat {
matcher,
..Default::default()
},
context.icu(),
context.intl_provider(),
);

// 11. Let type be ? GetOption(options, "type", string, « "conjunction", "disjunction", "unit" », "conjunction").
Expand All @@ -144,17 +144,17 @@ impl BuiltInConstructor for ListFormat {
let data_locale = DataLocale::from(&locale);
let formatter = match typ {
ListFormatType::Conjunction => ListFormatter::try_new_and_with_length_unstable(
context.icu().provider(),
context.intl_provider(),
&data_locale,
style,
),
ListFormatType::Disjunction => ListFormatter::try_new_or_with_length_unstable(
context.icu().provider(),
context.intl_provider(),
&data_locale,
style,
),
ListFormatType::Unit => ListFormatter::try_new_unit_with_length_unstable(
context.icu().provider(),
context.intl_provider(),
&data_locale,
style,
),
Expand Down
19 changes: 14 additions & 5 deletions core/engine/src/builtins/intl/locale/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,10 @@ impl BuiltInConstructor for Locale {
let region = get_option(options, utf16!("region"), context)?;

// 10. Set tag to ! CanonicalizeUnicodeLocaleId(tag).
context.icu().locale_canonicalizer().canonicalize(&mut tag);
context
.intl_provider()
.locale_canonicalizer()
.canonicalize(&mut tag);

// Skipping some boilerplate since this is easier to do using the `Locale` type, but putting the
// spec for completion.
Expand Down Expand Up @@ -280,7 +283,10 @@ impl BuiltInConstructor for Locale {
}

// 17. Return ! CanonicalizeUnicodeLocaleId(tag).
context.icu().locale_canonicalizer().canonicalize(&mut tag);
context
.intl_provider()
.locale_canonicalizer()
.canonicalize(&mut tag);
}

// 12. Let opt be a new Record.
Expand Down Expand Up @@ -363,7 +369,10 @@ impl BuiltInConstructor for Locale {
tag.extensions.unicode.keywords.set(key!("nu"), nu);
}

context.icu().locale_canonicalizer().canonicalize(&mut tag);
context
.intl_provider()
.locale_canonicalizer()
.canonicalize(&mut tag);

// 6. Let locale be ? OrdinaryCreateFromConstructor(NewTarget, "%Locale.prototype%", internalSlotsList).
let prototype =
Expand Down Expand Up @@ -403,7 +412,7 @@ impl Locale {
.clone();

// 3. Let maximal be the result of the Add Likely Subtags algorithm applied to loc.[[Locale]]. If an error is signaled, set maximal to loc.[[Locale]].
context.icu().locale_expander().maximize(&mut loc);
context.intl_provider().locale_expander().maximize(&mut loc);

// 4. Return ! Construct(%Locale%, maximal).
let prototype = context.intrinsics().constructors().locale().prototype();
Expand Down Expand Up @@ -439,7 +448,7 @@ impl Locale {
.clone();

// 3. Let minimal be the result of the Remove Likely Subtags algorithm applied to loc.[[Locale]]. If an error is signaled, set minimal to loc.[[Locale]].
context.icu().locale_expander().minimize(&mut loc);
context.intl_provider().locale_expander().minimize(&mut loc);

// 4. Return ! Construct(%Locale%, minimal).
let prototype = context.intrinsics().constructors().locale().prototype();
Expand Down
19 changes: 8 additions & 11 deletions core/engine/src/builtins/intl/locale/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
options::{IntlOptions, LocaleMatcher},
Service,
},
context::icu::{BoaProvider, Icu, StaticProviderAdapter},
context::icu::IntlProvider,
};

#[derive(Debug)]
Expand All @@ -30,7 +30,7 @@ impl Service for TestService {

type LocaleOptions = TestOptions;

fn resolve(locale: &mut Locale, options: &mut Self::LocaleOptions, provider: &BoaProvider) {
fn resolve(locale: &mut Locale, options: &mut Self::LocaleOptions, provider: &IntlProvider) {
let loc_hc = locale
.extensions
.unicode
Expand Down Expand Up @@ -73,11 +73,8 @@ impl Service for TestService {

#[test]
fn locale_resolution() {
let icu = Icu::new(BoaProvider::Buffer(Box::new(StaticProviderAdapter(
boa_icu_provider::buffer(),
))))
.unwrap();
let mut default = default_locale(icu.locale_canonicalizer());
let provider = IntlProvider::try_new_with_buffer_provider(boa_icu_provider::buffer()).unwrap();
let mut default = default_locale(provider.locale_canonicalizer());
default
.extensions
.unicode
Expand All @@ -91,7 +88,7 @@ fn locale_resolution() {
hc: Some(HourCycle::H11),
},
};
let locale = resolve_locale::<TestService>(&[], &mut options, &icu);
let locale = resolve_locale::<TestService>(&[], &mut options, &provider);
assert_eq!(locale, default);

// test best fit
Expand All @@ -102,10 +99,10 @@ fn locale_resolution() {
},
};

let locale = resolve_locale::<TestService>(&[], &mut options, &icu);
let locale = resolve_locale::<TestService>(&[], &mut options, &provider);
let best = best_locale_for_provider::<<TestService as Service>::LangMarker>(
default.id.clone(),
icu.provider(),
&provider,
)
.unwrap();
let mut best = Locale::from(best);
Expand All @@ -118,6 +115,6 @@ fn locale_resolution() {
service_options: TestOptions { hc: None },
};

let locale = resolve_locale::<TestService>(&[locale!("es-AR")], &mut options, &icu);
let locale = resolve_locale::<TestService>(&[locale!("es-AR")], &mut options, &provider);
assert_eq!(locale, "es-u-hc-h23".parse().unwrap());
}
64 changes: 34 additions & 30 deletions core/engine/src/builtins/intl/locale/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
options::get_option,
Array,
},
context::{icu::Icu, BoaProvider},
context::icu::IntlProvider,
js_string,
object::JsObject,
string::utf16,
Expand Down Expand Up @@ -138,7 +138,10 @@ pub(crate) fn canonicalize_locale_list(
};

// vi. Let canonicalizedTag be CanonicalizeUnicodeLocaleId(tag).
context.icu().locale_canonicalizer().canonicalize(&mut tag);
context
.intl_provider()
.locale_canonicalizer()
.canonicalize(&mut tag);

// vii. If canonicalizedTag is not an element of seen, append canonicalizedTag as the last element of seen.
seen.insert(tag);
Expand Down Expand Up @@ -314,9 +317,12 @@ pub(crate) fn best_locale_for_provider<M: KeyedDataMarker>(
/// in order to see if a certain [`Locale`] is supported.
///
/// [spec]: https://tc39.es/ecma402/#sec-lookupmatcher
fn lookup_matcher<M: KeyedDataMarker>(requested_locales: &[Locale], icu: &Icu) -> Locale
fn lookup_matcher<M: KeyedDataMarker>(
requested_locales: &[Locale],
provider: &IntlProvider,
) -> Locale
where
BoaProvider: DataProvider<M>,
IntlProvider: DataProvider<M>,
{
// 1. Let result be a new Record.
// 2. For each element locale of requestedLocales, do
Expand All @@ -329,7 +335,7 @@ where
locale.extensions.private.clear();

// b. Let availableLocale be ! BestAvailableLocale(availableLocales, noExtensionsLocale).
let available_locale = best_available_locale::<M>(id, icu.provider());
let available_locale = best_available_locale::<M>(id, provider);

// c. If availableLocale is not undefined, then
if let Some(available_locale) = available_locale {
Expand All @@ -349,7 +355,7 @@ where
// 3. Let defLocale be ! DefaultLocale().
// 4. Set result.[[locale]] to defLocale.
// 5. Return result.
default_locale(icu.locale_canonicalizer())
default_locale(provider.locale_canonicalizer())
}

/// Abstract operation [`BestFitMatcher ( availableLocales, requestedLocales )`][spec]
Expand All @@ -361,22 +367,25 @@ where
/// produced by the `LookupMatcher` abstract operation.
///
/// [spec]: https://tc39.es/ecma402/#sec-bestfitmatcher
fn best_fit_matcher<M: KeyedDataMarker>(requested_locales: &[Locale], icu: &Icu) -> Locale
fn best_fit_matcher<M: KeyedDataMarker>(
requested_locales: &[Locale],
provider: &IntlProvider,
) -> Locale
where
BoaProvider: DataProvider<M>,
IntlProvider: DataProvider<M>,
{
for mut locale in requested_locales
.iter()
.cloned()
.chain(std::iter::once_with(|| {
default_locale(icu.locale_canonicalizer())
default_locale(provider.locale_canonicalizer())
}))
{
let id = std::mem::take(&mut locale.id);
locale.extensions.transform.clear();
locale.extensions.private.clear();

if let Some(available) = best_locale_for_provider(id, icu.provider()) {
if let Some(available) = best_locale_for_provider(id, provider) {
locale.id = available;

return locale;
Expand All @@ -399,11 +408,11 @@ where
pub(in crate::builtins::intl) fn resolve_locale<S>(
requested_locales: &[Locale],
options: &mut IntlOptions<S::LocaleOptions>,
icu: &Icu,
provider: &IntlProvider,
) -> Locale
where
S: Service,
BoaProvider: DataProvider<S::LangMarker>,
IntlProvider: DataProvider<S::LangMarker>,
{
// 1. Let matcher be options.[[localeMatcher]].
// 2. If matcher is "lookup", then
Expand All @@ -412,9 +421,9 @@ where
// a. Let r be ! BestFitMatcher(availableLocales, requestedLocales).
// 4. Let foundLocale be r.[[locale]].
let mut found_locale = if options.matcher == LocaleMatcher::Lookup {
lookup_matcher::<S::LangMarker>(requested_locales, icu)
lookup_matcher::<S::LangMarker>(requested_locales, provider)
} else {
best_fit_matcher::<S::LangMarker>(requested_locales, icu)
best_fit_matcher::<S::LangMarker>(requested_locales, provider)
};

// From here, the spec differs significantly from the implementation,
Expand Down Expand Up @@ -469,12 +478,10 @@ where
// 11. Set result.[[locale]] to foundLocale.

// 12. Return result.
S::resolve(
&mut found_locale,
&mut options.service_options,
icu.provider(),
);
icu.locale_canonicalizer().canonicalize(&mut found_locale);
S::resolve(&mut found_locale, &mut options.service_options, provider);
provider
.locale_canonicalizer()
.canonicalize(&mut found_locale);
found_locale
}

Expand All @@ -493,7 +500,7 @@ where
/// [spec]: https://tc39.es/ecma402/#sec-lookupsupportedlocales
fn lookup_supported_locales<M: KeyedDataMarker>(
requested_locales: &[Locale],
provider: &impl DataProvider<M>,
provider: &(impl DataProvider<M> + ?Sized),
) -> Vec<Locale> {
// 1. Let subset be a new empty List.
// 2. For each element locale of requestedLocales, do
Expand All @@ -517,7 +524,7 @@ fn lookup_supported_locales<M: KeyedDataMarker>(
/// [spec]: https://tc39.es/ecma402/#sec-bestfitsupportedlocales
fn best_fit_supported_locales<M: KeyedDataMarker>(
requested_locales: &[Locale],
provider: &impl DataProvider<M>,
provider: &(impl DataProvider<M> + ?Sized),
) -> Vec<Locale> {
requested_locales
.iter()
Expand All @@ -538,7 +545,7 @@ pub(in crate::builtins::intl) fn supported_locales<M: KeyedDataMarker>(
context: &mut Context,
) -> JsResult<JsObject>
where
BoaProvider: DataProvider<M>,
IntlProvider: DataProvider<M>,
{
// 1. Set options to ? CoerceOptionsToObject(options).
let options = coerce_options_to_object(options, context)?;
Expand All @@ -550,12 +557,12 @@ where
// 4. Else,
// a. Let supportedLocales be LookupSupportedLocales(availableLocales, requestedLocales).
LocaleMatcher::Lookup => {
lookup_supported_locales(requested_locales, context.icu().provider())
lookup_supported_locales(requested_locales, context.intl_provider())
}
// 3. If matcher is "best fit", then
// a. Let supportedLocales be BestFitSupportedLocales(availableLocales, requestedLocales).
LocaleMatcher::BestFit => {
best_fit_supported_locales(requested_locales, context.icu().provider())
best_fit_supported_locales(requested_locales, context.intl_provider())
}
};

Expand Down Expand Up @@ -600,7 +607,7 @@ mod tests {
builtins::intl::locale::utils::{
best_available_locale, best_fit_matcher, default_locale, lookup_matcher,
},
context::icu::{BoaProvider, Icu, StaticProviderAdapter},
context::icu::IntlProvider,
};

#[test]
Expand All @@ -626,10 +633,7 @@ mod tests {

#[test]
fn lookup_match() {
let icu = Icu::new(BoaProvider::Buffer(Box::new(StaticProviderAdapter(
boa_icu_provider::buffer(),
))))
.unwrap();
let icu = IntlProvider::try_new_with_buffer_provider(boa_icu_provider::buffer()).unwrap();

// requested: []

Expand Down
4 changes: 2 additions & 2 deletions core/engine/src/builtins/intl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

use crate::{
builtins::{Array, BuiltInBuilder, BuiltInObject, IntrinsicObject},
context::{intrinsics::Intrinsics, BoaProvider},
context::{icu::IntlProvider, intrinsics::Intrinsics},
js_string,
object::JsObject,
property::Attribute,
Expand Down Expand Up @@ -172,7 +172,7 @@ trait Service {
fn resolve(
_locale: &mut icu_locid::Locale,
_options: &mut Self::LocaleOptions,
_provider: &BoaProvider,
_provider: &IntlProvider,
) {
}
}

0 comments on commit 52b39fa

Please sign in to comment.