Unverified Commit 32258e2e authored by Copilot's avatar Copilot Committed by GitHub
Browse files

Add Default trait to all Configuration types (#435)



* Initial plan

* Add Default derive/impl to all Configuration types

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

* Add tests for Configuration types Default and serialization

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

* Address code review feedback

Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>

---------

Co-authored-by: default avatarcopilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: default avatarNugine <30099658+Nugine@users.noreply.github.com>
parent 1bb770ee
Loading
Loading
Loading
Loading
+108 −2
Original line number Diff line number Diff line
@@ -437,6 +437,58 @@ fn collect_types_needing_serde(rust_types: &RustTypes) -> BTreeSet<String> {
    types_needing_serde
}

fn collect_types_needing_custom_default(rust_types: &RustTypes) -> BTreeSet<String> {
    let mut types_needing_custom_default = BTreeSet::new();

    // Start with Configuration types that can't derive Default
    for (name, rust_type) in rust_types {
        if name.ends_with("Configuration") {
            if let rust::Type::Struct(ty) = rust_type {
                if !can_derive_default(ty, rust_types) {
                    // Add this type and all its struct dependencies
                    collect_struct_dependencies(name, rust_types, &mut types_needing_custom_default);
                }
            }
        }
    }

    types_needing_custom_default
}

fn collect_struct_dependencies(type_name: &str, rust_types: &RustTypes, result: &mut BTreeSet<String>) {
    // Avoid infinite recursion
    if result.contains(type_name) {
        return;
    }

    // Only add this type if it can't derive Default
    if let Some(rust::Type::Struct(s)) = rust_types.get(type_name) {
        if !can_derive_default(s, rust_types) {
            result.insert(type_name.to_owned());

            // Recursively add struct dependencies that also can't derive Default
            for field in &s.fields {
                // Skip optional fields and list/map types (they already have Default)
                if field.option_type {
                    continue;
                }

                if let Some(field_type) = rust_types.get(&field.type_) {
                    match field_type {
                        rust::Type::Struct(_) => {
                            collect_struct_dependencies(&field.type_, rust_types, result);
                        }
                        rust::Type::List(_) | rust::Type::Map(_) => {
                            // Lists and maps already have Default, skip
                        }
                        _ => {}
                    }
                }
            }
        }
    }
}

fn collect_type_dependencies(type_name: &str, rust_types: &RustTypes, result: &mut BTreeSet<String>) {
    // Avoid infinite recursion
    if result.contains(type_name) {
@@ -476,6 +528,9 @@ pub fn codegen(rust_types: &RustTypes, ops: &Operations, patch: Option<Patch>) {
    // Collect types that need serde derives (Configuration types and their dependencies)
    let types_needing_serde = collect_types_needing_serde(rust_types);

    // Collect types that need custom Default implementations
    let types_needing_custom_default = collect_types_needing_custom_default(rust_types);

    g([
        "#![allow(clippy::empty_structs_with_brackets)]",
        "#![allow(clippy::too_many_lines)]",
@@ -515,7 +570,8 @@ pub fn codegen(rust_types: &RustTypes, ops: &Operations, patch: Option<Patch>) {
            }
            rust::Type::Struct(ty) => {
                let needs_serde = types_needing_serde.contains(&ty.name);
                codegen_struct(ty, rust_types, ops, needs_serde);
                let needs_custom_default = types_needing_custom_default.contains(&ty.name);
                codegen_struct(ty, rust_types, ops, needs_serde, needs_custom_default);
            }
            rust::Type::StructEnum(ty) => {
                let needs_serde = types_needing_serde.contains(&ty.name);
@@ -539,7 +595,7 @@ pub fn codegen(rust_types: &RustTypes, ops: &Operations, patch: Option<Patch>) {
    }
}

fn codegen_struct(ty: &rust::Struct, rust_types: &RustTypes, ops: &Operations, needs_serde: bool) {
fn codegen_struct(ty: &rust::Struct, rust_types: &RustTypes, ops: &Operations, needs_serde: bool, needs_custom_default: bool) {
    codegen_doc(ty.doc.as_deref());

    {
@@ -616,6 +672,11 @@ fn codegen_struct(ty: &rust::Struct, rust_types: &RustTypes, ops: &Operations, n
        g!("}}");
    }

    // Add custom Default implementation for types that need it
    if needs_custom_default {
        codegen_custom_default(ty, rust_types);
    }

    if is_op_input(&ty.name, ops) {
        g!("impl {} {{", ty.name);

@@ -628,6 +689,45 @@ fn codegen_struct(ty: &rust::Struct, rust_types: &RustTypes, ops: &Operations, n
    }
}

fn codegen_custom_default(ty: &rust::Struct, rust_types: &RustTypes) {
    g!("impl Default for {} {{", ty.name);
    g!("fn default() -> Self {{");
    g!("Self {{");
    for field in &ty.fields {
        if field.option_type {
            g!("{}: None,", field.name);
        } else if let Some(rust_type) = rust_types.get(&field.type_) {
            match rust_type {
                rust::Type::List(_) | rust::Type::Map(_) => {
                    g!("{}: default(),", field.name);
                }
                rust::Type::Alias(_) => {
                    // Type aliases to primitives implement Default
                    g!("{}: default(),", field.name);
                }
                rust::Type::StrEnum(_) => {
                    // StrEnum types need a string value, use empty string
                    g!("{}: String::new().into(),", field.name);
                }
                rust::Type::Struct(_) => {
                    // Try to use Default::default() for structs
                    g!("{}: default(),", field.name);
                }
                _ => {
                    g!("{}: default(),", field.name);
                }
            }
        } else {
            // Unknown type, try Default::default()
            g!("{}: default(),", field.name);
        }
    }
    g!("}}");
    g!("}}");
    g!("}}");
    g!();
}

fn codegen_str_enum(ty: &rust::StrEnum, _rust_types: &RustTypes, needs_serde: bool) {
    codegen_doc(ty.doc.as_deref());
    if needs_serde {
@@ -858,6 +958,12 @@ fn can_derive_default(ty: &rust::Struct, rust_types: &RustTypes) -> bool {
            }
            rust::Type::List(_) => return true,
            rust::Type::Map(_) => return true,
            rust::Type::Alias(alias_ty) => {
                // Type aliases to primitive types that have Default
                if matches!(alias_ty.type_.as_str(), "String" | "bool" | "i32" | "i64" | "f32" | "f64") {
                    return true;
                }
            }
            _ => {}
        }

+169 −87

File changed.

Preview size limit exceeded, changes collapsed.

+169 −87

File changed.

Preview size limit exceeded, changes collapsed.

+35 −1
Original line number Diff line number Diff line
use s3s::dto::GetObjectInput;
use s3s::dto::{
    AnalyticsConfiguration, BucketLifecycleConfiguration, GetObjectInput, IntelligentTieringConfiguration,
    InventoryConfiguration, LambdaFunctionConfiguration, MetadataTableConfiguration, MetricsConfiguration, QueueConfiguration,
    ReplicationConfiguration, RequestPaymentConfiguration, TopicConfiguration,
};

#[test]
fn builder() {
@@ -12,3 +16,33 @@ fn builder() {
    assert_eq!(input.bucket, "hello");
    assert_eq!(input.key, "world");
}

#[test]
fn configuration_types_have_default() {
    // Test the two types mentioned in the issue
    let _ = BucketLifecycleConfiguration::default();
    let _ = ReplicationConfiguration::default();

    // Test a few more Configuration types
    let _ = AnalyticsConfiguration::default();
    let _ = IntelligentTieringConfiguration::default();
    let _ = InventoryConfiguration::default();
    let _ = LambdaFunctionConfiguration::default();
    let _ = MetadataTableConfiguration::default();
    let _ = MetricsConfiguration::default();
    let _ = QueueConfiguration::default();
    let _ = RequestPaymentConfiguration::default();
    let _ = TopicConfiguration::default();
}

#[test]
fn configuration_serialization() {
    // Test that Configuration types can be serialized and deserialized
    let config = BucketLifecycleConfiguration::default();
    let json = serde_json::to_string(&config).expect("should serialize");
    let _: BucketLifecycleConfiguration = serde_json::from_str(&json).expect("should deserialize");

    let config = ReplicationConfiguration::default();
    let json = serde_json::to_string(&config).expect("should serialize");
    let _: ReplicationConfiguration = serde_json::from_str(&json).expect("should deserialize");
}