From 8d8e18afbc16f9c38e57031e397b3348bb263af0 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Tue, 21 Jul 2020 20:31:54 +0200 Subject: [PATCH] Make RoomVersionId a non-exhaustive enum --- ruma-events/src/room/aliases.rs | 20 ++--- ruma-events/src/room/create.rs | 5 +- ruma-events/tests/redacted.rs | 2 +- ruma-identifiers/CHANGELOG.md | 2 + ruma-identifiers/src/room_version_id.rs | 113 +++++++++++++++--------- 5 files changed, 84 insertions(+), 58 deletions(-) diff --git a/ruma-events/src/room/aliases.rs b/ruma-events/src/room/aliases.rs index 0f0cd61d..4fd42c40 100644 --- a/ruma-events/src/room/aliases.rs +++ b/ruma-events/src/room/aliases.rs @@ -32,16 +32,16 @@ impl AliasesEventContent { pub fn redact(self, version: RoomVersionId) -> RedactedAliasesEventContent { // We compare the long way to avoid pre version 6 behavior if/when // a new room version is introduced. - if version.is_version_1() - || version.is_version_2() - || version.is_version_3() - || version.is_version_4() - || version.is_version_5() - { - RedactedAliasesEventContent { aliases: Some(self.aliases) } - } else { - RedactedAliasesEventContent { aliases: None } - } + let aliases = match version { + RoomVersionId::Version1 + | RoomVersionId::Version2 + | RoomVersionId::Version3 + | RoomVersionId::Version4 + | RoomVersionId::Version5 => Some(self.aliases), + _ => None, + }; + + RedactedAliasesEventContent { aliases } } } diff --git a/ruma-events/src/room/create.rs b/ruma-events/src/room/create.rs index 453dda12..78174940 100644 --- a/ruma-events/src/room/create.rs +++ b/ruma-events/src/room/create.rs @@ -84,7 +84,7 @@ mod tests { let content = CreateEventContent { creator: UserId::try_from("@carl:example.com").unwrap(), federate: false, - room_version: RoomVersionId::version_4(), + room_version: RoomVersionId::Version4, predecessor: None, }; @@ -113,10 +113,9 @@ mod tests { CreateEventContent { creator, federate: true, - room_version, + room_version: RoomVersionId::Version4, predecessor: None, } if creator == "@carl:example.com" - && room_version == RoomVersionId::version_4() ); } } diff --git a/ruma-events/tests/redacted.rs b/ruma-events/tests/redacted.rs index cf96dc3f..3e047e51 100644 --- a/ruma-events/tests/redacted.rs +++ b/ruma-events/tests/redacted.rs @@ -323,7 +323,7 @@ fn redact_method_properly_redacts() { let event = from_json_value::>(ev).unwrap().deserialize().unwrap(); assert_matches!( - event.redact(redaction, RoomVersionId::version_6()), + event.redact(redaction, RoomVersionId::Version6), AnyRedactedMessageEvent::RoomMessage(RedactedMessageEvent { content: RedactedMessageEventContent, event_id, diff --git a/ruma-identifiers/CHANGELOG.md b/ruma-identifiers/CHANGELOG.md index fe1579e7..a095c02f 100644 --- a/ruma-identifiers/CHANGELOG.md +++ b/ruma-identifiers/CHANGELOG.md @@ -35,6 +35,8 @@ Breaking changes: * Change `DeviceId` from being an alias for `String` to being an alias for `str` * This means any string slice or string literal is a valid `&DeviceId` now * But to store one, you need to box it: `Box` +* Change `RoomVersionId` from being an opaque struct to a non-exhaustive enum + * The constructor functions and `is_` predicates are now deprecated Deprecations: diff --git a/ruma-identifiers/src/room_version_id.rs b/ruma-identifiers/src/room_version_id.rs index 5579fcd4..8ea7b5d6 100644 --- a/ruma-identifiers/src/room_version_id.rs +++ b/ruma-identifiers/src/room_version_id.rs @@ -26,12 +26,8 @@ const MAX_CODE_POINTS: usize = 32; /// assert_eq!(RoomVersionId::try_from("1").unwrap().as_ref(), "1"); /// ``` #[derive(Clone, Debug, PartialEq, Eq, Hash)] -pub struct RoomVersionId(InnerRoomVersionId); - -/// Possibile values for room version, distinguishing between official Matrix versions and custom -/// versions. -#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -enum InnerRoomVersionId { +#[non_exhaustive] +pub enum RoomVersionId { /// A version 1 room. Version1, @@ -51,38 +47,44 @@ enum InnerRoomVersionId { Version6, /// A custom room version. - Custom(Box), + Custom(CustomRoomVersion), } impl RoomVersionId { /// Creates a version 1 room ID. + #[deprecated = "use RoomVersionId::Version1 instead"] pub fn version_1() -> Self { - Self(InnerRoomVersionId::Version1) + Self::Version1 } /// Creates a version 2 room ID. + #[deprecated = "use RoomVersionId::Version2 instead"] pub fn version_2() -> Self { - Self(InnerRoomVersionId::Version2) + Self::Version2 } /// Creates a version 3 room ID. + #[deprecated = "use RoomVersionId::Version3 instead"] pub fn version_3() -> Self { - Self(InnerRoomVersionId::Version3) + Self::Version3 } /// Creates a version 4 room ID. + #[deprecated = "use RoomVersionId::Version4 instead"] pub fn version_4() -> Self { - Self(InnerRoomVersionId::Version4) + Self::Version4 } /// Creates a version 5 room ID. + #[deprecated = "use RoomVersionId::Version5 instead"] pub fn version_5() -> Self { - Self(InnerRoomVersionId::Version5) + Self::Version5 } /// Creates a version 6 room ID. + #[deprecated = "use RoomVersionId::Version6 instead"] pub fn version_6() -> Self { - Self(InnerRoomVersionId::Version6) + Self::Version6 } /// Whether or not this room version is an official one specified by the Matrix protocol. @@ -92,64 +94,70 @@ impl RoomVersionId { /// Whether or not this is a custom room version. pub fn is_custom(&self) -> bool { - matches!(self.0, InnerRoomVersionId::Custom(_)) + matches!(self, Self::Custom(_)) } /// Whether or not this is a version 1 room. + #[deprecated = "compare to RoomVersionId::Version1 instead"] pub fn is_version_1(&self) -> bool { - self.0 == InnerRoomVersionId::Version1 + matches!(self, Self::Version1) } /// Whether or not this is a version 2 room. + #[deprecated = "compare to RoomVersionId::Version2 instead"] pub fn is_version_2(&self) -> bool { - self.0 == InnerRoomVersionId::Version2 + matches!(self, Self::Version2) } /// Whether or not this is a version 3 room. + #[deprecated = "compare to RoomVersionId::Version3 instead"] pub fn is_version_3(&self) -> bool { - self.0 == InnerRoomVersionId::Version3 + matches!(self, Self::Version3) } /// Whether or not this is a version 4 room. + #[deprecated = "compare to RoomVersionId::Version4 instead"] pub fn is_version_4(&self) -> bool { - self.0 == InnerRoomVersionId::Version4 + matches!(self, Self::Version4) } /// Whether or not this is a version 5 room. + #[deprecated = "compare to RoomVersionId::Version5 instead"] pub fn is_version_5(&self) -> bool { - self.0 == InnerRoomVersionId::Version5 + matches!(self, Self::Version5) } /// Whether or not this is a version 6 room. + #[deprecated = "compare to RoomVersionId::Version6 instead"] pub fn is_version_6(&self) -> bool { - self.0 == InnerRoomVersionId::Version5 + matches!(self, Self::Version5) } } impl From for String { fn from(id: RoomVersionId) -> Self { - match id.0 { - InnerRoomVersionId::Version1 => "1".to_owned(), - InnerRoomVersionId::Version2 => "2".to_owned(), - InnerRoomVersionId::Version3 => "3".to_owned(), - InnerRoomVersionId::Version4 => "4".to_owned(), - InnerRoomVersionId::Version5 => "5".to_owned(), - InnerRoomVersionId::Version6 => "6".to_owned(), - InnerRoomVersionId::Custom(version) => version.into(), + match id { + RoomVersionId::Version1 => "1".to_owned(), + RoomVersionId::Version2 => "2".to_owned(), + RoomVersionId::Version3 => "3".to_owned(), + RoomVersionId::Version4 => "4".to_owned(), + RoomVersionId::Version5 => "5".to_owned(), + RoomVersionId::Version6 => "6".to_owned(), + RoomVersionId::Custom(version) => version.into(), } } } impl AsRef for RoomVersionId { fn as_ref(&self) -> &str { - match &self.0 { - InnerRoomVersionId::Version1 => "1", - InnerRoomVersionId::Version2 => "2", - InnerRoomVersionId::Version3 => "3", - InnerRoomVersionId::Version4 => "4", - InnerRoomVersionId::Version5 => "5", - InnerRoomVersionId::Version6 => "6", - InnerRoomVersionId::Custom(version) => version, + match &self { + Self::Version1 => "1", + Self::Version2 => "2", + Self::Version3 => "3", + Self::Version4 => "4", + Self::Version5 => "5", + Self::Version6 => "6", + Self::Custom(version) => version.as_ref(), } } } @@ -208,18 +216,18 @@ where S: AsRef + Into>, { let version = match room_version_id.as_ref() { - "1" => RoomVersionId(InnerRoomVersionId::Version1), - "2" => RoomVersionId(InnerRoomVersionId::Version2), - "3" => RoomVersionId(InnerRoomVersionId::Version3), - "4" => RoomVersionId(InnerRoomVersionId::Version4), - "5" => RoomVersionId(InnerRoomVersionId::Version5), + "1" => RoomVersionId::Version1, + "2" => RoomVersionId::Version2, + "3" => RoomVersionId::Version3, + "4" => RoomVersionId::Version4, + "5" => RoomVersionId::Version5, custom => { if custom.is_empty() { return Err(Error::MinimumLengthNotSatisfied); } else if custom.chars().count() > MAX_CODE_POINTS { return Err(Error::MaximumLengthExceeded); } else { - RoomVersionId(InnerRoomVersionId::Custom(room_version_id.into())) + RoomVersionId::Custom(CustomRoomVersion(room_version_id.into())) } } }; @@ -267,10 +275,26 @@ impl PartialEq for String { } } +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct CustomRoomVersion(Box); + +impl From for String { + fn from(v: CustomRoomVersion) -> Self { + v.0.into() + } +} + +impl AsRef for CustomRoomVersion { + fn as_ref(&self) -> &str { + &self.0 + } +} + #[cfg(test)] mod tests { use std::convert::TryFrom; + use matches::assert_matches; #[cfg(feature = "serde")] use serde_json::{from_str, to_string}; @@ -354,7 +378,7 @@ mod tests { let deserialized = from_str::(r#""1""#).expect("Failed to convert RoomVersionId to JSON."); - assert!(deserialized.is_version_1()); + assert_matches!(deserialized, RoomVersionId::Version1); assert!(deserialized.is_official()); assert_eq!( @@ -390,6 +414,7 @@ mod tests { } #[test] + #[allow(deprecated)] fn constructors() { assert!(RoomVersionId::version_1().is_version_1()); assert!(RoomVersionId::version_2().is_version_2()); @@ -399,7 +424,7 @@ mod tests { } #[test] - #[allow(clippy::cognitive_complexity)] + #[allow(deprecated, clippy::cognitive_complexity)] fn predicate_methods() { let version_1 = RoomVersionId::try_from("1").expect("Failed to create RoomVersionId."); let version_2 = RoomVersionId::try_from("2").expect("Failed to create RoomVersionId.");