Fix NSObjectProtocol ProtocolType implementation

Author: madsmtm

crates/icrate/examples/browser.rs

@@ -73,6 +73,8 @@ declare_class!(
         }
     }
 
+    unsafe impl NSObjectProtocol for Delegate {}
+
     unsafe impl NSApplicationDelegate for Delegate {
         #[method(applicationDidFinishLaunching:)]
         #[allow(non_snake_case)]
@@ -293,8 +295,6 @@ impl Delegate {
     }
 }
 
-unsafe impl NSObjectProtocol for Delegate {}
-
 fn main() {
     let mtm = MainThreadMarker::new().unwrap();
     let app = NSApplication::sharedApplication(mtm);

crates/icrate/examples/delegate.rs

@@ -50,6 +50,8 @@ declare_class!(
         }
     }
 
+    unsafe impl NSObjectProtocol for AppDelegate {}
+
     unsafe impl NSApplicationDelegate for AppDelegate {
         #[method(applicationDidFinishLaunching:)]
         fn did_finish_launching(&self, notification: &NSNotification) {
@@ -65,8 +67,6 @@ declare_class!(
     }
 );
 
-unsafe impl NSObjectProtocol for AppDelegate {}
-
 impl AppDelegate {
     pub fn new(ivar: u8, another_ivar: bool, mtm: MainThreadMarker) -> Id<Self> {
         unsafe { msg_send_id![mtm.alloc(), initWith: ivar, another: another_ivar] }

crates/icrate/examples/metal.rs

@@ -31,21 +31,21 @@ use objc2::{
 #[rustfmt::skip]
 const SHADERS: &str = r#"
     #include <metal_stdlib>
-        
+
     struct SceneProperties {
         float time;
-    };        
-    
+    };
+
     struct VertexInput {
         metal::packed_float3 position;
         metal::packed_float3 color;
     };
-    
+
     struct VertexOutput {
         metal::float4 position [[position]];
         metal::float4 color;
     };
-    
+
     vertex VertexOutput vertex_main(
         device const SceneProperties& properties [[buffer(0)]],
         device const VertexInput* vertices [[buffer(1)]],
@@ -64,7 +64,7 @@ const SHADERS: &str = r#"
         out.color = metal::float4(in.color, 1);
         return out;
     }
-    
+
     fragment metal::float4 fragment_main(VertexOutput in [[stage_in]]) {
         return in.color;
     }
@@ -155,6 +155,8 @@ declare_class!(
         }
     }
 
+    unsafe impl NSObjectProtocol for Delegate {}
+
     // define the delegate methods for the `NSApplicationDelegate` protocol
     unsafe impl NSApplicationDelegate for Delegate {
         #[method(applicationDidFinishLaunching:)]
@@ -356,8 +358,6 @@ declare_class!(
     }
 );
 
-unsafe impl NSObjectProtocol for Delegate {}
-
 impl Delegate {
     pub fn new(mtm: MainThreadMarker) -> Id<Self> {
         unsafe { msg_send_id![mtm.alloc(), init] }

crates/objc2/CHANGELOG.md

@@ -43,6 +43,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   ];
   ```
 
+### Fixed
+* Fixed the name of the protocol that `NSObjectProtocol` references.
+
 
 ## 0.4.1 - 2023-07-31
 

crates/objc2/src/__macro_helpers/mod.rs

@@ -580,32 +580,56 @@ impl ClassProtocolMethodsBuilder<'_, '_> {
         }
     }
 
-    #[inline]
+    #[cfg(all(debug_assertions, feature = "verify"))]
     pub fn __finish(self) {
-        #[cfg(all(debug_assertions, feature = "verify"))]
+        let superclass = self.builder.superclass();
+
         if let Some(protocol) = self.protocol {
             for desc in &self.required_instance_methods {
-                if !self.registered_instance_methods.contains(&desc.sel) {
-                    panic!(
-                        "must implement required protocol method -[{protocol} {}]",
-                        desc.sel
-                    )
+                if self.registered_instance_methods.contains(&desc.sel) {
+                    continue;
+                }
+
+                // TODO: Don't do this when `NS_PROTOCOL_REQUIRES_EXPLICIT_IMPLEMENTATION`
+                if superclass
+                    .and_then(|superclass| superclass.instance_method(desc.sel))
+                    .is_some()
+                {
+                    continue;
                 }
+
+                panic!(
+                    "must implement required protocol method -[{protocol} {}]",
+                    desc.sel
+                )
             }
         }
 
-        #[cfg(all(debug_assertions, feature = "verify"))]
         if let Some(protocol) = self.protocol {
             for desc in &self.required_class_methods {
-                if !self.registered_class_methods.contains(&desc.sel) {
-                    panic!(
-                        "must implement required protocol method +[{protocol} {}]",
-                        desc.sel
-                    )
+                if self.registered_class_methods.contains(&desc.sel) {
+                    continue;
+                }
+
+                // TODO: Don't do this when `NS_PROTOCOL_REQUIRES_EXPLICIT_IMPLEMENTATION`
+                if superclass
+                    .and_then(|superclass| superclass.class_method(desc.sel))
+                    .is_some()
+                {
+                    continue;
                 }
+
+                panic!(
+                    "must implement required protocol method +[{protocol} {}]",
+                    desc.sel
+                );
             }
         }
     }
+
+    #[inline]
+    #[cfg(not(all(debug_assertions, feature = "verify")))]
+    pub fn __finish(self) {}
 }
 
 #[cfg(test)]

crates/objc2/src/declare/mod.rs

@@ -356,7 +356,7 @@ impl ClassBuilder {
     }
 
     #[allow(unused)]
-    fn superclass(&self) -> Option<&AnyClass> {
+    pub(crate) fn superclass(&self) -> Option<&AnyClass> {
         // SAFETY: Though the class is not finalized, `class_getSuperclass` is
         // still safe to call.
         unsafe { AnyClass::superclass_raw(self.cls.as_ptr()) }
@@ -612,7 +612,9 @@ impl ClassBuilder {
     pub fn add_protocol(&mut self, proto: &AnyProtocol) {
         let success = unsafe { ffi::class_addProtocol(self.as_mut_ptr(), proto.as_ptr()) };
         let success = Bool::from_raw(success).as_bool();
-        assert!(success, "failed to add protocol {proto}");
+        if cfg!(not(feature = "gnustep-1-7")) {
+            assert!(success, "failed to add protocol {proto}");
+        }
     }
 
     // fn add_property(&self, name: &str, attributes: &[ffi::objc_property_attribute_t]);
@@ -770,7 +772,7 @@ mod tests {
     use crate::mutability::Immutable;
     use crate::rc::Id;
     use crate::runtime::{NSObject, NSObjectProtocol};
-    use crate::{declare_class, extern_methods, msg_send, test_utils, ClassType};
+    use crate::{declare_class, extern_methods, msg_send, test_utils, ClassType, ProtocolType};
 
     #[test]
     fn test_alignment() {
@@ -900,7 +902,43 @@ mod tests {
     }
 
     #[test]
-    #[should_panic = "failed to add protocol NSObject"]
+    fn inheriting_does_not_implement_protocols() {
+        let builder = ClassBuilder::new(
+            "TestClassBuilderInheritingDoesNotImplementProtocols",
+            NSObject::class(),
+        )
+        .unwrap();
+
+        let cls = builder.register();
+        let conforms = cls.conforms_to(<dyn NSObjectProtocol>::protocol().unwrap());
+        if cfg!(feature = "gnustep-1-7") {
+            // FIXME: GNUStep works differently here!
+            assert!(conforms);
+        } else {
+            assert!(!conforms);
+        }
+    }
+
+    #[test]
+    fn inherit_nsobject_add_protocol() {
+        let mut builder = ClassBuilder::new(
+            "TestClassBuilderInheritNSObjectAddProtocol",
+            NSObject::class(),
+        )
+        .unwrap();
+
+        let protocol = <dyn NSObjectProtocol>::protocol().unwrap();
+
+        builder.add_protocol(protocol);
+        let cls = builder.register();
+        assert!(cls.conforms_to(protocol));
+    }
+
+    #[test]
+    #[cfg_attr(
+        not(feature = "gnustep-1-7"),
+        should_panic = "failed to add protocol NSObject"
+    )]
     fn duplicate_protocol() {
         let cls = test_utils::custom_class();
         let mut builder = ClassBuilder::new("TestClassBuilderDuplicateProtocol", cls).unwrap();

crates/objc2/src/macros/declare_class.rs

@@ -259,6 +259,8 @@
 /// #       }
 ///     }
 ///
+///     unsafe impl NSObjectProtocol for MyCustomObject {}
+///
 ///     # #[cfg(available_in_icrate)]
 ///     unsafe impl NSCopying for MyCustomObject {
 ///         #[method_id(copyWithZone:)]
@@ -274,9 +276,6 @@
 ///     }
 /// );
 ///
-/// // TODO: Allow moving this inside `declare_class!`
-/// unsafe impl NSObjectProtocol for MyCustomObject {}
-///
 /// impl MyCustomObject {
 ///     pub fn new(foo: u8) -> Id<Self> {
 ///         unsafe { msg_send_id![Self::alloc(), initWithFoo: foo] }

crates/objc2/src/runtime/nsobject.rs

@@ -165,7 +165,7 @@ crate::__inner_extern_protocol!(
     ()
     (NSObjectProtocol)
     (dyn NSObjectProtocol)
-    ("NSCopying")
+    ("NSObject")
 );
 
 unsafe impl NSObjectProtocol for NSObject {}
@@ -372,4 +372,10 @@ mod tests {
 
         assert_eq!(ptr1, ptr2);
     }
+
+    #[test]
+    fn conforms_to_nsobjectprotocol() {
+        let protocol = <dyn NSObjectProtocol>::protocol().unwrap();
+        assert!(NSObject::class().conforms_to(protocol));
+    }
 }

crates/objc2/src/runtime/protocol_object.rs

@@ -253,21 +253,22 @@ mod tests {
             type Mutability = Mutable;
             const NAME: &'static str = "ProtocolTestsDummyClass";
         }
+
+        unsafe impl NSObjectProtocol for DummyClass {}
     );
 
+    unsafe impl Foo for DummyClass {}
+    unsafe impl Bar for DummyClass {}
+    unsafe impl FooBar for DummyClass {}
+    // unsafe impl FooFooBar for DummyClass {}
+
     extern_methods!(
         unsafe impl DummyClass {
             #[method_id(new)]
             fn new() -> Id<Self>;
         }
     );
 
-    unsafe impl NSObjectProtocol for DummyClass {}
-    unsafe impl Foo for DummyClass {}
-    unsafe impl Bar for DummyClass {}
-    unsafe impl FooBar for DummyClass {}
-    // unsafe impl FooFooBar for DummyClass {}
-
     #[test]
     fn impl_traits() {
         assert_impl_all!(NSObject: NSObjectProtocol);