Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify Icu API #3503

Merged
merged 4 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
) {
}
}
Loading
Loading