From a1306cc510c6a928ddd9374665fb6cc6aace9f6c Mon Sep 17 00:00:00 2001 From: "Victor M. Alvarez" Date: Thu, 5 Sep 2024 10:50:03 +0200 Subject: [PATCH] feat: implement support for repeated fields in constants This adds support for repeated fields in constants. Until know the following was failing with a parsing error: ```protobuf optional uint64 my_field = 1 [ (my_option) = { my_option_repeated_field: ["foo", "bar"] } ]; ``` This is valid for `protoc` and it's covered in the specification, but the parser wasn't accepting the `["foo", "bar"]` syntax for assigning a value to the repeated field. Some refactoring was required to implement this. Particularly, the functions `option_value_field_to_unknown_value` and `option_value_message_to_unknown_value`, which returned `UnknownValue`, now receive a mutable `UnknownFields` and add new fields to it. These function where renamed to more appropriate names. --- .../src/pure/convert/option_resolver.rs | 351 +++++++++--------- protobuf-parse/src/pure/model.rs | 26 +- protobuf-parse/src/pure/parser.rs | 38 +- 3 files changed, 243 insertions(+), 172 deletions(-) diff --git a/protobuf-parse/src/pure/convert/option_resolver.rs b/protobuf-parse/src/pure/convert/option_resolver.rs index 948d3e3b3..98ae74fcf 100644 --- a/protobuf-parse/src/pure/convert/option_resolver.rs +++ b/protobuf-parse/src/pure/convert/option_resolver.rs @@ -1,4 +1,3 @@ -use anyhow::Context; use protobuf::descriptor::DescriptorProto; use protobuf::descriptor::EnumDescriptorProto; use protobuf::descriptor::EnumValueDescriptorProto; @@ -343,7 +342,7 @@ impl<'a> OptionResoler<'a> { &self, scope: &ProtobufAbsPathRef, options_type: &MessageDescriptor, - options: &mut UnknownFields, + unknown_fields: &mut UnknownFields, option_name: &ProtobufOptionNamePart, option_name_rem: &[ProtobufOptionNamePart], option_value: &ProtobufConstant, @@ -357,18 +356,18 @@ impl<'a> OptionResoler<'a> { match field_type { TypeResolved::Message(message_name) => { let m = self.find_message_by_abs_name(&message_name)?; - let mut unknown_fields = UnknownFields::new(); + let mut message_unknown_fields = UnknownFields::new(); self.custom_option_ext_step( scope, &m, - &mut unknown_fields, + &mut message_unknown_fields, first, rem, option_value, )?; - options.add_length_delimited( + unknown_fields.add_length_delimited( field.proto().number() as u32, - unknown_fields.write_to_bytes(), + message_unknown_fields.write_to_bytes(), ); Ok(()) } @@ -383,26 +382,162 @@ impl<'a> OptionResoler<'a> { .into()), } } - None => { - let value = match self.option_value_to_unknown_value( + None => self + .add_option_value_to_unknown_fields( &field_type, + field.number() as u32, + unknown_fields, option_value, &format!("{}", option_name), - ) { - Ok(value) => value, - Err(e) => { - let e = e.context(format!( - "parsing custom option `{}` value `{}` at `{}`", - option_name, option_value, scope - )); - return Err(e.into()); - } - }; + ) + .map_err(|err| { + err.context(format!( + "parsing custom option `{}` value `{}` at `{}`", + option_name, option_value, scope + )) + }), + } + } - options.add_value(field.proto().number() as u32, value); - Ok(()) + fn add_option_value_to_unknown_fields( + &self, + field_type: &TypeResolved, + field_num: u32, + unknown_fields: &mut UnknownFields, + option_value: &ProtobufConstant, + option_name_for_diag: &str, + ) -> anyhow::Result<()> { + let error = || { + OptionResolverError::UnsupportedExtensionType( + option_name_for_diag.to_owned(), + format!("{:?}", field_type), + option_value.clone(), + ) + }; + + match option_value { + ProtobufConstant::U64(v) => match field_type { + TypeResolved::Fixed64 => unknown_fields.add_value(field_num, Self::fixed64(*v)?), + TypeResolved::Sfixed64 => unknown_fields.add_value(field_num, Self::sfixed64(*v)?), + TypeResolved::Fixed32 => unknown_fields.add_value(field_num, Self::fixed32(*v)?), + TypeResolved::Sfixed32 => unknown_fields.add_value(field_num, Self::sfixed32(*v)?), + TypeResolved::Int32 => unknown_fields.add_value(field_num, Self::int32(*v)?), + TypeResolved::Int64 => unknown_fields.add_value(field_num, Self::int64(*v)?), + TypeResolved::Uint64 => unknown_fields.add_value(field_num, Self::uint64(*v)?), + TypeResolved::Uint32 => unknown_fields.add_value(field_num, Self::uint32(*v)?), + TypeResolved::Sint64 => unknown_fields.add_value(field_num, Self::sint64(*v)?), + TypeResolved::Sint32 => unknown_fields.add_value(field_num, Self::sint32(*v)?), + TypeResolved::Float => { + unknown_fields.add_value(field_num, UnknownValue::float(*v as f32)) + } + TypeResolved::Double => { + unknown_fields.add_value(field_num, UnknownValue::double(*v as f64)) + } + _ => return Err(error().into()), + }, + ProtobufConstant::I64(v) => match field_type { + TypeResolved::Fixed64 => unknown_fields.add_value(field_num, Self::fixed64(*v)?), + TypeResolved::Sfixed64 => unknown_fields.add_value(field_num, Self::sfixed64(*v)?), + TypeResolved::Fixed32 => unknown_fields.add_value(field_num, Self::fixed32(*v)?), + TypeResolved::Sfixed32 => unknown_fields.add_value(field_num, Self::sfixed32(*v)?), + TypeResolved::Int32 => unknown_fields.add_value(field_num, Self::int32(*v)?), + TypeResolved::Int64 => unknown_fields.add_value(field_num, Self::int64(*v)?), + TypeResolved::Uint64 => unknown_fields.add_value(field_num, Self::uint64(*v)?), + TypeResolved::Uint32 => unknown_fields.add_value(field_num, Self::uint32(*v)?), + TypeResolved::Sint64 => unknown_fields.add_value(field_num, Self::sint64(*v)?), + TypeResolved::Sint32 => unknown_fields.add_value(field_num, Self::sint32(*v)?), + TypeResolved::Float => { + unknown_fields.add_value(field_num, UnknownValue::float(*v as f32)) + } + TypeResolved::Double => { + unknown_fields.add_value(field_num, UnknownValue::double(*v as f64)) + } + _ => return Err(error().into()), + }, + ProtobufConstant::F64(f) => match field_type { + TypeResolved::Float => { + unknown_fields.add_value(field_num, UnknownValue::float(*f as f32)) + } + TypeResolved::Double => { + unknown_fields.add_value(field_num, UnknownValue::double(*f)) + } + TypeResolved::Fixed32 => { + unknown_fields.add_value(field_num, UnknownValue::Fixed32(*f as u32)) + } + TypeResolved::Fixed64 => { + unknown_fields.add_value(field_num, UnknownValue::Fixed64(*f as u64)) + } + TypeResolved::Sfixed32 => { + unknown_fields.add_value(field_num, UnknownValue::sfixed32(*f as i32)) + } + TypeResolved::Sfixed64 => { + unknown_fields.add_value(field_num, UnknownValue::sfixed64(*f as i64)) + } + TypeResolved::Sint64 => { + unknown_fields.add_value(field_num, UnknownValue::sint64(*f as i64)) + } + TypeResolved::Sint32 => { + unknown_fields.add_value(field_num, UnknownValue::sint32(*f as i32)) + } + TypeResolved::Int32 | TypeResolved::Int64 => { + unknown_fields.add_value(field_num, UnknownValue::int64(*f as i64)) + } + TypeResolved::Uint32 | TypeResolved::Uint64 => { + unknown_fields.add_value(field_num, UnknownValue::Varint(*f as u64)) + } + _ => return Err(error().into()), + }, + ProtobufConstant::Bool(b) => match field_type { + TypeResolved::Bool => unknown_fields + .add_value(field_num, UnknownValue::Varint(if *b { 1 } else { 0 })), + _ => return Err(error().into()), + }, + ProtobufConstant::Ident(ident) => match field_type { + TypeResolved::Enum(abs_path) => { + let n = self + .resolver + .find_enum_by_abs_name(abs_path) + .map_err(OptionResolverError::OtherError)? + .values + .iter() + .find(|v| v.name == ident.to_string()) + .map(|v| v.number) + .ok_or_else(|| OptionResolverError::UnknownEnumValue(ident.to_string()))?; + + unknown_fields.add_value(field_num, UnknownValue::int32(n)); + } + _ => return Err(error().into()), + }, + ProtobufConstant::String(s) => match field_type { + TypeResolved::String => unknown_fields.add_value( + field_num, + UnknownValue::LengthDelimited(s.decode_utf8()?.into_bytes()), + ), + TypeResolved::Bytes => unknown_fields + .add_value(field_num, UnknownValue::LengthDelimited(s.decode_bytes()?)), + _ => return Err(error().into()), + }, + ProtobufConstant::Message(message) => self.add_option_value_message_to_unknown_fields( + field_type, + field_num, + unknown_fields, + message, + &option_name_for_diag, + )?, + ProtobufConstant::Repeated(list) => { + for v in list { + self.add_option_value_to_unknown_fields( + field_type, + field_num, + unknown_fields, + v, + option_name_for_diag, + )? + } } } + + Ok(()) } fn custom_option_ext( @@ -485,12 +620,14 @@ impl<'a> OptionResoler<'a> { Ok(UnknownValue::sint64(v.try_into()?)) } - fn option_value_message_to_unknown_value( + fn add_option_value_message_to_unknown_fields( &self, field_type: &TypeResolved, - value: &ProtobufConstantMessage, + field_num: u32, + options: &mut UnknownFields, + option_value: &ProtobufConstantMessage, option_name_for_diag: &str, - ) -> anyhow::Result { + ) -> anyhow::Result<()> { match &field_type { TypeResolved::Message(ma) => { let m = self @@ -499,27 +636,23 @@ impl<'a> OptionResoler<'a> { .map_err(OptionResolverError::OtherError)? .t; let mut unknown_fields = UnknownFields::new(); - for (n, v) in &value.fields { + for (n, v) in &option_value.fields { match n { ProtobufConstantMessageFieldName::Regular(n) => { - let f = match m.field_by_name(n.as_str()) { - Some(f) => f, - None => { - return Err( - OptionResolverError::UnknownFieldName(n.clone()).into() - ) - } - }; - let u = self - .option_value_field_to_unknown_value( - ma, - v, - n, - &f.typ, - option_name_for_diag, - ) - .map_err(OptionResolverError::OtherError)?; - unknown_fields.add_value(f.number as u32, u); + let field = m + .field_by_name(n.as_str()) + .ok_or_else(|| OptionResolverError::UnknownFieldName(n.clone()))?; + + let field_type = self.resolver.field_type(&ma, n, &field.typ)?; + + self.add_option_value_to_unknown_fields( + &field_type, + field.number as u32, + &mut unknown_fields, + v, + option_name_for_diag, + ) + .map_err(OptionResolverError::OtherError)?; } ProtobufConstantMessageFieldName::Extension(..) => { // TODO: implement extension fields in constants @@ -529,136 +662,16 @@ impl<'a> OptionResoler<'a> { } } } - Ok(UnknownValue::LengthDelimited( - unknown_fields.write_to_bytes(), - )) - } - _ => Err(OptionResolverError::MessageFieldRequiresMessageConstant.into()), - } - } - fn option_value_to_unknown_value( - &self, - field_type: &TypeResolved, - value: &model::ProtobufConstant, - option_name_for_diag: &str, - ) -> anyhow::Result { - match value { - &model::ProtobufConstant::Bool(b) => { - if field_type != &TypeResolved::Bool { - {} - } else { - return Ok(UnknownValue::Varint(if b { 1 } else { 0 })); - } - } - &model::ProtobufConstant::U64(v) => match field_type { - TypeResolved::Fixed64 => return Self::fixed64(v), - TypeResolved::Sfixed64 => return Self::sfixed64(v), - TypeResolved::Fixed32 => return Self::fixed32(v), - TypeResolved::Sfixed32 => return Self::sfixed32(v), - TypeResolved::Int32 => return Self::int32(v), - TypeResolved::Int64 => return Self::int64(v), - TypeResolved::Uint64 => return Self::uint64(v), - TypeResolved::Uint32 => return Self::uint32(v), - TypeResolved::Sint64 => return Self::sint64(v), - TypeResolved::Sint32 => return Self::sint32(v), - TypeResolved::Float => return Ok(UnknownValue::float(v as f32)), - TypeResolved::Double => return Ok(UnknownValue::double(v as f64)), - _ => {} - }, - &model::ProtobufConstant::I64(v) => match field_type { - TypeResolved::Fixed64 => return Self::fixed64(v), - TypeResolved::Sfixed64 => return Self::sfixed64(v), - TypeResolved::Fixed32 => return Self::fixed32(v), - TypeResolved::Sfixed32 => return Self::sfixed32(v), - TypeResolved::Int64 => return Self::int64(v), - TypeResolved::Int32 => return Self::int32(v), - TypeResolved::Uint64 => return Self::uint64(v), - TypeResolved::Uint32 => return Self::uint32(v), - TypeResolved::Sint64 => return Self::sint64(v), - TypeResolved::Sint32 => return Self::sint32(v), - TypeResolved::Float => return Ok(UnknownValue::float(v as f32)), - TypeResolved::Double => return Ok(UnknownValue::double(v as f64)), - _ => {} - }, - &model::ProtobufConstant::F64(f) => match field_type { - TypeResolved::Float => return Ok(UnknownValue::float(f as f32)), - TypeResolved::Double => return Ok(UnknownValue::double(f)), - TypeResolved::Fixed32 => return Ok(UnknownValue::Fixed32(f as u32)), - TypeResolved::Fixed64 => return Ok(UnknownValue::Fixed64(f as u64)), - TypeResolved::Sfixed32 => return Ok(UnknownValue::sfixed32(f as i32)), - TypeResolved::Sfixed64 => return Ok(UnknownValue::sfixed64(f as i64)), - TypeResolved::Int32 | TypeResolved::Int64 => { - return Ok(UnknownValue::int64(f as i64)) - } - TypeResolved::Uint32 | TypeResolved::Uint64 => { - return Ok(UnknownValue::Varint(f as u64)) - } - TypeResolved::Sint64 => return Ok(UnknownValue::sint64(f as i64)), - TypeResolved::Sint32 => return Ok(UnknownValue::sint32(f as i32)), - _ => {} - }, - &model::ProtobufConstant::String(ref s) => match field_type { - TypeResolved::String => { - return Ok(UnknownValue::LengthDelimited(s.decode_utf8()?.into_bytes())) - } - TypeResolved::Bytes => return Ok(UnknownValue::LengthDelimited(s.decode_bytes()?)), - _ => {} - }, - model::ProtobufConstant::Ident(ident) => match &field_type { - TypeResolved::Enum(e) => { - let e = self - .resolver - .find_enum_by_abs_name(e) - .map_err(OptionResolverError::OtherError)?; - let n = match e - .values - .iter() - .find(|v| v.name == format!("{}", ident)) - .map(|v| v.number) - { - Some(n) => n, - None => { - return Err( - OptionResolverError::UnknownEnumValue(ident.to_string()).into() - ) - } - }; - return Ok(UnknownValue::int32(n)); - } - _ => {} - }, - model::ProtobufConstant::Message(mo) => { - return self.option_value_message_to_unknown_value( - &field_type, - mo, - option_name_for_diag, + options.add_value( + field_num, + UnknownValue::LengthDelimited(unknown_fields.write_to_bytes()), ); - } - }; - - Err(match field_type { - _ => OptionResolverError::UnsupportedExtensionType( - option_name_for_diag.to_owned(), - format!("{:?}", field_type), - value.clone(), - ) - .into(), - }) - } - fn option_value_field_to_unknown_value( - &self, - scope: &ProtobufAbsPath, - value: &model::ProtobufConstant, - name: &str, - field_type: &model::FieldType, - option_name_for_diag: &str, - ) -> anyhow::Result { - let field_type = self.resolver.field_type(&scope, name, field_type)?; - Ok(self - .option_value_to_unknown_value(&field_type, value, option_name_for_diag) - .context("parsing custom option value")?) + Ok(()) + } + _ => Err(OptionResolverError::MessageFieldRequiresMessageConstant.into()), + } } fn custom_option_builtin( diff --git a/protobuf-parse/src/pure/model.rs b/protobuf-parse/src/pure/model.rs index 25a57de47..bcd59d13c 100644 --- a/protobuf-parse/src/pure/model.rs +++ b/protobuf-parse/src/pure/model.rs @@ -395,8 +395,16 @@ pub(crate) struct ProtobufConstantMessage { pub(crate) fields: IndexMap, } -/// constant = fullIdent | ( [ "-" | "+" ] intLit ) | ( [ "-" | "+" ] floatLit ) | -// strLit | boolLit +/// constant = fullIdent | +/// ( [ "-" | "+" ] intLit ) | +/// ( [ "-" | "+" ] floatLit ) | +/// strLit | +/// boolLit | +/// messageValue +/// +/// https://protobuf.dev/reference/protobuf/proto2-spec/#constant +/// https://protobuf.dev/reference/protobuf/proto3-spec/#constant +/// https://protobuf.dev/reference/protobuf/textformat-spec/#fields #[derive(Debug, Clone, PartialEq)] pub(crate) enum ProtobufConstant { U64(u64), @@ -406,6 +414,7 @@ pub(crate) enum ProtobufConstant { Ident(ProtobufPath), String(StrLit), Message(ProtobufConstantMessage), + Repeated(Vec), } impl fmt::Display for ProtobufConstant { @@ -419,6 +428,7 @@ impl fmt::Display for ProtobufConstant { ProtobufConstant::String(v) => write!(f, "{}", v), // TODO: text format explicitly ProtobufConstant::Message(v) => write!(f, "{:?}", v), + ProtobufConstant::Repeated(v) => write!(f, "{:?}", v), } } } @@ -448,6 +458,18 @@ impl ProtobufConstant { ProtobufConstant::Ident(ref i) => format!("{}", i), ProtobufConstant::String(ref s) => s.quoted(), ProtobufConstant::Message(ref s) => s.format(), + ProtobufConstant::Repeated(ref l) => { + let mut s = String::from("["); + let mut it = l.iter().peekable(); + while let Some(constant) = it.next() { + s.push_str(&constant.format()); + if it.peek().is_some() { + s.push(','); + } + } + s.push(']'); + s + } } } diff --git a/protobuf-parse/src/pure/parser.rs b/protobuf-parse/src/pure/parser.rs index a61a110b1..bad217fce 100644 --- a/protobuf-parse/src/pure/parser.rs +++ b/protobuf-parse/src/pure/parser.rs @@ -396,14 +396,38 @@ impl<'a> Parser<'a> { Ok(r) } + fn next_list_constant(&mut self) -> anyhow::Result> { + self.tokenizer.next_symbol_expect_eq('[', "list constant")?; + + let mut list = Vec::new(); + + // The list may be empty. + if self.tokenizer.next_symbol_if_eq(']')? { + return Ok(list); + } + + list.push(self.next_constant()?); + while self.tokenizer.next_symbol_if_eq(',')? { + list.push(self.next_constant()?); + } + + self.tokenizer.next_symbol_expect_eq(']', "list constant")?; + + Ok(list) + } + // constant = fullIdent | ( [ "-" | "+" ] intLit ) | ( [ "-" | "+" ] floatLit ) | - // strLit | boolLit + // strLit | boolLit | MessageValue fn next_constant(&mut self) -> anyhow::Result { // https://github.com/google/protobuf/blob/a21f225824e994ebd35e8447382ea4e0cd165b3c/src/google/protobuf/unittest_custom_options.proto#L350 if self.tokenizer.lookahead_is_symbol('{')? { return Ok(ProtobufConstant::Message(self.next_message_constant()?)); } + if self.tokenizer.lookahead_is_symbol('[')? { + return Ok(ProtobufConstant::Repeated(self.next_list_constant()?)); + } + if let Some(b) = self.next_bool_lit_opt()? { return Ok(ProtobufConstant::Bool(b)); } @@ -1369,6 +1393,18 @@ mod test { let msg = r#" (my_opt) = { my_field: "foo"} "#; let opt = parse(msg, |p| p.next_field_option()); assert_eq!(r#"{ my_field: "foo" }"#, opt.value.format()); + + let msg = r#" (my_opt) = { my_field: [] } "#; + let opt = parse(msg, |p| p.next_field_option()); + assert_eq!(r#"{ my_field: [] }"#, opt.value.format()); + + let msg = r#" (my_opt) = { my_field: ["foo", "bar"] } "#; + let opt = parse(msg, |p| p.next_field_option()); + assert_eq!(r#"{ my_field: ["foo","bar"] }"#, opt.value.format()); + + let msg = r#" (my_opt) = { my_field: [1, 2] } "#; + let opt = parse(msg, |p| p.next_field_option()); + assert_eq!(r#"{ my_field: [1,2] }"#, opt.value.format()); } #[test]