fix(gix): extend lifetime of iterators

Previously the iterators would return references with lifetimes that
were shorter than the actual lifetimes of the `gix::Reference`
themselves. This was dues to a footgun with `'_` (eliding the lifetime).

When a function returns an elided lifetime, this lifetime usually is the
lifetime of the `&self` parameter, but sometimes it is the lifetime of
the type itself (e.g. `Iter<'_>`).

I made the lifetimes explicit to ensure we were using the correct ones.

Closes #2103

Author: jalil-salame

gix-ref/src/store/file/overlay_iter.rs

@@ -189,11 +189,11 @@ impl Iterator for LooseThenPacked<'_, '_> {
     }
 }
 
-impl Platform<'_> {
+impl<'repo> Platform<'repo> {
     /// Return an iterator over all references, loose or packed, sorted by their name.
     ///
     /// Errors are returned similarly to what would happen when loose and packed refs were iterated by themselves.
-    pub fn all(&self) -> std::io::Result<LooseThenPacked<'_, '_>> {
+    pub fn all<'p>(&'p self) -> std::io::Result<LooseThenPacked<'p, 'repo>> {
         self.store.iter_packed(self.packed.as_ref().map(|b| &***b))
     }
 
@@ -205,14 +205,14 @@ impl Platform<'_> {
     /// starts with `foo`, like `refs/heads/foo` and `refs/heads/foobar`.
     ///
     /// Prefixes are relative paths with slash-separated components.
-    pub fn prefixed(&self, prefix: &RelativePath) -> std::io::Result<LooseThenPacked<'_, '_>> {
+    pub fn prefixed<'p>(&'p self, prefix: &RelativePath) -> std::io::Result<LooseThenPacked<'p, 'repo>> {
         self.store
             .iter_prefixed_packed(prefix, self.packed.as_ref().map(|b| &***b))
     }
 
     /// Return an iterator over the pseudo references, like `HEAD` or `FETCH_HEAD`, or anything else suffixed with `HEAD`
     /// in the root of the `.git` directory, sorted by name.
-    pub fn pseudo(&self) -> std::io::Result<LooseThenPacked<'_, '_>> {
+    pub fn pseudo<'p>(&'p self) -> std::io::Result<LooseThenPacked<'p, 'repo>> {
         self.store.iter_pseudo()
     }
 }

gix/src/reference/iter.rs

@@ -13,15 +13,15 @@ pub struct Platform<'r> {
 }
 
 /// An iterator over references, with or without filter.
-pub struct Iter<'r> {
-    inner: gix_ref::file::iter::LooseThenPacked<'r, 'r>,
+pub struct Iter<'p, 'r> {
+    inner: gix_ref::file::iter::LooseThenPacked<'p, 'r>,
     peel_with_packed: Option<gix_ref::file::packed::SharedBufferSnapshot>,
     peel: bool,
     repo: &'r crate::Repository,
 }
 
-impl<'r> Iter<'r> {
-    fn new(repo: &'r crate::Repository, platform: gix_ref::file::iter::LooseThenPacked<'r, 'r>) -> Self {
+impl<'p, 'r> Iter<'p, 'r> {
+    fn new(repo: &'r crate::Repository, platform: gix_ref::file::iter::LooseThenPacked<'p, 'r>) -> Self {
         Iter {
             inner: platform,
             peel_with_packed: None,
@@ -31,39 +31,55 @@ impl<'r> Iter<'r> {
     }
 }
 
-impl Platform<'_> {
+impl<'repo> Platform<'repo> {
     /// Return an iterator over all references in the repository, excluding
     /// pseudo references.
     ///
     /// Even broken or otherwise unparsable or inaccessible references are returned and have to be handled by the caller on a
     /// case by case basis.
-    pub fn all(&self) -> Result<Iter<'_>, init::Error> {
+    pub fn all<'p>(&'p self) -> Result<Iter<'p, 'repo>, init::Error> {
         Ok(Iter::new(self.repo, self.platform.all()?))
     }
 
     /// Return an iterator over all references that match the given `prefix`.
     ///
     /// These are of the form `refs/heads/` or `refs/remotes/origin`, and must not contain relative paths components like `.` or `..`.
-    pub fn prefixed<'a>(
-        &self,
+    pub fn prefixed<'p, 'a>(
+        &'p self,
         prefix: impl TryInto<&'a RelativePath, Error = gix_path::relative_path::Error>,
-    ) -> Result<Iter<'_>, init::Error> {
+    ) -> Result<Iter<'p, 'repo>, init::Error> {
         Ok(Iter::new(self.repo, self.platform.prefixed(prefix.try_into()?)?))
     }
 
     // TODO: tests
     /// Return an iterator over all references that are tags.
     ///
     /// They are all prefixed with `refs/tags`.
-    pub fn tags(&self) -> Result<Iter<'_>, init::Error> {
+    ///
+    /// ```rust
+    /// # // Regression test for https://github.com/GitoxideLabs/gitoxide/issues/2103
+    /// # // This only ensures we can return a reference, not that the code below is correct
+    /// /// Get the latest tag that isn't a pre-release version
+    /// fn latest_stable_tag(repo: &gix::Repository) -> Result<gix::Reference<'_>, Box<dyn std::error::Error>> {
+    ///     repo.references()?
+    ///         .tags()?
+    ///         .filter_map(|tag| tag.ok())
+    ///         // Warning: lexically sorting version numbers is incorrect, use the semver crate if
+    ///         // you want correct results
+    ///         .max_by_key(|tag| tag.name().shorten().to_owned())
+    ///         .ok_or(std::io::Error::other("latest tag not found"))
+    ///         .map_err(Into::into)
+    /// }
+    /// ```
+    pub fn tags<'p>(&'p self) -> Result<Iter<'p, 'repo>, init::Error> {
         Ok(Iter::new(self.repo, self.platform.prefixed(b"refs/tags/".try_into()?)?))
     }
 
     // TODO: tests
     /// Return an iterator over all local branches.
     ///
     /// They are all prefixed with `refs/heads`.
-    pub fn local_branches(&self) -> Result<Iter<'_>, init::Error> {
+    pub fn local_branches<'p>(&'p self) -> Result<Iter<'p, 'repo>, init::Error> {
         Ok(Iter::new(
             self.repo,
             self.platform.prefixed(b"refs/heads/".try_into()?)?,
@@ -72,23 +88,23 @@ impl Platform<'_> {
 
     // TODO: tests
     /// Return an iterator over all local pseudo references.
-    pub fn pseudo(&self) -> Result<Iter<'_>, init::Error> {
+    pub fn pseudo<'p>(&'p self) -> Result<Iter<'p, 'repo>, init::Error> {
         Ok(Iter::new(self.repo, self.platform.pseudo()?))
     }
 
     // TODO: tests
     /// Return an iterator over all remote branches.
     ///
     /// They are all prefixed with `refs/remotes`.
-    pub fn remote_branches(&self) -> Result<Iter<'_>, init::Error> {
+    pub fn remote_branches<'p>(&'p self) -> Result<Iter<'p, 'repo>, init::Error> {
         Ok(Iter::new(
             self.repo,
             self.platform.prefixed(b"refs/remotes/".try_into()?)?,
         ))
     }
 }
 
-impl Iter<'_> {
+impl Iter<'_, '_> {
     /// Automatically peel references before yielding them during iteration.
     ///
     /// This has the same effect as using `iter.map(|r| {r.peel_to_id_in_place(); r})`.
@@ -104,7 +120,7 @@ impl Iter<'_> {
     }
 }
 
-impl<'r> Iterator for Iter<'r> {
+impl<'r> Iterator for Iter<'_, 'r> {
     type Item = Result<crate::Reference<'r>, Box<dyn std::error::Error + Send + Sync + 'static>>;
 
     fn next(&mut self) -> Option<Self::Item> {