Change #[hint(typed_signals = false)] to #[godot_api(no_typed_signals)]

#[hint] should really be hints: injecting information that the proc-macro cannot infer, like with
#[hint(no_base)] fields. We could use #[hint(has_base_field = true, class_visibility = pub)] or so
in the future.

For now, disabling entire typed-signal API (including traits) can be done on #[godot_api] itself.

Author: Bromeon

godot-macros/src/class/data_models/inherent_impl.rs

@@ -19,7 +19,6 @@ use crate::{handle_mutually_exclusive_keys, util, ParseResult};
 use proc_macro2::{Delimiter, Group, Ident, TokenStream};
 use quote::spanned::Spanned;
 use quote::{format_ident, quote};
-use venial::Impl;
 
 /// Attribute for user-declared function.
 enum ItemAttrType {
@@ -72,19 +71,17 @@ struct SignalAttr {
     pub no_builder: bool,
 }
 
-#[derive(Default)]
-pub(crate) struct GodotApiHints {
-    pub has_typed_signals: Option<bool>,
-}
-
-// ----------------------------------------------------------------------------------------------------------------------------------------------
-
-pub struct InherentImplAttr {
+pub(crate) struct InherentImplAttr {
     /// For implementation reasons, there can be a single 'primary' impl block and 0 or more 'secondary' impl blocks.
     /// For now, this is controlled by a key in the 'godot_api' attribute.
     pub secondary: bool,
+
+    /// When typed signal generation is explicitly disabled by the user.
+    pub no_typed_signals: bool,
 }
 
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+
 /// Codegen for `#[godot_api] impl MyType`
 pub fn transform_inherent_impl(
     meta: InherentImplAttr,
@@ -95,8 +92,6 @@ pub fn transform_inherent_impl(
     let class_name_obj = util::class_name_obj(&class_name);
     let prv = quote! { ::godot::private };
 
-    let hints = extract_hint_attribute(&mut impl_block)?;
-
     // Can add extra functions to the end of the impl block.
     let (funcs, signals) = process_godot_fns(&class_name, &mut impl_block, meta.secondary)?;
     let consts = process_godot_constants(&mut impl_block)?;
@@ -115,8 +110,12 @@ pub fn transform_inherent_impl(
 
     // For each #[func] in this impl block, create one constant.
     let func_name_constants = make_funcs_collection_constants(&funcs, &class_name);
-    let (signal_registrations, signal_symbol_types) =
-        make_signal_registrations(&signals, &class_name, &class_name_obj, hints)?;
+    let (signal_registrations, signal_symbol_types) = make_signal_registrations(
+        &signals,
+        &class_name,
+        &class_name_obj,
+        meta.no_typed_signals,
+    )?;
 
     #[cfg(feature = "codegen-full")]
     let rpc_registrations = crate::class::make_rpc_registrations_fn(&class_name, &funcs);
@@ -214,19 +213,22 @@ pub fn transform_inherent_impl(
     }
 }
 
-fn extract_hint_attribute(impl_block: &mut Impl) -> ParseResult<GodotApiHints> {
-    // Could possibly be extended with #[hint(signal_vis = pub)] or so.
-
-    // #[hint(typed_signals)]
-    let has_typed_signals;
+/* Re-enable if we allow controlling declarative macros for signals (base_field_macro, visibility_macros).
+fn extract_hint_attribute(impl_block: &mut venial:: Impl) -> ParseResult<GodotApiHints> {
+    // #[hint(has_base_field = BOOL)]
+    let has_base_field;
     if let Some(mut hints) = KvParser::parse_remove(&mut impl_block.attributes, "hint")? {
-        has_typed_signals = hints.handle_bool("typed_signals")?;
+        has_base_field = hints.handle_bool("has_base_field")?;
     } else {
-        has_typed_signals = None;
+        has_base_field = None;
     }
 
-    Ok(GodotApiHints { has_typed_signals })
+    // #[hint(class_visibility = pub(crate))]
+    // ...
+
+    Ok(GodotApiHints { has_base_field })
 }
+*/
 
 fn process_godot_fns(
     class_name: &Ident,

godot-macros/src/class/data_models/signal.rs

@@ -7,7 +7,6 @@
 
 // Some duplication with godot-codegen/signals.rs; see comments there.
 
-use crate::class::GodotApiHints;
 use crate::util::bail;
 use crate::{util, ParseResult};
 use proc_macro2::{Delimiter, Ident, TokenStream, TokenTree};
@@ -183,7 +182,7 @@ pub fn make_signal_registrations(
     signals: &[SignalDefinition],
     class_name: &Ident,
     class_name_obj: &TokenStream,
-    hints: GodotApiHints,
+    no_typed_signals: bool,
 ) -> ParseResult<(Vec<TokenStream>, Option<TokenStream>)> {
     let mut signal_registrations = Vec::new();
 
@@ -212,8 +211,11 @@ pub fn make_signal_registrations(
         signal_registrations.push(registration);
     }
 
+    // Rewrite the above using #[cfg].
     #[cfg(since_api = "4.2")]
-    let signal_symbols = Some(make_signal_symbols(class_name, collection_api, hints));
+    let signal_symbols =
+        (!no_typed_signals).then(|| make_signal_symbols(class_name, collection_api));
+
     #[cfg(before_api = "4.2")]
     let signal_symbols = None;
 
@@ -389,13 +391,7 @@ fn make_signal_symbols(
     class_name: &Ident,
     collection_api: SignalCollection,
     // max_visibility: SignalVisibility,
-    hints: GodotApiHints,
 ) -> TokenStream {
-    // Return early if typed signals are explicitly disabled.
-    if hints.has_typed_signals == Some(false) {
-        return TokenStream::new();
-    }
-
     // Earlier implementation generated a simplified code when no #[signal] was declared: only WithSignals/WithUserSignals impl, but no own
     // collection, instead the associated type pointing to the base class. This has however some problems:
     // * Part of the reason for user-defined collection is to store UserSignalObject instead of Gd, which can store &mut self.

godot-macros/src/class/godot_api.rs

@@ -17,9 +17,20 @@ fn parse_inherent_impl_attr(meta: TokenStream) -> Result<super::InherentImplAttr
     let item = venial_parse_meta(&meta, format_ident!("godot_api"), &quote! { fn func(); })?;
     let mut attr = KvParser::parse_required(item.attributes(), "godot_api", &meta)?;
     let secondary = attr.handle_alone("secondary")?;
+    let no_typed_signals = attr.handle_alone("no_typed_signals")?;
     attr.finish()?;
 
-    Ok(super::InherentImplAttr { secondary })
+    if no_typed_signals && secondary {
+        return bail!(
+            meta,
+            "#[godot_api]: keys `secondary` and `no_typed_signals` are mutually exclusive; secondary blocks allow no signals anyway"
+        )?;
+    }
+
+    Ok(super::InherentImplAttr {
+        secondary,
+        no_typed_signals,
+    })
 }
 
 pub fn attribute_godot_api(

godot-macros/src/util/kv_parser.rs

@@ -64,6 +64,7 @@ impl KvParser {
     /// Like `parse()`, but removes the attribute from the list.
     ///
     /// Useful for `#[proc_macro_attributes]`, where handled attributes must not show up in resulting code.
+    // Currently unused.
     pub fn parse_remove(
         attributes: &mut Vec<venial::Attribute>,
         expected: &str,

itest/rust/src/object_tests/object_test.rs

@@ -997,8 +997,9 @@ pub mod object_test_gd {
     }
     use nested::ObjectTest;
 
-    #[godot_api]
-    #[hint(typed_signals = false)] // Allows nested::ObjectTest, which would fail otherwise due to generated decl-macro being out-of-scope.
+    // Disabling signals allows nested::ObjectTest, which would fail otherwise due to generated decl-macro being out-of-scope.
+    #[godot_api(no_typed_signals)]
+    // #[hint(has_base_field = false)] // if we allow more fine-grained control in the future
     impl nested::ObjectTest {
         #[func]
         fn pass_object(&self, object: Gd<Object>) -> i64 {