serde_keyvalue: support for tuple and struct enums

Up to now only unit enums were supported. Add support for tuple and
struct enums, which can deserialize the following enum

    enum VideoMode {
        Fullscreen,
        Window(u32, u32),
        Remote { host: String },
    }

from the following inputs:

    mode=fullscreen
    mode=window[640,480]
    mode=remote[host=workstation]

This allows us to stop using flattened or dedicated enums for parsing and also
results in a less ambiguous syntax for these cases.

BUG=b:248993755
TEST=cargo test -p serde_keyvalue
TEST=cargo test

Change-Id: I142fc91fab19f4a977adec3ee36094e5f95363db
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3915040
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
This commit is contained in:
Alexandre Courbot 2022-09-22 16:58:54 +09:00 committed by crosvm LUCI
parent 3573dfef69
commit e91b0fd604
2 changed files with 256 additions and 29 deletions

View file

@ -34,6 +34,7 @@ use num_traits::Num;
use remain::sorted;
use serde::de;
use serde::Deserialize;
use serde::Deserializer;
use thiserror::Error;
#[derive(Debug, Error, PartialEq)]
@ -274,6 +275,20 @@ impl<'de> KeyValueDeserializer<'de> {
Some(c)
}
/// Confirm that we have a separator (i.e. ',' or ']') character or have reached the end of the
/// input string.
fn confirm_separator(&mut self) -> Result<()> {
// We must have a comma or end of input after a value.
match self.peek_char() {
Some(',') => {
let _ = self.next_char();
Ok(())
}
Some(']') | None => Ok(()),
Some(_) => Err(self.error_here(ErrorKind::ExpectedComma)),
}
}
/// Attempts to parse an identifier, either for a key or for the value of an enum type.
pub fn parse_identifier(&mut self) -> Result<&'de str> {
let (remainder, res) = any_identifier(self.input)
@ -354,8 +369,9 @@ impl<'de> de::MapAccess<'de> for KeyValueDeserializer<'de> {
{
let has_next_identifier = self.next_identifier.is_some();
if self.peek_char().is_none() {
return Ok(None);
match self.peek_char() {
None | Some(']') => return Ok(None),
_ => (),
}
self.has_equal = false;
@ -377,7 +393,7 @@ impl<'de> de::MapAccess<'de> for KeyValueDeserializer<'de> {
Ok(val)
}
// Ok if we are parsing a boolean where an empty value means true.
Some(',') | None => Ok(val),
Some(',') | Some(']') | None => Ok(val),
Some(_) => Err(self.error_here(ErrorKind::ExpectedEqual)),
}
}
@ -388,14 +404,63 @@ impl<'de> de::MapAccess<'de> for KeyValueDeserializer<'de> {
{
let val = seed.deserialize(&mut *self)?;
// We must have a comma or end of input after a value.
match self.peek_char() {
Some(',') | None => {
let _ = self.next_char();
Ok(val)
}
Some(_) => Err(self.error_here(ErrorKind::ExpectedComma)),
}
self.confirm_separator()?;
Ok(val)
}
}
/// `MapAccess` for a map with no members specified.
///
/// This is used to allow a struct enum type to be specified without `[` and `]`, in which case
/// all its members will take their default value:
///
/// ```
/// # use serde_keyvalue::from_key_values;
/// # use serde::Deserialize;
/// #[derive(Deserialize, PartialEq, Debug)]
/// #[serde(rename_all = "kebab-case")]
/// enum FlipMode {
/// Active {
/// #[serde(default)]
/// switch1: bool,
/// #[serde(default)]
/// switch2: bool,
/// },
/// }
/// #[derive(Deserialize, PartialEq, Debug)]
/// struct TestStruct {
/// mode: FlipMode,
/// }
/// let res: TestStruct = from_key_values("mode=active").unwrap();
/// assert_eq!(
/// res,
/// TestStruct {
/// mode: FlipMode::Active {
/// switch1: false,
/// switch2: false
/// }
/// }
/// );
/// ```
struct EmptyMapAccess;
impl<'de> de::MapAccess<'de> for EmptyMapAccess {
type Error = ParseError;
fn next_key_seed<K>(&mut self, _seed: K) -> Result<Option<K::Value>>
where
K: de::DeserializeSeed<'de>,
{
Ok(None)
}
fn next_value_seed<V>(&mut self, _seed: V) -> Result<V::Value>
where
V: de::DeserializeSeed<'de>,
{
// Never reached because `next_key_seed` never returns a valid key.
unreachable!()
}
}
@ -426,18 +491,34 @@ impl<'a, 'de> de::VariantAccess<'de> for &'a mut KeyValueDeserializer<'de> {
unimplemented!()
}
fn tuple_variant<V>(self, _len: usize, _visitor: V) -> Result<V::Value>
fn tuple_variant<V>(self, len: usize, visitor: V) -> Result<V::Value>
where
V: de::Visitor<'de>,
{
unimplemented!()
self.deserialize_tuple(len, visitor)
}
fn struct_variant<V>(self, _fields: &'static [&'static str], _visitor: V) -> Result<V::Value>
fn struct_variant<V>(self, _fields: &'static [&'static str], visitor: V) -> Result<V::Value>
where
V: de::Visitor<'de>,
{
unimplemented!()
if self.peek_char() == Some('[') {
self.next_char();
let val = self.deserialize_map(visitor)?;
if self.peek_char() != Some(']') {
Err(self.error_here(ErrorKind::ExpectedCloseBracket))
} else {
self.next_char();
Ok(val)
}
} else {
// The `EmptyMapAccess` failing to parse means that this enum must take arguments, i.e.
// that an opening bracket is expected.
visitor
.visit_map(EmptyMapAccess)
.map_err(|_| self.error_here(ErrorKind::ExpectedOpenBracket))
}
}
}
@ -454,15 +535,9 @@ impl<'de> de::SeqAccess<'de> for KeyValueDeserializer<'de> {
let value = seed.deserialize(&mut *self)?;
// We must have a comma or end of input after a value.
match self.peek_char() {
Some(',') => {
let _ = self.next_char();
Ok(Some(value))
}
Some(']') | None => Ok(Some(value)),
_ => Err(self.error_here(ErrorKind::ExpectedComma)),
}
self.confirm_separator()?;
Ok(Some(value))
}
}
@ -1342,4 +1417,155 @@ mod tests {
}
);
}
#[test]
fn deserialize_struct_and_tuple_enum() {
#[derive(Deserialize, PartialEq, Debug)]
#[serde(rename_all = "kebab-case")]
enum VideoMode {
Fullscreen,
WindowAsTuple(u32, u32),
WindowAsStruct { width: u32, height: u32 },
}
#[derive(Deserialize, PartialEq, Debug)]
struct TestStruct {
mode: VideoMode,
}
let res: TestStruct = from_key_values("mode=fullscreen").unwrap();
assert_eq!(
res,
TestStruct {
mode: VideoMode::Fullscreen
}
);
let res: TestStruct = from_key_values("mode=window-as-tuple[640,480]").unwrap();
assert_eq!(
res,
TestStruct {
mode: VideoMode::WindowAsTuple(640, 480),
}
);
// Missing values
let err = from_key_values::<TestStruct>("mode=window-as-tuple").unwrap_err();
assert_eq!(
err,
ParseError {
kind: ErrorKind::ExpectedOpenBracket,
pos: 20,
}
);
let res: TestStruct =
from_key_values("mode=window-as-struct[width=800,height=600]").unwrap();
assert_eq!(
res,
TestStruct {
mode: VideoMode::WindowAsStruct {
width: 800,
height: 600,
}
}
);
// Missing values.
let err = from_key_values::<TestStruct>("mode=window-as-struct").unwrap_err();
assert_eq!(
err,
ParseError {
kind: ErrorKind::ExpectedOpenBracket,
pos: 21,
}
);
}
#[test]
fn deserialize_struct_enum_with_default() {
#[derive(Deserialize, PartialEq, Debug)]
#[serde(rename_all = "kebab-case")]
enum FlipMode {
Inactive,
Active {
#[serde(default)]
switch1: bool,
#[serde(default)]
switch2: bool,
},
}
#[derive(Deserialize, PartialEq, Debug)]
struct TestStruct {
mode: FlipMode,
}
// Only specify one member and expect the other to be default.
let res: TestStruct = from_key_values("mode=active[switch1=true]").unwrap();
assert_eq!(
res,
TestStruct {
mode: FlipMode::Active {
switch1: true,
switch2: false
}
}
);
// Specify boolean members without explicit value.
let res: TestStruct = from_key_values("mode=active[switch1,switch2]").unwrap();
assert_eq!(
res,
TestStruct {
mode: FlipMode::Active {
switch1: true,
switch2: true
}
}
);
// No member specified, braces present.
let res: TestStruct = from_key_values("mode=active[]").unwrap();
assert_eq!(
res,
TestStruct {
mode: FlipMode::Active {
switch1: false,
switch2: false
}
}
);
// No member specified and no braces.
let res: TestStruct = from_key_values("mode=active").unwrap();
assert_eq!(
res,
TestStruct {
mode: FlipMode::Active {
switch1: false,
switch2: false
}
}
);
// Non-struct variant should be recognized without braces.
let res: TestStruct = from_key_values("mode=inactive").unwrap();
assert_eq!(
res,
TestStruct {
mode: FlipMode::Inactive,
}
);
// Non-struct variant should not accept braces.
let err = from_key_values::<TestStruct>("mode=inactive[]").unwrap_err();
assert_eq!(
err,
ParseError {
kind: ErrorKind::ExpectedComma,
pos: 13,
}
);
}
}

View file

@ -208,8 +208,8 @@
//! );
//! ```
//!
//! Enums taking a single value should use the `flatten` field attribute in order to be inferred
//! from their variant key directly:
//! Enums taking a single value can use the `flatten` field attribute in order to be inferred from
//! their variant key directly:
//!
//! ```
//! # use serde_keyvalue::from_key_values;
@ -268,8 +268,8 @@
//! );
//! ```
//!
//! If an enum's variants are made of structs, it should take the `untagged` container attribute so
//! it can be inferred directly from the fields of the embedded structs:
//! If an enum's variants are made of structs, it can take the `untagged` container attribute to be
//! inferred directly from the fields of the embedded structs:
//!
//! ```
//! # use serde_keyvalue::from_key_values;
@ -331,8 +331,9 @@
//! is not available to the deserializer, it will try to determine the type of fields using the
//! input as its sole hint. For instance, any number will be returned as an integer type, and if the
//! parsed structure was actually expecting a number as a string, then an error will occur.
//! Struct enums also cannot be flattened and won't be recognized at all.
//!
//! For this reason it is discouraged to use `flatten` except when neither the embedding not the
//! For these reasons, it is discouraged to use `flatten` except when neither the embedding not the
//! flattened structs has a member of string type.
//!
//! Most of the time, similar functionality can be obtained by implementing a custom deserializer