Merge pull request #850 from godot-rust/qol/require-tool-for-virtual-extensions

Validate that virtual extension classes require `#[class(tool)]`

Author: Bromeon

godot-core/src/obj/script.rs

@@ -53,8 +53,9 @@ use self::bounded_ptr_list::BoundedPtrList;
 /// use godot::extras::{IScriptExtension, ScriptInstance};
 ///
 /// // 1) Define the script.
+/// // This needs #[class(tool)] since the script extension runs in the editor.
 /// #[derive(GodotClass)]
-/// #[class(init, base=ScriptExtension)]
+/// #[class(init, base=ScriptExtension, tool)]
 /// struct MyScript {
 ///    base: Base<ScriptExtension>,
 ///    // ... other fields

godot-core/src/registry/class.rs

@@ -348,6 +348,7 @@ fn fill_into<T>(dst: &mut Option<T>, src: Option<T>) -> Result<(), ()> {
 /// Registers a class with given the dynamic type information `info`.
 fn register_class_raw(mut info: ClassRegistrationInfo) {
     // First register class...
+    validate_class_constraints(&info);
 
     let class_name = info.class_name;
     let parent_class_name = info
@@ -428,6 +429,10 @@ fn register_class_raw(mut info: ClassRegistrationInfo) {
     }
 }
 
+fn validate_class_constraints(_class: &ClassRegistrationInfo) {
+    // TODO: if we add builder API, the proc-macro checks in parse_struct_attributes() etc. should be duplicated here.
+}
+
 fn unregister_class_raw(class: LoadedClass) {
     let class_name = class.name;
     out!("Unregister class: {class_name}");

godot-macros/src/class/derive_godot_class.rs

@@ -368,6 +368,8 @@ fn parse_struct_attributes(class: &venial::Struct) -> ParseResult<ClassAttribute
         parser.finish()?;
     }
 
+    post_validate(&base_ty, is_tool)?;
+
     Ok(ClassAttributes {
         base_ty,
         init_strategy,
@@ -557,3 +559,36 @@ fn handle_opposite_keys(
         ),
     }
 }
+
+/// Checks more logical combinations of attributes.
+fn post_validate(base_ty: &Ident, is_tool: bool) -> ParseResult<()> {
+    // TODO: this should be delegated to either:
+    // a) the type system: have a trait IsTool which is implemented when #[class(tool)] is set.
+    //    Then, for certain base classes, require a tool bound (e.g. generate method `fn type_check<T: IsTool>()`).
+    //    This would also allow moving the logic to godot-codegen.
+    // b) a runtime check in class.rs > register_class_raw() and validate_class_constraints().
+
+    let is_extension = is_class_virtual_extension(&base_ty.to_string());
+    if is_extension && !is_tool {
+        return bail!(
+            base_ty,
+            "Base class `{}` is a virtual extension class, which runs in the editor and thus requires #[class(tool)].",
+            base_ty
+        );
+    }
+
+    Ok(())
+}
+
+/// Whether a class exists primarily for GDExtension to overload virtual methods.
+// See post_validate(). Should be moved to godot-codegen > special_cases.rs.
+fn is_class_virtual_extension(godot_class_name: &str) -> bool {
+    // Heuristic: suffix, with some exceptions.
+    // Generally, a rule might also be "there is another class without that suffix", however that doesn't apply to e.g. OpenXRAPIExtension.
+
+    match godot_class_name {
+        "GDExtension" => false,
+
+        _ => godot_class_name.ends_with("Extension"),
+    }
+}

itest/rust/src/builtin_tests/script/script_instance_tests.rs

@@ -17,7 +17,7 @@ use godot::register::{godot_api, GodotClass};
 use godot::sys;
 
 #[derive(GodotClass)]
-#[class(base = ScriptExtension, init)]
+#[class(base = ScriptExtension, init, tool)]
 struct TestScript {
     base: Base<ScriptExtension>,
 }