From 3fc5ad76c43065360983ebf161f54b4905172ce9 Mon Sep 17 00:00:00 2001 From: "Victor M. Alvarez" Date: Fri, 16 Feb 2024 17:14:24 +0100 Subject: [PATCH 1/2] 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 | 62 ++++++++++++++++++++------ 3 files changed, 72 insertions(+), 31 deletions(-) diff --git a/protobuf-parse/src/pure/convert/mod.rs b/protobuf-parse/src/pure/convert/mod.rs index c98fcf554..3e4fdc22e 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..64ea644fc 100644 --- a/protobuf-parse/src/pure/model.rs +++ b/protobuf-parse/src/pure/model.rs @@ -5,7 +5,7 @@ use std::fmt; use std::fmt::Write; -use std::ops::Deref; +use std::ops::{Deref, RangeInclusive}; use indexmap::IndexMap; use protobuf::reflect::ReflectValueBox; @@ -213,14 +213,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)] @@ -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..4f848fd79 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,9 @@ 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 +906,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 +924,8 @@ impl<'a> Parser<'a> { name, values, options, + reserved_nums, + reserved_names }; Ok(Some(WithLoc { loc, @@ -1470,7 +1481,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"; @@ -1481,10 +1492,10 @@ 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 } + 4..=4, + 15..=15, + 17..=20, + 30..=30, ], mess.t.reserved_nums ); @@ -1495,6 +1506,29 @@ 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 { From b484d8a7025db760c4fcf6994bda39f73452c6d7 Mon Sep 17 00:00:00 2001 From: "Victor M. Alvarez" Date: Fri, 16 Feb 2024 17:28:53 +0100 Subject: [PATCH 2/2] Fix formatting issues. --- protobuf-parse/src/pure/convert/mod.rs | 8 ++++---- protobuf-parse/src/pure/model.rs | 1 - protobuf-parse/src/pure/parser.rs | 21 +++++---------------- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/protobuf-parse/src/pure/convert/mod.rs b/protobuf-parse/src/pure/convert/mod.rs index 3e4fdc22e..cae7fc06e 100644 --- a/protobuf-parse/src/pure/convert/mod.rs +++ b/protobuf-parse/src/pure/convert/mod.rs @@ -527,7 +527,7 @@ 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()); @@ -536,9 +536,9 @@ impl<'a> Resolver<'a> { reserved_range.set_end(*reserved.end()); output.reserved_range.push(reserved_range); } - - output.reserved_name = input.reserved_names.clone().into(); - + + 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 64ea644fc..9fbfad3aa 100644 --- a/protobuf-parse/src/pure/model.rs +++ b/protobuf-parse/src/pure/model.rs @@ -213,7 +213,6 @@ pub(crate) enum FieldOrOneOf { OneOf(OneOf), } - /// A protobuf message #[derive(Debug, Clone, Default)] pub(crate) struct Message { diff --git a/protobuf-parse/src/pure/parser.rs b/protobuf-parse/src/pure/parser.rs index 4f848fd79..27cdb6fec 100644 --- a/protobuf-parse/src/pure/parser.rs +++ b/protobuf-parse/src/pure/parser.rs @@ -1,4 +1,4 @@ -use std::ops::{RangeInclusive}; +use std::ops::RangeInclusive; use std::str; use protobuf_support::lexer::int; @@ -898,7 +898,6 @@ impl<'a> Parser<'a> { 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('}') { // emptyStatement @@ -911,7 +910,7 @@ impl<'a> Parser<'a> { reserved_names.extend(field_names); continue; } - + if let Some(o) = self.next_option_opt()? { options.push(o); continue; @@ -925,7 +924,7 @@ impl<'a> Parser<'a> { values, options, reserved_nums, - reserved_names + reserved_names, }; Ok(Some(WithLoc { loc, @@ -1491,12 +1490,7 @@ mod test { let mess = parse_opt(msg, |p| p.next_message_opt()); assert_eq!( - vec![ - 4..=4, - 15..=15, - 17..=20, - 30..=30, - ], + vec![4..=4, 15..=15, 17..=20, 30..=30,], mess.t.reserved_nums ); assert_eq!( @@ -1515,12 +1509,7 @@ mod test { let enum_ = parse_opt(msg, |p| p.next_enum_opt()); assert_eq!( - vec![ - 4..=4, - 15..=15, - 17..=20, - 30..=30, - ], + vec![4..=4, 15..=15, 17..=20, 30..=30,], enum_.t.reserved_nums ); assert_eq!(