Skip to content

Commit befc6cb

Browse files
fix(encode): support arrays and label Option<T> as optional (#855)
### Changelog - Added derive(Encode) support for arrays. - Marked Option<T> types as optional. ### Docs None ### Description - Added encoding for arrays following the same format as Vec - When an Option<T> is None, currently the field is not written at all but the field is still marked as Required. Changed it to optional. --------- Co-authored-by: Greg Smith <gasmith@foxglove.dev>
1 parent 2a11e13 commit befc6cb

4 files changed

Lines changed: 358 additions & 0 deletions

File tree

rust/foxglove/src/protobuf.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,13 @@ pub trait ProtobufField {
126126
false
127127
}
128128

129+
/// Indicates the type represents an optional field (like an Option).
130+
///
131+
/// By default, fields are not optional.
132+
fn optional() -> bool {
133+
false
134+
}
135+
129136
/// The length of the field to be written, in bytes (not including the tag).
130137
fn encoded_len(&self) -> usize;
131138

@@ -452,6 +459,10 @@ where
452459
true
453460
}
454461

462+
fn enum_descriptor() -> Option<prost_types::EnumDescriptorProto> {
463+
T::enum_descriptor()
464+
}
465+
455466
fn message_descriptor() -> Option<prost_types::DescriptorProto> {
456467
// The message descriptor of a vector is the message descriptor of the element type
457468
// the "repeating" property is set on the field that is repeating rather than the message
@@ -483,6 +494,70 @@ where
483494
}
484495
}
485496

497+
// implement a protobuf field for any array [T; N] where T implements ProtobufField
498+
impl<T, const N: usize> ProtobufField for [T; N]
499+
where
500+
T: ProtobufField,
501+
{
502+
fn field_type() -> ProstFieldType {
503+
T::field_type()
504+
}
505+
506+
fn wire_type() -> u32 {
507+
prost::encoding::WireType::LengthDelimited as u32
508+
}
509+
510+
fn write_tagged(&self, field_number: u32, buf: &mut impl bytes::BufMut) {
511+
// Non-packed repeated fields are encoded as a record for each element.
512+
// https://protobuf.dev/programming-guides/encoding/#repeated
513+
for value in self {
514+
value.write_tagged(field_number, buf);
515+
}
516+
}
517+
518+
fn write(&self, _buf: &mut impl bytes::BufMut) {
519+
panic!("[T; N] should always be written using write_tagged");
520+
}
521+
522+
fn repeating() -> bool {
523+
true
524+
}
525+
526+
fn enum_descriptor() -> Option<prost_types::EnumDescriptorProto> {
527+
T::enum_descriptor()
528+
}
529+
530+
fn message_descriptor() -> Option<prost_types::DescriptorProto> {
531+
// The message descriptor of an array is the message descriptor of the element type
532+
// the "repeating" property is set on the field that is repeating rather than the message
533+
// descriptor
534+
T::message_descriptor()
535+
}
536+
537+
fn file_descriptor() -> Option<prost_types::FileDescriptorProto> {
538+
T::file_descriptor()
539+
}
540+
541+
fn file_descriptors() -> Vec<prost_types::FileDescriptorProto> {
542+
T::file_descriptors()
543+
}
544+
545+
fn type_name() -> Option<String> {
546+
T::type_name()
547+
}
548+
549+
fn encoded_len(&self) -> usize {
550+
self.iter().map(|value| value.encoded_len()).sum()
551+
}
552+
553+
fn encoded_len_tagged(&self, field_number: u32) -> usize {
554+
// Each element is written with its own tag, so sum up the tagged lengths.
555+
self.iter()
556+
.map(|value| value.encoded_len_tagged(field_number))
557+
.sum()
558+
}
559+
}
560+
486561
// Implement ProtobufField for Option<T> where T implements ProtobufField.
487562
// In proto3, all fields are implicitly optional. None means the field is not written.
488563
impl<T> ProtobufField for Option<T>
@@ -532,6 +607,10 @@ where
532607
T::repeating()
533608
}
534609

610+
fn optional() -> bool {
611+
true
612+
}
613+
535614
fn encoded_len(&self) -> usize {
536615
match self {
537616
Some(value) => value.encoded_len(),

rust/foxglove_derive/src/lib.rs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,24 +37,67 @@ fn is_option(ty: &Type) -> bool {
3737
unwrap_generic_type(ty, "Option").is_some()
3838
}
3939

40+
/// Check if a type is `[T; N]` for some T and N.
41+
fn is_array(ty: &Type) -> bool {
42+
matches!(ty, Type::Array(_))
43+
}
44+
45+
/// Extract the element type from an array type `[T; N]`.
46+
fn unwrap_array_type(ty: &Type) -> Option<&Type> {
47+
match ty {
48+
Type::Array(arr) => Some(&arr.elem),
49+
_ => None,
50+
}
51+
}
52+
4053
/// Check if a type is `Vec<Option<T>>`, which is not supported because protobuf
4154
/// repeated fields cannot represent null/missing elements.
4255
fn is_vec_of_option(ty: &Type) -> bool {
4356
unwrap_generic_type(ty, "Vec").is_some_and(is_option)
4457
}
4558

59+
/// Check if a type is `[Option<T>; N]`, which is not supported because protobuf
60+
/// repeated fields cannot represent null/missing elements.
61+
fn is_array_of_option(ty: &Type) -> bool {
62+
unwrap_array_type(ty).is_some_and(is_option)
63+
}
64+
4665
/// Check if a type is `Option<Vec<T>>`, which is not supported because protobuf
4766
/// cannot distinguish between "not present" and "empty list".
4867
fn is_option_of_vec(ty: &Type) -> bool {
4968
unwrap_generic_type(ty, "Option").is_some_and(is_vec)
5069
}
5170

71+
/// Check if a type is `Option<[T; N]>`, which is not supported because protobuf
72+
/// cannot distinguish between "not present" and "empty array".
73+
fn is_option_of_array(ty: &Type) -> bool {
74+
unwrap_generic_type(ty, "Option").is_some_and(is_array)
75+
}
76+
5277
/// Check if a type is `Vec<Vec<T>>`, which is not supported because protobuf
5378
/// does not support nested repeated fields.
5479
fn is_vec_of_vec(ty: &Type) -> bool {
5580
unwrap_generic_type(ty, "Vec").is_some_and(is_vec)
5681
}
5782

83+
/// Check if a type is `[Vec<T>; N]`, which is not supported because protobuf
84+
/// does not support nested repeated fields.
85+
fn is_array_of_vec(ty: &Type) -> bool {
86+
unwrap_array_type(ty).is_some_and(is_vec)
87+
}
88+
89+
/// Check if a type is `Vec<[T; N]>`, which is not supported because protobuf
90+
/// does not support nested repeated fields.
91+
fn is_vec_of_array(ty: &Type) -> bool {
92+
unwrap_generic_type(ty, "Vec").is_some_and(is_array)
93+
}
94+
95+
/// Check if a type is `[[T; M]; N]`, which is not supported because protobuf
96+
/// does not support nested repeated fields.
97+
fn is_array_of_array(ty: &Type) -> bool {
98+
unwrap_array_type(ty).is_some_and(is_array)
99+
}
100+
58101
/// Derive macro for enums and structs allowing them to be logged to a Foxglove channel.
59102
#[proc_macro_derive(Encode)]
60103
pub fn derive_loggable(input: TokenStream) -> TokenStream {
@@ -209,6 +252,36 @@ fn derive_struct_impl(input: &DeriveInput, data: &DataStruct) -> TokenStream {
209252
});
210253
}
211254

255+
if is_array_of_option(field_type) {
256+
return TokenStream::from(quote! {
257+
compile_error!("[Option<T>; N] is not supported. Protobuf repeated fields cannot represent null/missing elements.");
258+
});
259+
}
260+
261+
if is_option_of_array(field_type) {
262+
return TokenStream::from(quote! {
263+
compile_error!("Option<[T; N]> is not supported. Protobuf cannot distinguish between absent and empty repeated fields.");
264+
});
265+
}
266+
267+
if is_array_of_array(field_type) {
268+
return TokenStream::from(quote! {
269+
compile_error!("[[T; M]; N] is not supported. Protobuf does not support nested repeated fields.");
270+
});
271+
}
272+
273+
if is_array_of_vec(field_type) {
274+
return TokenStream::from(quote! {
275+
compile_error!("[Vec<T>; N] is not supported. Protobuf does not support nested repeated fields.");
276+
});
277+
}
278+
279+
if is_vec_of_array(field_type) {
280+
return TokenStream::from(quote! {
281+
compile_error!("Vec<[T; N]> is not supported. Protobuf does not support nested repeated fields.");
282+
});
283+
}
284+
212285
field_tagged_lengths.push(quote! {
213286
::foxglove::protobuf::ProtobufField::encoded_len_tagged(&self.#field_name, #field_number)
214287
});
@@ -240,6 +313,8 @@ fn derive_struct_impl(input: &DeriveInput, data: &DataStruct) -> TokenStream {
240313

241314
if <#field_type as ::foxglove::protobuf::ProtobufField>::repeating() {
242315
field.label = Some(::foxglove::prost_types::field_descriptor_proto::Label::Repeated as i32);
316+
} else if <#field_type as ::foxglove::protobuf::ProtobufField>::optional() {
317+
field.label = Some(::foxglove::prost_types::field_descriptor_proto::Label::Optional as i32);
243318
} else {
244319
field.label = Some(::foxglove::prost_types::field_descriptor_proto::Label::Required as i32);
245320
}

rust/foxglove_derive/tests/enum_fields_test.rs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,16 @@ struct TestMessage {
2020
c: TestEnum,
2121
}
2222

23+
#[derive(Encode)]
24+
struct TestMessageVecEnum {
25+
values: Vec<TestEnum>,
26+
}
27+
28+
#[derive(Encode)]
29+
struct TestMessageArrayEnum {
30+
values: [TestEnum; 3],
31+
}
32+
2333
#[test]
2434
fn test_single_enum_field_serialization() {
2535
let test_struct = TestMessage {
@@ -58,6 +68,86 @@ fn test_single_enum_field_serialization() {
5868
}
5969
}
6070

71+
#[test]
72+
fn test_vec_of_enum_serialization() {
73+
let test_struct = TestMessageVecEnum {
74+
values: vec![TestEnum::ValueNeg, TestEnum::Value2, TestEnum::ValueOne],
75+
};
76+
77+
let mut buf = BytesMut::new();
78+
test_struct.encode(&mut buf).expect("Failed to encode");
79+
80+
let schema = TestMessageVecEnum::get_schema().expect("Failed to get schema");
81+
assert_eq!(schema.encoding, "protobuf");
82+
83+
let message_descriptor = get_message_descriptor(&schema);
84+
85+
// Verify the field is marked as repeated
86+
let field_descriptor = message_descriptor
87+
.get_field_by_name("values")
88+
.expect("Field 'values' not found");
89+
assert!(
90+
field_descriptor.is_list(),
91+
"Field should be a repeated list"
92+
);
93+
94+
let deserialized_message = DynamicMessage::decode(message_descriptor.clone(), buf.as_ref())
95+
.expect("Failed to deserialize");
96+
97+
let field_value = deserialized_message.get_field(&field_descriptor);
98+
let list_value = field_value.as_list().expect("Field value is not a list");
99+
100+
assert_eq!(list_value.len(), 3, "Vec should have 3 elements");
101+
102+
let expected_values = [-10, 2, 1];
103+
for (i, expected) in expected_values.iter().enumerate() {
104+
let value = list_value[i]
105+
.as_enum_number()
106+
.expect("List item should be an enum");
107+
assert_eq!(value, *expected, "Enum value at index {} is wrong", i);
108+
}
109+
}
110+
111+
#[test]
112+
fn test_array_of_enum_serialization() {
113+
let test_struct = TestMessageArrayEnum {
114+
values: [TestEnum::ValueOne, TestEnum::ValueNeg, TestEnum::Value2],
115+
};
116+
117+
let mut buf = BytesMut::new();
118+
test_struct.encode(&mut buf).expect("Failed to encode");
119+
120+
let schema = TestMessageArrayEnum::get_schema().expect("Failed to get schema");
121+
assert_eq!(schema.encoding, "protobuf");
122+
123+
let message_descriptor = get_message_descriptor(&schema);
124+
125+
// Verify the field is marked as repeated
126+
let field_descriptor = message_descriptor
127+
.get_field_by_name("values")
128+
.expect("Field 'values' not found");
129+
assert!(
130+
field_descriptor.is_list(),
131+
"Field should be a repeated list"
132+
);
133+
134+
let deserialized_message = DynamicMessage::decode(message_descriptor.clone(), buf.as_ref())
135+
.expect("Failed to deserialize");
136+
137+
let field_value = deserialized_message.get_field(&field_descriptor);
138+
let list_value = field_value.as_list().expect("Field value is not a list");
139+
140+
assert_eq!(list_value.len(), 3, "Array should have 3 elements");
141+
142+
let expected_values = [1, -10, 2];
143+
for (i, expected) in expected_values.iter().enumerate() {
144+
let value = list_value[i]
145+
.as_enum_number()
146+
.expect("List item should be an enum");
147+
assert_eq!(value, *expected, "Enum value at index {} is wrong", i);
148+
}
149+
}
150+
61151
fn get_message_descriptor(schema: &Schema) -> MessageDescriptor {
62152
let descriptor_set = prost_types::FileDescriptorSet::decode(schema.data.as_ref())
63153
.expect("Failed to decode descriptor set");

0 commit comments

Comments
 (0)