Merge pull request #91 from madsmtm/encode-fixes

`Encoding` comparison fixes

Author: madsmtm

objc2-encode/CHANGELOG.md

@@ -6,6 +6,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased - YYYY-MM-DD
 
+### Added
+* `Encoding::equivalent_to`, `Encoding::equivalent_to_str` and
+  `Encoding::equivalent_to_start_of_str` methods for more precise comparison
+  semantics.
+
+### Changed
+* Discourage comparing `str` with `Encoding` using `PartialEq`. This trait
+  impl might get removed in a future version.
+
 
 ## 2.0.0-beta.0 - 2021-11-22
 

objc2-encode/README.md

@@ -42,21 +42,21 @@ unsafe impl Encode for MyObject {
     );
 }
 
-assert_eq!(&MyObject::ENCODING, "{MyObject=fs}");
+assert!(MyObject::ENCODING.equivalent_to_str("{MyObject=fs}"));
 
 unsafe impl RefEncode for MyObject {
     const ENCODING_REF: Encoding<'static> = Encoding::Pointer(&Self::ENCODING);
 }
 
-assert_eq!(&MyObject::ENCODING_REF, "^{MyObject=fs}");
+assert!(MyObject::ENCODING_REF.equivalent_to_str("^{MyObject=fs}"));
 ```
 
 An `Encoding` can be compared with an encoding string from the Objective-C
 runtime:
 
 ```rust
 use objc2_encode::Encode;
-assert!(&i32::ENCODING == "i");
+assert!(i32::ENCODING.equivalent_to_str("i"));
 ```
 
 `Encoding` implements `Display` as its string representation. This can be

objc2-encode/examples/ns_uinteger.rs

@@ -26,6 +26,7 @@ unsafe impl RefEncode for NSUInteger {
 }
 
 fn main() {
-    assert_eq!(&NSUInteger::ENCODING, "Q");
-    assert_eq!(&<&NSUInteger>::ENCODING, "^Q");
+    assert!(NSUInteger::ENCODING.equivalent_to_str("Q"));
+    assert!(<&NSUInteger>::ENCODING.equivalent_to_str("^Q"));
+    assert!(<&NSUInteger>::ENCODING.equivalent_to_str("r^Q"));
 }

objc2-encode/examples/opaque_type.rs

@@ -28,7 +28,7 @@ unsafe impl RefEncode for NSDecimal {
 }
 
 fn main() {
-    assert_eq!(&NSDecimal::ENCODING_REF, "^{?=cCCC[38C]}");
+    assert!(NSDecimal::ENCODING_REF.equivalent_to_str("^{?=cCCC[38C]}"));
     // Does not compile:
     // println!("{:?}", NSDecimal::ENCODING);
 }

objc2-encode/src/encoding.rs

@@ -106,6 +106,39 @@ pub enum Encoding<'a> {
     // NSLog(@"Encoding: %s", @encode(const int*)); // -> r^i
 }
 
+impl Encoding<'_> {
+    /// Check if one encoding is equivalent to another.
+    pub fn equivalent_to(&self, other: &Self) -> bool {
+        // For now, because we don't allow representing qualifiers
+        self == other
+    }
+
+    /// Check if an encoding is equivalent to the given string representation.
+    pub fn equivalent_to_str(&self, s: &str) -> bool {
+        // if the given encoding can be successfully removed from the start
+        // and an empty string remains, they were fully equivalent!
+        if let Some(res) = self.equivalent_to_start_of_str(s) {
+            res.is_empty()
+        } else {
+            false
+        }
+    }
+
+    /// Check if an encoding is equivalent to the start of the given string
+    /// representation.
+    ///
+    /// If it is equivalent, the remaining part of the string is returned.
+    /// Otherwise this returns [`None`].
+    pub fn equivalent_to_start_of_str<'a>(&self, s: &'a str) -> Option<&'a str> {
+        // strip leading qualifiers
+        let s = s.trim_start_matches(parse::QUALIFIERS);
+
+        // TODO: Allow missing/"?" names in structs and unions?
+
+        parse::rm_enc_prefix(s, self)
+    }
+}
+
 impl fmt::Display for Encoding<'_> {
     fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
         use Encoding::*;
@@ -162,15 +195,37 @@ impl fmt::Display for Encoding<'_> {
     }
 }
 
+// TODO: Deprecate and remove these PartialEq impls
+
+/// Partial equality between an [`Encoding`] and a [`str`].
+///
+/// Using this is heavily discouraged, since it is not transitive; use
+/// [`Encoding::equivalent_to_str`] instead for more correct semantics.
 impl PartialEq<str> for Encoding<'_> {
+    /// Using this is discouraged.
     fn eq(&self, other: &str) -> bool {
-        parse::eq_enc(other, self)
+        self.equivalent_to_str(other)
+    }
+
+    /// Using this is discouraged.
+    fn ne(&self, other: &str) -> bool {
+        !self.eq(other)
     }
 }
 
+/// Partial equality between an [`Encoding`] and a [`str`].
+///
+/// Using this is heavily discouraged, since it is not transitive; use
+/// [`Encoding::equivalent_to_str`] instead for more correct semantics.
 impl PartialEq<Encoding<'_>> for str {
+    /// Using this is discouraged.
     fn eq(&self, other: &Encoding<'_>) -> bool {
-        parse::eq_enc(self, other)
+        other.equivalent_to_str(self)
+    }
+
+    /// Using this is discouraged.
+    fn ne(&self, other: &Encoding<'_>) -> bool {
+        !self.eq(other)
     }
 }
 
@@ -183,14 +238,14 @@ mod tests {
     fn test_array_display() {
         let e = Encoding::Array(12, &Encoding::Int);
         assert_eq!(e.to_string(), "[12i]");
-        assert_eq!(&e, "[12i]");
+        assert!(e.equivalent_to_str("[12i]"));
     }
 
     #[test]
     fn test_pointer_display() {
         let e = Encoding::Pointer(&Encoding::Int);
         assert_eq!(e.to_string(), "^i");
-        assert_eq!(&e, "^i");
+        assert!(e.equivalent_to_str("^i"));
     }
 
     #[test]
@@ -205,7 +260,7 @@ mod tests {
     #[test]
     fn test_int_display() {
         assert_eq!(Encoding::Int.to_string(), "i");
-        assert_eq!(&Encoding::Int, "i");
+        assert!(Encoding::Int.equivalent_to_str("i"));
     }
 
     #[test]
@@ -221,7 +276,7 @@ mod tests {
     fn test_struct_display() {
         let s = Encoding::Struct("CGPoint", &[Encoding::Char, Encoding::Int]);
         assert_eq!(s.to_string(), "{CGPoint=ci}");
-        assert_eq!(&s, "{CGPoint=ci}");
+        assert!(s.equivalent_to_str("{CGPoint=ci}"));
     }
 
     #[test]
@@ -235,7 +290,7 @@ mod tests {
     fn test_union_display() {
         let u = Encoding::Union("Onion", &[Encoding::Char, Encoding::Int]);
         assert_eq!(u.to_string(), "(Onion=ci)");
-        assert_eq!(&u, "(Onion=ci)");
+        assert!(u.equivalent_to_str("(Onion=ci)"));
     }
 
     #[test]

objc2-encode/src/parse.rs

@@ -3,7 +3,7 @@
 
 use crate::Encoding;
 
-const QUALIFIERS: &[char] = &[
+pub(crate) const QUALIFIERS: &[char] = &[
     'r', // const
     'n', // in
     'N', // inout
@@ -13,7 +13,7 @@ const QUALIFIERS: &[char] = &[
     'V', // oneway
 ];
 
-fn rm_enc_prefix<'a>(s: &'a str, enc: &Encoding<'_>) -> Option<&'a str> {
+pub(crate) fn rm_enc_prefix<'a>(s: &'a str, enc: &Encoding<'_>) -> Option<&'a str> {
     use Encoding::*;
     let code = match *enc {
         Char => "c",
@@ -93,15 +93,6 @@ fn rm_int_prefix(s: &str, other: usize) -> Option<&str> {
     chomp_int(s).and_then(|(n, t)| if other == n { Some(t) } else { None })
 }
 
-pub(crate) fn eq_enc(s: &str, enc: &Encoding<'_>) -> bool {
-    // strip qualifiers
-    let s = s.trim_start_matches(QUALIFIERS);
-
-    // if the given encoding can be successfully removed from the start
-    // and an empty string remains, they were equal!
-    rm_enc_prefix(s, enc).map_or(false, str::is_empty)
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -116,26 +107,31 @@ mod tests {
                 Encoding::Int,
             ],
         );
-        assert!(eq_enc("{A={B=ci}ci}", &enc));
-        assert!(!eq_enc("{A={B=ci}ci", &enc));
+        assert!(enc.equivalent_to_str("{A={B=ci}ci}"));
+        assert_eq!(
+            enc.equivalent_to_start_of_str("{A={B=ci}ci}def"),
+            Some("def")
+        );
+        assert!(!enc.equivalent_to_str("{A={B=ci}ci"));
     }
 
     #[test]
     fn test_bitfield() {
-        assert!(eq_enc("b32", &Encoding::BitField(32)));
-        assert!(!eq_enc("b", &Encoding::BitField(32)));
-        assert!(!eq_enc("b-32", &Encoding::BitField(32)));
+        assert!(Encoding::BitField(32).equivalent_to_str("b32"));
+        assert!(!Encoding::BitField(32).equivalent_to_str("b32a"));
+        assert!(!Encoding::BitField(32).equivalent_to_str("b"));
+        assert!(!Encoding::BitField(32).equivalent_to_str("b-32"));
     }
 
     #[test]
     fn test_qualifiers() {
-        assert!(eq_enc("Vv", &Encoding::Void));
-        assert!(eq_enc("r*", &Encoding::String));
+        assert!(Encoding::Void.equivalent_to_str("Vv"));
+        assert!(Encoding::String.equivalent_to_str("r*"));
     }
 
     #[test]
     fn test_unicode() {
         let fields = &[Encoding::Char, Encoding::Int];
-        assert!(eq_enc("{☃=ci}", &Encoding::Struct("☃", fields)));
+        assert!(Encoding::Struct("☃", fields).equivalent_to_str("{☃=ci}"));
     }
 }

objc2-foundation/src/value.rs

@@ -29,7 +29,10 @@ pub unsafe trait INSValue: INSObject {
     fn get(&self) -> Self::Value {
         if let Some(encoding) = self.encoding() {
             // TODO: This can be a debug assertion (?)
-            assert!(&Self::Value::ENCODING == encoding, "Wrong encoding");
+            assert!(
+                Self::Value::ENCODING.equivalent_to_str(encoding),
+                "Wrong encoding"
+            );
             unsafe { self.get_unchecked() }
         } else {
             panic!("Missing NSValue encoding");
@@ -108,7 +111,7 @@ mod tests {
     fn test_value() {
         let val = NSValue::new(13u32);
         assert_eq!(val.get(), 13);
-        assert!(&u32::ENCODING == val.encoding().unwrap());
+        assert!(u32::ENCODING.equivalent_to_str(val.encoding().unwrap()));
     }
 
     #[test]
@@ -162,7 +165,7 @@ mod tests {
     #[test]
     fn test_value_nsrange() {
         let val = NSValue::new(NSRange::from(1..2));
-        assert!(&NSRange::ENCODING == val.encoding().unwrap());
+        assert!(NSRange::ENCODING.equivalent_to_str(val.encoding().unwrap()));
         let range: NSRange = unsafe { objc2::msg_send![val, rangeValue] };
         assert_eq!(range, NSRange::from(1..2));
         // NSValue -getValue is broken on GNUStep for some types

objc2/examples/introspection.rs

@@ -32,7 +32,7 @@ fn main() {
     let hash_method = cls.instance_method(hash_sel).unwrap();
     let hash_return = hash_method.return_type();
     println!("-[NSObject hash] return type: {:?}", hash_return);
-    assert!(*hash_return == usize::ENCODING);
+    assert!(usize::ENCODING.equivalent_to_str(&hash_return));
 
     // Invoke a method on the object
     let hash: usize = unsafe { msg_send![obj, hash] };

objc2/src/message/verify.rs

@@ -70,7 +70,7 @@ where
 
     let ret = R::ENCODING;
     let expected_ret = method.return_type();
-    if ret != *expected_ret {
+    if !ret.equivalent_to_str(&*expected_ret) {
         return Err(VerificationError::MismatchedReturn(method, ret));
     }
 
@@ -85,7 +85,7 @@ where
 
     for (i, arg) in self_and_cmd.iter().chain(args).copied().enumerate() {
         let expected = method.argument_type(i).unwrap();
-        if arg != *expected {
+        if !arg.equivalent_to_str(&*expected) {
             return Err(VerificationError::MismatchedArgument(method, i, arg));
         }
     }

objc2/src/runtime.rs

@@ -490,7 +490,7 @@ impl RefUnwindSafe for Protocol {}
 fn get_ivar_offset<T: Encode>(cls: &Class, name: &str) -> isize {
     match cls.instance_variable(name) {
         Some(ivar) => {
-            assert!(ivar.type_encoding() == &T::ENCODING);
+            assert!(T::ENCODING.equivalent_to_str(ivar.type_encoding()));
             ivar.offset()
         }
         None => panic!("Ivar {} not found on class {:?}", name, cls),
@@ -604,7 +604,7 @@ mod tests {
         let cls = test_utils::custom_class();
         let ivar = cls.instance_variable("_foo").unwrap();
         assert_eq!(ivar.name(), "_foo");
-        assert!(ivar.type_encoding() == &<u32>::ENCODING);
+        assert!(<u32>::ENCODING.equivalent_to_str(ivar.type_encoding()));
         assert!(ivar.offset() > 0);
 
         let ivars = cls.instance_variables();
@@ -618,8 +618,8 @@ mod tests {
         let method = cls.instance_method(sel).unwrap();
         assert_eq!(method.name().name(), "foo");
         assert_eq!(method.arguments_count(), 2);
-        assert!(*method.return_type() == <u32>::ENCODING);
-        assert!(*method.argument_type(1).unwrap() == Sel::ENCODING);
+        assert!(<u32>::ENCODING.equivalent_to_str(&method.return_type()));
+        assert!(Sel::ENCODING.equivalent_to_str(&method.argument_type(1).unwrap()));
 
         let methods = cls.instance_methods();
         assert!(methods.len() > 0);
@@ -697,10 +697,10 @@ mod tests {
 
     #[test]
     fn test_encode() {
-        assert!(<&Object>::ENCODING.to_string() == "@");
-        assert!(<*mut Object>::ENCODING.to_string() == "@");
-        assert!(<&Class>::ENCODING.to_string() == "#");
-        assert!(Sel::ENCODING.to_string() == ":");
+        assert_eq!(<&Object>::ENCODING.to_string(), "@");
+        assert_eq!(<*mut Object>::ENCODING.to_string(), "@");
+        assert_eq!(<&Class>::ENCODING.to_string(), "#");
+        assert_eq!(Sel::ENCODING.to_string(), ":");
     }
 
     #[test]