Try #177:

Author: bors[bot]

godot-core/src/obj/traits.rs

@@ -13,6 +13,9 @@ 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; see the [documentation for the
+/// `derive(GodotClass)` macro](godot_macros::GodotClass).
 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,46 @@ 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 name = string_lit_contents(&name_lit).ok_or_else(|| {
+                    venial::Error::new(format!(
+                        "argument to {key} must be a string literal, got: {name_lit}"
+                    ))
+                })?;
+                GetterSetter::Custom(name)
+            }
+            Some(KvValue::Ident(ident)) => {
+                return Err(venial::Error::new(format!(
+                    "argument to {key} must be a string, got: {ident}"
+                )));
+            }
+        })
+    }
+}
+
 #[derive(Clone)]
 struct ExportHint {
     hint_type: Ident,
@@ -196,8 +231,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 +304,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(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(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,
+                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 +418,32 @@ 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 existence_check(ident: &Ident) -> TokenStream {
+    quote! {
+        #[allow(path_statements)]
+        Self::#ident;
+    }
+}
+
+/// Tests that the proc-macro doesn't panic. This is helpful because `RUST_BACKTRACE=1` does not
+/// affect panics in macro invocations (e.g. in our integration tests), but it works fine in unit
+/// tests.
+#[test]
+fn does_not_panic() {
+    let decl = venial::parse_declaration(quote! {
+        #[class(base=Node)]
+        struct HasProperty {
+            #[base]
+            base: Base<Node>,
+            #[export]
+            int_val_default: i32,
+            #[export(get = "get_int_val", set = "set_int_val")]
+            int_val: i32,
+        }
+    })
+    .unwrap();
+    transform(decl).unwrap();
+}