[Result migration] Don't return options for StringArray iterators

examples/tag.rs

@@ -65,7 +65,7 @@ fn run(args: &Args) -> Result<(), Error> {
     } else if args.flag_list {
         let pattern = args.arg_pattern.as_ref().map(|s| &s[..]).unwrap_or("*");
         for name in repo.tag_names(Some(pattern))?.iter() {
-            let name = name.expect("Not invalid utf8").expect("Not None");
+            let name = name.expect("Not invalid utf8");
             let obj = repo.revparse_single(name)?;
 
             if let Some(tag) = obj.as_tag() {

src/remote.rs

@@ -886,7 +886,7 @@ mod tests {
             assert_eq!(remotes.len(), 1);
             assert_eq!(remotes.get(0), Ok(Some("origin")));
             assert_eq!(remotes.iter().count(), 1);
-            assert_eq!(remotes.iter().next().unwrap(), Ok(Some("origin")));
+            assert_eq!(remotes.iter().next().unwrap(), Ok("origin"));
         }
 
         origin.connect(Direction::Push).unwrap();

src/string_array.rs

@@ -52,8 +52,8 @@ impl StringArray {
 
     /// Returns an iterator over the strings contained within this array.
     ///
-    /// The iterator yields `Option<&str>` as it is unknown whether the contents
-    /// are utf-8 or not.
+    /// The iterator yields `Result<&str, Error>` as it is unknown whether the
+    /// contents are utf-8 or not.
     pub fn iter(&self) -> Iter<'_> {
         Iter {
             range: 0..self.len(),
@@ -92,25 +92,43 @@ impl Binding for StringArray {
 }
 
 impl<'a> IntoIterator for &'a StringArray {
-    type Item = Result<Option<&'a str>, Error>;
+    type Item = Result<&'a str, Error>;
     type IntoIter = Iter<'a>;
     fn into_iter(self) -> Self::IntoIter {
         self.iter()
     }
 }
 
 impl<'a> Iterator for Iter<'a> {
-    type Item = Result<Option<&'a str>, Error>;
-    fn next(&mut self) -> Option<Result<Option<&'a str>, Error>> {
-        self.range.next().map(|i| self.arr.get(i))
+    type Item = Result<&'a str, Error>;
+    fn next(&mut self) -> Option<Result<&'a str, Error>> {
+        // Convert from Result<Option<&str>, Error> to just
+        // Result<&str, Error> - if Ok(None) was returned by self.arr.get()
+        // then something went wrong with the array and panicking is the
+        // right decision, see discussion on #1263.
+        self.range.next().map(|i| {
+            self.arr
+                .get(i)
+                .transpose()
+                .expect("StringArray should have values for all indices in its size")
+        })
     }
     fn size_hint(&self) -> (usize, Option<usize>) {
         self.range.size_hint()
     }
 }
 impl<'a> DoubleEndedIterator for Iter<'a> {
-    fn next_back(&mut self) -> Option<Result<Option<&'a str>, Error>> {
-        self.range.next_back().map(|i| self.arr.get(i))
+    fn next_back(&mut self) -> Option<Result<&'a str, Error>> {
+        // Convert from Result<Option<&str>, Error> to just
+        // Result<&str, Error> - if Ok(None) was returned by self.arr.get()
+        // then something went wrong with the array and panicking is the
+        // right decision, see discussion on #1263.
+        self.range.next_back().map(|i| {
+            self.arr
+                .get(i)
+                .transpose()
+                .expect("StringArray should have values for all indices in its size")
+        })
     }
 }
 impl<'a> FusedIterator for Iter<'a> {}

tests/add_extensions.rs

@@ -16,16 +16,16 @@ fn test_add_extensions() -> Result<(), Error> {
     assert_eq!(
         extensions,
         [
-            Some("custom"),
-            Some("noop"),
+            "custom",
+            "noop",
             // The objectformat extension was added in 1.6
-            Some("objectformat"),
+            "objectformat",
             // The preciousobjects extension was added in 1.9
-            Some("preciousobjects"),
+            "preciousobjects",
             // The relativeworktrees extension was added in 1.9.4
-            Some("relativeworktrees"),
+            "relativeworktrees",
             // The worktreeconfig extension was added in 1.8
-            Some("worktreeconfig")
+            "worktreeconfig"
         ]
     );
 

tests/get_extensions.rs

@@ -12,15 +12,15 @@ fn test_get_extensions() -> Result<(), Error> {
     assert_eq!(
         extensions,
         [
-            Some("noop"),
+            "noop",
             // The objectformat extension was added in 1.6
-            Some("objectformat"),
+            "objectformat",
             // The preciousobjects extension was added in 1.9
-            Some("preciousobjects"),
+            "preciousobjects",
             // The relativeworktrees extension was added in 1.9.4
-            Some("relativeworktrees"),
+            "relativeworktrees",
             // The worktreeconfig extension was added in 1.8
-            Some("worktreeconfig")
+            "worktreeconfig"
         ]
     );
 

tests/remove_extensions.rs

@@ -22,7 +22,7 @@ fn test_remove_extensions() -> Result<(), Error> {
     let extensions: Result<Vec<_>, Error> = extensions.iter().collect();
     let extensions = extensions.unwrap();
 
-    assert_eq!(extensions, [Some("custom"), Some("other")]);
+    assert_eq!(extensions, ["custom", "other"]);
 
     Ok(())
 }