fix: correct size calculation for field IDs

The calculation for required bytes of a field ID did not take the recently added field encoding into account, which could potentially result in too small values.
dnaka91 · Jan 8, 2024 · e56dd8d · e56dd8d
1 parent 069f788
commit e56dd8d
Show file tree

Hide file tree

Showing 12 changed files with 55 additions and 47 deletions.
diff --git a/crates/mabo-build/src/size.rs b/crates/mabo-build/src/size.rs
@@ -123,19 +123,19 @@ fn compile_variant(
     match fields.kind {
         FieldKind::Named => quote! {
             Self::#name{ #(#field_names,)* } => {
-                ::mabo::buf::size_id(#id) +
+                ::mabo::buf::size_variant_id(#id) +
                 #fields_body
             }
         },
         FieldKind::Unnamed => quote! {
             Self::#name(#(#field_names,)*) => {
-                ::mabo::buf::size_id(#id) +
+                ::mabo::buf::size_variant_id(#id) +
                 #fields_body
             }
         },
         FieldKind::Unit => quote! {
             Self::#name => {
-                ::mabo::buf::size_id(#id)
+                ::mabo::buf::size_variant_id(#id)
             }
         },
     }

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@enum_basic.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@enum_basic.mabo.snap
@@ -142,15 +142,15 @@ impl ::mabo::buf::Size for Sample {
     )]
     fn size(&self) -> usize {
         match self {
-            Self::One => ::mabo::buf::size_id(1),
+            Self::One => ::mabo::buf::size_variant_id(1),
             Self::Two(n0, n1) => {
-                ::mabo::buf::size_id(2)
+                ::mabo::buf::size_variant_id(2)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*n0) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_u64(*n1) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
             Self::Three { field1, field2 } => {
-                ::mabo::buf::size_id(3)
+                ::mabo::buf::size_variant_id(3)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*field1) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_bool(*field2) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@enum_generics.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@enum_generics.mabo.snap
@@ -155,14 +155,15 @@ where
     )]
     fn size(&self) -> usize {
         match self {
-            Self::One => ::mabo::buf::size_id(1),
+            Self::One => ::mabo::buf::size_variant_id(1),
             Self::Two(n0, n1) => {
-                ::mabo::buf::size_id(2) + ::mabo::buf::size_field(1, || { n0.size() })
+                ::mabo::buf::size_variant_id(2)
+                    + ::mabo::buf::size_field(1, || { n0.size() })
                     + ::mabo::buf::size_field(2, || { n1.size() })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
             Self::Three { field1, field2 } => {
-                ::mabo::buf::size_id(3)
+                ::mabo::buf::size_variant_id(3)
                     + ::mabo::buf::size_field(1, || { field1.size() })
                     + ::mabo::buf::size_field(2, || { field2.size() })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@enum_many_ws.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@enum_many_ws.mabo.snap
@@ -137,15 +137,15 @@ impl ::mabo::buf::Size for Sample {
     )]
     fn size(&self) -> usize {
         match self {
-            Self::One => ::mabo::buf::size_id(1),
+            Self::One => ::mabo::buf::size_variant_id(1),
             Self::Two(n0, n1) => {
-                ::mabo::buf::size_id(2)
+                ::mabo::buf::size_variant_id(2)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*n0) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_u64(*n1) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
             Self::Three { field1, field2 } => {
-                ::mabo::buf::size_id(3)
+                ::mabo::buf::size_variant_id(3)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*field1) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_bool(*field2) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@enum_min_ws.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@enum_min_ws.mabo.snap
@@ -173,16 +173,16 @@ where
     )]
     fn size(&self) -> usize {
         match self {
-            Self::One => ::mabo::buf::size_id(1),
+            Self::One => ::mabo::buf::size_variant_id(1),
             Self::Two(n0, n1, n2) => {
-                ::mabo::buf::size_id(2)
+                ::mabo::buf::size_variant_id(2)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*n0) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_u64(*n1) })
                     + ::mabo::buf::size_field(3, || { n2.size() })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
             Self::Three { field1, field2, field3 } => {
-                ::mabo::buf::size_id(3)
+                ::mabo::buf::size_variant_id(3)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*field1) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_bool(*field2) })
                     + ::mabo::buf::size_field(3, || { field3.size() })

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@mixed.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@mixed.mabo.snap
@@ -423,12 +423,12 @@ impl ::mabo::buf::Size for HouseNumber {
     fn size(&self) -> usize {
         match self {
             Self::Digit(n0) => {
-                ::mabo::buf::size_id(1)
+                ::mabo::buf::size_variant_id(1)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u16(*n0) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
             Self::Text(n0) => {
-                ::mabo::buf::size_id(2)
+                ::mabo::buf::size_variant_id(2)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_string(n0) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
@@ -574,21 +574,21 @@ pub mod birthday {
         fn size(&self) -> usize {
             match self {
                 Self::Specific { year, month, day } => {
-                    ::mabo::buf::size_id(1)
+                    ::mabo::buf::size_variant_id(1)
                         + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u16(*year) })
                         + ::mabo::buf::size_field(2, || { month.size() })
                         + ::mabo::buf::size_field(3, || { ::mabo::buf::size_u8(*day) })
                         + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
                 }
                 Self::Secret { reason } => {
-                    ::mabo::buf::size_id(2)
+                    ::mabo::buf::size_variant_id(2)
                         + ::mabo::buf::size_field_option(
                             1,
                             reason.as_ref(),
                             |v| { ::mabo::buf::size_string(v) },
                         ) + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
                 }
-                Self::Unknown => ::mabo::buf::size_id(3),
+                Self::Unknown => ::mabo::buf::size_variant_id(3),
             }
         }
     }
@@ -696,18 +696,18 @@ pub mod birthday {
         )]
         fn size(&self) -> usize {
             match self {
-                Self::January => ::mabo::buf::size_id(1),
-                Self::February => ::mabo::buf::size_id(2),
-                Self::March => ::mabo::buf::size_id(3),
-                Self::April => ::mabo::buf::size_id(4),
-                Self::May => ::mabo::buf::size_id(5),
-                Self::June => ::mabo::buf::size_id(6),
-                Self::July => ::mabo::buf::size_id(7),
-                Self::August => ::mabo::buf::size_id(8),
-                Self::September => ::mabo::buf::size_id(9),
-                Self::October => ::mabo::buf::size_id(10),
-                Self::November => ::mabo::buf::size_id(11),
-                Self::December => ::mabo::buf::size_id(12),
+                Self::January => ::mabo::buf::size_variant_id(1),
+                Self::February => ::mabo::buf::size_variant_id(2),
+                Self::March => ::mabo::buf::size_variant_id(3),
+                Self::April => ::mabo::buf::size_variant_id(4),
+                Self::May => ::mabo::buf::size_variant_id(5),
+                Self::June => ::mabo::buf::size_variant_id(6),
+                Self::July => ::mabo::buf::size_variant_id(7),
+                Self::August => ::mabo::buf::size_variant_id(8),
+                Self::September => ::mabo::buf::size_variant_id(9),
+                Self::October => ::mabo::buf::size_variant_id(10),
+                Self::November => ::mabo::buf::size_variant_id(11),
+                Self::December => ::mabo::buf::size_variant_id(12),
             }
         }
     }

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@module_basic.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@module_basic.mabo.snap
@@ -53,7 +53,7 @@ pub mod a {
             )]
             fn size(&self) -> usize {
                 match self {
-                    Self::One => ::mabo::buf::size_id(1),
+                    Self::One => ::mabo::buf::size_variant_id(1),
                 }
             }
         }

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@optional_ids.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@optional_ids.mabo.snap
@@ -353,15 +353,15 @@ impl ::mabo::buf::Size for SampleEnum {
     fn size(&self) -> usize {
         match self {
             Self::Named { field1, field2, field3 } => {
-                ::mabo::buf::size_id(1)
+                ::mabo::buf::size_variant_id(1)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*field1) })
                     + ::mabo::buf::size_field(100, || { ::mabo::buf::size_u32(*field2) })
                     + ::mabo::buf::size_field(101, || { ::mabo::buf::size_u32(*field3) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
-            Self::Unit => ::mabo::buf::size_id(50),
+            Self::Unit => ::mabo::buf::size_variant_id(50),
             Self::Unnamed(n0, n1, n2) => {
-                ::mabo::buf::size_id(51)
+                ::mabo::buf::size_variant_id(51)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*n0) })
                     + ::mabo::buf::size_field(100, || { ::mabo::buf::size_u32(*n1) })
                     + ::mabo::buf::size_field(101, || { ::mabo::buf::size_u32(*n2) })

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@schema_basic.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@schema_basic.mabo.snap
@@ -217,15 +217,15 @@ impl ::mabo::buf::Size for SampleEnum {
     )]
     fn size(&self) -> usize {
         match self {
-            Self::One => ::mabo::buf::size_id(1),
+            Self::One => ::mabo::buf::size_variant_id(1),
             Self::Two(n0, n1) => {
-                ::mabo::buf::size_id(2)
+                ::mabo::buf::size_variant_id(2)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*n0) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_u64(*n1) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
             Self::Three { field1, field2 } => {
-                ::mabo::buf::size_id(3)
+                ::mabo::buf::size_variant_id(3)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*field1) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_bool(*field2) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile@types_ref.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile@types_ref.mabo.snap
@@ -125,7 +125,7 @@ impl ::mabo::buf::Size for Test123 {
     )]
     fn size(&self) -> usize {
         match self {
-            Self::Value => ::mabo::buf::size_id(1),
+            Self::Value => ::mabo::buf::size_variant_id(1),
         }
     }
 }

diff --git a/crates/mabo-build/tests/snapshots/compiler__compile_extra@enum.mabo.snap b/crates/mabo-build/tests/snapshots/compiler__compile_extra@enum.mabo.snap
@@ -151,15 +151,15 @@ impl ::mabo::buf::Size for Sample {
     )]
     fn size(&self) -> usize {
         match self {
-            Self::Variant1 => ::mabo::buf::size_id(1),
+            Self::Variant1 => ::mabo::buf::size_variant_id(1),
             Self::Variant2(n0, n1) => {
-                ::mabo::buf::size_id(2)
+                ::mabo::buf::size_variant_id(2)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_u32(*n0) })
                     + ::mabo::buf::size_field(2, || { ::mabo::buf::size_u8(*n1) })
                     + ::mabo::buf::size_u32(::mabo::buf::END_MARKER)
             }
             Self::Variant3 { field1, field2 } => {
-                ::mabo::buf::size_id(3)
+                ::mabo::buf::size_variant_id(3)
                     + ::mabo::buf::size_field(1, || { ::mabo::buf::size_string(field1) })
                     + ::mabo::buf::size_field(
                         2,

diff --git a/crates/mabo/src/buf/size.rs b/crates/mabo/src/buf/size.rs
@@ -110,10 +110,17 @@ where
     size_u64(N as u64) + array.iter().map(size).sum::<usize>()
 }
 
-/// Calculate the size of a Mabo field or variant identifier.
+/// Calculate the size of a Mabo field identifier.
 #[inline(always)]
 #[must_use]
-pub fn size_id(id: u32) -> usize {
+pub fn size_field_id(id: u32) -> usize {
+    size_u32(id << 3)
+}
+
+/// Calculate the size of a Mabo variant identifier.
+#[inline(always)]
+#[must_use]
+pub fn size_variant_id(id: u32) -> usize {
     size_u32(id)
 }
 
@@ -123,7 +130,7 @@ pub fn size_field<S>(id: u32, size: S) -> usize
 where
     S: Fn() -> usize,
 {
-    size_id(id) + size()
+    size_field_id(id) + size()
 }
 
 /// Calculate the size of an optional Mabo struct or enum field.
@@ -132,7 +139,7 @@ pub fn size_field_option<T, S>(id: u32, option: Option<&T>, size: S) -> usize
 where
     S: Fn(&T) -> usize,
 {
-    option.map_or(0, |value| size_id(id) + size(value))
+    option.map_or(0, |value| size_field_id(id) + size(value))
 }
 
 /// Values that are able to calculate their encoded byte size, without actually encoding.