Merge remote-tracking branch 'origin/main' into feature/improve-build…

…-compatibility

Cargo.toml

@@ -1,7 +1,9 @@
 [workspace]
 members = [
-  "pbjson",
-  "pbjson-build",
-  "pbjson-test",
-  "pbjson-types",
+    "pbjson",
+    "pbjson-build",
+    "pbjson-test",
+    "pbjson-types",
 ]
+
+resolver = "2"

pbjson-build/Cargo.toml

@@ -11,8 +11,8 @@ repository = "https://github.com/influxdata/pbjson"
 
 [dependencies]
 heck = "0.4"
-prost = "0.11"
-prost-types = "0.11"
+prost = "0.12"
+prost-types = "0.12"
 itertools = "0.10"
 
 [dev-dependencies]

pbjson-build/src/descriptor.rs

@@ -12,7 +12,7 @@ use prost_types::{
     FileDescriptorProto, FileDescriptorSet, MessageOptions, OneofDescriptorProto,
 };
 
-use crate::escape::escape_ident;
+use crate::escape::{escape_ident, escape_type};
 
 #[derive(Debug, Clone, Ord, PartialOrd, Eq, PartialEq, Hash)]
 pub struct Package {
@@ -75,7 +75,7 @@ impl TypeName {
 
     pub fn to_upper_camel_case(&self) -> String {
         use heck::ToUpperCamelCase;
-        self.0.to_upper_camel_case()
+        escape_type(self.0.to_upper_camel_case())
     }
 }
 

pbjson-build/src/escape.rs

@@ -1,4 +1,4 @@
-///! Contains code to escape strings to avoid collisions with reserved Rust keywords
+//! Contains code to escape strings to avoid collisions with reserved Rust keywords
 
 pub fn escape_ident(mut ident: String) -> String {
     // Copied from prost-build::ident
@@ -24,3 +24,11 @@ pub fn escape_ident(mut ident: String) -> String {
     };
     ident
 }
+
+pub fn escape_type(mut ident: String) -> String {
+    // this keyword is not supported as a raw identifier and is therefore suffixed with an underscore.
+    if ident == "Self" {
+        ident += "_";
+    }
+    ident
+}

pbjson-build/src/generator/enumeration.rs

@@ -11,6 +11,7 @@ use super::{
 use crate::descriptor::{EnumDescriptor, TypePath};
 use crate::generator::write_fields_array;
 use crate::resolver::Resolver;
+use std::collections::HashSet;
 use std::io::{Result, Write};
 
 pub fn generate_enum<W: Write>(
@@ -22,9 +23,13 @@ pub fn generate_enum<W: Write>(
 ) -> Result<()> {
     let rust_type = resolver.rust_type(path);
 
+    let mut seen_numbers = HashSet::new();
     let variants: Vec<_> = descriptor
         .values
         .iter()
+        // Skip duplicates if we've seen the number before
+        // Protobuf's `allow_alias` option permits duplicates if set
+        .filter(|variant| seen_numbers.insert(variant.number()))
         .map(|variant| {
             let variant_name = variant.name.clone().unwrap();
             let variant_number = variant.number();

pbjson-build/src/generator/message.rs

@@ -30,6 +30,7 @@ use super::{
     Indent,
 };
 use crate::descriptor::TypePath;
+use crate::escape::escape_type;
 use crate::generator::write_fields_array;
 use crate::resolver::Resolver;
 
@@ -374,6 +375,11 @@ fn write_serialize_scalar_variable<W: Write>(
             )
         }
         _ => {
+            writeln!(
+                writer,
+                "{}#[allow(clippy::needless_borrow)]",
+                Indent(indent)
+            )?;
             writeln!(
                 writer,
                 "{}struct_ser.serialize_field(\"{}\", {}(&{}).as_str())?;",
@@ -517,7 +523,7 @@ fn write_deserialize_message<W: Write>(
 {indent}        formatter.write_str("struct {name}")
 {indent}    }}
 
-{indent}    fn visit_map<V>(self, mut map: V) -> std::result::Result<{rust_type}, V::Error>
+{indent}    fn visit_map<V>(self, mut map_: V) -> std::result::Result<{rust_type}, V::Error>
 {indent}        where
 {indent}            V: serde::de::MapAccess<'de>,
 {indent}    {{"#,
@@ -547,7 +553,7 @@ fn write_deserialize_message<W: Write>(
     if !message.fields.is_empty() || !message.one_ofs.is_empty() {
         writeln!(
             writer,
-            "{}while let Some(k) = map.next_key()? {{",
+            "{}while let Some(k) = map_.next_key()? {{",
             Indent(indent + 2)
         )?;
 
@@ -582,7 +588,7 @@ fn write_deserialize_message<W: Write>(
             )?;
             writeln!(
                 writer,
-                "{}let _ = map.next_value::<serde::de::IgnoredAny>()?;",
+                "{}let _ = map_.next_value::<serde::de::IgnoredAny>()?;",
                 Indent(indent + 5),
             )?;
             writeln!(writer, "{}}}", Indent(indent + 4))?;
@@ -593,12 +599,12 @@ fn write_deserialize_message<W: Write>(
     } else {
         writeln!(
             writer,
-            "{}while map.next_key::<GeneratedField>()?.is_some() {{",
+            "{}while map_.next_key::<GeneratedField>()?.is_some() {{",
             Indent(indent + 2)
         )?;
         writeln!(
             writer,
-            "{}let _ = map.next_value::<serde::de::IgnoredAny>()?;",
+            "{}let _ = map_.next_value::<serde::de::IgnoredAny>()?;",
             Indent(indent + 3)
         )?;
         writeln!(writer, "{}}}", Indent(indent + 2))?;
@@ -720,15 +726,15 @@ fn write_deserialize_field_name<W: Write>(
                     Indent(indent + 5),
                     json_name,
                     proto_name,
-                    type_name
+                    escape_type(type_name.to_string())
                 )?;
             } else {
                 writeln!(
                     writer,
                     "{}\"{}\" => Ok(GeneratedField::{}),",
                     Indent(indent + 5),
                     json_name,
-                    type_name
+                    escape_type(type_name.to_string())
                 )?;
             }
         }
@@ -784,7 +790,12 @@ fn write_fields_enum<'a, W: Write, I: Iterator<Item = &'a str>>(
     )?;
     writeln!(writer, "{}enum GeneratedField {{", Indent(indent))?;
     for type_name in fields {
-        writeln!(writer, "{}{},", Indent(indent + 1), type_name)?;
+        writeln!(
+            writer,
+            "{}{},",
+            Indent(indent + 1),
+            escape_type(type_name.to_string())
+        )?;
     }
 
     if ignore_unknown_fields {
@@ -837,7 +848,7 @@ fn write_deserialize_field<W: Write>(
                 Some(deserializer) => {
                     write!(
                         writer,
-                        "map.next_value::<::std::option::Option<{}>>()?.map(|x| {}::{}(x.0))",
+                        "map_.next_value::<::std::option::Option<{}>>()?.map(|x| {}::{}(x.0))",
                         deserializer,
                         resolver.rust_type(&one_of.path),
                         field.rust_type_name()
@@ -846,7 +857,7 @@ fn write_deserialize_field<W: Write>(
                 None => {
                     write!(
                         writer,
-                        "map.next_value::<::std::option::Option<_>>()?.map({}::{})",
+                        "map_.next_value::<::std::option::Option<_>>()?.map({}::{})",
                         resolver.rust_type(&one_of.path),
                         field.rust_type_name()
                     )?;
@@ -855,15 +866,15 @@ fn write_deserialize_field<W: Write>(
             FieldType::Enum(path) => {
                 write!(
                     writer,
-                    "map.next_value::<::std::option::Option<{}>>()?.map(|x| {}::{}(x as i32))",
+                    "map_.next_value::<::std::option::Option<{}>>()?.map(|x| {}::{}(x as i32))",
                     resolver.rust_type(path),
                     resolver.rust_type(&one_of.path),
                     field.rust_type_name()
                 )?;
             }
             FieldType::Message(_) => writeln!(
                 writer,
-                "map.next_value::<::std::option::Option<_>>()?.map({}::{})",
+                "map_.next_value::<::std::option::Option<_>>()?.map({}::{})",
                 resolver.rust_type(&one_of.path),
                 field.rust_type_name()
             )?,
@@ -878,21 +889,21 @@ fn write_deserialize_field<W: Write>(
                 FieldModifier::Optional => {
                     write!(
                         writer,
-                        "map.next_value::<::std::option::Option<{}>>()?.map(|x| x as i32)",
+                        "map_.next_value::<::std::option::Option<{}>>()?.map(|x| x as i32)",
                         resolver.rust_type(path)
                     )?;
                 }
                 FieldModifier::Repeated => {
                     write!(
                         writer,
-                        "Some(map.next_value::<Vec<{}>>()?.into_iter().map(|x| x as i32).collect())",
+                        "Some(map_.next_value::<Vec<{}>>()?.into_iter().map(|x| x as i32).collect())",
                         resolver.rust_type(path)
                     )?;
                 }
                 _ => {
                     write!(
                         writer,
-                        "Some(map.next_value::<{}>()? as i32)",
+                        "Some(map_.next_value::<{}>()? as i32)",
                         resolver.rust_type(path)
                     )?;
                 }
@@ -903,12 +914,12 @@ fn write_deserialize_field<W: Write>(
                 match btree_map {
                     true => write!(
                         writer,
-                        "{}map.next_value::<std::collections::BTreeMap<",
+                        "{}map_.next_value::<std::collections::BTreeMap<",
                         Indent(indent + 2),
                     )?,
                     false => write!(
                         writer,
-                        "{}map.next_value::<std::collections::HashMap<",
+                        "{}map_.next_value::<std::collections::HashMap<",
                         Indent(indent + 2),
                     )?,
                 }
@@ -970,9 +981,9 @@ fn write_deserialize_field<W: Write>(
             FieldType::Message(_) => match field.field_modifier {
                 FieldModifier::Repeated => {
                     // No explicit presence for repeated fields
-                    write!(writer, "Some(map.next_value()?)")?;
+                    write!(writer, "Some(map_.next_value()?)")?;
                 }
-                _ => write!(writer, "map.next_value()?")?,
+                _ => write!(writer, "map_.next_value()?")?,
             },
         },
     }
@@ -999,9 +1010,9 @@ fn write_encode_scalar_field<W: Write>(
         None => {
             return match field_modifier {
                 FieldModifier::Optional => {
-                    write!(writer, "map.next_value()?")
+                    write!(writer, "map_.next_value()?")
                 }
-                _ => write!(writer, "Some(map.next_value()?)"),
+                _ => write!(writer, "Some(map_.next_value()?)"),
             };
         }
     };
@@ -1012,15 +1023,15 @@ fn write_encode_scalar_field<W: Write>(
         FieldModifier::Optional => {
             writeln!(
                 writer,
-                "{}map.next_value::<::std::option::Option<{}>>()?.map(|x| x.0)",
+                "{}map_.next_value::<::std::option::Option<{}>>()?.map(|x| x.0)",
                 Indent(indent + 1),
                 deserializer
             )?;
         }
         FieldModifier::Repeated => {
             writeln!(
                 writer,
-                "{}Some(map.next_value::<Vec<{}>>()?",
+                "{}Some(map_.next_value::<Vec<{}>>()?",
                 Indent(indent + 1),
                 deserializer
             )?;
@@ -1033,7 +1044,7 @@ fn write_encode_scalar_field<W: Write>(
         _ => {
             writeln!(
                 writer,
-                "{}Some(map.next_value::<{}>()?.0)",
+                "{}Some(map_.next_value::<{}>()?.0)",
                 Indent(indent + 1),
                 deserializer
             )?;

pbjson-build/src/message.rs

@@ -10,7 +10,7 @@ use prost_types::{
 };
 
 use crate::descriptor::{Descriptor, DescriptorSet, MessageDescriptor, Syntax, TypeName, TypePath};
-use crate::escape::escape_ident;
+use crate::escape::{escape_ident, escape_type};
 
 #[derive(Debug, Clone, Copy)]
 pub enum ScalarType {
@@ -81,7 +81,7 @@ pub struct Field {
 impl Field {
     pub fn rust_type_name(&self) -> String {
         use heck::ToUpperCamelCase;
-        self.name.to_upper_camel_case()
+        escape_type(self.name.to_upper_camel_case())
     }
 
     pub fn rust_field_name(&self) -> String {
@@ -183,7 +183,7 @@ fn field_modifier(
     field: &FieldDescriptorProto,
     field_type: &FieldType,
 ) -> FieldModifier {
-    let label = Label::from_i32(field.label.expect("expected label")).expect("valid label");
+    let label = Label::try_from(field.label.expect("expected label")).expect("valid label");
     if field.proto3_optional.unwrap_or(false) {
         assert_eq!(label, Label::Optional);
         return FieldModifier::Optional;
@@ -217,7 +217,7 @@ fn field_type(descriptors: &DescriptorSet, field: &FieldDescriptorProto) -> Fiel
         Some(type_name) => resolve_type(descriptors, type_name.as_str()),
         None => {
             let scalar =
-                match Type::from_i32(field.r#type.expect("expected type")).expect("valid type") {
+                match Type::try_from(field.r#type.expect("expected type")).expect("valid type") {
                     Type::Double => ScalarType::F64,
                     Type::Float => ScalarType::F32,
                     Type::Int64 | Type::Sfixed64 | Type::Sint64 => ScalarType::I64,

pbjson-test/Cargo.toml

@@ -7,7 +7,7 @@ description = "Test resources for pbjson converion"
 repository = "https://github.com/influxdata/pbjson"
 
 [dependencies]
-prost = "0.11"
+prost = "0.12"
 pbjson = { path = "../pbjson" }
 pbjson-types = { path = "../pbjson-types" }
 serde = { version = "1.0", features = ["derive"] }
@@ -24,5 +24,5 @@ chrono = "0.4"
 serde_json = "1.0"
 
 [build-dependencies]
-prost-build = "0.11"
+prost-build = "0.12"
 pbjson-build = { path = "../pbjson-build" }

pbjson-test/build.rs

@@ -13,6 +13,7 @@ fn main() -> Result<()> {
         root.join("syntax3.proto"),
         root.join("common.proto"),
         root.join("duplicate_name.proto"),
+        root.join("duplicate_number.proto"),
         root.join("escape.proto"),
     ];
 
@@ -28,7 +29,7 @@ fn main() -> Result<()> {
         .compile_well_known_types()
         .extern_path(".google.protobuf", "::pbjson_types")
         .extern_path(".test.external", "crate")
-        .bytes(&[".test"])
+        .bytes([".test"])
         .protoc_arg("--experimental_allow_proto3_optional");
 
     if cfg!(feature = "btree") {

pbjson-test/protos/duplicate_number.proto

@@ -0,0 +1,16 @@
+syntax = "proto3";
+
+package test.duplicate_number;
+
+message Compressor {
+    enum CompressionLevel {
+        option allow_alias = true;
+
+        DEFAULT = 0;
+        BEST_SPEED = 1;
+        COMPRESSION_LEVEL_1 = 1;
+        COMPRESSION_LEVEL_2 = 2;
+        COMPRESSION_LEVEL_3 = 3;
+        BEST_COMPRESSION = 3;
+    }
+}