From 7f5a141180878eaea7ae0a00f3f1f194ff7784ee Mon Sep 17 00:00:00 2001 From: "Victor M. Alvarez" Date: Fri, 16 Feb 2024 17:14:24 +0100 Subject: [PATCH] Add support for `reserved` keyword in enums The `reserved` keyword was supported for messages but not for enums. Also, used this opportunity to remove the `FieldNumberRange` type and use `RangeInclusive` instead, as proposed by a `TODO` comment in the existing code. --- protobuf-parse/src/pure/convert/mod.rs | 21 ++++++++-- protobuf-parse/src/pure/model.rs | 20 ++++------ protobuf-parse/src/pure/parser.rs | 55 ++++++++++++++++++-------- 3 files changed, 63 insertions(+), 33 deletions(-) diff --git a/protobuf-parse/src/pure/convert/mod.rs b/protobuf-parse/src/pure/convert/mod.rs index c98fcf554..cae7fc06e 100644 --- a/protobuf-parse/src/pure/convert/mod.rs +++ b/protobuf-parse/src/pure/convert/mod.rs @@ -5,6 +5,7 @@ mod type_resolver; use protobuf; use protobuf::descriptor::descriptor_proto::ReservedRange; +use protobuf::descriptor::enum_descriptor_proto::EnumReservedRange; use protobuf::descriptor::field_descriptor_proto; use protobuf::descriptor::field_descriptor_proto::Type; use protobuf::descriptor::FieldDescriptorProto; @@ -296,8 +297,8 @@ impl<'a> Resolver<'a> { for ext in &input.extension_ranges { let mut extension_range = protobuf::descriptor::descriptor_proto::ExtensionRange::new(); - extension_range.set_start(ext.from); - extension_range.set_end(ext.to + 1); + extension_range.set_start(*ext.start()); + extension_range.set_end(*ext.end() + 1); output.extension_range.push(extension_range); } for ext in &input.extensions { @@ -313,8 +314,8 @@ impl<'a> Resolver<'a> { for reserved in &input.reserved_nums { let mut reserved_range = ReservedRange::new(); - reserved_range.set_start(reserved.from); - reserved_range.set_end(reserved.to + 1); + reserved_range.set_start(*reserved.start()); + reserved_range.set_end(*reserved.end() + 1); output.reserved_range.push(reserved_range); } output.reserved_name = input.reserved_names.clone().into(); @@ -526,6 +527,18 @@ impl<'a> Resolver<'a> { .iter() .map(|v| self.enum_value(scope, &v)) .collect::>()?; + + for reserved in &input.reserved_nums { + let mut reserved_range = EnumReservedRange::new(); + reserved_range.set_start(*reserved.start()); + // EnumReservedRange is inclusive, not like ExtensionRange and + // ReservedRange, which are exclusive. + reserved_range.set_end(*reserved.end()); + output.reserved_range.push(reserved_range); + } + + output.reserved_name = input.reserved_names.clone().into(); + Ok(output) } diff --git a/protobuf-parse/src/pure/model.rs b/protobuf-parse/src/pure/model.rs index 3efdc1136..8e2cb5fa9 100644 --- a/protobuf-parse/src/pure/model.rs +++ b/protobuf-parse/src/pure/model.rs @@ -6,6 +6,7 @@ use std::fmt; use std::fmt::Write; use std::ops::Deref; +use std::ops::RangeInclusive; use indexmap::IndexMap; use protobuf::reflect::ReflectValueBox; @@ -213,15 +214,6 @@ pub(crate) enum FieldOrOneOf { OneOf(OneOf), } -/// Extension range -#[derive(Default, Debug, Eq, PartialEq, Copy, Clone)] -pub(crate) struct FieldNumberRange { - /// First number - pub from: i32, - /// Inclusive - pub to: i32, -} - /// A protobuf message #[derive(Debug, Clone, Default)] pub(crate) struct Message { @@ -230,9 +222,7 @@ pub(crate) struct Message { /// Message fields and oneofs pub fields: Vec>, /// Message reserved numbers - /// - /// TODO: use RangeInclusive once stable - pub reserved_nums: Vec, + pub reserved_nums: Vec>, /// Message reserved names pub reserved_names: Vec, /// Nested messages @@ -242,7 +232,7 @@ pub(crate) struct Message { /// Non-builtin options pub options: Vec, /// Extension field numbers - pub extension_ranges: Vec, + pub extension_ranges: Vec>, /// Extensions pub extensions: Vec>, } @@ -318,6 +308,10 @@ pub(crate) struct Enumeration { pub values: Vec, /// enum options pub options: Vec, + /// enum reserved numbers + pub reserved_nums: Vec>, + /// enum reserved names + pub reserved_names: Vec, } /// A OneOf diff --git a/protobuf-parse/src/pure/parser.rs b/protobuf-parse/src/pure/parser.rs index 1c40f406f..27cdb6fec 100644 --- a/protobuf-parse/src/pure/parser.rs +++ b/protobuf-parse/src/pure/parser.rs @@ -1,3 +1,4 @@ +use std::ops::RangeInclusive; use std::str; use protobuf_support::lexer::int; @@ -21,7 +22,6 @@ use crate::pure::model::EnumValue; use crate::pure::model::Enumeration; use crate::pure::model::Extension; use crate::pure::model::Field; -use crate::pure::model::FieldNumberRange; use crate::pure::model::FieldOrOneOf; use crate::pure::model::FieldType; use crate::pure::model::FileDescriptor; @@ -264,12 +264,12 @@ impl MessageBodyParseMode { #[derive(Default)] pub(crate) struct MessageBody { pub fields: Vec>, - pub reserved_nums: Vec, + pub reserved_nums: Vec>, pub reserved_names: Vec, pub messages: Vec>, pub enums: Vec>, pub options: Vec, - pub extension_ranges: Vec, + pub extension_ranges: Vec>, pub extensions: Vec>, } @@ -775,7 +775,7 @@ impl<'a> Parser<'a> { // Extensions // range = intLit [ "to" ( intLit | "max" ) ] - fn next_range(&mut self) -> anyhow::Result { + fn next_range(&mut self) -> anyhow::Result> { let from = self.next_field_number()?; let to = if self.tokenizer.next_ident_if_eq("to")? { if self.tokenizer.next_ident_if_eq("max")? { @@ -786,11 +786,11 @@ impl<'a> Parser<'a> { } else { from }; - Ok(FieldNumberRange { from, to }) + Ok(from..=to) } // ranges = range { "," range } - fn next_ranges(&mut self) -> anyhow::Result> { + fn next_ranges(&mut self) -> anyhow::Result>> { let mut ranges = Vec::new(); ranges.push(self.next_range()?); while self.tokenizer.next_symbol_if_eq(',')? { @@ -800,7 +800,7 @@ impl<'a> Parser<'a> { } // extensions = "extensions" ranges ";" - fn next_extensions_opt(&mut self) -> anyhow::Result>> { + fn next_extensions_opt(&mut self) -> anyhow::Result>>> { if self.tokenizer.next_ident_if_eq("extensions")? { Ok(Some(self.next_ranges()?)) } else { @@ -815,7 +815,7 @@ impl<'a> Parser<'a> { // fieldNames = fieldName { "," fieldName } fn next_reserved_opt( &mut self, - ) -> anyhow::Result, Vec)>> { + ) -> anyhow::Result>, Vec)>> { if self.tokenizer.next_ident_if_eq("reserved")? { let (ranges, names) = if let &Token::StrLit(..) = self.tokenizer.lookahead_some()? { let mut names = Vec::new(); @@ -886,7 +886,7 @@ impl<'a> Parser<'a> { } // enum = "enum" enumName enumBody - // enumBody = "{" { option | enumField | emptyStatement } "}" + // enumBody = "{" { option | enumField | emptyStatement | reserved } "}" fn next_enum_opt(&mut self) -> anyhow::Result>> { let loc = self.tokenizer.lookahead_loc(); @@ -895,6 +895,8 @@ impl<'a> Parser<'a> { let mut values = Vec::new(); let mut options = Vec::new(); + let mut reserved_nums = Vec::new(); + let mut reserved_names = Vec::new(); self.tokenizer.next_symbol_expect_eq('{', "enum")?; while self.tokenizer.lookahead_if_symbol()? != Some('}') { @@ -903,6 +905,12 @@ impl<'a> Parser<'a> { continue; } + if let Some((field_nums, field_names)) = self.next_reserved_opt()? { + reserved_nums.extend(field_nums); + reserved_names.extend(field_names); + continue; + } + if let Some(o) = self.next_option_opt()? { options.push(o); continue; @@ -915,6 +923,8 @@ impl<'a> Parser<'a> { name, values, options, + reserved_nums, + reserved_names, }; Ok(Some(WithLoc { loc, @@ -1470,7 +1480,7 @@ mod test { } #[test] - fn test_reserved() { + fn test_reserved_in_message() { let msg = r#"message Sample { reserved 4, 15, 17 to 20, 30; reserved "foo", "bar"; @@ -1480,12 +1490,7 @@ mod test { let mess = parse_opt(msg, |p| p.next_message_opt()); assert_eq!( - vec![ - FieldNumberRange { from: 4, to: 4 }, - FieldNumberRange { from: 15, to: 15 }, - FieldNumberRange { from: 17, to: 20 }, - FieldNumberRange { from: 30, to: 30 } - ], + vec![4..=4, 15..=15, 17..=20, 30..=30,], mess.t.reserved_nums ); assert_eq!( @@ -1495,6 +1500,24 @@ mod test { assert_eq!(2, mess.t.fields.len()); } + #[test] + fn test_reserved_in_enum() { + let msg = r#"enum Sample { + reserved 4, 15, 17 to 20, 30; + reserved "foo", "bar"; + }"#; + + let enum_ = parse_opt(msg, |p| p.next_enum_opt()); + assert_eq!( + vec![4..=4, 15..=15, 17..=20, 30..=30,], + enum_.t.reserved_nums + ); + assert_eq!( + vec!["foo".to_string(), "bar".to_string()], + enum_.t.reserved_names + ); + } + #[test] fn test_default_value_int() { let msg = r#"message Sample {