Audit self by-copy vs by-ref on Borrowed types (#5977)

Fixes #5921


We should also land #5975 


<!--
Thank you for your pull request to ICU4X!

Reminder: try to use [Conventional
Comments](https://conventionalcomments.org/) to make comments clearer.

Please see
https://github.com/unicode-org/icu4x/blob/main/CONTRIBUTING.md for
general
information on contributing to ICU4X.
-->

components/datetime/src/provider/pattern/runtime/pattern.rs

@@ -143,7 +143,7 @@ impl<'data> PatternBorrowed<'data> {
         metadata: PatternMetadata::DEFAULT,
     };
 
-    pub(crate) fn as_pattern(&self) -> Pattern<'data> {
+    pub(crate) fn as_pattern(self) -> Pattern<'data> {
         Pattern {
             items: self.items.as_zerovec(),
             metadata: self.metadata,

components/normalizer/src/properties.rs

@@ -44,7 +44,7 @@ use icu_provider::prelude::*;
 /// However, this API is provided for callers such as HarfBuzz that specifically
 /// want access to the raw canonical composition operation e.g. for use in a
 /// glyph-availability-guided custom normalizer.
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub struct CanonicalCompositionBorrowed<'a> {
     canonical_compositions: &'a CanonicalCompositionsV1<'a>,
 }
@@ -100,7 +100,7 @@ impl CanonicalCompositionBorrowed<'_> {
     /// assert_eq!(comp.compose('가', 'ᆨ'), Some('각')); // Hangul LVT
     /// ```
     #[inline(always)]
-    pub fn compose(&self, starter: char, second: char) -> Option<char> {
+    pub fn compose(self, starter: char, second: char) -> Option<char> {
         crate::compose(
             self.canonical_compositions.canonical_compositions.iter(),
             starter,

components/properties/src/code_point_map.rs

@@ -263,7 +263,7 @@ impl<'a, T: TrieValue> CodePointMapDataBorrowed<'a, T> {
 
 impl CodePointMapDataBorrowed<'_, GeneralCategory> {
     /// TODO
-    pub fn get_set_for_value_group(&self, value: GeneralCategoryGroup) -> crate::CodePointSetData {
+    pub fn get_set_for_value_group(self, value: GeneralCategoryGroup) -> crate::CodePointSetData {
         let matching_gc_ranges = self
             .iter_ranges()
             .filter(|cpm_range| (1 << cpm_range.value as u32) & value.0 != 0)

components/properties/src/emoji.rs

@@ -120,13 +120,13 @@ impl EmojiSetDataBorrowed<'_> {
 
     /// Check if the set contains the code point.
     #[inline]
-    pub fn contains(&self, ch: char) -> bool {
+    pub fn contains(self, ch: char) -> bool {
         self.set.contains(ch)
     }
 
     /// See [`Self::contains`].
     #[inline]
-    pub fn contains32(&self, cp: u32) -> bool {
+    pub fn contains32(self, cp: u32) -> bool {
         self.set.contains32(cp)
     }
 }

components/properties/src/names.rs

@@ -150,7 +150,7 @@ impl<T: TrieValue> PropertyParserBorrowed<'_, T> {
     /// assert_eq!(lookup.get_strict_u16("UppercaseLetter"), None);
     /// ```
     #[inline]
-    pub fn get_strict_u16(&self, name: &str) -> Option<u16> {
+    pub fn get_strict_u16(self, name: &str) -> Option<u16> {
         get_strict_u16(self.map, name)
     }
 
@@ -176,7 +176,7 @@ impl<T: TrieValue> PropertyParserBorrowed<'_, T> {
     /// assert_eq!(lookup.get_strict("UppercaseLetter"), None);
     /// ```
     #[inline]
-    pub fn get_strict(&self, name: &str) -> Option<T> {
+    pub fn get_strict(self, name: &str) -> Option<T> {
         T::try_from_u32(self.get_strict_u16(name)? as u32).ok()
     }
 
@@ -206,7 +206,7 @@ impl<T: TrieValue> PropertyParserBorrowed<'_, T> {
     /// );
     /// ```
     #[inline]
-    pub fn get_loose_u16(&self, name: &str) -> Option<u16> {
+    pub fn get_loose_u16(self, name: &str) -> Option<u16> {
         get_loose_u16(self.map, name)
     }
 
@@ -236,7 +236,7 @@ impl<T: TrieValue> PropertyParserBorrowed<'_, T> {
     /// );
     /// ```
     #[inline]
-    pub fn get_loose(&self, name: &str) -> Option<T> {
+    pub fn get_loose(self, name: &str) -> Option<T> {
         T::try_from_u32(self.get_loose_u16(name)? as u32).ok()
     }
 }
@@ -411,7 +411,7 @@ impl<T: NamedEnumeratedProperty> PropertyNamesLong<T> {
     }
 }
 
-impl<T: NamedEnumeratedProperty> PropertyNamesLongBorrowed<'_, T> {
+impl<'a, T: NamedEnumeratedProperty> PropertyNamesLongBorrowed<'a, T> {
     /// Get the property name given a value
     ///
     /// # Example
@@ -431,7 +431,7 @@ impl<T: NamedEnumeratedProperty> PropertyNamesLongBorrowed<'_, T> {
     /// );
     /// ```
     #[inline]
-    pub fn get(&self, property: T) -> Option<&str> {
+    pub fn get(self, property: T) -> Option<&'a str> {
         self.map.get(property.to_u32())
     }
 }
@@ -544,7 +544,7 @@ impl<T: NamedEnumeratedProperty> PropertyNamesShort<T> {
     }
 }
 
-impl<T: NamedEnumeratedProperty> PropertyNamesShortBorrowed<'_, T> {
+impl<'a, T: NamedEnumeratedProperty> PropertyNamesShortBorrowed<'a, T> {
     /// Get the property name given a value
     ///
     /// # Example
@@ -558,7 +558,7 @@ impl<T: NamedEnumeratedProperty> PropertyNamesShortBorrowed<'_, T> {
     /// assert_eq!(lookup.get(CanonicalCombiningClass::AboveLeft), Some("AL"));
     /// ```
     #[inline]
-    pub fn get(&self, property: T) -> Option<&str> {
+    pub fn get(self, property: T) -> Option<&'a str> {
         self.map.get(property.to_u32())
     }
 }
@@ -603,7 +603,7 @@ impl PropertyNamesShortBorrowed<'_, Script> {
     /// );
     /// ```
     #[inline]
-    pub fn get_locale_script(&self, property: Script) -> Option<icu_locale_core::subtags::Script> {
+    pub fn get_locale_script(self, property: Script) -> Option<icu_locale_core::subtags::Script> {
         let prop = usize::try_from(property.to_u32()).ok()?;
         self.map.map.get(prop).and_then(|o| o.0)
     }

components/timezone/src/windows_tz.rs

@@ -79,7 +79,7 @@ impl WindowsTimeZoneMapper {
 }
 
 /// A borrowed wrapper around the windows time zone mapper data.
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub struct WindowsTimeZoneMapperBorrowed<'a> {
     data: &'a WindowsZonesToBcp47MapV1<'a>,
 }
@@ -125,7 +125,7 @@ impl WindowsTimeZoneMapperBorrowed<'_> {
     /// assert_eq!(bcp47_id, Some(TimeZoneBcp47Id(tinystr!(8, "cawnp"))));
     /// ```
     pub fn windows_tz_to_bcp47_id(
-        &self,
+        self,
         windows_tz: &str,
         region: Option<Region>,
     ) -> Option<TimeZoneBcp47Id> {

provider/core/src/request.rs

@@ -95,15 +95,15 @@ impl<'a> DataIdentifierBorrowed<'a> {
     }
 
     /// Converts this [`DataIdentifierBorrowed`] into a [`DataIdentifierCow<'static>`].
-    pub fn into_owned(&self) -> DataIdentifierCow<'static> {
+    pub fn into_owned(self) -> DataIdentifierCow<'static> {
         DataIdentifierCow {
             marker_attributes: Cow::Owned(self.marker_attributes.to_owned()),
             locale: Cow::Owned(self.locale.clone()),
         }
     }
 
     /// Borrows this [`DataIdentifierBorrowed`] as a [`DataIdentifierCow<'a>`].
-    pub fn as_cow(&self) -> DataIdentifierCow<'a> {
+    pub fn as_cow(self) -> DataIdentifierCow<'a> {
         DataIdentifierCow {
             marker_attributes: Cow::Borrowed(self.marker_attributes),
             locale: Cow::Borrowed(self.locale),

utils/zerovec/src/map/borrowed.rs

@@ -127,12 +127,12 @@ where
     }
 
     /// The number of elements in the [`ZeroMapBorrowed`]
-    pub fn len(&self) -> usize {
+    pub fn len(self) -> usize {
         self.values.zvl_len()
     }
 
     /// Whether the [`ZeroMapBorrowed`] is empty
-    pub fn is_empty(&self) -> bool {
+    pub fn is_empty(self) -> bool {
         self.values.zvl_len() == 0
     }
 }
@@ -160,7 +160,7 @@ where
     /// assert_eq!(borrowed.get(&1), Some("one"));
     /// assert_eq!(borrowed.get(&3), None);
     /// ```
-    pub fn get(&self, key: &K) -> Option<&'a V::GetType> {
+    pub fn get(self, key: &K) -> Option<&'a V::GetType> {
         let index = self.keys.zvl_binary_search(key).ok()?;
         self.values.zvl_get(index)
     }
@@ -181,7 +181,7 @@ where
     /// assert_eq!(borrowed.get_by(|probe| probe.cmp(&1)), Some("one"));
     /// assert_eq!(borrowed.get_by(|probe| probe.cmp(&3)), None);
     /// ```
-    pub fn get_by(&self, predicate: impl FnMut(&K) -> Ordering) -> Option<&'a V::GetType> {
+    pub fn get_by(self, predicate: impl FnMut(&K) -> Ordering) -> Option<&'a V::GetType> {
         let index = self.keys.zvl_binary_search_by(predicate).ok()?;
         self.values.zvl_get(index)
     }
@@ -198,7 +198,7 @@ where
     /// assert!(borrowed.contains_key(&1));
     /// assert!(!borrowed.contains_key(&3));
     /// ```
-    pub fn contains_key(&self, key: &K) -> bool {
+    pub fn contains_key(self, key: &K) -> bool {
         self.keys.zvl_binary_search(key).is_ok()
     }
 }
@@ -209,27 +209,25 @@ where
     V: ZeroMapKV<'a> + ?Sized,
 {
     /// Produce an ordered iterator over key-value pairs
-    pub fn iter<'b>(
-        &'b self,
+    pub fn iter(
+        self,
     ) -> impl Iterator<
         Item = (
             &'a <K as ZeroMapKV<'a>>::GetType,
             &'a <V as ZeroMapKV<'a>>::GetType,
         ),
-    > + 'b {
+    > {
         self.iter_keys().zip(self.iter_values())
     }
 
     /// Produce an ordered iterator over keys
-    pub fn iter_keys<'b>(&'b self) -> impl Iterator<Item = &'a <K as ZeroMapKV<'a>>::GetType> + 'b {
+    pub fn iter_keys(self) -> impl Iterator<Item = &'a <K as ZeroMapKV<'a>>::GetType> {
         #[allow(clippy::unwrap_used)] // idx in 0..keys.zvl_len()
         (0..self.keys.zvl_len()).map(move |idx| self.keys.zvl_get(idx).unwrap())
     }
 
     /// Produce an iterator over values, ordered by keys
-    pub fn iter_values<'b>(
-        &'b self,
-    ) -> impl Iterator<Item = &'a <V as ZeroMapKV<'a>>::GetType> + 'b {
+    pub fn iter_values(self) -> impl Iterator<Item = &'a <V as ZeroMapKV<'a>>::GetType> {
         #[allow(clippy::unwrap_used)] // idx in 0..keys.zvl_len() == values.zvl_len()
         (0..self.values.zvl_len()).map(move |idx| self.values.zvl_get(idx).unwrap())
     }
@@ -241,22 +239,22 @@ where
     V: ZeroMapKV<'a, Slice = ZeroSlice<V>> + AsULE + Copy + 'static,
 {
     /// For cases when `V` is fixed-size, obtain a direct copy of `V` instead of `V::ULE`
-    pub fn get_copied(&self, key: &K) -> Option<V> {
+    pub fn get_copied(self, key: &K) -> Option<V> {
         let index = self.keys.zvl_binary_search(key).ok()?;
         self.values.get(index)
     }
 
     /// For cases when `V` is fixed-size, obtain a direct copy of `V` instead of `V::ULE`
-    pub fn get_copied_by(&self, predicate: impl FnMut(&K) -> Ordering) -> Option<V> {
+    pub fn get_copied_by(self, predicate: impl FnMut(&K) -> Ordering) -> Option<V> {
         let index = self.keys.zvl_binary_search_by(predicate).ok()?;
         self.values.get(index)
     }
 
     /// Similar to [`Self::iter()`] except it returns a direct copy of the values instead of references
     /// to `V::ULE`, in cases when `V` is fixed-size
-    pub fn iter_copied_values<'b>(
-        &'b self,
-    ) -> impl Iterator<Item = (&'b <K as ZeroMapKV<'a>>::GetType, V)> {
+    pub fn iter_copied_values(
+        self,
+    ) -> impl Iterator<Item = (&'a <K as ZeroMapKV<'a>>::GetType, V)> {
         (0..self.keys.zvl_len()).map(move |idx| {
             (
                 #[allow(clippy::unwrap_used)] // idx in 0..keys.zvl_len()
@@ -276,16 +274,14 @@ where
     /// Similar to [`Self::iter()`] except it returns a direct copy of the keys values instead of references
     /// to `K::ULE` and `V::ULE`, in cases when `K` and `V` are fixed-size
     #[allow(clippy::needless_lifetimes)] // Lifetime is necessary in impl Trait
-    pub fn iter_copied<'b: 'a>(&'b self) -> impl Iterator<Item = (K, V)> + 'b {
-        let keys = &self.keys;
-        let values = &self.values;
+    pub fn iter_copied(self) -> impl Iterator<Item = (K, V)> + 'a {
         let len = self.keys.zvl_len();
         (0..len).map(move |idx| {
             (
                 #[allow(clippy::unwrap_used)] // idx in 0..keys.zvl_len()
-                ZeroSlice::get(keys, idx).unwrap(),
+                ZeroSlice::get(self.keys, idx).unwrap(),
                 #[allow(clippy::unwrap_used)] // idx in 0..keys.zvl_len() = values.zvl_len()
-                ZeroSlice::get(values, idx).unwrap(),
+                ZeroSlice::get(self.values, idx).unwrap(),
             )
         })
     }