Fix off-by-one bug in serde impl for Symbol types

Author: caspark

src/serde_impl.rs

@@ -93,16 +93,22 @@ macro_rules! impl_serde_for_symbol {
                 &self,
                 serializer: T,
             ) -> ::core::result::Result<T::Ok, T::Error> {
-                self.value.serialize(serializer)
+                // When the value is deserialized below, it will be converted back to a non-zero
+                // type by adding 1 to the value, so we need to account for that here by subtracting
+                // 1 from the value.
+                let zeroable_value = self.value.get() - 1;
+                zeroable_value.serialize(serializer)
             }
         }
 
         impl<'de> ::serde::Deserialize<'de> for $crate::symbol::$name {
             fn deserialize<D: ::serde::Deserializer<'de>>(
                 deserializer: D,
             ) -> ::core::result::Result<Self, D::Error> {
-                let index = <$ty as ::serde::Deserialize<'de>>::deserialize(deserializer)?;
-                let ::core::option::Option::Some(symbol) = Self::new(index) else {
+                let zeroable_value = <$ty as ::serde::Deserialize<'de>>::deserialize(deserializer)?;
+                // Symbol::new() will add 1 to the value to guarantee that the resulting underlying
+                // index will be non-zero.
+                let ::core::option::Option::Some(symbol) = Self::new(zeroable_value) else {
                     return ::core::result::Result::Err(<D::Error as ::serde::de::Error>::custom(
                         ::core::concat!(
                             "invalid index value for `",
@@ -119,3 +125,61 @@ macro_rules! impl_serde_for_symbol {
 impl_serde_for_symbol!(SymbolU16, u16);
 impl_serde_for_symbol!(SymbolU32, u32);
 impl_serde_for_symbol!(SymbolUsize, usize);
+
+#[cfg(test)]
+mod tests {
+    use crate::{
+        symbol::{SymbolU16, SymbolU32, SymbolUsize},
+        Symbol,
+    };
+    use serde_json;
+
+    fn symbol_round_trip_serializes<S>(symbol: S) -> bool
+    where
+        S: Symbol + std::fmt::Debug + serde::Serialize + serde::de::DeserializeOwned + PartialEq,
+    {
+        let serialized = serde_json::to_string(&symbol).expect("serialization should succeed");
+        let deserialized: S =
+            serde_json::from_str(&serialized).expect("deserialization should succeed");
+        symbol == deserialized
+    }
+
+    #[test]
+    fn symbol_u16_round_trips() {
+        assert!(symbol_round_trip_serializes(
+            SymbolU16::try_from_usize(0).unwrap()
+        ));
+        assert!(symbol_round_trip_serializes(
+            SymbolU16::try_from_usize(42).unwrap()
+        ));
+        assert!(symbol_round_trip_serializes(
+            SymbolU16::try_from_usize(u16::MAX as usize - 1).unwrap()
+        ));
+    }
+
+    #[test]
+    fn symbol_u32_round_trips() {
+        assert!(symbol_round_trip_serializes(
+            SymbolU32::try_from_usize(0).unwrap()
+        ));
+        assert!(symbol_round_trip_serializes(
+            SymbolU32::try_from_usize(42).unwrap()
+        ));
+        assert!(symbol_round_trip_serializes(
+            SymbolU32::try_from_usize(u32::MAX as usize - 1).unwrap()
+        ));
+    }
+
+    #[test]
+    fn symbol_usize_round_trips() {
+        assert!(symbol_round_trip_serializes(
+            SymbolUsize::try_from_usize(0).unwrap()
+        ));
+        assert!(symbol_round_trip_serializes(
+            SymbolUsize::try_from_usize(42).unwrap()
+        ));
+        assert!(symbol_round_trip_serializes(
+            SymbolUsize::try_from_usize(usize::MAX as usize - 1).unwrap()
+        ));
+    }
+}