From cd7b9d4d120f3e1a5a80c7036177bf3dbeff11de Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 4 Oct 2021 23:51:19 +0200 Subject: [PATCH] Rename NSArray and NSDictionary methods to better match std naming Also add a few bound checks since throwing exceptions from Objective-C is still UB --- objc2_foundation/examples/basic_usage.rs | 6 +- objc2_foundation/src/array.rs | 241 +++++++++++++---------- objc2_foundation/src/dictionary.rs | 62 +++--- 3 files changed, 167 insertions(+), 142 deletions(-) diff --git a/objc2_foundation/examples/basic_usage.rs b/objc2_foundation/examples/basic_usage.rs index 6af8fb83c..2d4510772 100644 --- a/objc2_foundation/examples/basic_usage.rs +++ b/objc2_foundation/examples/basic_usage.rs @@ -17,7 +17,7 @@ fn main() { for obj in array.object_enumerator() { println!("{:?}", obj); } - println!("{}", array.count()); + println!("{}", array.len()); // Turn the NSArray back into a Vec let mut objs = NSArray::into_vec(array); @@ -33,6 +33,6 @@ fn main() { let keys = &[&*string]; let vals = vec![obj]; let dict = NSDictionary::from_keys_and_objects(keys, vals); - println!("{:?}", dict.object_for(&string)); - println!("{}", dict.count()); + println!("{:?}", dict.get(&string)); + println!("{}", dict.len()); } diff --git a/objc2_foundation/src/array.rs b/objc2_foundation/src/array.rs index 405eb782a..393dd5d3d 100644 --- a/objc2_foundation/src/array.rs +++ b/objc2_foundation/src/array.rs @@ -86,37 +86,70 @@ pub trait INSArray: INSObject { type Item: INSObject; type ItemOwnership: Ownership; - fn count(&self) -> usize { + #[doc(alias = "count")] + fn len(&self) -> usize { unsafe { msg_send![self, count] } } - fn object_at(&self, index: usize) -> &Self::Item { - unsafe { - let obj: *const Self::Item = msg_send![self, objectAtIndex: index]; - &*obj + #[doc(alias = "objectAtIndex:")] + fn get(&self, index: usize) -> Option<&Self::Item> { + // TODO: Replace this check with catching the thrown NSRangeException + if index < self.len() { + // SAFETY: The index is checked to be in bounds. + Some(unsafe { msg_send![self, objectAtIndex: index] }) + } else { + None } } - fn first_object(&self) -> Option<&Self::Item> { - unsafe { - let obj: *const Self::Item = msg_send![self, firstObject]; - if obj.is_null() { - None - } else { - Some(&*obj) - } + #[doc(alias = "objectAtIndex:")] + fn get_mut(&mut self, index: usize) -> Option<&mut Self::Item> + where + Self: INSArray, + { + // TODO: Replace this check with catching the thrown NSRangeException + if index < self.len() { + // SAFETY: The index is checked to be in bounds. + Some(unsafe { msg_send![self, objectAtIndex: index] }) + } else { + None } } - fn last_object(&self) -> Option<&Self::Item> { - unsafe { - let obj: *const Self::Item = msg_send![self, lastObject]; - if obj.is_null() { - None - } else { - Some(&*obj) - } - } + #[doc(alias = "objectAtIndex:")] + fn get_retained(&self, index: usize) -> Id + where + Self: INSArray, + { + let obj = self.get(index).unwrap(); + // SAFETY: The object is originally shared (see `where` bound). + unsafe { Id::retain(obj.into()) } + } + + #[doc(alias = "firstObject")] + fn first(&self) -> Option<&Self::Item> { + unsafe { msg_send![self, firstObject] } + } + + #[doc(alias = "firstObject")] + fn first_mut(&mut self) -> Option<&mut Self::Item> + where + Self: INSArray, + { + unsafe { msg_send![self, firstObject] } + } + + #[doc(alias = "lastObject")] + fn last(&self) -> Option<&Self::Item> { + unsafe { msg_send![self, lastObject] } + } + + #[doc(alias = "lastObject")] + fn last_mut(&mut self) -> Option<&mut Self::Item> + where + Self: INSArray, + { + unsafe { msg_send![self, lastObject] } } fn object_enumerator(&self) -> NSEnumerator { @@ -142,7 +175,7 @@ pub trait INSArray: INSObject { } fn to_vec(&self) -> Vec<&Self::Item> { - self.objects_in_range(0..self.count()) + self.objects_in_range(0..self.len()) } // TODO: Take Id ? @@ -154,24 +187,6 @@ pub trait INSArray: INSObject { .collect() } - fn mut_object_at(&mut self, index: usize) -> &mut Self::Item - where - Self: INSArray, - { - unsafe { - let result: *mut Self::Item = msg_send![self, objectAtIndex: index]; - &mut *result - } - } - - fn shared_object_at(&self, index: usize) -> Id - where - Self: INSArray, - { - let obj = self.object_at(index); - unsafe { Id::retain(obj.into()) } - } - fn from_slice(slice: &[Id]) -> Id where Self: INSArray, @@ -241,30 +256,41 @@ impl Index for NSArray { type Output = T; fn index(&self, index: usize) -> &T { - self.object_at(index) + self.get(index).unwrap() } } pub trait INSMutableArray: INSArray { - fn add_object(&mut self, obj: Id) { - unsafe { - let _: () = msg_send![self, addObject: &*obj]; - } - } - - fn insert_object_at(&mut self, index: usize, obj: Id) { - unsafe { - let _: () = msg_send![self, insertObject: &*obj, atIndex: index]; + #[doc(alias = "addObject:")] + fn push(&mut self, obj: Id) { + // SAFETY: The object is not nil + unsafe { msg_send![self, addObject: &*obj] } + } + + #[doc(alias = "insertObject:atIndex:")] + fn insert(&mut self, index: usize, obj: Id) { + // TODO: Replace this check with catching the thrown NSRangeException + let len = self.len(); + if index < len { + // SAFETY: The object is not nil and the index is checked to be in + // bounds. + unsafe { msg_send![self, insertObject: &*obj, atIndex: index] } + } else { + panic!( + "insertion index (is {}) should be <= len (is {})", + index, len + ); } } - fn replace_object_at( + #[doc(alias = "replaceObjectAtIndex:withObject:")] + fn replace( &mut self, index: usize, obj: Id, ) -> Id { let old_obj = unsafe { - let obj = self.object_at(index); + let obj = self.get(index).unwrap(); Id::retain(obj.into()) }; unsafe { @@ -277,10 +303,12 @@ pub trait INSMutableArray: INSArray { old_obj } - fn remove_object_at(&mut self, index: usize) -> Id { - let obj = unsafe { - let obj = self.object_at(index); - Id::retain(obj.into()) + #[doc(alias = "removeObjectAtIndex:")] + fn remove(&mut self, index: usize) -> Id { + let obj = if let Some(obj) = self.get(index) { + unsafe { Id::retain(obj.into()) } + } else { + panic!("removal index should be < len"); }; unsafe { let _: () = msg_send![self, removeObjectAtIndex: index]; @@ -288,24 +316,23 @@ pub trait INSMutableArray: INSArray { obj } - fn remove_last_object(&mut self) -> Id { - let obj = self - .last_object() - .map(|obj| unsafe { Id::retain(obj.into()) }); - unsafe { - let _: () = msg_send![self, removeLastObject]; - } - // removeLastObject would have failed if the array is empty, - // so we know this won't be None - obj.unwrap() + #[doc(alias = "removeLastObject")] + fn pop(&mut self) -> Option> { + self.last().map(|obj| { + let obj = unsafe { Id::retain(obj.into()) }; + unsafe { + let _: () = msg_send![self, removeLastObject]; + } + obj + }) } - fn remove_all_objects(&mut self) { - unsafe { - let _: () = msg_send![self, removeAllObjects]; - } + #[doc(alias = "removeAllObjects")] + fn clear(&mut self) { + unsafe { msg_send![self, removeAllObjects] } } + #[doc(alias = "sortUsingFunction:context:")] fn sort_by(&mut self, compare: F) where F: FnMut(&Self::Item, &Self::Item) -> Ordering, @@ -376,7 +403,7 @@ impl Index for NSMutableArray { type Output = T; fn index(&self, index: usize) -> &T { - self.object_at(index) + self.get(index).unwrap() } } @@ -398,24 +425,24 @@ mod tests { } #[test] - fn test_count() { + fn test_len() { let empty_array = NSArray::::new(); - assert!(empty_array.count() == 0); + assert_eq!(empty_array.len(), 0); let array = sample_array(4); - assert!(array.count() == 4); + assert_eq!(array.len(), 4); } #[test] - fn test_object_at() { + fn test_get() { let array = sample_array(4); - assert!(array.object_at(0) != array.object_at(3)); - assert!(array.first_object().unwrap() == array.object_at(0)); - assert!(array.last_object().unwrap() == array.object_at(3)); + assert_ne!(array.get(0), array.get(3)); + assert_eq!(array.first(), array.get(0)); + assert_eq!(array.last(), array.get(3)); let empty_array: Id, _> = INSObject::new(); - assert!(empty_array.first_object().is_none()); - assert!(empty_array.last_object().is_none()); + assert!(empty_array.first().is_none()); + assert!(empty_array.last().is_none()); } #[test] @@ -426,7 +453,7 @@ mod tests { assert!(array .object_enumerator() .enumerate() - .all(|(i, obj)| obj == array.object_at(i))); + .all(|(i, obj)| Some(obj) == array.get(i))); } #[test] @@ -434,15 +461,15 @@ mod tests { let array = sample_array(4); let middle_objs = array.objects_in_range(1..3); - assert!(middle_objs.len() == 2); - assert!(middle_objs[0] == array.object_at(1)); - assert!(middle_objs[1] == array.object_at(2)); + assert_eq!(middle_objs.len(), 2); + assert_eq!(middle_objs[0], array.get(1).unwrap()); + assert_eq!(middle_objs[1], array.get(2).unwrap()); let empty_objs = array.objects_in_range(1..1); assert!(empty_objs.is_empty()); let all_objs = array.objects_in_range(0..4); - assert!(all_objs.len() == 4); + assert_eq!(all_objs.len(), 4); } #[test] @@ -450,49 +477,49 @@ mod tests { let array = sample_array(4); let vec = INSArray::into_vec(array); - assert!(vec.len() == 4); + assert_eq!(vec.len(), 4); } #[test] - fn test_add_object() { + fn test_adding() { let mut array = NSMutableArray::new(); let obj = NSObject::new(); - array.add_object(obj); + array.push(obj); - assert!(array.count() == 1); - assert!(array.object_at(0) == array.object_at(0)); + assert_eq!(array.len(), 1); + assert_eq!(array.get(0), array.get(0)); let obj = NSObject::new(); - array.insert_object_at(0, obj); - assert!(array.count() == 2); + array.insert(0, obj); + assert_eq!(array.len(), 2); } #[test] - fn test_replace_object() { + fn test_replace() { let mut array = NSMutableArray::new(); let obj = NSObject::new(); - array.add_object(obj); + array.push(obj); let obj = NSObject::new(); - let old_obj = array.replace_object_at(0, obj); - assert!(&*old_obj != array.object_at(0)); + let old_obj = array.replace(0, obj); + assert_ne!(&*old_obj, array.get(0).unwrap()); } #[test] - fn test_remove_object() { + fn test_remove() { let mut array = NSMutableArray::new(); for _ in 0..4 { - array.add_object(NSObject::new()); + array.push(NSObject::new()); } - array.remove_object_at(1); - assert!(array.count() == 3); + let _ = array.remove(1); + assert_eq!(array.len(), 3); - array.remove_last_object(); - assert!(array.count() == 2); + let _ = array.pop(); + assert_eq!(array.len(), 2); - array.remove_all_objects(); - assert!(array.count() == 0); + array.clear(); + assert_eq!(array.len(), 0); } #[test] @@ -501,7 +528,7 @@ mod tests { let mut strings = NSMutableArray::from_vec(strings); strings.sort_by(|s1, s2| s1.as_str().len().cmp(&s2.as_str().len())); - assert!(strings[0].as_str() == "hi"); - assert!(strings[1].as_str() == "hello"); + assert_eq!(strings[0].as_str(), "hi"); + assert_eq!(strings[1].as_str(), "hello"); } } diff --git a/objc2_foundation/src/dictionary.rs b/objc2_foundation/src/dictionary.rs index c07289bbc..d8eb57d32 100644 --- a/objc2_foundation/src/dictionary.rs +++ b/objc2_foundation/src/dictionary.rs @@ -32,23 +32,19 @@ pub trait INSDictionary: INSObject { type Value: INSObject; type ValueOwnership: Ownership; - fn count(&self) -> usize { + #[doc(alias = "count")] + fn len(&self) -> usize { unsafe { msg_send![self, count] } } - fn object_for(&self, key: &Self::Key) -> Option<&Self::Value> { - unsafe { - let obj: *mut Self::Value = msg_send![self, objectForKey: key]; - if obj.is_null() { - None - } else { - Some(&*obj) - } - } + #[doc(alias = "objectForKey:")] + fn get(&self, key: &Self::Key) -> Option<&Self::Value> { + unsafe { msg_send![self, objectForKey: key] } } + #[doc(alias = "getObjects:andKeys:")] fn keys(&self) -> Vec<&Self::Key> { - let len = self.count(); + let len = self.len(); let mut keys = Vec::with_capacity(len); unsafe { let _: () = msg_send![ @@ -61,8 +57,9 @@ pub trait INSDictionary: INSObject { keys } + #[doc(alias = "getObjects:andKeys:")] fn values(&self) -> Vec<&Self::Value> { - let len = self.count(); + let len = self.len(); let mut vals = Vec::with_capacity(len); unsafe { let _: () = msg_send![ @@ -75,8 +72,9 @@ pub trait INSDictionary: INSObject { vals } + #[doc(alias = "getObjects:andKeys:")] fn keys_and_objects(&self) -> (Vec<&Self::Key>, Vec<&Self::Value>) { - let len = self.count(); + let len = self.len(); let mut keys = Vec::with_capacity(len); let mut objs = Vec::with_capacity(len); unsafe { @@ -162,7 +160,7 @@ impl<'a, K: INSObject, V: INSObject> Index<&'a K> for NSDictionary { type Output = V; fn index(&self, index: &K) -> &V { - self.object_for(index).unwrap() + self.get(index).unwrap() } } @@ -181,20 +179,20 @@ mod tests { } #[test] - fn test_count() { + fn test_len() { let dict = sample_dict("abcd"); - assert!(dict.count() == 1); + assert_eq!(dict.len(), 1); } #[test] - fn test_object_for() { + fn test_get() { let dict = sample_dict("abcd"); let string = NSString::from_str("abcd"); - assert!(dict.object_for(&string).is_some()); + assert!(dict.get(&string).is_some()); let string = NSString::from_str("abcde"); - assert!(dict.object_for(&string).is_none()); + assert!(dict.get(&string).is_none()); } #[test] @@ -202,8 +200,8 @@ mod tests { let dict = sample_dict("abcd"); let keys = dict.keys(); - assert!(keys.len() == 1); - assert!(keys[0].as_str() == "abcd"); + assert_eq!(keys.len(), 1); + assert_eq!(keys[0].as_str(), "abcd"); } #[test] @@ -211,7 +209,7 @@ mod tests { let dict = sample_dict("abcd"); let vals = dict.values(); - assert!(vals.len() == 1); + assert_eq!(vals.len(), 1); } #[test] @@ -219,23 +217,23 @@ mod tests { let dict = sample_dict("abcd"); let (keys, objs) = dict.keys_and_objects(); - assert!(keys.len() == 1); - assert!(objs.len() == 1); - assert!(keys[0].as_str() == "abcd"); - assert!(objs[0] == dict.object_for(keys[0]).unwrap()); + assert_eq!(keys.len(), 1); + assert_eq!(objs.len(), 1); + assert_eq!(keys[0].as_str(), "abcd"); + assert_eq!(objs[0], dict.get(keys[0]).unwrap()); } #[test] fn test_key_enumerator() { let dict = sample_dict("abcd"); - assert!(dict.key_enumerator().count() == 1); - assert!(dict.key_enumerator().next().unwrap().as_str() == "abcd"); + assert_eq!(dict.key_enumerator().count(), 1); + assert_eq!(dict.key_enumerator().next().unwrap().as_str(), "abcd"); } #[test] fn test_object_enumerator() { let dict = sample_dict("abcd"); - assert!(dict.object_enumerator().count() == 1); + assert_eq!(dict.object_enumerator().count(), 1); } #[test] @@ -243,10 +241,10 @@ mod tests { let dict = sample_dict("abcd"); let keys = dict.keys_array(); - assert!(keys.count() == 1); - assert!(keys.object_at(0).as_str() == "abcd"); + assert_eq!(keys.len(), 1); + assert_eq!(keys[0].as_str(), "abcd"); // let objs = INSDictionary::into_values_array(dict); - // assert!(objs.count() == 1); + // assert_eq!(objs.len(), 1); } }