Merge pull request #839 from godot-rust/qol/export-restrictions

Disallow `Export` if class doesn't inherit `Node` or `Resource`

Author: Bromeon

godot-codegen/src/context.rs

@@ -329,13 +329,32 @@ impl InheritanceTree {
 
     /// Returns all base classes, without the class itself, in order from nearest to furthest (object).
     pub fn collect_all_bases(&self, derived_name: &TyName) -> Vec<TyName> {
-        let mut maybe_base = derived_name;
+        let mut upgoer = derived_name;
         let mut result = vec![];
 
-        while let Some(base) = self.derived_to_base.get(maybe_base) {
+        while let Some(base) = self.derived_to_base.get(upgoer) {
             result.push(base.clone());
-            maybe_base = base;
+            upgoer = base;
         }
         result
     }
+
+    /// Whether a class is a direct or indirect subclass of another (true for derived == base).
+    pub fn inherits(&self, derived: &TyName, base_name: &str) -> bool {
+        // Reflexive: T inherits T.
+        if derived.godot_ty == base_name {
+            return true;
+        }
+
+        let mut upgoer = derived;
+
+        while let Some(next_base) = self.derived_to_base.get(upgoer) {
+            if next_base.godot_ty == base_name {
+                return true;
+            }
+            upgoer = next_base;
+        }
+
+        false
+    }
 }

godot-codegen/src/generator/classes.rs

@@ -155,7 +155,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
     // notify() and notify_reversed() are added after other methods, to list others first in docs.
     let notify_methods = notifications::make_notify_methods(class_name, ctx);
 
-    let (assoc_memory, assoc_dyn_memory) = make_bounds(class);
+    let (assoc_memory, assoc_dyn_memory, is_exportable) = make_bounds(class, ctx);
 
     let internal_methods = quote! {
         fn __checked_id(&self) -> Option<crate::obj::InstanceId> {
@@ -228,6 +228,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
                 type Memory = crate::obj::bounds::#assoc_memory;
                 type DynMemory = crate::obj::bounds::#assoc_dyn_memory;
                 type Declarer = crate::obj::bounds::DeclEngine;
+                type Exportable = crate::obj::bounds::#is_exportable;
             }
 
             #(
@@ -420,8 +421,10 @@ fn make_deref_impl(class_name: &TyName, base_ty: &TokenStream) -> TokenStream {
     }
 }
 
-fn make_bounds(class: &Class) -> (Ident, Ident) {
-    let assoc_dyn_memory = if class.name().rust_ty == "Object" {
+fn make_bounds(class: &Class, ctx: &mut Context) -> (Ident, Ident, Ident) {
+    let c = class.name();
+
+    let assoc_dyn_memory = if c.rust_ty == "Object" {
         ident("MemDynamic")
     } else if class.is_refcounted {
         ident("MemRefCounted")
@@ -435,7 +438,14 @@ fn make_bounds(class: &Class) -> (Ident, Ident) {
         ident("MemManual")
     };
 
-    (assoc_memory, assoc_dyn_memory)
+    let tree = ctx.inheritance_tree();
+    let is_exportable = if tree.inherits(c, "Node") || tree.inherits(c, "Resource") {
+        ident("Yes")
+    } else {
+        ident("No")
+    };
+
+    (assoc_memory, assoc_dyn_memory, is_exportable)
 }
 
 fn make_class_methods(

godot-core/src/obj/bounds.rs

@@ -58,7 +58,7 @@ use private::Sealed;
 // Sealed trait
 
 pub(super) mod private {
-    use super::{Declarer, DynMemory, Memory};
+    use super::{Declarer, DynMemory, Exportable, Memory};
 
     // Bounds trait declared here for code locality; re-exported in crate::obj.
 
@@ -97,6 +97,12 @@ pub(super) mod private {
         /// Whether this class is a core Godot class provided by the engine, or declared by the user as a Rust struct.
         // TODO what about GDScript user classes?
         type Declarer: Declarer;
+
+        /// True if *either* `T: Inherits<Node>` *or* `T: Inherits<Resource>` is fulfilled.
+        ///
+        /// Enables `#[export]` for those classes.
+        #[doc(hidden)]
+        type Exportable: Exportable;
     }
 
     /// Implements [`Bounds`] for a user-defined class.
@@ -130,6 +136,7 @@ pub(super) mod private {
                 type Memory = <<$UserClass as $crate::obj::GodotClass>::Base as $crate::obj::Bounds>::Memory;
                 type DynMemory = <<$UserClass as $crate::obj::GodotClass>::Base as $crate::obj::Bounds>::DynMemory;
                 type Declarer = $crate::obj::bounds::DeclUser;
+                type Exportable = <<$UserClass as $crate::obj::GodotClass>::Base as $crate::obj::Bounds>::Exportable;
             }
         };
     }
@@ -417,3 +424,19 @@ impl Declarer for DeclUser {
         }
     }
 }
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+// Exportable bounds (still hidden)
+
+#[doc(hidden)]
+pub trait Exportable: Sealed {}
+
+#[doc(hidden)]
+pub enum Yes {}
+impl Sealed for Yes {}
+impl Exportable for Yes {}
+
+#[doc(hidden)]
+pub enum No {}
+impl Sealed for No {}
+impl Exportable for No {}

godot-core/src/obj/gd.rs

@@ -738,16 +738,27 @@ impl<T: GodotClass> GodotType for Gd<T> {
 
 impl<T: GodotClass> ArrayElement for Gd<T> {
     fn element_type_string() -> String {
-        match Self::export_hint().hint {
-            hint @ (PropertyHint::RESOURCE_TYPE | PropertyHint::NODE_TYPE) => {
-                format!(
-                    "{}/{}:{}",
-                    VariantType::OBJECT.ord(),
-                    hint.ord(),
-                    T::class_name()
-                )
-            }
-            _ => format!("{}:", VariantType::OBJECT.ord()),
+        // See also impl Export for Gd<T>.
+
+        let hint = if T::inherits::<classes::Resource>() {
+            Some(PropertyHint::RESOURCE_TYPE)
+        } else if T::inherits::<classes::Node>() {
+            Some(PropertyHint::NODE_TYPE)
+        } else {
+            None
+        };
+
+        // Exportable classes (Resource/Node based) include the {RESOURCE|NODE}_TYPE hint + the class name.
+        if let Some(export_hint) = hint {
+            format!(
+                "{variant}/{hint}:{class}",
+                variant = VariantType::OBJECT.ord(),
+                hint = export_hint.ord(),
+                class = T::class_name()
+            )
+        } else {
+            // Previous impl: format!("{variant}:", variant = VariantType::OBJECT.ord())
+            unreachable!("element_type_string() should only be invoked for exportable classes")
         }
     }
 }
@@ -793,14 +804,17 @@ impl<T: GodotClass> Var for Gd<T> {
     }
 }
 
-impl<T: GodotClass> Export for Gd<T> {
+impl<T> Export for Gd<T>
+where
+    T: GodotClass + Bounds<Exportable = bounds::Yes>,
+{
     fn export_hint() -> PropertyHintInfo {
         let hint = if T::inherits::<classes::Resource>() {
             PropertyHint::RESOURCE_TYPE
         } else if T::inherits::<classes::Node>() {
             PropertyHint::NODE_TYPE
         } else {
-            PropertyHint::NONE
+            unreachable!("classes not inheriting from Resource or Node should not be exportable")
         };
 
         // Godot does this by default too; the hint is needed when the class is a resource/node,

godot-core/src/obj/traits.rs

@@ -80,6 +80,7 @@ unsafe impl Bounds for NoBase {
     type Memory = bounds::MemManual;
     type DynMemory = bounds::MemManual;
     type Declarer = bounds::DeclEngine;
+    type Exportable = bounds::No;
 }
 
 /// Non-strict inheritance relationship in the Godot class hierarchy.

godot-core/src/registry/property.rs

@@ -22,8 +22,8 @@ use crate::meta::{FromGodot, GodotConvert, GodotType, PropertyHintInfo, ToGodot}
 /// This does not require [`FromGodot`] or [`ToGodot`], so that something can be used as a property even if it can't be used in function
 /// arguments/return types.
 
-// We also mention #[export] here, because we can't control the order of error messages. Missing Export often also means missing Var trait,
-// and so the Var error message appears first.
+// on_unimplemented: we also mention #[export] here, because we can't control the order of error messages.
+// Missing Export often also means missing Var trait, and so the Var error message appears first.
 #[diagnostic::on_unimplemented(
     message = "`#[var]` properties require `Var` trait; #[export] ones require `Export` trait",
     label = "type cannot be used as a property",
@@ -41,7 +41,10 @@ pub trait Var: GodotConvert {
 }
 
 /// Trait implemented for types that can be used as `#[export]` fields.
-// Mentioning both Var + Export: see above.
+///
+/// `Export` is only implemented for objects `Gd<T>` if either `T: Inherits<Node>` or `T: Inherits<Resource>`, just like GDScript.
+/// This means you cannot use `#[export]` with `Gd<RefCounted>`, for example.
+// on_unimplemented: mentioning both Var + Export; see above.
 #[diagnostic::on_unimplemented(
     message = "`#[var]` properties require `Var` trait; #[export] ones require `Export` trait",
     label = "type cannot be used as a property",
@@ -414,6 +417,7 @@ mod export_impls {
 
     // Dictionary: will need to be done manually once they become typed.
     impl_property_by_godot_convert!(Dictionary);
+    impl_property_by_godot_convert!(Variant);
 
     // Packed arrays: we manually implement `Export`.
     impl_property_by_godot_convert!(PackedByteArray, no_export);

godot-macros/src/class/derive_godot_class.rs

@@ -129,6 +129,7 @@ pub fn derive_godot_class(item: venial::Item) -> ParseResult<TokenStream> {
             type Memory = <<Self as ::godot::obj::GodotClass>::Base as ::godot::obj::Bounds>::Memory;
             type DynMemory = <<Self as ::godot::obj::GodotClass>::Base as ::godot::obj::Bounds>::DynMemory;
             type Declarer = ::godot::obj::bounds::DeclUser;
+            type Exportable = <<Self as ::godot::obj::GodotClass>::Base as ::godot::obj::Bounds>::Exportable;
         }
 
         #godot_init_impl