From bfa9bdc36e5f8f593802ea90cdaa7be48ae1ce9a Mon Sep 17 00:00:00 2001 From: Mingwei Samuel Date: Mon, 20 Jun 2022 20:43:15 -0700 Subject: [PATCH] Add deny-unknown-enum-variants features Add `NONE = -1` variant to Champions --- riven/Cargo.toml | 8 ++++ riven/src/consts/champion.rs | 6 ++- riven/src/consts/game_mode.rs | 4 +- riven/src/consts/macros.rs | 70 ++++++++++++++++++++++++++--- riven/src/consts/map.rs | 4 -- riven/src/consts/queue.rs | 4 -- riven/src/consts/queue_type.rs | 6 ++- riven/src/consts/season.rs | 4 -- riven/srcgen/consts/champion.rs.dt | 6 ++- riven/srcgen/consts/game_mode.rs.dt | 4 +- riven/srcgen/consts/map.rs.dt | 4 -- riven/srcgen/consts/queue.rs.dt | 4 -- riven/srcgen/consts/season.rs.dt | 4 -- riven/tests/tests_la1.rs | 2 +- test-full.bash | 2 +- 15 files changed, 89 insertions(+), 43 deletions(-) diff --git a/riven/Cargo.toml b/riven/Cargo.toml index 5ed3a8b..053a3ad 100644 --- a/riven/Cargo.toml +++ b/riven/Cargo.toml @@ -28,7 +28,15 @@ default-tls = [ "reqwest/default-tls" ] native-tls = [ "reqwest/native-tls" ] rustls-tls = [ "reqwest/rustls-tls" ] +deny-unknown = [ "deny-unknown-fields", "deny-unknown-enum-variants" ] +# If enabled, extra unknown fields encountered during deserialization will +# cause an error instead of being ignored. deny-unknown-fields = [] +# If enabled, deserialization of unknown enum variants will cause an error +# instead of being deserialized to `UNKNOWN` or other integer variants. +deny-unknown-enum-variants = [ "deny-unknown-enum-variants-strings", "deny-unknown-enum-variants-integers" ] +deny-unknown-enum-variants-strings = [] +deny-unknown-enum-variants-integers = [] [dependencies] lazy_static = "1.4" diff --git a/riven/src/consts/champion.rs b/riven/src/consts/champion.rs index 05efb27..d6f750d 100644 --- a/riven/src/consts/champion.rs +++ b/riven/src/consts/champion.rs @@ -15,6 +15,7 @@ newtype_enum! { /// /// Field | Name | Identifier | Id /// ---|---|---|--- + /// `NONE` | None (no ban) | | -1 /// `AATROX` | "Aatrox" | "Aatrox" | 266 /// `AHRI` | "Ahri" | "Ahri" | 103 /// `AKALI` | "Akali" | "Akali" | 84 @@ -175,9 +176,10 @@ newtype_enum! { /// `ZILEAN` | "Zilean" | "Zilean" | 26 /// `ZOE` | "Zoe" | "Zoe" | 142 /// `ZYRA` | "Zyra" | "Zyra" | 143 - #[derive(serde::Serialize, serde::Deserialize)] - #[serde(transparent)] pub newtype_enum Champion(i16) { + /// `-1`, none. Appears when a champion ban is not used in champ select. + NONE = -1, + /// `266`. AATROX = 266, /// `103`. diff --git a/riven/src/consts/game_mode.rs b/riven/src/consts/game_mode.rs index 9a77052..96ea27b 100644 --- a/riven/src/consts/game_mode.rs +++ b/riven/src/consts/game_mode.rs @@ -6,14 +6,14 @@ // // /////////////////////////////////////////////// -use strum_macros::{ EnumString, IntoStaticStr }; +use strum_macros::{ EnumString, EnumVariantNames, IntoStaticStr }; /// League of Legends game mode, such as Classic, /// ARAM, URF, One For All, Ascension, etc. #[non_exhaustive] #[derive(Debug, Clone)] #[derive(Eq, PartialEq, Hash)] -#[derive(EnumString, IntoStaticStr)] +#[derive(EnumString, EnumVariantNames, IntoStaticStr)] #[repr(u8)] pub enum GameMode { /// Catch-all variant for new, unknown game modes. diff --git a/riven/src/consts/macros.rs b/riven/src/consts/macros.rs index 4a681e6..2b4bd30 100644 --- a/riven/src/consts/macros.rs +++ b/riven/src/consts/macros.rs @@ -3,7 +3,7 @@ /// Macro for deriving `Serialize` and `Deserialize` for string enums with an /// `UNKNOWN(String)` variant. /// -/// Enum should have `#[derive(EnumString, IntoStaticStr)]` included. +/// Enum should have `#[derive(EnumString, EnumVariantNames, IntoStaticStr)]` included. /// /// Also implements `AsRef`, `Display`, and `From<&str>`. macro_rules! serde_strum_unknown { @@ -40,7 +40,23 @@ macro_rules! serde_strum_unknown { where D: serde::de::Deserializer<'de> { - <&str>::deserialize(deserializer).map(Into::into) + #[cfg(not(feature = "deny-unknown-enum-variants-strings"))] + { + <&str>::deserialize(deserializer).map(Into::into) + } + #[cfg(feature = "deny-unknown-enum-variants-strings")] + { + <&str>::deserialize(deserializer).map(Into::into) + .and_then(|item| { + match item { + Self::UNKNOWN(unknown) => Err(serde::de::Error::unknown_variant( + &*unknown, + ::VARIANTS, + )), + other => Ok(other), + } + }) + } } } } @@ -56,6 +72,13 @@ macro_rules! arr { } } +/// Macro for newtype "enums" with integer values. +/// +/// For serde, use the following: +/// ```ignore +/// #[derive(Serialize, Deserialize)] +/// #[serde(from = "$repr", into = "$repr")] +/// ``` macro_rules! newtype_enum { { $( #[$attr:meta] )* @@ -97,15 +120,49 @@ macro_rules! newtype_enum { } } + impl std::convert::From<$name> for $repr { + fn from(value: $name ) -> Self { + value.0 + } + } + impl serde::ser::Serialize for $name { + fn serialize(&self, serializer: S) -> Result + where + S: serde::ser::Serializer, + { + <$repr>::serialize(&self.0, serializer) + } + } + impl std::convert::From<$repr> for $name { fn from(value: $repr ) -> Self { Self(value) } } - - impl std::convert::From<$name> for $repr { - fn from(value: $name ) -> Self { - value.0 + impl<'de> serde::de::Deserialize<'de> for $name { + fn deserialize(deserializer: D) -> Result + where + D: serde::de::Deserializer<'de> + { + #[cfg(not(feature = "deny-unknown-enum-variants-integers"))] + { + <$repr>::deserialize(deserializer).map(Into::into) + } + #[cfg(feature = "deny-unknown-enum-variants-integers")] + { + <$repr>::deserialize(deserializer).map(Into::into) + .and_then(|item: Self| { + if !item.is_known() { + Err(serde::de::Error::custom(format!( + "Unknown integer enum variant: {} (\"deny-unknown-enum-variants-integers\" feature is enabled).\nExpected one of the following: {:?}", + item, Self::ALL_KNOWN + ))) + } + else { + Ok(item) + } + }) + } } } @@ -114,7 +171,6 @@ macro_rules! newtype_enum { self.0.fmt(f) } } - impl std::fmt::Debug for $name { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{}({}{})", stringify!($name), self.0, if self.is_known() { "" } else { "?" }) diff --git a/riven/src/consts/map.rs b/riven/src/consts/map.rs index f832315..5fb82f4 100644 --- a/riven/src/consts/map.rs +++ b/riven/src/consts/map.rs @@ -6,12 +6,8 @@ // // /////////////////////////////////////////////// -use serde::{ Serialize, Deserialize }; - newtype_enum! { /// A League of Legends map. - #[derive(Serialize, Deserialize)] - #[serde(transparent)] pub newtype_enum Map(u8) { /// `1`. /// Summoner's Rift diff --git a/riven/src/consts/queue.rs b/riven/src/consts/queue.rs index bf7aff8..f67090d 100644 --- a/riven/src/consts/queue.rs +++ b/riven/src/consts/queue.rs @@ -6,12 +6,8 @@ // // /////////////////////////////////////////////// -use serde::{ Serialize, Deserialize }; - newtype_enum! { /// A League of Legends matchmaking queue. - #[derive(Serialize, Deserialize)] - #[serde(transparent)] pub newtype_enum Queue(u16) { /// `0`. /// Games on Custom games diff --git a/riven/src/consts/queue_type.rs b/riven/src/consts/queue_type.rs index 65b687b..7044574 100644 --- a/riven/src/consts/queue_type.rs +++ b/riven/src/consts/queue_type.rs @@ -1,10 +1,10 @@ -use strum_macros::{ EnumString, IntoStaticStr }; +use strum_macros::{ EnumString, EnumVariantNames, IntoStaticStr }; /// LoL or TFT ranked queue types. #[non_exhaustive] #[derive(Debug, Clone)] #[derive(Eq, PartialEq, Hash)] -#[derive(EnumString, IntoStaticStr)] +#[derive(EnumString, EnumVariantNames, IntoStaticStr)] pub enum QueueType { /// Catch-all variant for new, unknown queue types. #[strum(default)] @@ -62,6 +62,8 @@ mod test { } #[test] + // Note: this test is often not run due to this condition below. + #[cfg(not(feature = "deny-unknown-enum-variants-strings"))] fn check_deserialize() { use std::collections::BTreeMap; diff --git a/riven/src/consts/season.rs b/riven/src/consts/season.rs index 37d8405..634f6d0 100644 --- a/riven/src/consts/season.rs +++ b/riven/src/consts/season.rs @@ -6,12 +6,8 @@ // // /////////////////////////////////////////////// -use serde::{ Serialize, Deserialize }; - newtype_enum! { /// A League of Legends season for competitive matchmaking. - #[derive(Serialize, Deserialize)] - #[serde(transparent)] pub newtype_enum Season(u8) { /// `0`. PRESEASON_3 = 0, diff --git a/riven/srcgen/consts/champion.rs.dt b/riven/srcgen/consts/champion.rs.dt index eadf5ee..8a47f6d 100644 --- a/riven/srcgen/consts/champion.rs.dt +++ b/riven/srcgen/consts/champion.rs.dt @@ -17,6 +17,7 @@ newtype_enum! { /// /// Field | Name | Identifier | Id /// ---|---|---|--- + /// `NONE` | None (no ban) | | -1 {{ for (const { id, alias, name } of champions) { }} @@ -24,9 +25,10 @@ newtype_enum! { {{ } }} - #[derive(serde::Serialize, serde::Deserialize)] - #[serde(transparent)] pub newtype_enum Champion(i16) { + /// `-1`, none. Appears when a champion ban is not used in champ select. + NONE = -1, + {{ for (const { id, alias, name } of champions) { }} diff --git a/riven/srcgen/consts/game_mode.rs.dt b/riven/srcgen/consts/game_mode.rs.dt index 74fdebd..abba75b 100644 --- a/riven/srcgen/consts/game_mode.rs.dt +++ b/riven/srcgen/consts/game_mode.rs.dt @@ -3,14 +3,14 @@ const gameModes = require('./.gameModes.json'); }}{{= dotUtils.preamble() }} -use strum_macros::{ EnumString, IntoStaticStr }; +use strum_macros::{ EnumString, EnumVariantNames, IntoStaticStr }; /// League of Legends game mode, such as Classic, /// ARAM, URF, One For All, Ascension, etc. #[non_exhaustive] #[derive(Debug, Clone)] #[derive(Eq, PartialEq, Hash)] -#[derive(EnumString, IntoStaticStr)] +#[derive(EnumString, EnumVariantNames, IntoStaticStr)] #[repr(u8)] pub enum GameMode { /// Catch-all variant for new, unknown game modes. diff --git a/riven/srcgen/consts/map.rs.dt b/riven/srcgen/consts/map.rs.dt index e394321..d97f647 100644 --- a/riven/srcgen/consts/map.rs.dt +++ b/riven/srcgen/consts/map.rs.dt @@ -3,12 +3,8 @@ const maps = require('./.maps.json'); }}{{= dotUtils.preamble() }} -use serde::{ Serialize, Deserialize }; - newtype_enum! { /// A League of Legends map. - #[derive(Serialize, Deserialize)] - #[serde(transparent)] pub newtype_enum Map(u8) { {{ for (const e of maps) { diff --git a/riven/srcgen/consts/queue.rs.dt b/riven/srcgen/consts/queue.rs.dt index 0f0d063..3922b66 100644 --- a/riven/srcgen/consts/queue.rs.dt +++ b/riven/srcgen/consts/queue.rs.dt @@ -3,12 +3,8 @@ const queues = require('./.queues.json'); }}{{= dotUtils.preamble() }} -use serde::{ Serialize, Deserialize }; - newtype_enum! { /// A League of Legends matchmaking queue. - #[derive(Serialize, Deserialize)] - #[serde(transparent)] pub newtype_enum Queue(u16) { {{ for (const e of queues) { diff --git a/riven/srcgen/consts/season.rs.dt b/riven/srcgen/consts/season.rs.dt index cec30e4..0eb5f31 100644 --- a/riven/srcgen/consts/season.rs.dt +++ b/riven/srcgen/consts/season.rs.dt @@ -3,12 +3,8 @@ const seasons = require('./.seasons.json'); }}{{= dotUtils.preamble() }} -use serde::{ Serialize, Deserialize }; - newtype_enum! { /// A League of Legends season for competitive matchmaking. - #[derive(Serialize, Deserialize)] - #[serde(transparent)] pub newtype_enum Season(u8) { {{ for (const e of seasons) { diff --git a/riven/tests/tests_la1.rs b/riven/tests/tests_la1.rs index 349a62f..b56e8fd 100644 --- a/riven/tests/tests_la1.rs +++ b/riven/tests/tests_la1.rs @@ -40,7 +40,7 @@ async_tests! { // Spot check 10% for `player-data`. for entry in leaderboard.iter().step_by(10) { - let player_data = RIOT_API.lol_challenges_v1().get_player_data(ROUTE, &*entry.puuid) + let _player_data = RIOT_API.lol_challenges_v1().get_player_data(ROUTE, &*entry.puuid) .await.map_err(|e| format!("Failed to get player data PUUID {}: {}", entry.puuid, e))?; } diff --git a/test-full.bash b/test-full.bash index 88bb381..c1a6519 100755 --- a/test-full.bash +++ b/test-full.bash @@ -9,4 +9,4 @@ cargo +stable test --no-run --features tracing cargo +nightly test --no-run --features nightly,tracing # Run tests on nightly. -RGAPI_KEY="$(cat apikey.txt)" RUST_BACKTRACE=1 RUST_LOG=riven=trace cargo +nightly test --features nightly,deny-unknown-fields -- --nocapture +RGAPI_KEY="$(cat apikey.txt)" RUST_BACKTRACE=1 RUST_LOG=riven=trace cargo +nightly test --features nightly,deny-unknown -- --nocapture