Support default methods on builtins; rework API

Author: Bromeon

godot-codegen/src/context.rs

@@ -210,7 +210,7 @@ impl<'a> Context<'a> {
         methods: &[JsonBuiltinMethod],
         ctx: &mut Context,
     ) {
-        let builtin_ty = TyName::from_godot(builtin.name.as_str());
+        let builtin_ty = TyName::from_godot_builtin(builtin);
         if special_cases::is_builtin_type_deleted(&builtin_ty) {
             return;
         }

godot-codegen/src/conv/type_conversions.rs

@@ -50,7 +50,7 @@ fn to_hardcoded_rust_ident(full_ty: &GodotTy) -> Option<&str> {
         ("double", None) => "f64",
         ("double", Some(meta)) => panic!("unhandled type double with meta {meta:?}"),
 
-        // Others
+        // Others. Keep in sync with BuiltinClass::from_json().
         ("bool", None) => "bool",
         ("String", None) => "GString",
         ("Array", None) => "VariantArray",

godot-codegen/src/generator/builtins.rs

@@ -23,11 +23,14 @@ use crate::{conv, util, SubmitFn};
 // Shared with native_structures.rs.
 pub struct GeneratedBuiltin {
     pub code: TokenStream,
+    pub has_sidecar_module: bool,
 }
 
 pub struct GeneratedBuiltinModule {
-    pub symbol_ident: Ident,
+    pub outer_builtin: Ident,
+    pub inner_builtin: Ident,
     pub module_name: ModName,
+    pub is_pub_sidecar: bool,
 }
 
 pub fn generate_builtin_class_files(
@@ -49,16 +52,18 @@ pub fn generate_builtin_class_files(
         let module_name = class.mod_name();
 
         let variant_shout_name = util::ident(variant.godot_shout_name());
-        let generated_class = make_builtin_class(class, &variant_shout_name, ctx);
-        let file_contents = generated_class.code;
+        let generated_builtin = make_builtin_class(class, &variant_shout_name, ctx);
+        let file_contents = generated_builtin.code;
 
         let out_path = gen_path.join(format!("{}.rs", module_name.rust_mod));
 
         submit_fn(out_path, file_contents);
 
         modules.push(GeneratedBuiltinModule {
-            symbol_ident: class.inner_name().clone(),
+            outer_builtin: class.name().rust_ty.clone(),
+            inner_builtin: class.inner_name().clone(),
             module_name: module_name.clone(),
+            is_pub_sidecar: generated_builtin.has_sidecar_module,
         });
     }
 
@@ -71,14 +76,21 @@ pub fn generate_builtin_class_files(
 pub fn make_builtin_module_file(classes_and_modules: Vec<GeneratedBuiltinModule>) -> TokenStream {
     let decls = classes_and_modules.iter().map(|m| {
         let GeneratedBuiltinModule {
+            outer_builtin,
+            inner_builtin,
             module_name,
-            symbol_ident,
-            ..
+            is_pub_sidecar,
         } = m;
 
+        // Module is public if it has default extenders (Ex* builders). Enums do not contribute, they're all manually redefined.
+        let vis = is_pub_sidecar.then_some(quote! { pub });
+
+        let doc = format!("Default extenders for builtin type [`{outer_builtin}`][crate::builtin::{outer_builtin}].");
+
         quote! {
-            mod #module_name;
-            pub use #module_name::#symbol_ident;
+            #[doc = #doc]
+            #vis mod #module_name;
+            pub use #module_name::#inner_builtin;
         }
     });
 
@@ -103,30 +115,39 @@ fn make_builtin_class(
     else {
         panic!("Rust type `{godot_name}` categorized wrong")
     };
-    let inner_class = class.inner_name();
+    let inner_builtin = class.inner_name();
 
-    // Note: builders are currently disabled for builtins, even though default parameters exist (e.g. String::substr).
-    // We just use the full signature instead. For outer APIs (user-facing), try to find idiomatic APIs.
+    // Enable `Ex*` builders for builtins to provide consistent API with classes.
+    // By default, builders are only placed on outer methods (for methods that are exposed per special_cases.rs), to reduce codegen.
+    // However it's possible to enable builders on the inner type, too, which is why both `inner_builders` + `outer_builders` exist.
     #[rustfmt::skip]
     let (
-        FnDefinitions { functions: inner_methods, .. },
-        FnDefinitions { functions: outer_methods, .. },
+        FnDefinitions { functions: inner_methods, builders: inner_builders },
+        FnDefinitions { functions: outer_methods, builders: outer_builders },
     ) = make_builtin_methods(class, variant_shout_name, &class.methods, ctx);
 
     let imports = util::make_imports();
     let enums = enums::make_enums(&class.enums, &TokenStream::new());
     let special_constructors = make_special_builtin_methods(class.name(), ctx);
 
-    // mod re_export needed, because class should not appear inside the file module, and we can't re-export private struct as pub
+    // `mod re_export` needed for builder structs to reference the Inner* type, similar to how classes use `re_export` for the class type.
     let code = quote! {
         #imports
 
-        #[repr(transparent)]
-        pub struct #inner_class<'a> {
-            _outer_lifetime: std::marker::PhantomData<&'a ()>,
-            sys_ptr: sys::GDExtensionTypePtr,
+        pub(super) mod re_export {
+            use super::*;
+
+            // Do *not* try to limit visibility, because inner types are used in PackedArrayElement::Inner<'a> associated type.
+            // Need to redesign that trait otherwise, and split into private/public parts.
+            #[doc(hidden)]
+            #[repr(transparent)]
+            pub struct #inner_builtin<'inner> {
+                pub(super) _outer_lifetime: std::marker::PhantomData<&'inner ()>,
+                pub(super) sys_ptr: sys::GDExtensionTypePtr,
+            }
         }
-        impl<'a> #inner_class<'a> {
+
+        impl<'inner> re_export::#inner_builtin<'inner> {
             pub fn from_outer(outer: &#outer_class) -> Self {
                 Self {
                     _outer_lifetime: std::marker::PhantomData,
@@ -136,16 +157,29 @@ fn make_builtin_class(
             #special_constructors
             #inner_methods
         }
+
+        // Re-export Inner* type for convenience.
+        pub use re_export::#inner_builtin;
+
         // Selected APIs appear directly in the outer class.
         impl #outer_class {
             #outer_methods
         }
 
+        #inner_builders
+        #outer_builders
         #enums
     };
     // note: TypePtr -> ObjectPtr conversion OK?
 
-    GeneratedBuiltin { code }
+    // If any exposed builders are present, generate a sidecar module for the builtin.
+    // Do not care about inner Ex* builders, or builtin enums (enums are manually defined).
+    let has_sidecar_module = !outer_builders.is_empty();
+
+    GeneratedBuiltin {
+        code,
+        has_sidecar_module,
+    }
 }
 
 /// Returns 2 definition packs, one for the `Inner*` methods, and one for those ending up directly in the public-facing (outer) class.

godot-codegen/src/generator/default_parameters.rs

@@ -33,7 +33,7 @@ pub fn make_function_definition_with_defaults(
     let default_parameter_usage = format!("To set the default parameters, use [`Self::{extended_fn_name}`] and its builder methods.  See [the book](https://godot-rust.github.io/book/godot-api/functions.html#default-parameters) for detailed usage instructions.");
     let vis = functions_common::make_vis(sig.is_private());
 
-    let (builder_doc, surround_class_prefix) = make_extender_doc(sig, &extended_fn_name);
+    let (builder_doc, surround_class_path) = make_extender_doc(sig, &extended_fn_name);
 
     let ExtenderReceiver {
         object_fn_param,
@@ -62,26 +62,26 @@ pub fn make_function_definition_with_defaults(
     let return_decl = &sig.return_value().decl;
 
     // If either the builder has a lifetime (non-static/global method), or one of its parameters is a reference,
-    // then we need to annotate the _ex() function with an explicit lifetime. Also adjust &self -> &'a self.
+    // then we need to annotate the _ex() function with an explicit lifetime. Also adjust &self -> &'ex self.
     let receiver_self = &code.receiver.self_prefix;
     let simple_receiver_param = &code.receiver.param;
-    let extended_receiver_param = &code.receiver.param_lifetime_a;
+    let extended_receiver_param = &code.receiver.param_lifetime_ex;
 
     let builders = quote! {
         #[doc = #builder_doc]
         #[must_use]
         #cfg_attributes
-        #vis struct #builder_ty<'a> {
-            _phantom: std::marker::PhantomData<&'a ()>,
+        #vis struct #builder_ty<'ex> {
+            _phantom: std::marker::PhantomData<&'ex ()>,
             #( #builder_field_decls, )*
         }
 
         // #[allow] exceptions:
         // - wrong_self_convention:     to_*() and from_*() are taken from Godot
         // - redundant_field_names:     'value: value' is a possible initialization pattern
-        // - needless-update:           Remainder expression '..self' has nothing left to change
+        // - needless_update:           Remainder expression '..self' has nothing left to change
         #[allow(clippy::wrong_self_convention, clippy::redundant_field_names, clippy::needless_update)]
-        impl<'a> #builder_ty<'a> {
+        impl<'ex> #builder_ty<'ex> {
             fn new(
                 //#object_param
                 #( #builder_ctor_params, )*
@@ -98,7 +98,7 @@ pub fn make_function_definition_with_defaults(
             #[inline]
             pub fn done(self) #return_decl {
                 let Self { _phantom, #( #builder_field_names, )* } = self;
-                #surround_class_prefix #full_fn_name(
+                #surround_class_path::#full_fn_name(
                     #( #full_fn_args, )* // includes `surround_object` if present
                 )
             }
@@ -122,10 +122,10 @@ pub fn make_function_definition_with_defaults(
         // _ex() function:
         // Lifetime is set if any parameter is a reference OR if the method is not static/global (and thus can refer to self).
         #[inline]
-        #vis fn #extended_fn_name<'a> (
+        #vis fn #extended_fn_name<'ex> (
             #extended_receiver_param
             #( #class_method_required_params_lifetimed, )*
-        ) -> #builder_ty<'a> {
+        ) -> #builder_ty<'ex> {
             #builder_ty::new(
                 #object_arg
                 #( #class_method_required_args, )*
@@ -137,11 +137,30 @@ pub fn make_function_definition_with_defaults(
 }
 
 pub fn function_uses_default_params(sig: &dyn Function) -> bool {
-    sig.params().iter().any(|arg| arg.default_value.is_some())
+    // For builtin types, strip "Inner" prefix, while avoiding collision with classes that might start with "Inner".
+    let class_or_builtin = sig.surrounding_class().map(|ty| {
+        let rust_name = ty.rust_ty.to_string();
+
+        // If it’s builtin and starts with "Inner", drop that prefix; otherwise keep original string.
+        match (sig.is_builtin(), rust_name.strip_prefix("Inner")) {
+            (true, Some(rest)) => rest.to_string(),
+            _ => rust_name,
+        }
+    });
+
+    let fn_declares_default_params = sig.params().iter().any(|arg| arg.default_value.is_some())
         && !special_cases::is_method_excluded_from_default_params(
-            sig.surrounding_class(),
+            class_or_builtin.as_deref(),
             sig.name(),
-        )
+        );
+
+    // For builtins, only generate `Ex*` builders if the method is exposed on the outer type.
+    // This saves on code generation and compile time for `Inner*` methods.
+    if fn_declares_default_params && sig.is_builtin() {
+        return sig.is_exposed_outer_builtin();
+    }
+
+    fn_declares_default_params
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
@@ -179,11 +198,17 @@ fn make_extender_doc(sig: &dyn Function, extended_fn_name: &Ident) -> (String, T
     #[allow(clippy::uninlined_format_args)]
     match sig.surrounding_class() {
         Some(TyName { rust_ty, .. }) => {
-            surround_class_prefix = quote! { re_export::#rust_ty:: };
+            surround_class_prefix = make_qualified_type(sig, rust_ty, true);
+            let path = if sig.is_builtin() {
+                format!("crate::builtin::{rust_ty}")
+            } else {
+                format!("super::{rust_ty}")
+            };
             builder_doc = format!(
-                "Default-param extender for [`{class}::{method}`][super::{class}::{method}].",
+                "Default-param extender for [`{class}::{method}`][{path}::{method}].",
                 class = rust_ty,
                 method = extended_fn_name,
+                path = path,
             );
         }
         None => {
@@ -213,13 +238,13 @@ fn make_extender_receiver(sig: &dyn Function) -> ExtenderReceiver {
     // Only add it if the method is not global or static.
     match sig.surrounding_class() {
         Some(surrounding_class) if !sig.qualifier().is_static_or_global() => {
-            let class = &surrounding_class.rust_ty;
+            let ty = make_qualified_type(sig, &surrounding_class.rust_ty, false);
 
             ExtenderReceiver {
                 object_fn_param: Some(FnParam {
                     name: ident("surround_object"),
                     type_: RustTy::ExtenderReceiver {
-                        tokens: quote! { &'a #builder_mut re_export::#class },
+                        tokens: quote! { &'ex #builder_mut #ty },
                     },
                     default_value: None,
                 }),
@@ -331,3 +356,23 @@ fn make_extender(
         class_method_required_args,
     }
 }
+
+/// Returns a qualified type path for builtin or class.
+///
+/// Type categories:
+/// - Exposed outer builtins (like `GString`): direct type reference without `re_export::`.
+/// - `Inner\*` types (like `InnerString`): `re_export::Type<'ex>`, with lifetime if `with_inner_lifetime` is true.
+/// - Classes (like `Node`): `re_export::Type` without lifetime.
+fn make_qualified_type(
+    sig: &dyn Function,
+    class_or_builtin: &Ident,
+    with_inner_lifetime: bool,
+) -> TokenStream {
+    if sig.is_exposed_outer_builtin() {
+        quote! { #class_or_builtin }
+    } else if with_inner_lifetime && sig.is_builtin() {
+        quote! { re_export::#class_or_builtin<'ex> }
+    } else {
+        quote! { re_export::#class_or_builtin }
+    }
+}