Skip to content

Commit

Permalink
Make Debug impl optional for types (#797)
Browse files Browse the repository at this point in the history
* Make Debug impl optional on derive(Message)

* Config to optionally skip debug for messages

* Inline things again

* Put more things behind if

* Optionally skip debug in oneofs too

* Optionally skip debug for all types

* fixup! Optionally skip debug for all types

* Fix merge

* Remove superfluous push_indent

* Add some tests for the macro property

* Fix test

* Test generated protobuf

* Unconditional Debug

* Import

---------

Co-authored-by: Lucio Franco <[email protected]>
  • Loading branch information
audunska and LucioFranco authored Sep 1, 2023
1 parent 6180f9f commit f9a3cff
Show file tree
Hide file tree
Showing 7 changed files with 216 additions and 42 deletions.
28 changes: 25 additions & 3 deletions prost-build/src/code_generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ impl<'a> CodeGenerator<'a> {
"#[derive(Clone, PartialEq, {}::Message)]\n",
self.config.prost_path.as_deref().unwrap_or("::prost")
));
self.append_skip_debug(&fq_message_name);
self.push_indent();
self.buf.push_str("pub struct ");
self.buf.push_str(&to_upper_camel(&message_name));
Expand Down Expand Up @@ -280,6 +281,19 @@ impl<'a> CodeGenerator<'a> {
}
}

fn should_skip_debug(&self, fq_message_name: &str) -> bool {
assert_eq!(b'.', fq_message_name.as_bytes()[0]);
self.config.skip_debug.get(fq_message_name).next().is_some()
}

fn append_skip_debug(&mut self, fq_message_name: &str) {
if self.should_skip_debug(fq_message_name) {
push_indent(self.buf, self.depth);
self.buf.push_str("#[prost(skip_debug)]");
self.buf.push('\n');
}
}

fn append_enum_attributes(&mut self, fq_message_name: &str) {
assert_eq!(b'.', fq_message_name.as_bytes()[0]);
for attribute in self.config.enum_attributes.get(fq_message_name) {
Expand Down Expand Up @@ -536,6 +550,7 @@ impl<'a> CodeGenerator<'a> {
"#[derive(Clone, PartialEq, {}::Oneof)]\n",
self.config.prost_path.as_deref().unwrap_or("::prost")
));
self.append_skip_debug(&fq_message_name);
self.push_indent();
self.buf.push_str("pub enum ");
self.buf.push_str(&to_upper_camel(oneof.name()));
Expand Down Expand Up @@ -647,9 +662,16 @@ impl<'a> CodeGenerator<'a> {
self.append_type_attributes(&fq_proto_enum_name);
self.append_enum_attributes(&fq_proto_enum_name);
self.push_indent();
self.buf.push_str(
&format!("#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, {}::Enumeration)]\n",self.config.prost_path.as_deref().unwrap_or("::prost")),
);
let dbg = if self.should_skip_debug(&fq_proto_enum_name) {
""
} else {
"Debug, "
};
self.buf.push_str(&format!(
"#[derive(Clone, Copy, {}PartialEq, Eq, Hash, PartialOrd, Ord, {}::Enumeration)]\n",
dbg,
self.config.prost_path.as_deref().unwrap_or("::prost"),
));
self.push_indent();
self.buf.push_str("#[repr(i32)]\n");
self.push_indent();
Expand Down
16 changes: 16 additions & 0 deletions prost-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ pub struct Config {
default_package_filename: String,
protoc_args: Vec<OsString>,
disable_comments: PathMap<()>,
skip_debug: PathMap<()>,
skip_protoc_run: bool,
include_file: Option<PathBuf>,
prost_path: Option<String>,
Expand Down Expand Up @@ -629,6 +630,19 @@ impl Config {
self
}

/// Skips generating `impl Debug` for types
pub fn skip_debug<I, S>(&mut self, paths: I) -> &mut Self
where
I: IntoIterator<Item = S>,
S: AsRef<str>,
{
self.skip_debug.clear();
for matcher in paths {
self.skip_debug.insert(matcher.as_ref().to_string(), ());
}
self
}

/// Declare an externally provided Protobuf package or type.
///
/// `extern_path` allows `prost` types in external crates to be referenced in generated code.
Expand Down Expand Up @@ -1245,6 +1259,7 @@ impl default::Default for Config {
default_package_filename: "_".to_string(),
protoc_args: Vec::new(),
disable_comments: PathMap::default(),
skip_debug: PathMap::default(),
skip_protoc_run: false,
include_file: None,
prost_path: None,
Expand All @@ -1269,6 +1284,7 @@ impl fmt::Debug for Config {
.field("default_package_filename", &self.default_package_filename)
.field("protoc_args", &self.protoc_args)
.field("disable_comments", &self.disable_comments)
.field("skip_debug", &self.skip_debug)
.field("prost_path", &self.prost_path)
.finish()
}
Expand Down
107 changes: 68 additions & 39 deletions prost-derive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ fn try_message(input: TokenStream) -> Result<TokenStream, Error> {

let ident = input.ident;

syn::custom_keyword!(skip_debug);
let skip_debug = input
.attrs
.into_iter()
.any(|a| a.path().is_ident("prost") && a.parse_args::<skip_debug>().is_ok());

let variant_data = match input.data {
Data::Struct(variant_data) => variant_data,
Data::Enum(..) => bail!("Message can not be derived for an enum"),
Expand Down Expand Up @@ -165,26 +171,6 @@ fn try_message(input: TokenStream) -> Result<TokenStream, Error> {
}
};

let debugs = unsorted_fields.iter().map(|&(ref field_ident, ref field)| {
let wrapper = field.debug(quote!(self.#field_ident));
let call = if is_struct {
quote!(builder.field(stringify!(#field_ident), &wrapper))
} else {
quote!(builder.field(&wrapper))
};
quote! {
let builder = {
let wrapper = #wrapper;
#call
};
}
});
let debug_builder = if is_struct {
quote!(f.debug_struct(stringify!(#ident)))
} else {
quote!(f.debug_tuple(stringify!(#ident)))
};

let expanded = quote! {
impl #impl_generics ::prost::Message for #ident #ty_generics #where_clause {
#[allow(unused_variables)]
Expand Down Expand Up @@ -223,14 +209,44 @@ fn try_message(input: TokenStream) -> Result<TokenStream, Error> {
#default
}
}
};
let expanded = if skip_debug {
expanded
} else {
let debugs = unsorted_fields.iter().map(|&(ref field_ident, ref field)| {
let wrapper = field.debug(quote!(self.#field_ident));
let call = if is_struct {
quote!(builder.field(stringify!(#field_ident), &wrapper))
} else {
quote!(builder.field(&wrapper))
};
quote! {
let builder = {
let wrapper = #wrapper;
#call
};
}
});
let debug_builder = if is_struct {
quote!(f.debug_struct(stringify!(#ident)))
} else {
quote!(f.debug_tuple(stringify!(#ident)))
};
quote! {
#expanded

impl #impl_generics ::core::fmt::Debug for #ident #ty_generics #where_clause {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
let mut builder = #debug_builder;
#(#debugs;)*
builder.finish()
impl #impl_generics ::core::fmt::Debug for #ident #ty_generics #where_clause {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
let mut builder = #debug_builder;
#(#debugs;)*
builder.finish()
}
}
}
};

let expanded = quote! {
#expanded

#methods
};
Expand Down Expand Up @@ -358,6 +374,12 @@ fn try_oneof(input: TokenStream) -> Result<TokenStream, Error> {

let ident = input.ident;

syn::custom_keyword!(skip_debug);
let skip_debug = input
.attrs
.into_iter()
.any(|a| a.path().is_ident("prost") && a.parse_args::<skip_debug>().is_ok());

let variants = match input.data {
Data::Enum(DataEnum { variants, .. }) => variants,
Data::Struct(..) => bail!("Oneof can not be derived for a struct"),
Expand Down Expand Up @@ -440,16 +462,6 @@ fn try_oneof(input: TokenStream) -> Result<TokenStream, Error> {
quote!(#ident::#variant_ident(ref value) => #encoded_len)
});

let debug = fields.iter().map(|&(ref variant_ident, ref field)| {
let wrapper = field.debug(quote!(*value));
quote!(#ident::#variant_ident(ref value) => {
let wrapper = #wrapper;
f.debug_tuple(stringify!(#variant_ident))
.field(&wrapper)
.finish()
})
});

let expanded = quote! {
impl #impl_generics #ident #ty_generics #where_clause {
/// Encodes the message to a buffer.
Expand Down Expand Up @@ -483,10 +495,27 @@ fn try_oneof(input: TokenStream) -> Result<TokenStream, Error> {
}
}

impl #impl_generics ::core::fmt::Debug for #ident #ty_generics #where_clause {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
match *self {
#(#debug,)*
};
let expanded = if skip_debug {
expanded
} else {
let debug = fields.iter().map(|&(ref variant_ident, ref field)| {
let wrapper = field.debug(quote!(*value));
quote!(#ident::#variant_ident(ref value) => {
let wrapper = #wrapper;
f.debug_tuple(stringify!(#variant_ident))
.field(&wrapper)
.finish()
})
});
quote! {
#expanded

impl #impl_generics ::core::fmt::Debug for #ident #ty_generics #where_clause {
fn fmt(&self, f: &mut ::core::fmt::Formatter) -> ::core::fmt::Result {
match *self {
#(#debug,)*
}
}
}
}
Expand Down
5 changes: 5 additions & 0 deletions tests/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ fn main() {
config.field_attribute("Foo.Custom.Attrs.AnotherEnum.D", "/// The D docs");
config.field_attribute("Foo.Custom.Attrs.Msg.field.a", "/// Oneof A docs");
config.field_attribute("Foo.Custom.Attrs.Msg.field.b", "/// Oneof B docs");
config.skip_debug(["custom_debug.Msg"]);

config.file_descriptor_set_path(
PathBuf::from(env::var("OUT_DIR").expect("OUT_DIR environment variable not set"))
Expand Down Expand Up @@ -90,6 +91,10 @@ fn main() {
.compile_protos(&[src.join("default_string_escape.proto")], includes)
.unwrap();

config
.compile_protos(&[src.join("custom_debug.proto")], includes)
.unwrap();

prost_build::Config::new()
.protoc_arg("--experimental_allow_proto3_optional")
.compile_protos(&[src.join("proto3_presence.proto")], includes)
Expand Down
17 changes: 17 additions & 0 deletions tests/src/custom_debug.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
syntax = "proto3";

package custom_debug;

message Msg {
int32 a = 1;
string b = 2;
oneof c {
AnEnum d = 3;
string e = 4;
};
}

enum AnEnum {
A = 0;
B = 1;
};
14 changes: 14 additions & 0 deletions tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ mod message_encoding;
#[cfg(test)]
mod no_unused_results;
#[cfg(test)]
#[cfg(feature = "std")]
mod skip_debug;
#[cfg(test)]
mod well_known_types;

pub mod foo {
Expand All @@ -59,6 +62,17 @@ pub mod recursive_oneof {
include!(concat!(env!("OUT_DIR"), "/recursive_oneof.rs"));
}

#[cfg(feature = "std")]
pub mod custom_debug {
use std::fmt;
include!(concat!(env!("OUT_DIR"), "/custom_debug.rs"));
impl fmt::Debug for Msg {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("Msg {..}")
}
}
}

/// This tests the custom attributes support by abusing docs.
///
/// Docs really are full-blown attributes. So we use them to ensure we can place them on everything
Expand Down
71 changes: 71 additions & 0 deletions tests/src/skip_debug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
//! Tests for skipping the default Debug implementation.

use crate::custom_debug::{msg, AnEnum, Msg};
use crate::message_encoding::BasicEnumeration;
use prost::alloc::{format, string::String};
use std::fmt;

/// A special case with a tuple struct
#[test]
fn tuple_struct_custom_debug() {
#[derive(Clone, PartialEq, prost::Message)]
#[prost(skip_debug)]
struct NewType(#[prost(enumeration = "BasicEnumeration", tag = "5")] i32);
impl fmt::Debug for NewType {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("NewType(custom_debug)")
}
}
assert_eq!(
format!("{:?}", NewType(BasicEnumeration::TWO as i32)),
"NewType(custom_debug)"
);
assert_eq!(format!("{:?}", NewType(42)), "NewType(custom_debug)");
}

#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, prost::Oneof)]
#[prost(skip_debug)]
pub enum OneofWithEnumCustomDebug {
#[prost(int32, tag = "8")]
Int(i32),
#[prost(string, tag = "9")]
String(String),
#[prost(enumeration = "BasicEnumeration", tag = "10")]
Enumeration(i32),
}

#[derive(Clone, PartialEq, prost::Message)]
#[prost(skip_debug)]
struct MessageWithOneofCustomDebug {
#[prost(oneof = "OneofWithEnumCustomDebug", tags = "8, 9, 10")]
of: Option<OneofWithEnumCustomDebug>,
}

impl fmt::Debug for MessageWithOneofCustomDebug {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.write_str("MessageWithOneofCustomDebug {..}")
}
}

/// Enumerations inside oneofs
#[test]
fn oneof_with_enum_custom_debug() {
let msg = MessageWithOneofCustomDebug {
of: Some(OneofWithEnumCustomDebug::Enumeration(
BasicEnumeration::TWO as i32,
)),
};
assert_eq!(format!("{:?}", msg), "MessageWithOneofCustomDebug {..}");
}

/// Generated protobufs
#[test]
fn test_proto_msg_custom_debug() {
let msg = Msg {
a: 0,
b: "".to_string(),
c: Some(msg::C::D(AnEnum::A as i32)),
};
assert_eq!(format!("{:?}", msg), "Msg {..}");
}

0 comments on commit f9a3cff

Please sign in to comment.