Minor improvements

rspotify-model/src/idtypes.rs

@@ -126,7 +126,7 @@ pub enum IdError {
 ///
 /// [module level documentation]: [`crate::idtypes`]
 #[enum_dispatch]
-pub trait Id<'a> {
+pub trait Id {
     /// Returns the inner Spotify object ID, which is guaranteed to be valid for
     /// its type.
     fn id(&self) -> &str;
@@ -324,7 +324,6 @@ macro_rules! define_idtypes {
                 /// long enough when using `Into<Cow<str>>`, so the only
                 /// sensible choice is to just use a `&str`.
                 pub fn from_id_or_uri(id_or_uri: &'a str) -> Result<Self, IdError> {
-                    let id_or_uri = id_or_uri.into();
                     match Self::from_uri(id_or_uri) {
                         Ok(id) => Ok(id),
                         Err(IdError::InvalidPrefix) => Self::from_id(id_or_uri),
@@ -335,7 +334,7 @@ macro_rules! define_idtypes {
                 /// This creates an ID with the underlying `&str` variant from a
                 /// reference. Useful to use an ID multiple times without having
                 /// to clone it.
-                pub fn as_ref(&'a self) -> $name<'a> {
+                pub fn as_ref(&'a self) -> Self {
                     Self(Cow::Borrowed(self.0.as_ref()))
                 }
 
@@ -352,7 +351,7 @@ macro_rules! define_idtypes {
                 }
             }
 
-            impl<'a> Id<'a> for $name<'a> {
+            impl Id for $name<'_> {
                 fn id(&self) -> &str {
                     &self.0
                 }
@@ -484,14 +483,32 @@ pub enum PlayContextId<'a> {
 }
 // These don't work with `enum_dispatch`, unfortunately.
 impl<'a> PlayContextId<'a> {
-    pub fn as_ref(&'a self) -> PlayContextId<'a> {
+    pub fn as_ref(&'a self) -> Self {
         match self {
             PlayContextId::Artist(x) => PlayContextId::Artist(x.as_ref()),
             PlayContextId::Album(x) => PlayContextId::Album(x.as_ref()),
             PlayContextId::Playlist(x) => PlayContextId::Playlist(x.as_ref()),
             PlayContextId::Show(x) => PlayContextId::Show(x.as_ref()),
         }
     }
+
+    pub fn into_static(self) -> PlayContextId<'static> {
+        match self {
+            PlayContextId::Artist(x) => PlayContextId::Artist(x.into_static()),
+            PlayContextId::Album(x) => PlayContextId::Album(x.into_static()),
+            PlayContextId::Playlist(x) => PlayContextId::Playlist(x.into_static()),
+            PlayContextId::Show(x) => PlayContextId::Show(x.into_static()),
+        }
+    }
+
+    pub fn clone_static(&'a self) -> PlayContextId<'static> {
+        match self {
+            PlayContextId::Artist(x) => PlayContextId::Artist(x.clone_static()),
+            PlayContextId::Album(x) => PlayContextId::Album(x.clone_static()),
+            PlayContextId::Playlist(x) => PlayContextId::Playlist(x.clone_static()),
+            PlayContextId::Show(x) => PlayContextId::Show(x.clone_static()),
+        }
+    }
 }
 
 /// Grouping up multiple kinds of IDs to treat them generically. This also
@@ -503,12 +520,26 @@ pub enum PlayableId<'a> {
 }
 // These don't work with `enum_dispatch`, unfortunately.
 impl<'a> PlayableId<'a> {
-    pub fn as_ref(&'a self) -> PlayableId<'a> {
+    pub fn as_ref(&'a self) -> Self {
         match self {
             PlayableId::Track(x) => PlayableId::Track(x.as_ref()),
             PlayableId::Episode(x) => PlayableId::Episode(x.as_ref()),
         }
     }
+
+    pub fn into_static(self) -> PlayableId<'static> {
+        match self {
+            PlayableId::Track(x) => PlayableId::Track(x.into_static()),
+            PlayableId::Episode(x) => PlayableId::Episode(x.into_static()),
+        }
+    }
+
+    pub fn clone_static(&'a self) -> PlayableId<'static> {
+        match self {
+            PlayableId::Track(x) => PlayableId::Track(x.clone_static()),
+            PlayableId::Episode(x) => PlayableId::Episode(x.clone_static()),
+        }
+    }
 }
 
 #[cfg(test)]

rspotify-model/src/lib.rs

@@ -53,6 +53,7 @@ impl PlayableItem {
     ///
     /// Note that if it's a track and if it's local, it may not have an ID, in
     /// which case this function will return `None`.
+    #[must_use]
     pub fn id(&self) -> Option<PlayableId<'_>> {
         match self {
             PlayableItem::Track(t) => t.id.as_ref().map(|t| PlayableId::Track(t.as_ref())),

src/lib.rs

@@ -286,18 +286,8 @@ pub(in crate) fn generate_random_string(length: usize, alphabet: &[u8]) -> Strin
         .collect()
 }
 
-// TODO: is it possible to take a `IntoIterator<Item = T>` instead of a
-// `IntoIterator<Item = &'a>`? Now we have `Id<'a>` instead of `&'a T`, so
-// technically `IntoIterator<Item = &'a T>` is the same as `IntoIterator<Item =
-// T<'a>>`. Otherwise, this function is taking a double reference, and requires
-// more boilerplate.
-//
-// However, this seems to be impossible because then the function would own the
-// Ids, and then we can't call `Id::id`.
-//
-// Hack: turning this into a macro (best if avoided).
 #[inline]
-pub(in crate) fn join_ids<'a, T: Id<'a> + 'a>(ids: impl IntoIterator<Item = T>) -> String {
+pub(in crate) fn join_ids<'a, T: Id + 'a>(ids: impl IntoIterator<Item = T>) -> String {
     let ids = ids.into_iter().collect::<Vec<_>>();
     ids.iter().map(Id::id).collect::<Vec<_>>().join(",")
 }