From c0afef6b9d92acdb6418a96772ef60cab0989cb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Sat, 22 Apr 2023 01:59:22 +0200 Subject: [PATCH 1/6] rename Badge::name and Badge::version to set_id and id --- src/badges.rs | 28 ++++++++++++++-------------- src/into_static.rs | 4 ++-- src/messages/privmsg.rs | 16 ++++++++-------- 3 files changed, 24 insertions(+), 24 deletions(-) diff --git a/src/badges.rs b/src/badges.rs index 77c6c6e..caf6e47 100644 --- a/src/badges.rs +++ b/src/badges.rs @@ -13,8 +13,8 @@ use crate::{ /// /// let input = "broadcaster/1,foo/bar"; /// let expected = [ -/// Badge{ name: Cow::Borrowed("broadcaster".into()), version: Cow::Borrowed("1".into()) }, -/// Badge{ name: Cow::Borrowed("foo".into()), version: Cow::Borrowed("bar".into()) }, +/// Badge{ set_id: Cow::Borrowed("broadcaster".into()), id: Cow::Borrowed("1".into()) }, +/// Badge{ set_id: Cow::Borrowed("foo".into()), id: Cow::Borrowed("bar".into()) }, /// ]; /// for (i, badge) in parse_badges(input).enumerate() { /// assert_eq!(expected[i], badge) @@ -26,24 +26,24 @@ pub fn parse_badges(input: &str) -> impl Iterator> + '_ { input .split(',') .flat_map(|badge| badge.split_once('/')) - .map(|(name, version)| { - let mut version = Cow::Borrowed(version); - Badge::unescape(&mut version); + .map(|(set_id, id)| { + let mut id = Cow::Borrowed(id); + Badge::unescape(&mut id); Badge { - name: Cow::Borrowed(name.into()), - version: IntoCow::into_cow(version), + set_id: Cow::Borrowed(set_id.into()), + id: IntoCow::into_cow(id), } }) } /// A badge attached to a message -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] pub struct Badge<'a> { - /// The name of the badge - pub name: Cow<'a, BadgeSetIdRef>, - /// The version (or, more specifically the metadata) for the badge - pub version: Cow<'a, ChatBadgeIdRef>, + /// The set_id or name of the badge + pub set_id: Cow<'a, BadgeSetIdRef>, + /// The id, version or metadata for the badge version + pub id: Cow<'a, ChatBadgeIdRef>, } impl<'a> Badge<'a> { @@ -55,8 +55,8 @@ impl<'a> Badge<'a> { /// /// let tags = Tags::builder().add("badges", "broadcaster/1,foo/bar").finish(); /// let expected = [ - /// Badge{ name: Cow::Borrowed("broadcaster".into()), version: Cow::Borrowed("1".into()) }, - /// Badge{ name: Cow::Borrowed("foo".into()), version: Cow::Borrowed("bar".into()) }, + /// Badge{ set_id: Cow::Borrowed("broadcaster".into()), id: Cow::Borrowed("1".into()) }, + /// Badge{ set_id: Cow::Borrowed("foo".into()), id: Cow::Borrowed("bar".into()) }, /// ]; /// for (i, badge) in Badge::from_tags(&tags).enumerate() { /// assert_eq!(expected[i], badge) diff --git a/src/into_static.rs b/src/into_static.rs index a059d65..30cc5e6 100644 --- a/src/into_static.rs +++ b/src/into_static.rs @@ -543,8 +543,8 @@ impl<'a> IntoStatic for Badge<'a> { fn into_static(self) -> Self::Output { Badge { - name: self.name.into_static(), - version: self.version.into_static(), + set_id: self.set_id.into_static(), + id: self.id.into_static(), } } } diff --git a/src/messages/privmsg.rs b/src/messages/privmsg.rs index 1f8a29f..2df5ecb 100644 --- a/src/messages/privmsg.rs +++ b/src/messages/privmsg.rs @@ -127,45 +127,45 @@ impl<'a> Privmsg<'a> { /// The message is from the broadcaster of the channel pub fn is_from_broadcaster(&self) -> bool { self.badges() - .any(|badge| badge.name.as_str() == "broadcaster") + .any(|badge| badge.set_id.as_str() == "broadcaster") } /// The message is from a moderator in the channel pub fn is_from_moderator(&self) -> bool { self.badges() - .any(|badge| badge.name.as_str() == "moderator") + .any(|badge| badge.set_id.as_str() == "moderator") } /// The message is from a VIP in the channel pub fn is_from_vip(&self) -> bool { - self.badges().any(|badge| badge.name.as_str() == "vip") + self.badges().any(|badge| badge.set_id.as_str() == "vip") } /// The message is from a subscriber of the channel pub fn is_from_subscriber(&self) -> bool { self.badges() - .any(|badge| badge.name.as_str() == "subscriber") + .any(|badge| badge.set_id.as_str() == "subscriber") } /// The message is from Twitch staff pub fn is_from_staff(&self) -> bool { - self.badges().any(|badge| badge.name.as_str() == "staff") + self.badges().any(|badge| badge.set_id.as_str() == "staff") } /// The message is from a turbo user pub fn is_from_turbo(&self) -> bool { - self.badges().any(|badge| badge.name.as_str() == "turbo") + self.badges().any(|badge| badge.set_id.as_str() == "turbo") } /// The message is from a global moderator pub fn is_from_global_moderator(&self) -> bool { self.badges() - .any(|badge| badge.name.as_str() == "global_mod") + .any(|badge| badge.set_id.as_str() == "global_mod") } /// The message is from a admin pub fn is_from_admin(&self) -> bool { - self.badges().any(|badge| badge.name.as_str() == "admin") + self.badges().any(|badge| badge.set_id.as_str() == "admin") } /// A builder for constructing a [`Message`](crate::messages::Message) or [`Privmsg`](crate::messages::Privmsg) From 30ece7c477b55197dfcac3a7e6b37f62437775d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Sat, 22 Apr 2023 03:18:10 +0200 Subject: [PATCH 2/6] fix ordering of badges --- src/badges.rs | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/src/badges.rs b/src/badges.rs index caf6e47..7ca1571 100644 --- a/src/badges.rs +++ b/src/badges.rs @@ -42,10 +42,60 @@ pub fn parse_badges(input: &str) -> impl Iterator> + '_ { pub struct Badge<'a> { /// The set_id or name of the badge pub set_id: Cow<'a, BadgeSetIdRef>, - /// The id, version or metadata for the badge version + /// The id, version or metadata for the badge pub id: Cow<'a, ChatBadgeIdRef>, } +impl<'a> PartialOrd for Badge<'a> { + fn partial_cmp(&self, other: &Self) -> Option { + static KNOWN_BADGES: &[&'_ str] = &[ + "staff", + "admin", + "global_mod", + + "broadcaster", + "vip", + "moderator", + + "partner", + + "subscriber", + + "artist-badge", + + "turbo", + "premium", + + "bits-leader", + "bits", + ]; + match self.set_id.partial_cmp(&other.set_id) { + Some(core::cmp::Ordering::Equal) => {} + // XXX: order known badges like first-party site + // FIXME: is the reflexive and transitive? as needed by Ord + ord => match ( + KNOWN_BADGES.iter().position(|b| *b == self.set_id.as_str()), + KNOWN_BADGES + .iter() + .position(|b| *b == other.set_id.as_str()), + ) { + (Some(s), Some(other)) => return s.partial_cmp(&other), + (Some(_), None) => return Some(core::cmp::Ordering::Less), + (None, Some(_)) => return Some(core::cmp::Ordering::Greater), + _ => return ord, + }, + } + // XXX: numerical partial_cmp on self.id, so that subscriber/12 > subscriber/9 + match ( + self.id.as_str().parse::(), + other.id.as_str().parse::(), + ) { + (Ok(s), Ok(other)) => s.partial_cmp(&other), + (_, _) => self.id.partial_cmp(&other.id), + } + } +} + impl<'a> Badge<'a> { /// Parse badges from a [`Tags`] /// @@ -95,3 +145,22 @@ impl Badge<'_> { .into(); } } + +#[cfg(test)] +#[test] +fn badge_ordering() { + #[track_caller] + fn parse(s: &str) -> Badge<'_> { + parse_badges(s).next().unwrap() + } + assert!(parse("subscriber/12") > parse("subscriber/9")); + assert!(parse("subscriber/0") < parse("subscriber/1")); + let mut badges: Vec<_> = parse_badges("bits/1,staff/1,broadcaster/1").collect(); + badges.sort(); + assert_eq!(badges.iter().map(|b| b.set_id.as_str()).collect::>(), vec!["staff", "broadcaster", "bits"]); + + let mut badges: Vec<_> = parse_badges("aaa/1,zzz/1,staff/1").collect(); + badges.sort(); + assert_eq!(badges.iter().map(|b| b.set_id.as_str()).collect::>(), vec!["staff", "aaa", "zzz"]); + +} From 6adc9f60679527b2ab864ac2be36692b3f9cc5b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Sat, 22 Apr 2023 04:20:05 +0200 Subject: [PATCH 3/6] add fuzzing --- fuzz/.gitignore | 3 + fuzz/Cargo.lock | 145 ++++++++++++++++++++++++++++++++ fuzz/Cargo.toml | 26 ++++++ fuzz/fuzz_targets/badge_fuzz.rs | 74 ++++++++++++++++ 4 files changed, 248 insertions(+) create mode 100644 fuzz/.gitignore create mode 100644 fuzz/Cargo.lock create mode 100644 fuzz/Cargo.toml create mode 100644 fuzz/fuzz_targets/badge_fuzz.rs diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 0000000..a092511 --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,3 @@ +target +corpus +artifacts diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock new file mode 100644 index 0000000..b261675 --- /dev/null +++ b/fuzz/Cargo.lock @@ -0,0 +1,145 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "arbitrary" +version = "1.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e2d098ff73c1ca148721f37baad5ea6a465a13f9573aba8641fbbbae8164a54e" + +[[package]] +name = "cc" +version = "1.0.79" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f" +dependencies = [ + "jobserver", +] + +[[package]] +name = "either" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91" + +[[package]] +name = "itertools" +version = "0.10.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" +dependencies = [ + "either", +] + +[[package]] +name = "jobserver" +version = "0.1.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "936cfd212a0155903bcbc060e316fb6cc7cbf2e1907329391ebadc1fe0ce77c2" +dependencies = [ + "libc", +] + +[[package]] +name = "libc" +version = "0.2.142" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6a987beff54b60ffa6d51982e1aa1146bc42f19bd26be28b0586f252fccf5317" + +[[package]] +name = "libfuzzer-sys" +version = "0.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "beb09950ae85a0a94b27676cccf37da5ff13f27076aa1adbc6545dd0d0e1bd4e" +dependencies = [ + "arbitrary", + "cc", + "once_cell", +] + +[[package]] +name = "once_cell" +version = "1.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7e5500299e16ebb147ae15a00a942af264cf3688f47923b8fc2cd5858f23ad3" + +[[package]] +name = "proc-macro2" +version = "1.0.56" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b63bdb0cd06f1f4dedf69b254734f9b45af66e4a031e42a7480257d9898b435" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.26" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4424af4bf778aae2051a77b60283332f386554255d722233d09fbfc7e30da2fc" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "serde" +version = "1.0.160" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb2f3770c8bce3bcda7e149193a069a0f4365bda1fa5cd88e03bca26afc1216c" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.160" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "291a097c63d8497e00160b166a967a4a79c64f3facdd01cbd7502231688d77df" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "syn" +version = "2.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a34fcf3e8b60f57e6a14301a2e916d323af98b0ea63c599441eec8558660c822" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "twitch_message" +version = "0.1.2" +dependencies = [ + "twitch_types", +] + +[[package]] +name = "twitch_message-fuzz" +version = "0.0.0" +dependencies = [ + "itertools", + "libfuzzer-sys", + "twitch_message", +] + +[[package]] +name = "twitch_types" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a741b3a7c79a497c0164e00838ca3a50bc57e4e7378d38befe418b9fbc4df54a" +dependencies = [ + "serde", +] + +[[package]] +name = "unicode-ident" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5464a87b239f13a63a501f2701565754bae92d243d4bb7eb12f6d57d2269bf4" diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 0000000..25f80d1 --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,26 @@ +[package] +name = "twitch_message-fuzz" +version = "0.0.0" +authors = ["Automatically generated"] +publish = false +edition = "2018" + +[package.metadata] +cargo-fuzz = true + +[dependencies] +itertools = "0.10.5" +libfuzzer-sys = "0.4" + +[dependencies.twitch_message] +path = ".." + +# Prevent this from interfering with workspaces +[workspace] +members = ["."] + +[[bin]] +name = "badge_fuzz" +path = "fuzz_targets/badge_fuzz.rs" +test = false +doc = false diff --git a/fuzz/fuzz_targets/badge_fuzz.rs b/fuzz/fuzz_targets/badge_fuzz.rs new file mode 100644 index 0000000..793a32c --- /dev/null +++ b/fuzz/fuzz_targets/badge_fuzz.rs @@ -0,0 +1,74 @@ +#![no_main] +use libfuzzer_sys::fuzz_target; + +use itertools::Itertools; +fuzz_target!(|data: &str| { fun(data) }); + +// notes on ord: https://doc.rust-lang.org/std/cmp/trait.Ord.html +// Implementations must be consistent with the PartialOrd implementation, and ensure max, min, and clamp are consistent with cmp: +// +// partial_cmp(a, b) == Some(cmp(a, b)). +// max(a, b) == max_by(a, b, cmp) (ensured by the default implementation). +// min(a, b) == min_by(a, b, cmp) (ensured by the default implementation). +// For a.clamp(min, max), see the method docs (ensured by the default implementation). +// +// It’s easy to accidentally make cmp and partial_cmp disagree by deriving some of the traits and manually implementing others. +// Corollaries +// +// From the above and the requirements of PartialOrd, it follows that < defines a strict total order. This means that for all a, b and c: +// +// exactly one of a < b, a == b or a > b is true; and +// < is transitive: a < b and b < c implies a < c. The same must hold for both == and >. + +pub fn fun(data: &str) { + if data.is_empty() + || data.chars().any(|c: char| { + !c.is_ascii_alphanumeric() && c != '_' && c != '-' && c != '/' && c != ',' + }) + { + return; + } + let Ok(badges) = std::panic::catch_unwind(|| twitch_message::parse_badges(data).collect::>()) else { + return; + }; + for badge in &badges { + if badge.set_id.as_str().is_empty() || badge.id.as_str().is_empty() { + return; + } + } + if badges.len() == 2 { + for (a, b) in [(0, 1), (1, 0)] { + //dbg!(badges[a].partial_cmp(&badges[b]), Some(badges[a].cmp(&badges[b]))); + assert!(badges[a].partial_cmp(&badges[b]) == Some(badges[a].cmp(&badges[b]))) + } + } + if badges.len() != 3 { + return; + } + + for vec in [0, 1, 2].iter().permutations(3) { + let a = &badges[*vec[0]]; + let b = &badges[*vec[1]]; + let c = &badges[*vec[2]]; + let res = [a > b, a == b, a < b]; + assert!( + res.iter().filter(|x| **x).count() == 1, + "{a:?} {b:?} {c:?} {:?}", + res + ); + let res = [c > b, c == b, c < b]; + assert!( + res.iter().filter(|x| **x).count() == 1, + "{a:?} {b:?} {c:?} {:?}", + res + ); + + if a < b && b < c { + assert!(a < c, "{a:?} < {b:?} && {b:?} < {c:?}", a = a); + } else if a == b && b == c { + assert!(a == c, "{a:?} == {b:?} && {b:?} == {c:?}", a = a); + } else if a > b && b > c { + assert!(a > c, "{a:?} > {b:?} && {b:?} > {c:?}", a = a); + } + } +} From 78089284b65c63f4e781de9666600a72874ece8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emil=20Gardstr=C3=B6m?= Date: Sat, 22 Apr 2023 04:20:36 +0200 Subject: [PATCH 4/6] fix wrong cmp when comparing `01` and `1` --- src/badges.rs | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/src/badges.rs b/src/badges.rs index 7ca1571..e0c7c82 100644 --- a/src/badges.rs +++ b/src/badges.rs @@ -37,7 +37,7 @@ pub fn parse_badges(input: &str) -> impl Iterator> + '_ { } /// A badge attached to a message -#[derive(Debug, Clone, PartialEq, Eq, Ord, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] pub struct Badge<'a> { /// The set_id or name of the badge @@ -45,6 +45,11 @@ pub struct Badge<'a> { /// The id, version or metadata for the badge pub id: Cow<'a, ChatBadgeIdRef>, } +impl Ord for Badge<'_> { + fn cmp(&self, other: &Self) -> std::cmp::Ordering { + self.partial_cmp(other).expect("badges are fully") + } +} impl<'a> PartialOrd for Badge<'a> { fn partial_cmp(&self, other: &Self) -> Option { @@ -52,20 +57,14 @@ impl<'a> PartialOrd for Badge<'a> { "staff", "admin", "global_mod", - "broadcaster", "vip", "moderator", - "partner", - "subscriber", - "artist-badge", - "turbo", "premium", - "bits-leader", "bits", ]; @@ -90,7 +89,7 @@ impl<'a> PartialOrd for Badge<'a> { self.id.as_str().parse::(), other.id.as_str().parse::(), ) { - (Ok(s), Ok(other)) => s.partial_cmp(&other), + (Ok(s), Ok(other)) if s != other => s.partial_cmp(&other), (_, _) => self.id.partial_cmp(&other.id), } } @@ -157,10 +156,15 @@ fn badge_ordering() { assert!(parse("subscriber/0") < parse("subscriber/1")); let mut badges: Vec<_> = parse_badges("bits/1,staff/1,broadcaster/1").collect(); badges.sort(); - assert_eq!(badges.iter().map(|b| b.set_id.as_str()).collect::>(), vec!["staff", "broadcaster", "bits"]); + assert_eq!( + badges.iter().map(|b| b.set_id.as_str()).collect::>(), + vec!["staff", "broadcaster", "bits"] + ); let mut badges: Vec<_> = parse_badges("aaa/1,zzz/1,staff/1").collect(); badges.sort(); - assert_eq!(badges.iter().map(|b| b.set_id.as_str()).collect::>(), vec!["staff", "aaa", "zzz"]); - + assert_eq!( + badges.iter().map(|b| b.set_id.as_str()).collect::>(), + vec!["staff", "aaa", "zzz"] + ); } From 6e7320b97a81a9386387ebfcc135a2ae35ca6a5a Mon Sep 17 00:00:00 2001 From: museun Date: Sat, 22 Apr 2023 15:46:21 -0400 Subject: [PATCH 5/6] Update src/badges.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Emil Gardström --- src/badges.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/badges.rs b/src/badges.rs index e0c7c82..aad0517 100644 --- a/src/badges.rs +++ b/src/badges.rs @@ -47,7 +47,7 @@ pub struct Badge<'a> { } impl Ord for Badge<'_> { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.partial_cmp(other).expect("badges are fully") + self.partial_cmp(other).expect("badges are fully comparable") } } From 50bcecb212e467a87e824a7dc2a99eca7758ccbb Mon Sep 17 00:00:00 2001 From: museun Date: Sat, 22 Apr 2023 16:12:14 -0400 Subject: [PATCH 6/6] fix ordering of numerical vs non-numerical ids --- src/badges.rs | 113 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 82 insertions(+), 31 deletions(-) diff --git a/src/badges.rs b/src/badges.rs index aad0517..ad1bf21 100644 --- a/src/badges.rs +++ b/src/badges.rs @@ -45,15 +45,17 @@ pub struct Badge<'a> { /// The id, version or metadata for the badge pub id: Cow<'a, ChatBadgeIdRef>, } + impl Ord for Badge<'_> { fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.partial_cmp(other).expect("badges are fully comparable") + self.partial_cmp(other) + .expect("badges are fully comparable") } } impl<'a> PartialOrd for Badge<'a> { fn partial_cmp(&self, other: &Self) -> Option { - static KNOWN_BADGES: &[&'_ str] = &[ + static KNOWN_BADGES: &[&str] = &[ "staff", "admin", "global_mod", @@ -68,8 +70,11 @@ impl<'a> PartialOrd for Badge<'a> { "bits-leader", "bits", ]; + + use core::cmp::Ordering; + match self.set_id.partial_cmp(&other.set_id) { - Some(core::cmp::Ordering::Equal) => {} + Some(Ordering::Equal) => {} // XXX: order known badges like first-party site // FIXME: is the reflexive and transitive? as needed by Ord ord => match ( @@ -78,18 +83,30 @@ impl<'a> PartialOrd for Badge<'a> { .iter() .position(|b| *b == other.set_id.as_str()), ) { - (Some(s), Some(other)) => return s.partial_cmp(&other), - (Some(_), None) => return Some(core::cmp::Ordering::Less), - (None, Some(_)) => return Some(core::cmp::Ordering::Greater), + (Some(left), Some(right)) => return left.partial_cmp(&right), + (Some(_), None) => return Some(Ordering::Less), + (None, Some(_)) => return Some(Ordering::Greater), _ => return ord, }, } // XXX: numerical partial_cmp on self.id, so that subscriber/12 > subscriber/9 - match ( - self.id.as_str().parse::(), - other.id.as_str().parse::(), - ) { - (Ok(s), Ok(other)) if s != other => s.partial_cmp(&other), + + let is_all_digit = |s: &Self| s.id.as_str().chars().all(|c| c.is_ascii_digit()); + let parse_num = |s: &Self| { + is_all_digit(s) + .then(|| s.id.as_str().parse::().ok()) + .flatten() + }; + + if !is_all_digit(self) { + return None; + } + if !is_all_digit(other) { + return None; + } + + match (parse_num(self), parse_num(other)) { + (Some(left), Some(right)) if left != right => left.partial_cmp(&right), (_, _) => self.id.partial_cmp(&other.id), } } @@ -146,25 +163,59 @@ impl Badge<'_> { } #[cfg(test)] -#[test] -fn badge_ordering() { - #[track_caller] - fn parse(s: &str) -> Badge<'_> { - parse_badges(s).next().unwrap() +mod tests { + use super::*; + + #[test] + fn ordering() { + let badges = "U/1U,U/11,U/5"; + + let mut badges: Vec<_> = parse_badges(badges).collect(); + + let expected = [("U", "1U"), ("U", "11"), ("U", "5")] + .into_iter() + .map(|(k, v)| Badge { + set_id: Cow::from(BadgeSetIdRef::from_static(k)), + id: Cow::from(ChatBadgeIdRef::from_static(v)), + }) + .collect::>(); + + assert_eq!(badges, expected); + + badges.sort(); + + let expected = [("U", "1U"), ("U", "5"), ("U", "11")] + .into_iter() + .map(|(k, v)| Badge { + set_id: Cow::from(BadgeSetIdRef::from_static(k)), + id: Cow::from(ChatBadgeIdRef::from_static(v)), + }) + .collect::>(); + + assert_eq!(badges, expected); + } + + #[test] + fn badge_ordering() { + #[track_caller] + fn parse(s: &str) -> Badge<'_> { + parse_badges(s).next().unwrap() + } + + assert!(parse("subscriber/12") > parse("subscriber/9")); + assert!(parse("subscriber/0") < parse("subscriber/1")); + let mut badges: Vec<_> = parse_badges("bits/1,staff/1,broadcaster/1").collect(); + badges.sort(); + assert_eq!( + badges.iter().map(|b| b.set_id.as_str()).collect::>(), + vec!["staff", "broadcaster", "bits"] + ); + + let mut badges: Vec<_> = parse_badges("aaa/1,zzz/1,staff/1").collect(); + badges.sort(); + assert_eq!( + badges.iter().map(|b| b.set_id.as_str()).collect::>(), + vec!["staff", "aaa", "zzz"] + ); } - assert!(parse("subscriber/12") > parse("subscriber/9")); - assert!(parse("subscriber/0") < parse("subscriber/1")); - let mut badges: Vec<_> = parse_badges("bits/1,staff/1,broadcaster/1").collect(); - badges.sort(); - assert_eq!( - badges.iter().map(|b| b.set_id.as_str()).collect::>(), - vec!["staff", "broadcaster", "bits"] - ); - - let mut badges: Vec<_> = parse_badges("aaa/1,zzz/1,staff/1").collect(); - badges.sort(); - assert_eq!( - badges.iter().map(|b| b.set_id.as_str()).collect::>(), - vec!["staff", "aaa", "zzz"] - ); }