martin-g commented on code in PR #561:
URL: https://github.com/apache/avro-rs/pull/561#discussion_r3479823022
##########
avro_derive/src/enums/plain.rs:
##########
@@ -17,49 +17,128 @@
use crate::attributes::{NamedTypeOptions, VariantOptions};
use crate::case::RenameRule;
-use crate::enums::default_enum_variant;
+use crate::implementation::Implementation;
+use crate::utils::json_value_expr;
use crate::{aliases, preserve_optional};
-use proc_macro2::{Span, TokenStream};
+use proc_macro2::{Ident, Span};
use quote::quote;
+use serde_json::Value;
use syn::spanned::Spanned;
-use syn::{DataEnum, Fields};
+use syn::{Attribute, DataEnum, Fields, Generics, Meta};
-pub fn schema_def(
- container_attrs: &NamedTypeOptions,
- data_enum: &DataEnum,
- ident_span: Span,
-) -> Result<TokenStream, Vec<syn::Error>> {
+pub fn to_implementation(
+ input_span: Span,
+ ident: Ident,
+ generics: Generics,
+ container_attrs: NamedTypeOptions,
+ data: DataEnum,
+) -> Result<Implementation, Vec<syn::Error>> {
let doc = preserve_optional(container_attrs.doc.as_ref());
let enum_aliases = aliases(&container_attrs.aliases);
- if data_enum.variants.iter().all(|v| Fields::Unit == v.fields) {
- let default_value = default_enum_variant(data_enum, ident_span)?;
- let default = preserve_optional(default_value);
- let mut symbols = Vec::new();
- for variant in &data_enum.variants {
- let field_attrs = VariantOptions::new(&variant.attrs,
variant.span())?;
- let name = match (field_attrs.rename, container_attrs.rename_all) {
- (Some(rename), _) => rename,
+
+ let mut errors = Vec::new();
+ let mut symbols = Vec::new();
+ let mut default = match &container_attrs.default {
+ None => None,
+ Some(Value::String(s)) => Some(s.clone()),
+ Some(_) => {
+ errors.push(syn::Error::new(
+ input_span,
+ "Expected a JSON string for an enum default",
+ ));
+ None
+ }
+ };
+
+ for variant in data.variants {
+ let variant_attrs = match VariantOptions::new(&variant.attrs,
variant.span()) {
+ Ok(attrs) => attrs,
+ Err(errs) => {
+ errors.extend(errs);
+ continue;
+ }
+ };
+
+ // Check the skip attribute before the check if it is a unit variant,
because a skipped (un)named
+ // variant is not a problem
+ if variant_attrs.skip {
+ continue;
+ }
+
+ if !matches!(variant.fields, Fields::Unit) {
+ errors.push(syn::Error::new(
+ variant.span(),
+ "AvroSchema: derive does not work for enums with non unit
structs",
+ ));
+ continue;
+ }
+
+ // When a variant has the `#[default]` attribute and we don't have a
default yet, we assign
+ // it as the default. If an enum has multiple variants with the
default attribute, the compiler
+ // throws an error anyway so we can ignore that situation. If the
default attribute is on a
+ // skipped variant, we ignore it as Serde will throw an error if we
try to deserialize that
+ // variant.
+ if default.is_none() && has_default_attr(&variant.attrs) {
Review Comment:
```suggestion
if has_default_attr(&variant.attrs) {
```
shouldn't the variant's `#[default]` be used even when there is a container
default ?
##########
avro_derive/tests/derive.rs:
##########
@@ -2570,3 +2570,18 @@ fn
avro_rs_476_skip_serializing_fielddefault_trait_none() {
},
}
}
+
+#[test]
+fn avro_rs_561_transparent_struct_with_default_override() {
+ #[derive(AvroSchema)]
+ #[serde(transparent)]
+ struct T {
+ #[avro(default = "0")]
Review Comment:
let's use non-zero value for the default, to make it more obvious that it
does not initialize to the primitive's default value as some other languages do
##########
avro_derive/src/enums/mod.rs:
##########
@@ -18,50 +18,30 @@
mod plain;
use crate::attributes::NamedTypeOptions;
-use proc_macro2::{Ident, Span, TokenStream};
-use syn::{Attribute, DataEnum, Fields, Meta};
+use crate::implementation::Implementation;
+use proc_macro2::{Ident, Span};
+use syn::{DataEnum, Fields, Generics};
/// Generate a schema definition for a enum.
-pub fn get_data_enum_schema_def(
- container_attrs: &NamedTypeOptions,
- data_enum: &DataEnum,
- ident_span: Span,
-) -> Result<TokenStream, Vec<syn::Error>> {
- if data_enum.variants.iter().all(|v| Fields::Unit == v.fields) {
- plain::schema_def(container_attrs, data_enum, ident_span)
+pub fn to_implementation(
+ input_span: Span,
+ ident: Ident,
+ generics: Generics,
+ container_attrs: NamedTypeOptions,
+ data: DataEnum,
+) -> Result<Implementation, Vec<syn::Error>> {
+ if container_attrs.transparent {
+ return Err(vec![syn::Error::new(
+ input_span,
+ "AvroSchema: `#[serde(transparent)]` is only supported on structs",
+ )]);
+ }
+ if data.variants.iter().all(|v| Fields::Unit == v.fields) {
Review Comment:
This check contradicts with the check for `.skip` at enums/plain.rs line 61:
https://github.com/apache/avro-rs/pull/561/changes#diff-96375cb0af5acdd83b14cd2b9a9100754ce6dc2c50a7684a04ae82a6db4e15a5R62-R64
##########
avro_derive/src/utils.rs:
##########
@@ -65,3 +66,65 @@ pub fn field_aliases(op: &[impl AsRef<str>]) -> TokenStream {
.collect();
quote! {vec![#(#items),*]}
}
+
+/// Convert a `Value` into a `TokenStream`.
+///
+/// The `Value` is constructed, not parsed from a JSON string.
+pub fn json_value_expr(value: Value) -> TokenStream {
+ match value {
+ Value::Null => quote! {
+ ::serde_json::Value::Null
+ },
+ Value::Bool(bool) => quote! {
+ ::serde_json::Value::Bool(#bool)
+ },
+ Value::Number(number) => {
+ if let Some(n) = number.as_u64() {
+ quote! {
+ ::serde_json::Value::Number(::serde_json::Number::from(#n))
+ }
+ } else if let Some(n) = number.as_i64() {
+ quote! {
+ ::serde_json::Value::Number(::serde_json::Number::from(#n))
+ }
+ } else if let Some(n) = number.as_f64() {
+ quote! {
+
::serde_json::Value::Number(::serde_json::Number::from_f64(#n).expect("Unreachable,
f64 is finite"))
+ }
+ } else {
+ // This is needed when the arbitrary-precision feature flag is
enabled on serde_json
+ let s = number.to_string();
+ quote! {
+
::serde_json::Value::Number(#s.parse().expect(concat!("This was a valid number
at compile time: ", #s)))
+ }
+ }
+ }
+ Value::String(string) => quote! {
+ ::serde_json::Value::String(::std::string::String::from(#string))
+ },
+ Value::Array(array) => {
+ let array = array.into_iter().map(json_value_expr);
+ quote! {
+ ::serde_json::Value::Array(vec![#(#array),*])
+ }
+ }
+ Value::Object(object) => {
+ let len = object.len();
+
+ let mut keys = Vec::with_capacity(len);
+ let mut values = Vec::with_capacity(len);
+ for (key, value) in object {
+ keys.push(key);
+ values.push(json_value_expr(value));
+ }
+
+ quote! {
+ ::serde_json::Value::Object({
+ let mut map = ::serde_json::Map::with_capacity(#len);
+ #(map.insert(::std::string::String::from(#keys),
#values));*;
Review Comment:
```suggestion
#(map.insert(::std::string::String::from(#keys),
#values);)*
```
to avoid empty statements (stray semicolons) when the object is empty
--
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]
