Merge pull request #508 from madsmtm/fix-nsobjectprotocol

Fix protocol names

crates/header-translator/src/stmt.rs

@@ -369,6 +369,7 @@ pub enum Stmt {
     /// extern_protocol!
     ProtocolDecl {
         id: ItemIdentifier,
+        actual_name: Option<String>,
         availability: Availability,
         protocols: BTreeSet<ItemIdentifier>,
         methods: Vec<Method>,
@@ -797,9 +798,14 @@ impl Stmt {
                 .collect()
             }
             EntityKind::ObjCProtocolDecl => {
-                let id = ItemIdentifier::new(entity, context)
-                    .map_name(|name| context.replace_protocol_name(name));
-                let data = context.protocol_data.get(&id.name);
+                let actual_id = ItemIdentifier::new(entity, context);
+                let data = context.protocol_data.get(&actual_id.name);
+                let actual_name = data
+                    .map(|data| data.renamed.is_some())
+                    .unwrap_or_default()
+                    .then(|| actual_id.name.clone());
+
+                let id = actual_id.map_name(|name| context.replace_protocol_name(name));
 
                 if data.map(|data| data.skipped).unwrap_or_default() {
                     return vec![];
@@ -830,6 +836,7 @@ impl Stmt {
 
                 vec![Self::ProtocolDecl {
                     id,
+                    actual_name,
                     availability,
                     protocols,
                     methods,
@@ -1616,6 +1623,7 @@ impl fmt::Display for Stmt {
             }
             Self::ProtocolDecl {
                 id,
+                actual_name,
                 availability,
                 protocols,
                 methods,
@@ -1683,7 +1691,13 @@ impl fmt::Display for Stmt {
                 }
                 writeln!(f, "    }}")?;
                 writeln!(f)?;
-                writeln!(f, "    unsafe impl ProtocolType for dyn {} {{}}", id.name)?;
+                writeln!(f, "    unsafe impl ProtocolType for dyn {} {{", id.name)?;
+                if let Some(actual_name) = actual_name {
+                    writeln!(f)?;
+                    writeln!(f, "        const NAME: &'static str = {actual_name:?};")?;
+                    write!(f, "    ")?;
+                }
+                writeln!(f, "}}")?;
                 writeln!(f, ");")?;
             }
             Self::StructDecl {

crates/icrate/CHANGELOG.md

@@ -65,6 +65,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   to use in such hashing collections.
 * **BREAKING**: Added `HasStableHash` requirement on `NSDictionary` and
   `NSSet` creation methods, fixing a long-standing soundess issue.
+* Fixed the protocol names of `NSAccessibilityElementProtocol`,
+  `NSTextAttachmentCellProtocol` and `NSFileProviderItemProtocol`.
 
 
 ## icrate 0.0.4 - 2023-07-31

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/icrate/src/fixes/Foundation/copying.rs

@@ -41,9 +41,7 @@ extern_protocol!(
             Self: CounterpartOrSelf;
     }
 
-    unsafe impl ProtocolType for dyn NSCopying {
-        const NAME: &'static str = "NSCopying";
-    }
+    unsafe impl ProtocolType for dyn NSCopying {}
 );
 
 // FIXME: Remove this hack which makes NSMutableDictionary tests work
@@ -86,7 +84,5 @@ extern_protocol!(
             Self: CounterpartOrSelf;
     }
 
-    unsafe impl ProtocolType for dyn NSMutableCopying {
-        const NAME: &'static str = "NSMutableCopying";
-    }
+    unsafe impl ProtocolType for dyn NSMutableCopying {}
 );

crates/icrate/tests/renamed_protocols.rs

@@ -0,0 +1,9 @@
+#![cfg(feature = "AppKit")]
+use icrate::AppKit::NSAccessibilityElementProtocol;
+use objc2::ProtocolType;
+
+#[test]
+fn accessibility_element_protocol() {
+    let actual: &str = <dyn NSAccessibilityElementProtocol>::NAME;
+    assert_eq!(actual, "NSAccessibilityElement");
+}

crates/objc2/CHANGELOG.md

@@ -43,6 +43,13 @@ 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.
+
+### Removed
+* **BREAKING**: Removed `ProtocolType` implementation for `NSObject`.
+  Use the more precise `NSObjectProtocol` trait instead!
+
 
 ## 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/protocol_type.rs

@@ -23,10 +23,10 @@ use crate::runtime::AnyProtocol;
 ///
 /// ```
 /// use objc2::ProtocolType;
-/// use objc2::runtime::NSObject;
-/// // Get a protocol object representing `NSObject`
-/// let protocol = NSObject::protocol().expect("NSObject to have a protocol");
-/// assert_eq!(NSObject::NAME, protocol.name());
+/// use objc2::runtime::NSObjectProtocol;
+/// // Get a protocol object representing the `NSObject` protocol
+/// let protocol = <dyn NSObjectProtocol>::protocol().expect("NSObject to have a protocol");
+/// assert_eq!(<dyn NSObjectProtocol>::NAME, protocol.name());
 /// ```
 ///
 /// Use the [`extern_protocol!`] macro to implement this trait for a type.