Skip to content

Commit

Permalink
Add support for reserved keyword in enums
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
plusvic authored and stepancheg committed Feb 25, 2024
1 parent 78ea5ef commit 42c8c79
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 33 deletions.
21 changes: 17 additions & 4 deletions protobuf-parse/src/pure/convert/mod.rs
Expand Up @@ -5,6 +5,7 @@ mod type_resolver;

use protobuf;

Check warning on line 6 in protobuf-parse/src/pure/convert/mod.rs

View workflow job for this annotation

GitHub Actions / linux nightly (all features)

the item `protobuf` is imported redundantly
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;
Expand Down Expand Up @@ -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 {
Expand All @@ -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();
Expand Down Expand Up @@ -526,6 +527,18 @@ impl<'a> Resolver<'a> {
.iter()
.map(|v| self.enum_value(scope, &v))
.collect::<Result<_, _>>()?;

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)
}

Expand Down
20 changes: 7 additions & 13 deletions protobuf-parse/src/pure/model.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand All @@ -230,9 +222,7 @@ pub(crate) struct Message {
/// Message fields and oneofs
pub fields: Vec<WithLoc<FieldOrOneOf>>,
/// Message reserved numbers
///
/// TODO: use RangeInclusive once stable
pub reserved_nums: Vec<FieldNumberRange>,
pub reserved_nums: Vec<RangeInclusive<i32>>,
/// Message reserved names
pub reserved_names: Vec<String>,
/// Nested messages
Expand All @@ -242,7 +232,7 @@ pub(crate) struct Message {
/// Non-builtin options
pub options: Vec<ProtobufOption>,
/// Extension field numbers
pub extension_ranges: Vec<FieldNumberRange>,
pub extension_ranges: Vec<RangeInclusive<i32>>,
/// Extensions
pub extensions: Vec<WithLoc<Extension>>,
}
Expand Down Expand Up @@ -318,6 +308,10 @@ pub(crate) struct Enumeration {
pub values: Vec<EnumValue>,
/// enum options
pub options: Vec<ProtobufOption>,
/// enum reserved numbers
pub reserved_nums: Vec<RangeInclusive<i32>>,
/// enum reserved names
pub reserved_names: Vec<String>,
}

/// A OneOf
Expand Down
55 changes: 39 additions & 16 deletions protobuf-parse/src/pure/parser.rs
@@ -1,3 +1,4 @@
use std::ops::RangeInclusive;
use std::str;

use protobuf_support::lexer::int;
Expand All @@ -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;
Expand Down Expand Up @@ -264,12 +264,12 @@ impl MessageBodyParseMode {
#[derive(Default)]
pub(crate) struct MessageBody {
pub fields: Vec<WithLoc<FieldOrOneOf>>,
pub reserved_nums: Vec<FieldNumberRange>,
pub reserved_nums: Vec<RangeInclusive<i32>>,
pub reserved_names: Vec<String>,
pub messages: Vec<WithLoc<Message>>,
pub enums: Vec<WithLoc<Enumeration>>,
pub options: Vec<ProtobufOption>,
pub extension_ranges: Vec<FieldNumberRange>,
pub extension_ranges: Vec<RangeInclusive<i32>>,
pub extensions: Vec<WithLoc<Extension>>,
}

Expand Down Expand Up @@ -775,7 +775,7 @@ impl<'a> Parser<'a> {
// Extensions

// range = intLit [ "to" ( intLit | "max" ) ]
fn next_range(&mut self) -> anyhow::Result<FieldNumberRange> {
fn next_range(&mut self) -> anyhow::Result<RangeInclusive<i32>> {
let from = self.next_field_number()?;
let to = if self.tokenizer.next_ident_if_eq("to")? {
if self.tokenizer.next_ident_if_eq("max")? {
Expand All @@ -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<Vec<FieldNumberRange>> {
fn next_ranges(&mut self) -> anyhow::Result<Vec<RangeInclusive<i32>>> {
let mut ranges = Vec::new();
ranges.push(self.next_range()?);
while self.tokenizer.next_symbol_if_eq(',')? {
Expand All @@ -800,7 +800,7 @@ impl<'a> Parser<'a> {
}

// extensions = "extensions" ranges ";"
fn next_extensions_opt(&mut self) -> anyhow::Result<Option<Vec<FieldNumberRange>>> {
fn next_extensions_opt(&mut self) -> anyhow::Result<Option<Vec<RangeInclusive<i32>>>> {
if self.tokenizer.next_ident_if_eq("extensions")? {
Ok(Some(self.next_ranges()?))
} else {
Expand All @@ -815,7 +815,7 @@ impl<'a> Parser<'a> {
// fieldNames = fieldName { "," fieldName }
fn next_reserved_opt(
&mut self,
) -> anyhow::Result<Option<(Vec<FieldNumberRange>, Vec<String>)>> {
) -> anyhow::Result<Option<(Vec<RangeInclusive<i32>>, Vec<String>)>> {
if self.tokenizer.next_ident_if_eq("reserved")? {
let (ranges, names) = if let &Token::StrLit(..) = self.tokenizer.lookahead_some()? {
let mut names = Vec::new();
Expand Down Expand Up @@ -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<Option<WithLoc<Enumeration>>> {
let loc = self.tokenizer.lookahead_loc();

Expand All @@ -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('}') {
Expand All @@ -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;
Expand All @@ -915,6 +923,8 @@ impl<'a> Parser<'a> {
name,
values,
options,
reserved_nums,
reserved_names,
};
Ok(Some(WithLoc {
loc,
Expand Down Expand Up @@ -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";
Expand All @@ -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!(
Expand All @@ -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 {
Expand Down

0 comments on commit 42c8c79

Please sign in to comment.