Merge #177

177: More complete #[export] implementation r=Bromeon a=ttencate

Also:

- Remove unused `property` argument.
- Rename arguments `getter` and `setter` to `get` and `set` to stay closer to GDScript and save keystrokes.
- Check at compilation time that the referenced getter and setter actually exist (otherwise Godot gives a cryptic "invalid get/set index" error).

See #3.

TBD:

- [ ] ~~`strip_quotes` should go away, not sure if it even works correctly if using e.g. raw string literals. Use an actual Rust parser? Omit the quotes from the argument instead, i.e. `get = get_my_field` instead of `get = "get_my_field"`?~~ See discussion below.
- [ ] ~~Make `KvParser::parse` take a closure so we can check that all fields have been consumed~~ See discussion below.
- [x] Omitting one of getter/setter should make field write/read only
- [x] Use `get`/`set` without arguments to generate a default one
- [x] Make generated getters and setters `pub` since they're public to Godot anyway

Co-authored-by: Thomas ten Cate <ttencate@gmail.com>

Author: bors[bot]

godot-core/src/obj/traits.rs

@@ -13,6 +13,8 @@ use godot_ffi as sys;
 /// Makes `T` eligible to be managed by Godot and stored in [`Gd<T>`][crate::obj::Gd] pointers.
 ///
 /// The behavior of types implementing this trait is influenced by the associated types; check their documentation for information.
+///
+/// You wouldn't usually implement this trait yourself; use the [`GodotClass`](godot_macros::GodotClass) derive macro instead.
 pub trait GodotClass: 'static
 where
     Self: Sized,

godot-macros/src/derive_godot_class.rs

@@ -4,7 +4,7 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use crate::util::{bail, ident, KvParser};
+use crate::util::{bail, ident, string_lit_contents, KvParser, KvValue};
 use crate::ParseResult;
 use proc_macro2::{Ident, Punct, TokenStream};
 use quote::{format_ident, quote};
@@ -68,7 +68,7 @@ fn parse_struct_attributes(class: &Struct) -> ParseResult<ClassAttributes> {
     let mut base_ty = ident("RefCounted");
     let mut has_generated_init = false;
 
-    // #[func] attribute on struct
+    // #[class] attribute on struct
     if let Some(mut parser) = KvParser::parse(&class.attributes, "class")? {
         if let Some(base) = parser.handle_ident("base")? {
             base_ty = base;
@@ -174,11 +174,45 @@ impl Field {
 
 struct ExportedField {
     field: Field,
-    getter: String,
-    setter: String,
+    getter: GetterSetter,
+    setter: GetterSetter,
     hint: Option<ExportHint>,
 }
 
+#[derive(Clone, Debug, Eq, PartialEq)]
+enum GetterSetter {
+    /// Getter/setter should be omitted, field is write/read only.
+    Omitted,
+    /// Trivial getter/setter should be autogenerated.
+    Generated,
+    /// Getter/setter is hand-written by the user, and here is its name.
+    Custom(String),
+}
+
+impl GetterSetter {
+    fn parse(parser: &mut KvParser, key: &str) -> ParseResult<Self> {
+        Ok(match parser.handle_any(key) {
+            // No `get` argument
+            None => GetterSetter::Omitted,
+            // `get` without value
+            Some(KvValue::None) => GetterSetter::Generated,
+            // `get = literal`
+            Some(KvValue::Lit(name_lit)) => {
+                let Some(name) = string_lit_contents(&name_lit) else {
+                    return bail(format!("argument to {key} must be a string literal, got: {name_lit}"), parser.span());
+                };
+                GetterSetter::Custom(name)
+            }
+            Some(KvValue::Ident(ident)) => {
+                return bail(
+                    format!("argument to {key} must be a string, got: {ident}"),
+                    parser.span(),
+                );
+            }
+        })
+    }
+}
+
 #[derive(Clone)]
 struct ExportHint {
     hint_type: Ident,
@@ -196,8 +230,12 @@ impl ExportHint {
 
 impl ExportedField {
     pub fn new_from_kv(field: Field, parser: &mut KvParser) -> ParseResult<ExportedField> {
-        let getter = parser.handle_lit_required("getter")?;
-        let setter = parser.handle_lit_required("setter")?;
+        let mut getter = GetterSetter::parse(parser, "get")?;
+        let mut setter = GetterSetter::parse(parser, "set")?;
+        if getter == GetterSetter::Omitted && setter == GetterSetter::Omitted {
+            getter = GetterSetter::Generated;
+            setter = GetterSetter::Generated;
+        }
 
         let hint = parser
             .handle_ident("hint")?
@@ -265,51 +303,109 @@ fn make_deref_impl(class_name: &Ident, fields: &Fields) -> TokenStream {
 }
 
 fn make_exports_impl(class_name: &Ident, fields: &Fields) -> TokenStream {
-    let export_tokens = fields
-        .exported_fields
-        .iter()
-        .map(|exported_field: &ExportedField| {
-            use std::str::FromStr;
-            let name = exported_field.field.name.to_string();
-            let getter = proc_macro2::Literal::from_str(&exported_field.getter).unwrap();
-            let setter = proc_macro2::Literal::from_str(&exported_field.setter).unwrap();
-            let field_type = exported_field.field.ty.clone();
-
-            let ExportHint {
-                hint_type,
-                description,
-            } = exported_field.hint.clone().unwrap_or_else(ExportHint::none);
-
-            // trims '"' and '\' from both ends of the hint description.
-            let description = description.trim_matches(|c| c == '\\' || c == '"');
-
-            quote! {
-                use ::godot::builtin::meta::VariantMetadata;
-
-                let class_name = ::godot::builtin::StringName::from(#class_name::CLASS_NAME);
-                let property_info = ::godot::builtin::meta::PropertyInfo::new(
-                    <#field_type>::variant_type(),
-                    ::godot::builtin::meta::ClassName::of::<#class_name>(),
-                    ::godot::builtin::StringName::from(#name),
-                    ::godot::engine::global::PropertyHint::#hint_type,
-                    GodotString::from(#description),
+    let mut getter_setter_impls = Vec::new();
+    let mut export_tokens = Vec::with_capacity(fields.exported_fields.len());
+
+    for exported_field in &fields.exported_fields {
+        let field_name = exported_field.field.name.to_string();
+        let field_ident = ident(&field_name);
+        let field_type = exported_field.field.ty.clone();
+
+        let ExportHint {
+            hint_type,
+            description,
+        } = exported_field.hint.clone().unwrap_or_else(ExportHint::none);
+
+        // trims '"' and '\' from both ends of the hint description.
+        let description = description.trim_matches(|c| c == '\\' || c == '"');
+
+        let getter_name;
+        match &exported_field.getter {
+            GetterSetter::Omitted => {
+                getter_name = "".to_owned();
+            }
+            GetterSetter::Generated => {
+                getter_name = format!("get_{field_name}");
+                let getter_ident = ident(&getter_name);
+                let signature = quote! {
+                    fn #getter_ident(&self) -> #field_type
+                };
+                getter_setter_impls.push(quote! {
+                    pub #signature {
+                        self.#field_ident
+                    }
+                });
+                export_tokens.push(quote! {
+                    ::godot::private::gdext_register_method!(#class_name, #signature);
+                });
+            }
+            GetterSetter::Custom(name) => {
+                getter_name = name.clone();
+                let getter_ident = ident(&getter_name);
+                export_tokens.push(make_existence_check(&getter_ident));
+            }
+        }
+
+        let setter_name;
+        match &exported_field.setter {
+            GetterSetter::Omitted => {
+                setter_name = "".to_owned();
+            }
+            GetterSetter::Generated => {
+                setter_name = format!("set_{field_name}");
+                let setter_ident = ident(&setter_name);
+                let signature = quote! {
+                    fn #setter_ident(&mut self, #field_ident: #field_type)
+                };
+                getter_setter_impls.push(quote! {
+                    pub #signature {
+                        self.#field_ident = #field_ident;
+                    }
+                });
+                export_tokens.push(quote! {
+                    ::godot::private::gdext_register_method!(#class_name, #signature);
+                });
+            }
+            GetterSetter::Custom(name) => {
+                setter_name = name.clone();
+                let setter_ident = ident(&setter_name);
+                export_tokens.push(make_existence_check(&setter_ident));
+            }
+        };
+
+        export_tokens.push(quote! {
+            use ::godot::builtin::meta::VariantMetadata;
+
+            let class_name = ::godot::builtin::StringName::from(#class_name::CLASS_NAME);
+
+            let property_info = ::godot::builtin::meta::PropertyInfo::new(
+                <#field_type>::variant_type(),
+                ::godot::builtin::meta::ClassName::of::<#class_name>(),
+                ::godot::builtin::StringName::from(#field_name),
+                ::godot::engine::global::PropertyHint::#hint_type,
+                ::godot::builtin::GodotString::from(#description),
+            );
+            let property_info_sys = property_info.property_sys();
+
+            let getter_name = ::godot::builtin::StringName::from(#getter_name);
+            let setter_name = ::godot::builtin::StringName::from(#setter_name);
+            unsafe {
+                ::godot::sys::interface_fn!(classdb_register_extension_class_property)(
+                    ::godot::sys::get_library(),
+                    class_name.string_sys(),
+                    std::ptr::addr_of!(property_info_sys),
+                    setter_name.string_sys(),
+                    getter_name.string_sys(),
                 );
-                let property_info_sys = property_info.property_sys();
-
-                let getter_string_name = ::godot::builtin::StringName::from(#getter);
-                let setter_string_name = ::godot::builtin::StringName::from(#setter);
-                unsafe {
-                    ::godot::sys::interface_fn!(classdb_register_extension_class_property)(
-                        ::godot::sys::get_library(),
-                        class_name.string_sys(),
-                        std::ptr::addr_of!(property_info_sys),
-                        setter_string_name.string_sys(),
-                        getter_string_name.string_sys(),
-                    );
-                }
             }
         });
+    }
+
     quote! {
+        impl #class_name {
+            #(#getter_setter_impls)*
+        }
+
         impl ::godot::obj::cap::ImplementsGodotExports for #class_name {
             fn __register_exports() {
                 #(
@@ -321,3 +417,12 @@ fn make_exports_impl(class_name: &Ident, fields: &Fields) -> TokenStream {
         }
     }
 }
+
+/// Checks at compile time that a function with the given name exists on `Self`.
+#[must_use]
+fn make_existence_check(ident: &Ident) -> TokenStream {
+    quote! {
+        #[allow(path_statements)]
+        Self::#ident;
+    }
+}

godot-macros/src/lib.rs

@@ -93,7 +93,137 @@ mod godot_api;
 mod itest;
 mod util;
 
-#[proc_macro_derive(GodotClass, attributes(class, property, export, base, signal))]
+/// Derive macro for [`GodotClass`](godot_core::obj::GodotClass) on structs. You should normally use
+/// this macro, rather than implement `GodotClass` manually for your type.
+///
+/// # Construction
+///
+/// To generate a constructor that will let you call `MyStruct.new()` from GDScript, annotate your
+/// struct with `#[class(init)]`:
+///
+/// ```
+/// # use godot_macros::GodotClass;
+/// #[derive(GodotClass)]
+/// #[class(init)]
+/// struct MyStruct {
+///     // ...
+/// }
+/// ```
+///
+/// # Inheritance
+///
+/// Unlike C++, Rust doesn't really have inheritance, but the GDExtension API lets us "inherit"
+/// from a built-in engine class.
+///
+/// By default, classes created with this library inherit from `RefCounted`.
+///
+/// To specify a different class to inherit from, add `#[class(base = Base)]` as an annotation on
+/// your `struct`:
+///
+/// ```
+/// use godot::prelude::*;
+///
+/// #[derive(GodotClass)]
+/// #[class(base = Node2D)]
+/// struct MyStruct {
+///     // ...
+/// }
+/// ```
+///
+/// If you need a reference to the base class, you can add a field of type `Gd<Base>` and annotate
+/// it with `#[base]`:
+///
+/// ```
+/// use godot::prelude::*;
+///
+/// #[derive(GodotClass)]
+/// #[class(base = Node2D)]
+/// struct MyStruct {
+///     #[base]
+///     base: Gd<Node2D>,
+/// }
+/// ```
+///
+/// # Exported properties
+///
+/// In GDScript, there is a distinction between
+/// [properties](https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/gdscript_basics.html#properties-setters-and-getters)
+/// (fields with a `get` or `set` declaration) and
+/// [exports](https://docs.godotengine.org/en/stable/tutorials/scripting/gdscript/gdscript_exports.html)
+/// (fields annotated with `@export`). In the GDExtension API, these two concepts are merged into
+/// one.
+///
+/// You can export fields of your struct using the `#[export]` annotation:
+///
+/// ```
+/// use godot::prelude::*;
+///
+/// #[derive(GodotClass)]
+/// struct MyStruct {
+///     #[export]
+///     my_field: i64,
+/// }
+/// ```
+///
+/// This makes the field accessible in GDScript using `my_struct.my_field` syntax. Additionally, it
+/// generates a trivial getter and setter named `get_my_field` and `set_my_field`, respectively.
+/// These are `pub` in Rust, since they're exposed from GDScript anyway.
+///
+/// If you want to implement your own getter and/or setter, write those as a function on your Rust
+/// type, expose it using `#[func]`, and annotate the field with
+/// `#[export(get = "...", set = "...")]`:
+///
+/// ```
+/// use godot::prelude::*;
+///
+/// #[derive(GodotClass)]
+/// struct MyStruct {
+///     #[export(get = "get_my_field", set = "set_my_field")]
+///     my_field: i64,
+/// }
+///
+/// #[godot_api]
+/// impl MyStruct {
+///     #[func]
+///     pub fn get_my_field(&self) -> i64 {
+///         self.my_field
+///     }
+///
+///     #[func]
+///     pub fn set_my_field(&mut self, value: i64) {
+///         self.my_field = value;
+///     }
+/// }
+/// ```
+///
+/// If you specify only `get`, no setter is generated, making the field read-only. If you specify
+/// only `set`, no getter is generated, making the field write-only (rarely useful). To add a
+/// generated getter or setter in these cases anyway, use `get` or `set` without a value:
+///
+/// ```
+/// use godot::prelude::*;
+///
+/// #[derive(GodotClass)]
+/// struct MyStruct {
+///     // Default getter, custom setter.
+///     #[export(get, set = "set_my_field")]
+///     my_field: i64,
+/// }
+///
+/// #[godot_api]
+/// impl MyStruct {
+///     #[func]
+///     pub fn set_my_field(&mut self, value: i64) {
+///         self.my_field = value;
+///     }
+/// }
+/// ```
+///
+/// # Signals
+///
+/// The `#[signal]` attribute is accepted, but not yet implemented. See [issue
+/// #8](https://github.com/godot-rust/gdext/issues/8).
+#[proc_macro_derive(GodotClass, attributes(class, export, base, signal))]
 pub fn derive_native_class(input: TokenStream) -> TokenStream {
     translate(input, derive_godot_class::transform)
 }