Remove duplicate field_names/variant_names fields

MrGVSV · MrGVSV · commit 4fe42a222f8f · 2022-09-19T17:05:10.000-07:00
diff --git a/crates/bevy_reflect/src/enums/enum_trait.rs b/crates/bevy_reflect/src/enums/enum_trait.rs
@@ -136,7 +136,6 @@ pub struct EnumInfo {
     type_name: &'static str,
     type_id: TypeId,
     variants: Box<[VariantInfo]>,
-    variant_names: Box<[&'static str]>,
     variant_indices: HashMap<&'static str, usize>,
 }
 
@@ -155,17 +154,11 @@ impl EnumInfo {
             .map(|(index, variant)| (variant.name(), index))
             .collect::<HashMap<_, _>>();
 
-        let variant_names = variants
-            .iter()
-            .map(|variant| variant.name())
-            .collect::<Vec<_>>();
-
         Self {
             name,
             type_name: std::any::type_name::<TEnum>(),
             type_id: TypeId::of::<TEnum>(),
             variants: variants.to_vec().into_boxed_slice(),
-            variant_names: variant_names.into_boxed_slice(),
             variant_indices,
         }
     }
@@ -177,11 +170,6 @@ impl EnumInfo {
             .map(|index| &self.variants[*index])
     }
 
-    /// A slice containing the names of all variants in order.
-    pub fn variant_names(&self) -> &[&'static str] {
-        &self.variant_names
-    }
-
     /// Get a variant at the given index.
     pub fn variant_at(&self, index: usize) -> Option<&VariantInfo> {
         self.variants.get(index)
diff --git a/crates/bevy_reflect/src/enums/variants.rs b/crates/bevy_reflect/src/enums/variants.rs
@@ -79,19 +79,16 @@ impl VariantInfo {
 pub struct StructVariantInfo {
     name: &'static str,
     fields: Box<[NamedField]>,
-    field_names: Box<[&'static str]>,
     field_indices: HashMap<&'static str, usize>,
 }
 
 impl StructVariantInfo {
     /// Create a new [`StructVariantInfo`].
     pub fn new(name: &'static str, fields: &[NamedField]) -> Self {
         let field_indices = Self::collect_field_indices(fields);
-        let field_names = fields.iter().map(|field| field.name()).collect::<Vec<_>>();
         Self {
             name,
             fields: fields.to_vec().into_boxed_slice(),
-            field_names: field_names.into_boxed_slice(),
             field_indices,
         }
     }
@@ -108,11 +105,6 @@ impl StructVariantInfo {
             .map(|index| &self.fields[*index])
     }
 
-    /// A slice containing the names of all fields in order.
-    pub fn field_names(&self) -> &[&'static str] {
-        &self.field_names
-    }
-
     /// Get the field at the given index.
     pub fn field_at(&self, index: usize) -> Option<&NamedField> {
         self.fields.get(index)
diff --git a/crates/bevy_reflect/src/serde/de.rs b/crates/bevy_reflect/src/serde/de.rs
@@ -13,7 +13,8 @@ use serde::de::{
 use serde::Deserialize;
 use std::any::TypeId;
 use std::fmt;
-use std::fmt::Formatter;
+use std::fmt::{Debug, Display, Formatter};
+use std::slice::Iter;
 
 pub trait DeserializeValue {
     fn deserialize(
@@ -25,7 +26,7 @@ pub trait DeserializeValue {
 trait StructLikeInfo {
     fn get_name(&self) -> &str;
     fn get_field(&self, name: &str) -> Option<&NamedField>;
-    fn get_field_names(&self) -> &[&'static str];
+    fn iter_fields(&self) -> Iter<'_, NamedField>;
 }
 
 trait TupleLikeInfo {
@@ -43,8 +44,8 @@ impl StructLikeInfo for StructInfo {
         self.field(name)
     }
 
-    fn get_field_names(&self) -> &[&'static str] {
-        self.field_names()
+    fn iter_fields(&self) -> Iter<'_, NamedField> {
+        self.iter()
     }
 }
 
@@ -57,8 +58,8 @@ impl StructLikeInfo for StructVariantInfo {
         self.field(name)
     }
 
-    fn get_field_names(&self) -> &[&'static str] {
-        self.field_names()
+    fn iter_fields(&self) -> Iter<'_, NamedField> {
+        self.iter()
     }
 }
 
@@ -90,6 +91,39 @@ impl TupleLikeInfo for TupleVariantInfo {
     }
 }
 
+/// A debug struct used for error messages that displays a list of expected values.
+///
+/// # Example
+///
+/// ```ignore
+/// let expected = vec!["foo", "bar", "baz"];
+/// assert_eq!("`foo`, `bar`, `baz`", format!("{}", ExpectedValues(expected)));
+/// ```
+struct ExpectedValues<TIntoIter, TIter, TItem>(TIntoIter)
+where
+    TIntoIter: IntoIterator<IntoIter = TIter> + Clone,
+    TIter: Iterator<Item = TItem> + ExactSizeIterator,
+    TItem: Display;
+
+impl<TIntoIter, TIter, TItem> Debug for ExpectedValues<TIntoIter, TIter, TItem>
+where
+    TIntoIter: IntoIterator<IntoIter = TIter> + Clone,
+    TIter: Iterator<Item = TItem> + ExactSizeIterator,
+    TItem: Display,
+{
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        let iter = self.0.clone().into_iter();
+        let len = iter.len();
+        for (index, item) in iter.enumerate() {
+            write!(f, "`{}`", item)?;
+            if index < len - 1 {
+                write!(f, ", ")?;
+            }
+        }
+        Ok(())
+    }
+}
+
 /// Represents a simple reflected identifier.
 #[derive(Debug, Clone, Eq, PartialEq)]
 struct Ident(String);
@@ -255,7 +289,8 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
             TypeInfo::Struct(struct_info) => {
                 let mut dynamic_struct = deserializer.deserialize_struct(
                     struct_info.name(),
-                    struct_info.field_names(),
+                    // Field names are mainly just a hint, we don't necessarily need to store and pass that data
+                    &[],
                     StructVisitor {
                         struct_info,
                         registry: self.registry,
@@ -324,7 +359,8 @@ impl<'a, 'de> DeserializeSeed<'de> for TypedReflectDeserializer<'a> {
                 } else {
                     deserializer.deserialize_enum(
                         enum_info.name(),
-                        enum_info.variant_names(),
+                        // Variant names are mainly just a hint, we don't necessarily need to store and pass that data
+                        &[],
                         EnumVisitor {
                             enum_info,
                             registry: self.registry,
@@ -583,15 +619,20 @@ impl<'a, 'de> Visitor<'de> for EnumVisitor<'a> {
     {
         let mut dynamic_enum = DynamicEnum::default();
         let (Ident(variant_name), variant) = data.variant().unwrap();
-        let variant_info = self
-            .enum_info
-            .variant(&variant_name)
-            .ok_or_else(|| Error::unknown_variant(&variant_name, self.enum_info.variant_names()))?;
+        let variant_info = self.enum_info.variant(&variant_name).ok_or_else(|| {
+            let names = self.enum_info.iter().map(|variant| variant.name());
+            Error::custom(format_args!(
+                "unknown variant `{}`, expected one of {:?}",
+                variant_name,
+                ExpectedValues(names)
+            ))
+        })?;
         let value: DynamicVariant = match variant_info {
             VariantInfo::Unit(..) => variant.unit_variant()?.into(),
             VariantInfo::Struct(struct_info) => variant
                 .struct_variant(
-                    struct_info.field_names(),
+                    // Field names are mainly just a hint, we don't necessarily need to store and pass that data
+                    &[],
                     StructVariantVisitor {
                         struct_info,
                         registry: self.registry,
@@ -727,9 +768,14 @@ where
 {
     let mut dynamic_struct = DynamicStruct::default();
     while let Some(Ident(key)) = map.next_key::<Ident>()? {
-        let field = info
-            .get_field(&key)
-            .ok_or_else(|| Error::unknown_field(&key, info.get_field_names()))?;
+        let field = info.get_field(&key).ok_or_else(|| {
+            let fields = info.iter_fields().map(|field| field.name());
+            Error::custom(format_args!(
+                "unknown field `{}`, expected one of {:?}",
+                key,
+                ExpectedValues(fields)
+            ))
+        })?;
         let registration = get_registration(field.type_id(), field.type_name(), registry)?;
         let value = map.next_value_seed(TypedReflectDeserializer {
             registration,
diff --git a/crates/bevy_reflect/src/struct_trait.rs b/crates/bevy_reflect/src/struct_trait.rs
@@ -73,7 +73,6 @@ pub struct StructInfo {
     type_name: &'static str,
     type_id: TypeId,
     fields: Box<[NamedField]>,
-    field_names: Box<[&'static str]>,
     field_indices: HashMap<&'static str, usize>,
 }
 
@@ -91,13 +90,11 @@ impl StructInfo {
             .enumerate()
             .map(|(index, field)| (field.name(), index))
             .collect::<HashMap<_, _>>();
-        let field_names = fields.iter().map(|field| field.name()).collect::<Vec<_>>();
 
         Self {
             name,
             type_name: std::any::type_name::<T>(),
             type_id: TypeId::of::<T>(),
-            field_names: field_names.into_boxed_slice(),
             fields: fields.to_vec().into_boxed_slice(),
             field_indices,
         }
@@ -110,11 +107,6 @@ impl StructInfo {
             .map(|index| &self.fields[*index])
     }
 
-    /// A slice containing the names of all fields in order.
-    pub fn field_names(&self) -> &[&'static str] {
-        &self.field_names
-    }
-
     /// Get the field at the given index.
     pub fn field_at(&self, index: usize) -> Option<&NamedField> {
         self.fields.get(index)
