From cb34daac6db647e6e40e01c648138d59861fc0cb Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Sun, 16 Jan 2022 21:04:24 -0800 Subject: [PATCH 1/6] pbjson-build/generator: escape idents in GeneratedMessage --- pbjson-build/src/escape.rs | 9 +++++++++ pbjson-build/src/generator/message.rs | 10 ++++++++-- pbjson-build/src/message.rs | 4 ++-- pbjson-build/src/resolver.rs | 5 +++-- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/pbjson-build/src/escape.rs b/pbjson-build/src/escape.rs index 554ac85..4d5031e 100644 --- a/pbjson-build/src/escape.rs +++ b/pbjson-build/src/escape.rs @@ -24,3 +24,12 @@ pub fn escape_ident(mut ident: String) -> String { }; ident } + +/// Converts a `snake_case` identifier to an `UpperCamel` case Rust type identifier. +pub fn escape_camel_case(mut ident: String) -> String { + // Suffix an underscore for the `Self` Rust keyword as it is not allowed as raw identifier. + if ident == "Self" { + ident += "_"; + } + ident +} diff --git a/pbjson-build/src/generator/message.rs b/pbjson-build/src/generator/message.rs index 795018f..a9bf9ea 100644 --- a/pbjson-build/src/generator/message.rs +++ b/pbjson-build/src/generator/message.rs @@ -30,6 +30,7 @@ use super::{ Indent, }; use crate::descriptor::TypePath; +use crate::escape::escape_camel_case; use crate::generator::write_fields_array; use crate::resolver::Resolver; @@ -591,7 +592,7 @@ fn write_deserialize_field_name( "{}\"{}\" => Ok(GeneratedField::{}),", Indent(indent + 5), json_name, - type_name + escape_camel_case(type_name.to_string()), )?; } writeln!( @@ -631,7 +632,12 @@ fn write_fields_enum<'a, W: Write, I: Iterator>( )?; writeln!(writer, "{}enum GeneratedField {{", Indent(indent))?; for type_name in fields { - writeln!(writer, "{}{},", Indent(indent + 1), type_name)?; + writeln!( + writer, + "{}{},", + Indent(indent + 1), + escape_camel_case(type_name.to_string()) + )?; } writeln!(writer, "{}}}", Indent(indent)) } diff --git a/pbjson-build/src/message.rs b/pbjson-build/src/message.rs index 7c3e5bc..430418c 100644 --- a/pbjson-build/src/message.rs +++ b/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_camel_case, escape_ident}; #[derive(Debug, Clone, Copy)] pub enum ScalarType { @@ -86,7 +86,7 @@ pub struct Field { impl Field { pub fn rust_type_name(&self) -> String { use heck::CamelCase; - self.name.to_camel_case() + escape_camel_case(self.name.to_camel_case()) } pub fn rust_field_name(&self) -> String { diff --git a/pbjson-build/src/resolver.rs b/pbjson-build/src/resolver.rs index 24152b7..3e82fdc 100644 --- a/pbjson-build/src/resolver.rs +++ b/pbjson-build/src/resolver.rs @@ -1,4 +1,5 @@ use crate::descriptor::{Package, TypePath}; +use crate::escape::{escape_camel_case, escape_ident}; #[derive(Debug)] pub struct Resolver<'a> { @@ -75,11 +76,11 @@ impl<'a> Resolver<'a> { while let Some(i) = iter.next() { match iter.peek() { Some(_) => { - ret.push_str(i.to_snake_case().as_str()); + ret.push_str(escape_ident(i.to_snake_case()).as_str()); ret.push_str("::"); } None => { - ret.push_str(i.to_camel_case().as_str()); + ret.push_str(escape_camel_case(i.to_camel_case()).as_str()); } } } From 1d220c81fa626fff708b78eaaf182fe77928e1d8 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Sun, 16 Jan 2022 22:24:52 -0800 Subject: [PATCH 2/6] fix resolver to use take_while rather than filter --- pbjson-build/src/resolver.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pbjson-build/src/resolver.rs b/pbjson-build/src/resolver.rs index 3e82fdc..58f233b 100644 --- a/pbjson-build/src/resolver.rs +++ b/pbjson-build/src/resolver.rs @@ -58,7 +58,7 @@ impl<'a> Resolver<'a> { .path() .iter() .zip(path.path()) - .filter(|(a, b)| a == b) + .take_while(|(a, b)| a == b) .count(); let super_count = self.package.path().len() - shared_prefix; @@ -111,6 +111,16 @@ mod tests { use super::*; use crate::descriptor::TypeName; + #[test] + fn test_complicated_resolver() { + let resolver_package = Package::new("envoy.service.health.v3"); + let resolver = Resolver::new(&[], &resolver_package, false); + + let to_resolve = TypePath::new(Package::new("envoy.config.core.v3")).child(TypeName::new("HealthStatus")); + + assert_eq!("super::super::super::config::core::v3::HealthStatus", resolver.rust_type(&to_resolve)); + } + #[test] fn test_resolver() { let resolver_package = Package::new("test.syntax3"); From bb3c8a417ed78f80219457f3421c973c590eb027 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Sun, 16 Jan 2022 23:14:27 -0800 Subject: [PATCH 3/6] message: make it less likely field name collides with pbjson_map arg --- pbjson-build/src/generator/message.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/pbjson-build/src/generator/message.rs b/pbjson-build/src/generator/message.rs index a9bf9ea..96b55b1 100644 --- a/pbjson-build/src/generator/message.rs +++ b/pbjson-build/src/generator/message.rs @@ -441,7 +441,7 @@ fn write_deserialize_message( {indent} formatter.write_str("struct {name}") {indent} }} -{indent} fn visit_map(self, mut map: V) -> Result<{rust_type}, V::Error> +{indent} fn visit_map(self, mut pbjson_map: V) -> Result<{rust_type}, V::Error> {indent} where {indent} V: serde::de::MapAccess<'de>, {indent} {{"#, @@ -471,7 +471,7 @@ fn write_deserialize_message( if !message.fields.is_empty() || !message.one_ofs.is_empty() { writeln!( writer, - "{}while let Some(k) = map.next_key()? {{", + "{}while let Some(k) = pbjson_map.next_key()? {{", Indent(indent + 2) )?; @@ -492,7 +492,7 @@ fn write_deserialize_message( } else { writeln!( writer, - "{}while map.next_key::()?.is_some() {{}}", + "{}while pbjson_map.next_key::()?.is_some() {{}}", Indent(indent + 2) )?; } @@ -695,14 +695,14 @@ fn write_deserialize_field( FieldModifier::Repeated => { write!( writer, - "map.next_value::>()?.into_iter().map(|x| x as i32).collect()", + "pbjson_map.next_value::>()?.into_iter().map(|x| x as i32).collect()", resolver.rust_type(path) )?; } _ => { write!( writer, - "map.next_value::<{}>()? as i32", + "pbjson_map.next_value::<{}>()? as i32", resolver.rust_type(path) )?; } @@ -711,7 +711,7 @@ fn write_deserialize_field( writeln!(writer)?; write!( writer, - "{}map.next_value::( write!(writer, "{}", Indent(indent + 1))?; } _ => { - write!(writer, "map.next_value()?",)?; + write!(writer, "pbjson_map.next_value()?",)?; } }; @@ -791,7 +791,7 @@ fn write_encode_scalar_field( let deserializer = match scalar { ScalarType::Bytes => "BytesDeserialize", _ if scalar.is_numeric() => "NumberDeserialize", - _ => return write!(writer, "map.next_value()?",), + _ => return write!(writer, "pbjson_map.next_value()?",), }; writeln!(writer)?; @@ -800,7 +800,7 @@ fn write_encode_scalar_field( FieldModifier::Repeated => { writeln!( writer, - "{}map.next_value::>>()?", + "{}pbjson_map.next_value::>>()?", Indent(indent + 1), deserializer )?; @@ -813,7 +813,7 @@ fn write_encode_scalar_field( _ => { writeln!( writer, - "{}map.next_value::<::pbjson::private::{}<_>>()?.0", + "{}pbjson_map.next_value::<::pbjson::private::{}<_>>()?.0", Indent(indent + 1), deserializer )?; From a362f9ab4240ee42f011fe6c471d65a70eedc1fc Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Sun, 16 Jan 2022 23:15:24 -0800 Subject: [PATCH 4/6] handle enums with `allow_alias` --- pbjson-build/src/generator/enumeration.rs | 11 ++++++++++- pbjson-build/src/resolver.rs | 8 ++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pbjson-build/src/generator/enumeration.rs b/pbjson-build/src/generator/enumeration.rs index faa8b78..8bd7a65 100644 --- a/pbjson-build/src/generator/enumeration.rs +++ b/pbjson-build/src/generator/enumeration.rs @@ -4,6 +4,9 @@ //! An enumeration should be decode-able from the full string variant name //! or its integer tag number, and should encode to the string representation +use std::collections::HashSet; +use std::io::{Result, Write}; + use super::{ write_deserialize_end, write_deserialize_start, write_serialize_end, write_serialize_start, Indent, @@ -11,7 +14,6 @@ use super::{ use crate::descriptor::{EnumDescriptor, TypePath}; use crate::generator::write_fields_array; use crate::resolver::Resolver; -use std::io::{Result, Write}; pub fn generate_enum( resolver: &Resolver<'_>, @@ -21,9 +23,16 @@ pub fn generate_enum( ) -> Result<()> { let rust_type = resolver.rust_type(path); + let mut numbers = HashSet::new(); + let variants: Vec<_> = descriptor .values .iter() + .filter(|variant| { + // Skip duplicate enum values. Protobuf allows this when the + // 'allow_alias' option is set. + numbers.insert(variant.number()) + }) .map(|variant| { let variant_name = variant.name.clone().unwrap(); let rust_variant = resolver.rust_variant(path, &variant_name); diff --git a/pbjson-build/src/resolver.rs b/pbjson-build/src/resolver.rs index 58f233b..7c97c7b 100644 --- a/pbjson-build/src/resolver.rs +++ b/pbjson-build/src/resolver.rs @@ -116,9 +116,13 @@ mod tests { let resolver_package = Package::new("envoy.service.health.v3"); let resolver = Resolver::new(&[], &resolver_package, false); - let to_resolve = TypePath::new(Package::new("envoy.config.core.v3")).child(TypeName::new("HealthStatus")); + let to_resolve = TypePath::new(Package::new("envoy.config.core.v3")) + .child(TypeName::new("HealthStatus")); - assert_eq!("super::super::super::config::core::v3::HealthStatus", resolver.rust_type(&to_resolve)); + assert_eq!( + "super::super::super::config::core::v3::HealthStatus", + resolver.rust_type(&to_resolve) + ); } #[test] From c83c9ac1491e36def07fd7d1195f4fd762070ca7 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Thu, 10 Aug 2023 09:15:30 -0700 Subject: [PATCH 5/6] rename to escape_type Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com> --- pbjson-build/src/escape.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pbjson-build/src/escape.rs b/pbjson-build/src/escape.rs index 4d5031e..6d0aab8 100644 --- a/pbjson-build/src/escape.rs +++ b/pbjson-build/src/escape.rs @@ -26,7 +26,7 @@ pub fn escape_ident(mut ident: String) -> String { } /// Converts a `snake_case` identifier to an `UpperCamel` case Rust type identifier. -pub fn escape_camel_case(mut ident: String) -> String { +pub fn escape_type(mut ident: String) -> String { // Suffix an underscore for the `Self` Rust keyword as it is not allowed as raw identifier. if ident == "Self" { ident += "_"; From e4759cf710759bd2c6b9c654ed8d6b9f12e79387 Mon Sep 17 00:00:00 2001 From: Bobby Powers Date: Thu, 10 Aug 2023 09:20:45 -0700 Subject: [PATCH 6/6] update call sites --- pbjson-build/src/generator/message.rs | 6 +++--- pbjson-build/src/message.rs | 4 ++-- pbjson-build/src/resolver.rs | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pbjson-build/src/generator/message.rs b/pbjson-build/src/generator/message.rs index 96b55b1..32c3b32 100644 --- a/pbjson-build/src/generator/message.rs +++ b/pbjson-build/src/generator/message.rs @@ -30,7 +30,7 @@ use super::{ Indent, }; use crate::descriptor::TypePath; -use crate::escape::escape_camel_case; +use crate::escape::escape_type; use crate::generator::write_fields_array; use crate::resolver::Resolver; @@ -592,7 +592,7 @@ fn write_deserialize_field_name( "{}\"{}\" => Ok(GeneratedField::{}),", Indent(indent + 5), json_name, - escape_camel_case(type_name.to_string()), + escape_type(type_name.to_string()), )?; } writeln!( @@ -636,7 +636,7 @@ fn write_fields_enum<'a, W: Write, I: Iterator>( writer, "{}{},", Indent(indent + 1), - escape_camel_case(type_name.to_string()) + escape_type(type_name.to_string()) )?; } writeln!(writer, "{}}}", Indent(indent)) diff --git a/pbjson-build/src/message.rs b/pbjson-build/src/message.rs index bd4c2a4..1980be3 100644 --- a/pbjson-build/src/message.rs +++ b/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_camel_case, escape_ident}; +use crate::escape::{escape_type, escape_ident}; #[derive(Debug, Clone, Copy)] pub enum ScalarType { @@ -86,7 +86,7 @@ pub struct Field { impl Field { pub fn rust_type_name(&self) -> String { use heck::ToUpperCamelCase; - escape_camel_case(self.name.to_upper_camel_case()) + escape_type(self.name.to_upper_camel_case()) } pub fn rust_field_name(&self) -> String { diff --git a/pbjson-build/src/resolver.rs b/pbjson-build/src/resolver.rs index 598f07f..b5ef499 100644 --- a/pbjson-build/src/resolver.rs +++ b/pbjson-build/src/resolver.rs @@ -1,5 +1,5 @@ use crate::descriptor::{Package, TypePath}; -use crate::escape::{escape_camel_case, escape_ident}; +use crate::escape::{escape_type, escape_ident}; #[derive(Debug)] pub struct Resolver<'a> { @@ -80,7 +80,7 @@ impl<'a> Resolver<'a> { ret.push_str("::"); } None => { - ret.push_str(escape_camel_case(i.to_upper_camel_case()).as_str()); + ret.push_str(escape_type(i.to_upper_camel_case()).as_str()); } } }