Skip to content

Correctly handle "true" subtags in enum_keywords macro #6698

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Correctly handle "true" subtags in enum_keywords macro #6698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
121 changes: 87 additions & 34 deletions components/locale_core/src/extensions/unicode/value.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,16 @@ use core::str::FromStr;
/// "islamic-civil"
/// );
///
/// // The value "true" has the special, empty string representation
/// // The value "true" has the special, empty string representation,
/// // making it an "implicit" subtag.
/// assert_eq!(value!("true").to_string(), "");
///
/// // However, when the "true" subtag is part of a multiple subtag Value, it becomes
/// // an explicit subtag.
/// assert_writeable_eq!(
/// "true-value".parse::<Value>().unwrap(),
/// "true-value"
/// );
/// ```
#[derive(Debug, PartialEq, Eq, Clone, Hash, PartialOrd, Ord, Default)]
pub struct Value(ShortBoxSlice<Subtag>);
Expand Down Expand Up @@ -63,49 +71,68 @@ impl Value {
let mut v = ShortBoxSlice::new();

if !code_units.is_empty() {
for chunk in SubtagIterator::new(code_units) {
let subtag = Subtag::try_from_utf8(chunk)?;
if subtag != TRUE_VALUE {
v.push(subtag);
let mut iter = SubtagIterator::new(code_units);
match (iter.next(), iter.next()) {
(Some(single), None) => {
let subtag = Subtag::try_from_utf8(single)?;
if subtag != TRUE_VALUE {
v.push(subtag);
}
}
(Some(first), Some(second)) => {
v.push(Subtag::try_from_utf8(first)?);
v.push(Subtag::try_from_utf8(second)?);

for chunk in iter {
let subtag = Subtag::try_from_utf8(chunk)?;
v.push(subtag);
}
}
_ => {
// Should technically be unreachable thanks to the `code_units.is_empty()` check above.
}
}
}
Ok(Self(v))
}

/// Returns a reference to a single [`Subtag`] if the [`Value`] contains exactly one
/// subtag, or `None` otherwise.
/// explicit subtag, or `None` otherwise.
///
/// # Examples
///
/// ```
/// use core::str::FromStr;
/// use icu::locale::extensions::unicode::Value;
///
/// let value1 = Value::from_str("foo").expect("failed to parse a Value");
/// let value2 = Value::from_str("foo-bar").expect("failed to parse a Value");
/// let value1 = Value::default();
/// let value2 = Value::from_str("foo").expect("failed to parse a Value");
/// let value3 = Value::from_str("foo-bar").expect("failed to parse a Value");
///
/// assert!(value1.as_single_subtag().is_some());
/// assert!(value2.as_single_subtag().is_none());
/// assert!(value1.as_single_subtag().is_none());
/// assert!(value2.as_single_subtag().is_some());
/// assert!(value3.as_single_subtag().is_none());
/// ```
pub const fn as_single_subtag(&self) -> Option<&Subtag> {
self.0.single()
}

/// Destructs into a single [`Subtag`] if the [`Value`] contains exactly one
/// subtag, or returns `None` otherwise.
/// explicit subtag, or returns `None` otherwise.
///
/// # Examples
///
/// ```
/// use core::str::FromStr;
/// use icu::locale::extensions::unicode::Value;
///
/// let value1 = Value::from_str("foo").expect("failed to parse a Value");
/// let value2 = Value::from_str("foo-bar").expect("failed to parse a Value");
/// let value1 = Value::default();
/// let value2 = Value::from_str("foo").expect("failed to parse a Value");
/// let value3 = Value::from_str("foo-bar").expect("failed to parse a Value");
///
/// assert!(value1.into_single_subtag().is_some());
/// assert!(value2.into_single_subtag().is_none());
/// assert!(value1.into_single_subtag().is_none());
/// assert!(value2.into_single_subtag().is_some());
/// assert!(value3.into_single_subtag().is_none());
/// ```
pub fn into_single_subtag(self) -> Option<Subtag> {
self.0.into_single()
Expand All @@ -116,34 +143,43 @@ impl Value {
&self.0
}

/// Appends a subtag to the back of a [`Value`].
/// Appends a subtag to the back of a [`Value`]. If the value is empty,
/// this will push an explicit `true` subtag to the value.
///
/// # Examples
///
/// ```
/// use icu::locale::{extensions::unicode::Value, subtags::subtag};
///
/// let mut v = Value::default();
/// v.push_subtag(subtag!("foo"));
/// let mut v = Value::from_subtag(Some(subtag!("foo")));
/// v.push_subtag(subtag!("bar"));
/// assert_eq!(v, "foo-bar");
///
/// let mut v = Value::default();
/// v.push_subtag(subtag!("bar"));
/// assert_eq!(v, "true-bar");
/// ```
#[cfg(feature = "alloc")]
pub fn push_subtag(&mut self, subtag: Subtag) {
if self.is_empty() {
self.0.push(subtag!("true"));
}
self.0.push(subtag);
}

/// Returns the number of subtags in the [`Value`].
/// Returns the number of explicit subtags in the [`Value`], returning
/// 0 if the value is "true".
///
/// # Examples
///
/// ```
/// use icu::locale::{extensions::unicode::Value, subtags::subtag};
///
/// let mut v = Value::default();
/// assert_eq!(v.subtag_count(), 0);
/// v.push_subtag(subtag!("foo"));
/// assert_eq!(v.subtag_count(), 1);
/// let v1 = Value::default();
/// assert_eq!(v1.subtag_count(), 0);
///
/// let v2 = Value::from_subtag(Some(subtag!("foo")));
/// assert_eq!(v2.subtag_count(), 1);
/// ```
pub fn subtag_count(&self) -> usize {
self.0.len()
Expand All @@ -162,15 +198,18 @@ impl Value {
Self(ShortBoxSlice::new())
}

/// Returns `true` if the Value has no subtags.
/// Returns `true` if the Value has no explicit subtags.
///
/// # Examples
///
/// ```
/// use icu::locale::{extensions::unicode::Value, subtags::subtag};
///
/// let mut v = Value::default();
/// assert_eq!(v.is_empty(), true);
/// let v1 = Value::default();
/// assert_eq!(v1.is_empty(), true);
///
/// let v2 = Value::from_subtag(Some(subtag!("foo")));
/// assert_eq!(v2.is_empty(), false);
/// ```
pub fn is_empty(&self) -> bool {
self.0.is_empty()
Expand All @@ -179,23 +218,38 @@ impl Value {
/// Removes and returns the subtag at position `index` within the value,
/// shifting all subtags after it to the left.
///
/// If after removing the subtag the value only contains an explicit
/// "true" subtag, this will automatically empty the value.
///
/// # Examples
///
/// ```
/// use icu::locale::{extensions::unicode::Value, subtags::subtag};
/// let mut v = Value::default();
/// v.push_subtag(subtag!("foo"));
/// v.push_subtag(subtag!("bar"));
/// v.push_subtag(subtag!("baz"));
///
/// assert_eq!(v.remove_subtag(1), Some(subtag!("bar")));
/// assert_eq!(v, "foo-baz");
/// let mut v1 = Value::from_subtag(Some(subtag!("foo")));
/// v1.push_subtag(subtag!("bar"));
/// v1.push_subtag(subtag!("baz"));
///
/// assert_eq!(v1.remove_subtag(1), Some(subtag!("bar")));
/// assert_eq!(v1, "foo-baz");
///
/// let mut v2 = Value::default();
/// v2.push_subtag(subtag!("bar"));
/// v2.push_subtag(subtag!("baz"));
///
/// assert_eq!(v2.get_subtag(0), Some(&subtag!("true")));
/// assert_eq!(v2.remove_subtag(2), Some(subtag!("baz")));
/// assert_eq!(v2.remove_subtag(1), Some(subtag!("bar")));
/// assert_eq!(v2.is_empty(), true);
/// ```
pub fn remove_subtag(&mut self, idx: usize) -> Option<Subtag> {
if self.0.len() < idx {
None
} else {
let item = self.0.remove(idx);
if self.subtag_count() == 1 && self.get_subtag(0) == Some(&TRUE_VALUE) {
*self = Self::new_empty();
}
Some(item)
}
}
Expand All @@ -206,8 +260,7 @@ impl Value {
///
/// ```
/// use icu::locale::{extensions::unicode::Value, subtags::subtag};
/// let mut v = Value::default();
/// v.push_subtag(subtag!("foo"));
/// let mut v = Value::from_subtag(Some(subtag!("foo")));
/// v.push_subtag(subtag!("bar"));
/// v.push_subtag(subtag!("baz"));
///
Expand Down
49 changes: 48 additions & 1 deletion components/locale_core/src/preferences/extensions/unicode/macros/enum_keyword.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ macro_rules! __enum_keyword {
// complaints.
#[allow(non_snake_case)]
Self::$variant $(($v2))? => {
input.push_subtag($crate::subtags::subtag!($key));
*input = $crate::extensions::unicode::Value::from_subtag(Some($crate::subtags::subtag!($key)));

$(
if let Some(v2) = $v2 {
Expand Down Expand Up @@ -317,4 +317,51 @@ mod tests {
"sub-rare"
);
}

#[test]
fn enum_keywords_boolean_test() {
enum_keyword!(DummyTrueKeyword { Standard, Rare });

enum_keyword!(DummyKeyword {
("false" => False),
("true" => True(DummyTrueKeyword) {
("standard" => Standard),
("rare" => Rare)
})
}, "dk");

let v = unicode::Value::from_str("false").unwrap();
let dk = DummyKeyword::try_from(&v).unwrap();
assert_eq!(dk, DummyKeyword::False);
assert_eq!(unicode::Value::from(dk), v);

let v = unicode::Value::from_str("true").unwrap();
let dk = DummyKeyword::try_from(&v).unwrap();
assert_eq!(dk, DummyKeyword::True(None));
assert_eq!(unicode::Value::from(dk), v);

let v = unicode::Value::from_str("foo").unwrap();
let dk = DummyKeyword::try_from(&v);
assert!(dk.is_err());

let v = unicode::Value::from_str("true-standard").unwrap();
let dk = DummyKeyword::try_from(&v).unwrap();
assert_eq!(dk, DummyKeyword::True(Some(DummyTrueKeyword::Standard)));
assert_eq!(unicode::Value::from(dk), v);

let v = unicode::Value::from_str("true-rare").unwrap();
let dk = DummyKeyword::try_from(&v).unwrap();
assert_eq!(dk, DummyKeyword::True(Some(DummyTrueKeyword::Rare)));
assert_eq!(unicode::Value::from(dk), v);

let v = unicode::Value::from_str("true-foo").unwrap();
let dk = DummyKeyword::try_from(&v).unwrap();
assert_eq!(dk, DummyKeyword::True(None));
assert_eq!(unicode::Value::from(dk), unicode::value!("true"));

assert_eq!(
DummyKeyword::True(Some(DummyTrueKeyword::Rare)).as_str(),
"true-rare"
);
}
}