martin-g commented on code in PR #2954:
URL: https://github.com/apache/avro/pull/2954#discussion_r1665674276


##########
lang/rust/avro_derive/src/lib.rs:
##########
@@ -281,6 +286,29 @@ fn type_to_schema_expr(ty: &Type) -> Result<TokenStream, 
Vec<syn::Error>> {
     }
 }
 
+fn default_enum_variant(
+    data_enum: &syn::DataEnum,
+    error_span: Span,
+) -> Result<Option<String>, Vec<syn::Error>> {
+    match data_enum
+        .variants
+        .iter()
+        .filter(|v| v.attrs.iter().any(is_default_attr))
+        .collect::<Vec<_>>()[..]
+    {
+        [] => Ok(None),
+        [default] => Ok(Some(default.ident.to_string())),
+        _ => Err(vec![syn::Error::new(

Review Comment:
   ```suggestion
           multiple => Err(vec![syn::Error::new(
   ```
   Could we catch here the values and print them at line 303 ?



##########
lang/rust/avro_derive/src/lib.rs:
##########
@@ -433,6 +461,89 @@ mod tests {
         };
     }
 
+    #[test]
+    fn test_basic_enum_with_default() {
+        let basic_enum = quote! {
+            enum Basic {
+                #[default]
+                A,
+                B,
+                C,
+                D
+            }
+        };
+        match syn::parse2::<DeriveInput>(basic_enum) {
+            Ok(mut input) => {
+                let derived = derive_avro_schema(&mut input);
+                assert!(derived.is_ok());
+                assert_eq!(derived.unwrap().to_string(), quote! {
+                    impl apache_avro::schema::derive::AvroSchemaComponent for 
Basic {
+                        fn get_schema_in_ctxt(
+                            named_schemas: &mut std::collections::HashMap<
+                                apache_avro::schema::Name,
+                                apache_avro::schema::Schema
+                            >,
+                            enclosing_namespace: &Option<String>
+                        ) -> apache_avro::schema::Schema {
+                            let name = apache_avro::schema::Name::new("Basic")
+                                .expect(&format!("Unable to parse schema name 
{}", "Basic")[..])
+                                .fully_qualified_name(enclosing_namespace);
+                            let enclosing_namespace = &name.namespace;
+                            if named_schemas.contains_key(&name) {
+                                apache_avro::schema::Schema::Ref { name: 
name.clone() }
+                            } else {
+                                named_schemas.insert(
+                                    name.clone(),
+                                    apache_avro::schema::Schema::Ref { name: 
name.clone() }
+                                );
+                                
apache_avro::schema::Schema::Enum(apache_avro::schema::EnumSchema {
+                                    name: 
apache_avro::schema::Name::new("Basic").expect(
+                                        &format!("Unable to parse enum name 
for schema {}", "Basic")[..]
+                                    ),
+                                    aliases: None,
+                                    doc: None,
+                                    symbols: vec![
+                                        "A".to_owned(),
+                                        "B".to_owned(),
+                                        "C".to_owned(),
+                                        "D".to_owned()
+                                    ],
+                                    default: Some("A".into()),
+                                    attributes: Default::default(),
+                                })
+                            }
+                        }
+                    }
+                }.to_string());
+            }
+            Err(error) => panic!(
+                "Failed to parse as derive input when it should be able to. 
Error: {error:?}"
+            ),
+        };
+    }
+
+    #[test]
+    fn test_basic_enum_with_default_twice() {
+        let non_basic_enum = quote! {
+            enum Basic {
+                #[default]
+                A,
+                B,
+                #[default]
+                C,
+                D
+            }
+        };
+        match syn::parse2::<DeriveInput>(non_basic_enum) {
+            Ok(mut input) => {
+                assert!(derive_avro_schema(&mut input).is_err())

Review Comment:
   Please assert the error message here.



##########
lang/rust/avro_derive/src/lib.rs:
##########
@@ -433,6 +461,89 @@ mod tests {
         };
     }
 
+    #[test]
+    fn test_basic_enum_with_default() {
+        let basic_enum = quote! {
+            enum Basic {
+                #[default]
+                A,
+                B,
+                C,
+                D
+            }
+        };
+        match syn::parse2::<DeriveInput>(basic_enum) {
+            Ok(mut input) => {
+                let derived = derive_avro_schema(&mut input);
+                assert!(derived.is_ok());
+                assert_eq!(derived.unwrap().to_string(), quote! {
+                    impl apache_avro::schema::derive::AvroSchemaComponent for 
Basic {
+                        fn get_schema_in_ctxt(
+                            named_schemas: &mut std::collections::HashMap<
+                                apache_avro::schema::Name,
+                                apache_avro::schema::Schema
+                            >,
+                            enclosing_namespace: &Option<String>
+                        ) -> apache_avro::schema::Schema {
+                            let name = apache_avro::schema::Name::new("Basic")
+                                .expect(&format!("Unable to parse schema name 
{}", "Basic")[..])
+                                .fully_qualified_name(enclosing_namespace);
+                            let enclosing_namespace = &name.namespace;
+                            if named_schemas.contains_key(&name) {
+                                apache_avro::schema::Schema::Ref { name: 
name.clone() }
+                            } else {
+                                named_schemas.insert(
+                                    name.clone(),
+                                    apache_avro::schema::Schema::Ref { name: 
name.clone() }
+                                );
+                                
apache_avro::schema::Schema::Enum(apache_avro::schema::EnumSchema {
+                                    name: 
apache_avro::schema::Name::new("Basic").expect(
+                                        &format!("Unable to parse enum name 
for schema {}", "Basic")[..]
+                                    ),
+                                    aliases: None,
+                                    doc: None,
+                                    symbols: vec![
+                                        "A".to_owned(),
+                                        "B".to_owned(),
+                                        "C".to_owned(),
+                                        "D".to_owned()
+                                    ],
+                                    default: Some("A".into()),
+                                    attributes: Default::default(),
+                                })
+                            }
+                        }
+                    }
+                }.to_string());
+            }
+            Err(error) => panic!(
+                "Failed to parse as derive input when it should be able to. 
Error: {error:?}"
+            ),
+        };
+    }
+
+    #[test]
+    fn test_basic_enum_with_default_twice() {

Review Comment:
   ```suggestion
       fn avro_3687_basic_enum_with_default_twice() {
   ```



##########
lang/rust/avro_derive/src/lib.rs:
##########
@@ -433,6 +461,89 @@ mod tests {
         };
     }
 
+    #[test]
+    fn test_basic_enum_with_default() {

Review Comment:
   ```suggestion
       fn avro_3687_basic_enum_with_default() {
   ```



-- 
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]

Reply via email to