Merge #203

203: Make `get` and `set` arguments take identifiers instead of strings r=Bromeon a=ttencate Refactors `KvParser` to accept some expressions, as long as they don't contain a comma at the top level outside any pair of `[]`, `()` or `{}`. Also tightens up spans in error messages quite a bit. Prelude for #199. Co-authored-by: Thomas ten Cate <[email protected]>
godot-rust · Mar 27, 2023 · c1c2d60 · c1c2d60
2 parents afa2e59 + 1b32155
commit c1c2d60
Show file tree

Hide file tree

Showing 4 changed files with 331 additions and 258 deletions.
diff --git a/godot-macros/src/derive_godot_class.rs b/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, string_lit_contents, KvParser, KvValue};
+use crate::util::{bail, ident, KvParser};
 use crate::ParseResult;
 use proc_macro2::{Ident, Punct, TokenStream};
 use quote::{format_ident, quote};
@@ -185,45 +185,36 @@ enum GetterSetter {
     Omitted,
     /// Trivial getter/setter should be autogenerated.
     Generated,
-    /// Getter/setter is hand-written by the user, and here is its name.
-    Custom(String),
+    /// Getter/setter is hand-written by the user, and here is its identifier.
+    Custom(Ident),
 }
 
 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(),
-                );
-            }
+            Some(value) => match value {
+                // `get` without value
+                None => GetterSetter::Generated,
+                // `get = expr`
+                Some(value) => GetterSetter::Custom(value.ident()?),
+            },
         })
     }
 }
 
 #[derive(Clone)]
 struct ExportHint {
     hint_type: Ident,
-    description: String,
+    description: TokenStream,
 }
 
 impl ExportHint {
     fn none() -> Self {
         Self {
             hint_type: ident("PROPERTY_HINT_NONE"),
-            description: "".to_string(),
+            description: quote!(""),
         }
     }
 }
@@ -242,7 +233,7 @@ impl ExportedField {
             .map(|hint_type| {
                 Ok(ExportHint {
                     hint_type,
-                    description: parser.handle_lit_required("hint_desc")?,
+                    description: parser.handle_expr_required("hint_desc")?,
                 })
             })
             .transpose()?;
@@ -316,9 +307,6 @@ fn make_exports_impl(class_name: &Ident, fields: &Fields) -> TokenStream {
             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 => {
@@ -339,10 +327,9 @@ fn make_exports_impl(class_name: &Ident, fields: &Fields) -> TokenStream {
                     ::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));
+            GetterSetter::Custom(getter_ident) => {
+                getter_name = getter_ident.to_string();
+                export_tokens.push(make_existence_check(getter_ident));
             }
         }
 
@@ -366,10 +353,9 @@ fn make_exports_impl(class_name: &Ident, fields: &Fields) -> TokenStream {
                     ::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));
+            GetterSetter::Custom(setter_ident) => {
+                setter_name = setter_ident.to_string();
+                export_tokens.push(make_existence_check(setter_ident));
             }
         };
 

diff --git a/godot-macros/src/lib.rs b/godot-macros/src/lib.rs
@@ -171,14 +171,14 @@ mod util;
 ///
 /// 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 = "...")]`:
+/// `#[export(get = ..., set = ...)]`:
 ///
 /// ```
 /// use godot::prelude::*;
 ///
 /// #[derive(GodotClass)]
 /// struct MyStruct {
-///     #[export(get = "get_my_field", set = "set_my_field")]
+///     #[export(get = get_my_field, set = set_my_field)]
 ///     my_field: i64,
 /// }
 ///
@@ -206,7 +206,7 @@ mod util;
 /// #[derive(GodotClass)]
 /// struct MyStruct {
 ///     // Default getter, custom setter.
-///     #[export(get, set = "set_my_field")]
+///     #[export(get, set = set_my_field)]
 ///     my_field: i64,
 /// }
 ///