martin-g commented on code in PR #398:
URL: https://github.com/apache/avro-rs/pull/398#discussion_r2681150479
##########
avro_derive/src/lib.rs:
##########
@@ -264,6 +283,69 @@ fn get_data_enum_schema_def(
}
}
+/// Generate a schema definition for a type marked transparent.
+fn get_transparent_data_schema_def(
+ data: &syn::Data,
+ input_span: Span,
+) -> Result<TokenStream, Vec<syn::Error>> {
+ match data {
+ syn::Data::Struct(data_struct) => match &data_struct.fields {
+ syn::Fields::Named(fields_named) => {
+ if fields_named.named.len() != 1 {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is only allowed on structs with
one field",
+ )]);
+ }
+ let field = fields_named
+ .named
+ .first()
+ .expect("There is exactly one field");
+ let field_attrs = FieldOptions::new(&field.attrs,
field.span())?;
+ if field_attrs != FieldOptions::default() {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is incompatible with all other
attributes",
+ )]);
+ }
+ let ty = &field.ty;
+ Ok(quote! {
+ #ty::get_schema_in_ctxt(named_schemas, enclosing_namespace)
+ })
+ }
+ syn::Fields::Unnamed(_) => Err(vec![syn::Error::new(
+ input_span,
+ "AvroSchema derive does not work for tuple structs",
+ )]),
+ syn::Fields::Unit => Err(vec![syn::Error::new(
+ input_span,
+ "AvroSchema derive does not work for unit structs",
+ )]),
+ },
+ syn::Data::Enum(data_enum) => {
+ if data_enum
+ .variants
+ .iter()
+ .all(|v| syn::Fields::Unit == v.fields)
+ {
+ Err(vec![syn::Error::new(
+ input_span,
+ "AvroSchema derive does not support #[serde(transparent)]
on simple enums",
+ )])
+ } else {
+ Err(vec![syn::Error::new(
+ input_span,
+ "AvroSchema derive does not work for enums with non unit
structs",
+ )])
+ }
+ }
+ syn::Data::Union(_) => Err(vec![syn::Error::new(
+ input_span,
+ "AvroSchema derive only works for structs and simple enums",
Review Comment:
```suggestion
"AvroSchema derive only works for structs with named fields",
```
We just errored above that enums are not supported, simple or not.
##########
avro_derive/src/lib.rs:
##########
@@ -221,21 +235,26 @@ fn get_data_struct_schema_def(
})
}
+/// Generate a schema definition for a enum.
fn get_data_enum_schema_def(
full_schema_name: &str,
doc: Option<String>,
aliases: Vec<String>,
rename_all: RenameRule,
- e: &syn::DataEnum,
+ data_enum: &syn::DataEnum,
error_span: Span,
Review Comment:
Above you renamed `error_span` to `ident_span`. Should we do the same here
too ? For consistency
##########
avro_derive/src/lib.rs:
##########
@@ -264,6 +283,69 @@ fn get_data_enum_schema_def(
}
}
+/// Generate a schema definition for a type marked transparent.
+fn get_transparent_data_schema_def(
+ data: &syn::Data,
+ input_span: Span,
+) -> Result<TokenStream, Vec<syn::Error>> {
+ match data {
+ syn::Data::Struct(data_struct) => match &data_struct.fields {
+ syn::Fields::Named(fields_named) => {
+ if fields_named.named.len() != 1 {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is only allowed on structs with
one field",
+ )]);
+ }
+ let field = fields_named
+ .named
+ .first()
+ .expect("There is exactly one field");
+ let field_attrs = FieldOptions::new(&field.attrs,
field.span())?;
+ if field_attrs != FieldOptions::default() {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is incompatible with all other
attributes",
+ )]);
+ }
+ let ty = &field.ty;
+ Ok(quote! {
+ #ty::get_schema_in_ctxt(named_schemas, enclosing_namespace)
+ })
+ }
+ syn::Fields::Unnamed(_) => Err(vec![syn::Error::new(
Review Comment:
> #[serde(transparent)]
Serialize and deserialize a newtype struct or a braced struct with one field
exactly the same as if its one field were serialized and deserialized by
itself.
Maybe we should support `newtype struct` too ?!
##########
avro_derive/src/lib.rs:
##########
@@ -264,6 +283,69 @@ fn get_data_enum_schema_def(
}
}
+/// Generate a schema definition for a type marked transparent.
+fn get_transparent_data_schema_def(
+ data: &syn::Data,
+ input_span: Span,
+) -> Result<TokenStream, Vec<syn::Error>> {
+ match data {
+ syn::Data::Struct(data_struct) => match &data_struct.fields {
+ syn::Fields::Named(fields_named) => {
+ if fields_named.named.len() != 1 {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is only allowed on structs with
one field",
+ )]);
+ }
+ let field = fields_named
+ .named
+ .first()
+ .expect("There is exactly one field");
+ let field_attrs = FieldOptions::new(&field.attrs,
field.span())?;
+ if field_attrs != FieldOptions::default() {
+ return Err(vec![syn::Error::new(
+ input_span,
Review Comment:
```suggestion
field.span(),
```
This would be more accurate, no ?
##########
avro_derive/src/lib.rs:
##########
@@ -264,6 +283,69 @@ fn get_data_enum_schema_def(
}
}
+/// Generate a schema definition for a type marked transparent.
+fn get_transparent_data_schema_def(
+ data: &syn::Data,
+ input_span: Span,
+) -> Result<TokenStream, Vec<syn::Error>> {
+ match data {
+ syn::Data::Struct(data_struct) => match &data_struct.fields {
+ syn::Fields::Named(fields_named) => {
+ if fields_named.named.len() != 1 {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is only allowed on structs with
one field",
+ )]);
+ }
+ let field = fields_named
+ .named
+ .first()
+ .expect("There is exactly one field");
+ let field_attrs = FieldOptions::new(&field.attrs,
field.span())?;
+ if field_attrs != FieldOptions::default() {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is incompatible with all other
attributes",
+ )]);
+ }
+ let ty = &field.ty;
+ Ok(quote! {
+ #ty::get_schema_in_ctxt(named_schemas, enclosing_namespace)
+ })
+ }
+ syn::Fields::Unnamed(_) => Err(vec![syn::Error::new(
+ input_span,
+ "AvroSchema derive does not work for tuple structs",
+ )]),
+ syn::Fields::Unit => Err(vec![syn::Error::new(
+ input_span,
+ "AvroSchema derive does not work for unit structs",
+ )]),
+ },
+ syn::Data::Enum(data_enum) => {
+ if data_enum
Review Comment:
Do we need the if/else at all ?
Should we just return an Err with a message like `#[serde(transparent)] is
not supported on enums` ?
##########
avro_derive/src/lib.rs:
##########
@@ -264,6 +283,69 @@ fn get_data_enum_schema_def(
}
}
+/// Generate a schema definition for a type marked transparent.
+fn get_transparent_data_schema_def(
+ data: &syn::Data,
+ input_span: Span,
+) -> Result<TokenStream, Vec<syn::Error>> {
+ match data {
+ syn::Data::Struct(data_struct) => match &data_struct.fields {
+ syn::Fields::Named(fields_named) => {
+ if fields_named.named.len() != 1 {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is only allowed on structs with
one field",
+ )]);
+ }
+ let field = fields_named
+ .named
+ .first()
+ .expect("There is exactly one field");
+ let field_attrs = FieldOptions::new(&field.attrs,
field.span())?;
+ if field_attrs != FieldOptions::default() {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "#[serde(transparent)] is incompatible with all other
attributes",
+ )]);
+ }
+ let ty = &field.ty;
Review Comment:
Do we need to check the type of `ty` before doing the rest ?
E.g. in `#type_to_schema_expr()` we handle Type::Path, Type::Reference and
Type::Array.
Here I think we should proceed only when it is Type::Path.
##########
avro_derive/src/attributes/mod.rs:
##########
@@ -89,6 +84,19 @@ impl NamedTypeOptions {
"#[avro(rename_all = \"..\")] must match #[serde(rename_all =
\"..\")], it's also deprecated. Please use only `#[serde(rename_all =
\"..\")]`",
));
}
+ if serde.transparent
+ && (serde.rename.is_some()
+ || avro.namespace.is_some()
Review Comment:
```suggestion
|| avro.name.is_some()
|| avro.namespace.is_some()
```
`avro.name` is deprecated but still possible to be used, so maybe we should
check it too ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
