martin-g commented on code in PR #377:
URL: https://github.com/apache/avro-rs/pull/377#discussion_r2649624377
##########
avro_derive/src/attributes/avro.rs:
##########
@@ -0,0 +1,40 @@
+use crate::case::RenameRule;
+
Review Comment:
Let's add some rustdoc for the `pub`lic structs and their fields.
##########
avro_derive/src/attributes/serde.rs:
##########
@@ -0,0 +1,298 @@
+//! Attribute parsing for Serde attributes
+//!
+//! # Serde attributes
+//! This module only parses the minimal amount to be able to read Serde
attributes. This means
+//! most attributes are decoded as an `Option<String>` as the actual content
is not relevant.
+//! Attributes which are not needed by the derive are prefixed with a `_` as
we can't ignore them
+//! (this would be a compile error).
+//!
+//! If Serde adds new attributes they need to be added here too.
+
+use darling::{FromAttributes, FromMeta};
+use syn::Expr;
+
+use crate::case::RenameRule;
+
+// from_expr is used for `rename_all = ".."`
+// FromMeta is used for `rename_all(serialize = "..", ..)`
+#[derive(Debug, Default, FromMeta, PartialEq, Eq)]
+#[darling(from_expr = RenameAll::from_expr)]
+pub struct RenameAll {
+ #[darling(default)]
+ pub serialize: RenameRule,
+ #[darling(default)]
+ pub deserialize: RenameRule,
+}
+
+impl From<RenameRule> for RenameAll {
+ fn from(value: RenameRule) -> Self {
+ Self {
+ serialize: value,
+ deserialize: value,
+ }
+ }
+}
+
+impl RenameAll {
+ fn from_expr(expr: &Expr) -> darling::Result<Self> {
+ let Expr::Lit(lit) = expr else {
+ return Err(darling::Error::custom("Expected a string or a
tuple!"));
+ };
+ let rule = RenameRule::from_value(&lit.lit)?;
+ Ok(RenameAll::from(rule))
+ }
+}
+
+#[derive(Debug, FromMeta, PartialEq)]
+#[darling(from_expr = |expr| Ok(SerdeDefault::Expr(expr.clone())))]
+pub enum SerdeDefault {
+ #[darling(word, skip)]
+ UseTrait,
+ Expr(Expr),
+}
+
+/// All Serde attributes that a container can have.
+#[derive(Debug, FromAttributes)]
+#[darling(attributes(serde))]
+pub struct ContainerAttributes {
+ #[darling(rename = "rename")]
+ /// Rename this container.
+ pub _rename: Option<String>,
+ /// Rename all the fields (if this is a struct) or variants (if this is an
enum) according to the given case convention.
+ #[darling(default)]
+ pub rename_all: RenameAll,
+ /// Rename all the fields of the struct variants in this enum.
+ #[darling(default, rename = "rename_all_fields")]
+ pub _rename_all_fields: RenameAll,
+ /// Error when encountering unknown fields when deserialising.
+ #[darling(default, rename = "deny_unknown_fields")]
+ pub _deny_unknown_fields: bool,
+ /// Add/expect a tag during serialisation.
+ ///
+ /// When used on a struct, this adds an extra field which is not in the
schema definition.
+ /// When used on an enum, serde transforms it into a struct which does not
match the schema definition.
+ pub tag: Option<String>,
+ /// Put the content in a field with this name.
+ pub content: Option<String>,
+ /// This makes the enum transparent, (de)serializing based on the variant
directly.
+ ///
+ /// This does not match the schema definition.
+ #[darling(default)]
+ pub untagged: bool,
+ /// Add a bound to the Serialize/Deserialize trait.
+ #[darling(default, rename = "bound")]
+ pub _bound: Option<String>,
+ /// When deserializing, any missing fields should be filled in from the
struct's implementation of `Default`.
+ #[darling(rename = "default")]
+ pub _default: Option<SerdeDefault>,
+ /// This type is the serde implementation for a "remote" type.
+ ///
+ /// This makes the (de)serialisation use/return a different type.
+ pub remote: Option<String>,
+ /// Directly use the inner type for (de)serialisation.
+ #[darling(default)]
+ pub transparent: bool,
+ /// Deserialize using the given type and then convert to this type with
`From`.
+ #[darling(default, rename = "from")]
+ pub _from: Option<String>,
+ /// Deserialize using the given type and then convert to this type with
`TryFrom`.
+ #[darling(default, rename = "try_from")]
+ pub _try_from: Option<String>,
+ /// Convert this type to the given type using `Into` and then serialize
using the given type.
+ #[darling(default, rename = "into")]
+ pub _into: Option<String>,
+ /// Use the Serde API at this path.
+ #[darling(default, rename = "crate")]
+ pub _crate: Option<String>,
+ /// Custom error text.
+ #[darling(default, rename = "expecting")]
+ pub _expecting: Option<String>,
+ /// This does something with tags, which are incompatible.
+ #[darling(default)]
+ pub variant_identifier: bool,
+ /// This does something with tags, `` which are incompatible.
Review Comment:
What is the purpose of the empty backticks here ?
##########
avro_derive/src/attributes/mod.rs:
##########
@@ -0,0 +1,202 @@
+use darling::FromAttributes;
+use proc_macro2::Span;
+use syn::Attribute;
+
+use crate::{case::RenameRule, darling_to_syn};
+
+pub mod avro;
+pub mod serde;
+
+#[derive(Default)]
+pub struct NamedTypeOptions {
+ pub name: Option<String>,
+ pub namespace: Option<String>,
+ pub doc: Option<String>,
+ pub alias: Vec<String>,
+ pub rename_all: RenameRule,
+}
+
+impl NamedTypeOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
+
avro::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
+
serde::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.tag.is_some()
+ || serde.content.is_some()
+ || serde.untagged
+ || serde.variant_identifier
+ || serde.field_identifier
+ {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support changing the tagging Serde
generates (`tag`, `content`, `untagged`, `variant_identifier`,
`field_identifier`)",
+ ));
+ }
+ if serde.remote.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `remote`
attribute",
+ ));
+ }
+ if serde.transparent {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support Serde transparent",
+ ))
+ }
+ if serde.rename_all.deserialize != serde.rename_all.serialize {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support different rename rules for
serializing and deserializing (`rename_all(serialize = \"..\", deserialize =
\"..\")`)"
+ ))
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.rename_all != RenameRule::None && serde.rename_all.serialize
!= avro.rename_all {
+ errors.push(syn::Error::new(
+ span,
+ "#[avro(rename_all = \"..\")] must match #[serde(rename_all =
\"..\")], it's also deprecated. Please use only `#[serde(rename_all =
\"..\")]`",
+ ))
+ }
+
+ if !errors.is_empty() {
+ return Err(errors);
+ }
+
+ Ok(Self {
+ name: avro.name,
+ namespace: avro.namespace,
+ doc: avro.doc,
+ alias: avro.alias,
+ rename_all: serde.rename_all.serialize,
+ })
+ }
+}
+
+pub struct FieldOptions {
+ pub doc: Option<String>,
+ pub default: Option<String>,
+ pub alias: Vec<String>,
+ pub rename: Option<String>,
+ pub skip: bool,
+ pub flatten: bool,
+}
+
+impl FieldOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
avro::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
serde::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.getter.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `getter`
attribute",
+ ));
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.skip && !serde.skip {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(skip)]` requires `#[serde(skip)]`, it's also
deprecated. Please use only `#[serde(skip)]`"
+ ));
+ }
+ if avro.flatten && !serde.flatten {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(flatten)]` requires `#[serde(flatten)]`, it's also
deprecated. Please use only `#[serde(flatten)]`"
+ ));
+ }
+ // TODO: rename and alias checking can be relaxed with a more complex
check, would require the field name
+ if avro.rename.is_some() && serde.rename != avro.rename {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(rename = \"..\")]` must match `#[serde(rename =
\"..\")]`, it's also deprecated. Please use only `#[serde(rename = \"..\")]`"
+ ));
+ }
+ if serde.alias != avro.alias {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(alias = \"..\")]` must match `#[serde(alias =
\"..\")]`, it's also deprecated. Please use only `#[serde(alias = \"..\")]`"
+ ));
+ }
+ if serde.skip_serializing && serde.skip_deserializing {
+ errors.push(syn::Error::new(
+ span,
+ "Use `#[serde(skip)]` instead of `#[serde(skip_serializing,
skip_deserializing)]`",
+ ));
+
+ // Don't want to suggest default for skip_serializing as it's not
needed for skip
+ return Err(errors);
+ }
+ if ((serde.skip_serializing) || serde.skip_serializing_if.is_some())
+ && avro.default.is_none()
+ {
+ errors.push(syn::Error::new(
+ span,
+ "`#[serde(skip_serializing)]` and
`#[serde(skip_serializing_if)]` require `#[avro(default = \"..\")]"
Review Comment:
```suggestion
"`#[serde(skip_serializing)]` and
`#[serde(skip_serializing_if)]` require `#[avro(default = \"..\")]`"
```
##########
avro_derive/src/case.rs:
##########
@@ -47,6 +51,15 @@ pub enum RenameRule {
ScreamingKebabCase,
}
+impl FromMeta for RenameRule {
+ fn from_value(value: &Lit) -> darling::Result<Self> {
+ let Lit::Str(litstr) = value else {
+ panic!("Only expected a string");
Review Comment:
```suggestion
return Err(darling::Error::unexpected_lit_type(value));
```
##########
avro_derive/src/attributes/serde.rs:
##########
@@ -0,0 +1,298 @@
+//! Attribute parsing for Serde attributes
+//!
+//! # Serde attributes
+//! This module only parses the minimal amount to be able to read Serde
attributes. This means
+//! most attributes are decoded as an `Option<String>` as the actual content
is not relevant.
+//! Attributes which are not needed by the derive are prefixed with a `_` as
we can't ignore them
+//! (this would be a compile error).
+//!
+//! If Serde adds new attributes they need to be added here too.
+
+use darling::{FromAttributes, FromMeta};
+use syn::Expr;
+
+use crate::case::RenameRule;
+
+// from_expr is used for `rename_all = ".."`
+// FromMeta is used for `rename_all(serialize = "..", ..)`
+#[derive(Debug, Default, FromMeta, PartialEq, Eq)]
+#[darling(from_expr = RenameAll::from_expr)]
+pub struct RenameAll {
+ #[darling(default)]
+ pub serialize: RenameRule,
+ #[darling(default)]
+ pub deserialize: RenameRule,
+}
+
+impl From<RenameRule> for RenameAll {
+ fn from(value: RenameRule) -> Self {
+ Self {
+ serialize: value,
+ deserialize: value,
+ }
+ }
+}
+
+impl RenameAll {
+ fn from_expr(expr: &Expr) -> darling::Result<Self> {
+ let Expr::Lit(lit) = expr else {
+ return Err(darling::Error::custom("Expected a string or a
tuple!"));
+ };
+ let rule = RenameRule::from_value(&lit.lit)?;
+ Ok(RenameAll::from(rule))
+ }
+}
+
+#[derive(Debug, FromMeta, PartialEq)]
+#[darling(from_expr = |expr| Ok(SerdeDefault::Expr(expr.clone())))]
+pub enum SerdeDefault {
+ #[darling(word, skip)]
+ UseTrait,
+ Expr(Expr),
+}
+
+/// All Serde attributes that a container can have.
+#[derive(Debug, FromAttributes)]
+#[darling(attributes(serde))]
+pub struct ContainerAttributes {
+ #[darling(rename = "rename")]
+ /// Rename this container.
+ pub _rename: Option<String>,
+ /// Rename all the fields (if this is a struct) or variants (if this is an
enum) according to the given case convention.
+ #[darling(default)]
+ pub rename_all: RenameAll,
+ /// Rename all the fields of the struct variants in this enum.
+ #[darling(default, rename = "rename_all_fields")]
+ pub _rename_all_fields: RenameAll,
+ /// Error when encountering unknown fields when deserialising.
+ #[darling(default, rename = "deny_unknown_fields")]
+ pub _deny_unknown_fields: bool,
+ /// Add/expect a tag during serialisation.
+ ///
+ /// When used on a struct, this adds an extra field which is not in the
schema definition.
+ /// When used on an enum, serde transforms it into a struct which does not
match the schema definition.
+ pub tag: Option<String>,
+ /// Put the content in a field with this name.
+ pub content: Option<String>,
+ /// This makes the enum transparent, (de)serializing based on the variant
directly.
+ ///
+ /// This does not match the schema definition.
+ #[darling(default)]
+ pub untagged: bool,
+ /// Add a bound to the Serialize/Deserialize trait.
+ #[darling(default, rename = "bound")]
+ pub _bound: Option<String>,
+ /// When deserializing, any missing fields should be filled in from the
struct's implementation of `Default`.
+ #[darling(rename = "default")]
+ pub _default: Option<SerdeDefault>,
+ /// This type is the serde implementation for a "remote" type.
+ ///
+ /// This makes the (de)serialisation use/return a different type.
+ pub remote: Option<String>,
+ /// Directly use the inner type for (de)serialisation.
+ #[darling(default)]
+ pub transparent: bool,
+ /// Deserialize using the given type and then convert to this type with
`From`.
+ #[darling(default, rename = "from")]
+ pub _from: Option<String>,
+ /// Deserialize using the given type and then convert to this type with
`TryFrom`.
+ #[darling(default, rename = "try_from")]
+ pub _try_from: Option<String>,
+ /// Convert this type to the given type using `Into` and then serialize
using the given type.
+ #[darling(default, rename = "into")]
+ pub _into: Option<String>,
+ /// Use the Serde API at this path.
+ #[darling(default, rename = "crate")]
+ pub _crate: Option<String>,
+ /// Custom error text.
+ #[darling(default, rename = "expecting")]
+ pub _expecting: Option<String>,
+ /// This does something with tags, which are incompatible.
+ #[darling(default)]
+ pub variant_identifier: bool,
+ /// This does something with tags, `` which are incompatible.
+ #[darling(default)]
+ pub field_identifier: bool,
+}
+
+/// All Serde attributes that a enum variant can have.
+#[derive(Debug, FromAttributes)]
+#[darling(attributes(serde))]
+pub struct VariantAttributes {
+ /// Rename the variant.
+ #[darling(default)]
+ pub rename: Option<String>,
+ /// Aliases for this variant, only used during deserialisation.
+ #[darling(multiple, rename = "alias")]
+ pub _alias: Vec<String>,
+ #[darling(default, rename = "rename_all")]
+ pub _rename_all: RenameAll,
+ /// Do not serialize or deserialize this variant.
+ #[darling(default, rename = "skip")]
+ pub _skip: bool,
+ /// Do not serialize this variant.
+ #[darling(default, rename = "skip_serializing")]
+ pub _skip_serializing: bool,
+ /// Do not deserialize this variant.
+ #[darling(default, rename = "skip_deserializing")]
+ pub _skip_deserializing: bool,
+ /// Use this function for serializing.
+ #[darling(rename = "serialize_with")]
+ pub _serialize_with: Option<String>,
+ /// Use this function for deserializing.
+ #[darling(rename = "deserialize_with")]
+ pub _deserialize_with: Option<String>,
+ /// Use this module for (de)serializing.
+ #[darling(rename = "with")]
+ pub _with: Option<String>,
+ /// Put trait bounds on the implementations.
+ #[darling(rename = "bound")]
+ pub _bound: Option<String>,
+ /// Put bounds on the lifetimes.
+ #[darling(rename = "borrow")]
+ pub _borrow: Option<SerdeBorrow>,
+ /// This does something with tags, which are incompatible.
+ #[darling(default)]
+ pub other: bool,
+ /// (De)serialize this variant as if it was not part of the enum.
+ pub untagged: Option<String>,
Review Comment:
Is this the correct way to read it ?!
`#[serde(untagged)]` has no String value.
##########
avro_derive/src/attributes/mod.rs:
##########
@@ -0,0 +1,202 @@
+use darling::FromAttributes;
+use proc_macro2::Span;
+use syn::Attribute;
+
+use crate::{case::RenameRule, darling_to_syn};
+
+pub mod avro;
+pub mod serde;
+
+#[derive(Default)]
+pub struct NamedTypeOptions {
+ pub name: Option<String>,
+ pub namespace: Option<String>,
+ pub doc: Option<String>,
+ pub alias: Vec<String>,
+ pub rename_all: RenameRule,
+}
+
+impl NamedTypeOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
+
avro::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
+
serde::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.tag.is_some()
+ || serde.content.is_some()
+ || serde.untagged
+ || serde.variant_identifier
+ || serde.field_identifier
+ {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support changing the tagging Serde
generates (`tag`, `content`, `untagged`, `variant_identifier`,
`field_identifier`)",
+ ));
+ }
+ if serde.remote.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `remote`
attribute",
+ ));
+ }
+ if serde.transparent {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support Serde transparent",
+ ))
+ }
+ if serde.rename_all.deserialize != serde.rename_all.serialize {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support different rename rules for
serializing and deserializing (`rename_all(serialize = \"..\", deserialize =
\"..\")`)"
+ ))
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.rename_all != RenameRule::None && serde.rename_all.serialize
!= avro.rename_all {
+ errors.push(syn::Error::new(
+ span,
+ "#[avro(rename_all = \"..\")] must match #[serde(rename_all =
\"..\")], it's also deprecated. Please use only `#[serde(rename_all =
\"..\")]`",
+ ))
+ }
+
+ if !errors.is_empty() {
+ return Err(errors);
+ }
+
+ Ok(Self {
+ name: avro.name,
+ namespace: avro.namespace,
+ doc: avro.doc,
+ alias: avro.alias,
+ rename_all: serde.rename_all.serialize,
+ })
+ }
+}
+
+pub struct FieldOptions {
+ pub doc: Option<String>,
+ pub default: Option<String>,
+ pub alias: Vec<String>,
+ pub rename: Option<String>,
+ pub skip: bool,
+ pub flatten: bool,
+}
+
+impl FieldOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
avro::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
serde::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.getter.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `getter`
attribute",
+ ));
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.skip && !serde.skip {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(skip)]` requires `#[serde(skip)]`, it's also
deprecated. Please use only `#[serde(skip)]`"
+ ));
+ }
+ if avro.flatten && !serde.flatten {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(flatten)]` requires `#[serde(flatten)]`, it's also
deprecated. Please use only `#[serde(flatten)]`"
+ ));
+ }
+ // TODO: rename and alias checking can be relaxed with a more complex
check, would require the field name
+ if avro.rename.is_some() && serde.rename != avro.rename {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(rename = \"..\")]` must match `#[serde(rename =
\"..\")]`, it's also deprecated. Please use only `#[serde(rename = \"..\")]`"
+ ));
+ }
+ if serde.alias != avro.alias {
Review Comment:
This would fail if the order of the items differ but I think it is a minor
issue that the users can easily resolve by using the same order.
##########
avro_derive/src/attributes/mod.rs:
##########
@@ -0,0 +1,202 @@
+use darling::FromAttributes;
+use proc_macro2::Span;
+use syn::Attribute;
+
+use crate::{case::RenameRule, darling_to_syn};
+
+pub mod avro;
+pub mod serde;
+
+#[derive(Default)]
+pub struct NamedTypeOptions {
+ pub name: Option<String>,
+ pub namespace: Option<String>,
+ pub doc: Option<String>,
+ pub alias: Vec<String>,
+ pub rename_all: RenameRule,
+}
+
+impl NamedTypeOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
+
avro::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
+
serde::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.tag.is_some()
+ || serde.content.is_some()
+ || serde.untagged
+ || serde.variant_identifier
+ || serde.field_identifier
+ {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support changing the tagging Serde
generates (`tag`, `content`, `untagged`, `variant_identifier`,
`field_identifier`)",
+ ));
+ }
+ if serde.remote.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `remote`
attribute",
+ ));
+ }
+ if serde.transparent {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support Serde transparent",
+ ))
+ }
+ if serde.rename_all.deserialize != serde.rename_all.serialize {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support different rename rules for
serializing and deserializing (`rename_all(serialize = \"..\", deserialize =
\"..\")`)"
+ ))
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.rename_all != RenameRule::None && serde.rename_all.serialize
!= avro.rename_all {
+ errors.push(syn::Error::new(
+ span,
+ "#[avro(rename_all = \"..\")] must match #[serde(rename_all =
\"..\")], it's also deprecated. Please use only `#[serde(rename_all =
\"..\")]`",
+ ))
+ }
+
+ if !errors.is_empty() {
+ return Err(errors);
+ }
+
+ Ok(Self {
+ name: avro.name,
+ namespace: avro.namespace,
+ doc: avro.doc,
+ alias: avro.alias,
+ rename_all: serde.rename_all.serialize,
+ })
+ }
+}
+
+pub struct FieldOptions {
+ pub doc: Option<String>,
+ pub default: Option<String>,
+ pub alias: Vec<String>,
+ pub rename: Option<String>,
+ pub skip: bool,
+ pub flatten: bool,
+}
+
+impl FieldOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
avro::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
serde::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.getter.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `getter`
attribute",
+ ));
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.skip && !serde.skip {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(skip)]` requires `#[serde(skip)]`, it's also
deprecated. Please use only `#[serde(skip)]`"
+ ));
+ }
+ if avro.flatten && !serde.flatten {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(flatten)]` requires `#[serde(flatten)]`, it's also
deprecated. Please use only `#[serde(flatten)]`"
+ ));
+ }
+ // TODO: rename and alias checking can be relaxed with a more complex
check, would require the field name
+ if avro.rename.is_some() && serde.rename != avro.rename {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(rename = \"..\")]` must match `#[serde(rename =
\"..\")]`, it's also deprecated. Please use only `#[serde(rename = \"..\")]`"
+ ));
+ }
+ if serde.alias != avro.alias {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(alias = \"..\")]` must match `#[serde(alias =
\"..\")]`, it's also deprecated. Please use only `#[serde(alias = \"..\")]`"
+ ));
+ }
+ if serde.skip_serializing && serde.skip_deserializing {
+ errors.push(syn::Error::new(
+ span,
+ "Use `#[serde(skip)]` instead of `#[serde(skip_serializing,
skip_deserializing)]`",
+ ));
+
+ // Don't want to suggest default for skip_serializing as it's not
needed for skip
+ return Err(errors);
+ }
+ if ((serde.skip_serializing) || serde.skip_serializing_if.is_some())
Review Comment:
```suggestion
if (serde.skip_serializing || serde.skip_serializing_if.is_some())
```
##########
avro_derive/src/attributes/mod.rs:
##########
@@ -0,0 +1,202 @@
+use darling::FromAttributes;
+use proc_macro2::Span;
+use syn::Attribute;
+
+use crate::{case::RenameRule, darling_to_syn};
+
+pub mod avro;
+pub mod serde;
+
+#[derive(Default)]
+pub struct NamedTypeOptions {
+ pub name: Option<String>,
+ pub namespace: Option<String>,
+ pub doc: Option<String>,
+ pub alias: Vec<String>,
+ pub rename_all: RenameRule,
+}
+
+impl NamedTypeOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
+
avro::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
+
serde::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.tag.is_some()
+ || serde.content.is_some()
+ || serde.untagged
+ || serde.variant_identifier
+ || serde.field_identifier
+ {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support changing the tagging Serde
generates (`tag`, `content`, `untagged`, `variant_identifier`,
`field_identifier`)",
+ ));
+ }
+ if serde.remote.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `remote`
attribute",
+ ));
+ }
+ if serde.transparent {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support Serde transparent",
+ ))
+ }
+ if serde.rename_all.deserialize != serde.rename_all.serialize {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support different rename rules for
serializing and deserializing (`rename_all(serialize = \"..\", deserialize =
\"..\")`)"
+ ))
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.rename_all != RenameRule::None && serde.rename_all.serialize
!= avro.rename_all {
+ errors.push(syn::Error::new(
+ span,
+ "#[avro(rename_all = \"..\")] must match #[serde(rename_all =
\"..\")], it's also deprecated. Please use only `#[serde(rename_all =
\"..\")]`",
+ ))
+ }
+
+ if !errors.is_empty() {
+ return Err(errors);
+ }
+
+ Ok(Self {
+ name: avro.name,
+ namespace: avro.namespace,
+ doc: avro.doc,
+ alias: avro.alias,
+ rename_all: serde.rename_all.serialize,
+ })
+ }
+}
+
+pub struct FieldOptions {
+ pub doc: Option<String>,
+ pub default: Option<String>,
+ pub alias: Vec<String>,
+ pub rename: Option<String>,
+ pub skip: bool,
+ pub flatten: bool,
+}
+
+impl FieldOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
avro::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
serde::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.getter.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `getter`
attribute",
+ ));
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.skip && !serde.skip {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(skip)]` requires `#[serde(skip)]`, it's also
deprecated. Please use only `#[serde(skip)]`"
Review Comment:
Is there a way to report the usage of a deprecated `avro(attribute)` as a
warning without depending on the value of its respective `serde` counter-part ?
I.e. pseudo code:
```
if avro.skip { report a warning like "#[avro(skip)] is deprecated. Use
#[serde(skip)] instead" }
if avro.skip != serde.skip {report an error, without the ", it's also
deprecated" text}
```
Currently the user will be notified that a deprecated avro attribute is used
only if the avro value differs from the serde's one.
##########
avro_derive/src/attributes/mod.rs:
##########
@@ -0,0 +1,202 @@
+use darling::FromAttributes;
+use proc_macro2::Span;
+use syn::Attribute;
+
+use crate::{case::RenameRule, darling_to_syn};
+
+pub mod avro;
+pub mod serde;
+
+#[derive(Default)]
+pub struct NamedTypeOptions {
+ pub name: Option<String>,
+ pub namespace: Option<String>,
+ pub doc: Option<String>,
+ pub alias: Vec<String>,
+ pub rename_all: RenameRule,
+}
+
+impl NamedTypeOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
+
avro::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
+
serde::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.tag.is_some()
+ || serde.content.is_some()
+ || serde.untagged
+ || serde.variant_identifier
+ || serde.field_identifier
+ {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support changing the tagging Serde
generates (`tag`, `content`, `untagged`, `variant_identifier`,
`field_identifier`)",
+ ));
+ }
+ if serde.remote.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `remote`
attribute",
+ ));
+ }
+ if serde.transparent {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support Serde transparent",
Review Comment:
```suggestion
"AvroSchema derive does not support Serde `transparent`
attribute",
```
##########
avro_derive/src/attributes/mod.rs:
##########
@@ -0,0 +1,202 @@
+use darling::FromAttributes;
+use proc_macro2::Span;
+use syn::Attribute;
+
+use crate::{case::RenameRule, darling_to_syn};
+
+pub mod avro;
+pub mod serde;
+
+#[derive(Default)]
+pub struct NamedTypeOptions {
+ pub name: Option<String>,
+ pub namespace: Option<String>,
+ pub doc: Option<String>,
+ pub alias: Vec<String>,
+ pub rename_all: RenameRule,
+}
+
+impl NamedTypeOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
+
avro::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
+
serde::ContainerAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.tag.is_some()
+ || serde.content.is_some()
+ || serde.untagged
+ || serde.variant_identifier
+ || serde.field_identifier
+ {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support changing the tagging Serde
generates (`tag`, `content`, `untagged`, `variant_identifier`,
`field_identifier`)",
+ ));
+ }
+ if serde.remote.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `remote`
attribute",
+ ));
+ }
+ if serde.transparent {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support Serde transparent",
+ ))
+ }
+ if serde.rename_all.deserialize != serde.rename_all.serialize {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support different rename rules for
serializing and deserializing (`rename_all(serialize = \"..\", deserialize =
\"..\")`)"
+ ))
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.rename_all != RenameRule::None && serde.rename_all.serialize
!= avro.rename_all {
+ errors.push(syn::Error::new(
+ span,
+ "#[avro(rename_all = \"..\")] must match #[serde(rename_all =
\"..\")], it's also deprecated. Please use only `#[serde(rename_all =
\"..\")]`",
+ ))
+ }
+
+ if !errors.is_empty() {
+ return Err(errors);
+ }
+
+ Ok(Self {
+ name: avro.name,
+ namespace: avro.namespace,
+ doc: avro.doc,
+ alias: avro.alias,
+ rename_all: serde.rename_all.serialize,
+ })
+ }
+}
+
+pub struct FieldOptions {
+ pub doc: Option<String>,
+ pub default: Option<String>,
+ pub alias: Vec<String>,
+ pub rename: Option<String>,
+ pub skip: bool,
+ pub flatten: bool,
+}
+
+impl FieldOptions {
+ pub fn new(attributes: &[Attribute], span: Span) -> Result<Self,
Vec<syn::Error>> {
+ let avro =
avro::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+ let serde =
serde::FieldAttributes::from_attributes(attributes).map_err(darling_to_syn)?;
+
+ // Collect errors so user gets all feedback at once
+ let mut errors = Vec::new();
+
+ // Check for any Serde attributes that are hard errors
+ if serde.getter.is_some() {
+ errors.push(syn::Error::new(
+ span,
+ "AvroSchema derive does not support the Serde `getter`
attribute",
+ ));
+ }
+
+ // Check for conflicts between Serde and Avro
+ if avro.skip && !serde.skip {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(skip)]` requires `#[serde(skip)]`, it's also
deprecated. Please use only `#[serde(skip)]`"
+ ));
+ }
+ if avro.flatten && !serde.flatten {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(flatten)]` requires `#[serde(flatten)]`, it's also
deprecated. Please use only `#[serde(flatten)]`"
+ ));
+ }
+ // TODO: rename and alias checking can be relaxed with a more complex
check, would require the field name
+ if avro.rename.is_some() && serde.rename != avro.rename {
+ errors.push(syn::Error::new(
+ span,
+ "`#[avro(rename = \"..\")]` must match `#[serde(rename =
\"..\")]`, it's also deprecated. Please use only `#[serde(rename = \"..\")]`"
+ ));
+ }
+ if serde.alias != avro.alias {
Review Comment:
This would fail if only `#[serde(alias= ...)]` is used because `avro.alias`
will be resolved to an empty Vec.
```suggestion
if !avro.alias.is_empty() && serde.alias != avro.alias {
```
--
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]
