Repository: rust-itertools/itertools (language: rust)

Issue:
Should 'ExactlyOneError' implement std-'Error'?
The `exactly_one()` function was added in https://github.com/rust-itertools/itertools/pull/310:
```rust
exactly_one(self) -> Result<Self::Item, ExactlyOneError<Self>> 
```

For using the early return operator `?` on its result, it would be nice if `ExactlyOneError` would implement `Error`. Or is there any reason not to?

cc @Xaeroxe @Centril @bluss 


Discussion / hints:
Fine by me
@njam Would you like to file a PR? This would be sufficient:
```rust
impl<I: Iterator> Error for ExactlyOneError<I> {}
```

I want to PR for a small change to the innards of the thing, so I'll get it.

Fix patch:
diff --git a/src/exactly_one_err.rs b/src/exactly_one_err.rs
index e4925ffb7..63485c993 100644
--- a/src/exactly_one_err.rs
+++ b/src/exactly_one_err.rs
@@ -1,5 +1,11 @@
+#[cfg(feature = "use_std")]
+use std::error::Error;
+use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
+
 use std::iter::ExactSizeIterator;
 
+use either::Either;
+
 use crate::size_hint;
 
 /// Iterator returned for the error case of `IterTools::exactly_one()`
@@ -10,12 +16,12 @@ use crate::size_hint;
 ///
 /// This is very similar to PutBackN except this iterator only supports 0-2 elements and does not
 /// use a `Vec`.
-#[derive(Debug, Clone)]
+#[derive(Clone)]
 pub struct ExactlyOneError<I>
 where
     I: Iterator,
 {
-    first_two: (Option<I::Item>, Option<I::Item>),
+    first_two: Option<Either<[I::Item; 2], I::Item>>,
     inner: I,
 }
 
@@ -24,9 +30,17 @@ where
     I: Iterator,
 {
     /// Creates a new `ExactlyOneErr` iterator.
-    pub(crate) fn new(first_two: (Option<I::Item>, Option<I::Item>), inner: I) -> Self {
+    pub(crate) fn new(first_two: Option<Either<[I::Item; 2], I::Item>>, inner: I) -> Self {
         Self { first_two, inner }
     }
+
+    fn additional_len(&self) -> usize {
+        match self.first_two {
+            Some(Either::Left(_)) => 2,
+            Some(Either::Right(_)) => 1,
+            None => 0,
+        }
+    }
 }
 
 impl<I> Iterator for ExactlyOneError<I>
@@ -36,23 +50,61 @@ where
     type Item = I::Item;
 
     fn next(&mut self) -> Option<Self::Item> {
-        self.first_two
-            .0
-            .take()
-            .or_else(|| self.first_two.1.take())
-            .or_else(|| self.inner.next())
+        match self.first_two.take() {
+            Some(Either::Left([first, second])) => {
+                self.first_two = Some(Either::Right(second));
+                Some(first)
+            },
+            Some(Either::Right(second)) => {
+                Some(second)
+            }
+            None => {
+                self.inner.next()
+            }
+        }
     }
 
     fn size_hint(&self) -> (usize, Option<usize>) {
-        let mut additional_len = 0;
-        if self.first_two.0.is_some() {
-            additional_len += 1;
+        size_hint::add_scalar(self.inner.size_hint(), self.additional_len())
+    }
+}
+
+
+impl<I> ExactSizeIterator for ExactlyOneError<I> where I: ExactSizeIterator {}
+
+impl<I> Display for ExactlyOneError<I> 
+    where I: Iterator,
+{
+    fn fmt(&self, f: &mut Formatter) -> FmtResult {
+        let additional = self.additional_len();
+        if additional > 0 {
+            write!(f, "got at least 2 elements when exactly one was expected")
+        } else {
+            write!(f, "got zero elements when exactly one was expected")
         }
-        if self.first_two.1.is_some() {
-            additional_len += 1;
+    }
+}
+
+impl<I> Debug for ExactlyOneError<I> 
+    where I: Iterator + Debug,
+          I::Item: Debug,
+{
+    fn fmt(&self, f: &mut Formatter) -> FmtResult {
+        match &self.first_two {
+            Some(Either::Left([first, second])) => {
+                write!(f, "ExactlyOneError[First: {:?}, Second: {:?}, RemainingIter: {:?}]", first, second, self.inner)
+            },
+            Some(Either::Right(second)) => {
+                write!(f, "ExactlyOneError[Second: {:?}, RemainingIter: {:?}]", second, self.inner)
+            }
+            None => {
+                write!(f, "ExactlyOneError[RemainingIter: {:?}]", self.inner)
+            }
         }
-        size_hint::add_scalar(self.inner.size_hint(), additional_len)
     }
 }
 
-impl<I> ExactSizeIterator for ExactlyOneError<I> where I: ExactSizeIterator {}
+#[cfg(feature = "use_std")]
+impl<I> Error for ExactlyOneError<I>  where I: Iterator + Debug, I::Item: Debug, {}
+
+
diff --git a/src/lib.rs b/src/lib.rs
index 4568f649b..9ce015209 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -2791,14 +2791,14 @@ pub trait Itertools : Iterator {
             Some(first) => {
                 match self.next() {
                     Some(second) => {
-                        Err(ExactlyOneError::new((Some(first), Some(second)), self))
+                        Err(ExactlyOneError::new(Some(Either::Left([first, second])), self))
                     }
                     None => {
                         Ok(first)
                     }
                 }
             }
-            None => Err(ExactlyOneError::new((None, None), self)),
+            None => Err(ExactlyOneError::new(None, self)),
         }
     }
 


Test patch:
diff --git a/tests/test_std.rs b/tests/test_std.rs
index 9918592cb..24d0f0888 100644
--- a/tests/test_std.rs
+++ b/tests/test_std.rs
@@ -1,6 +1,7 @@
 use permutohedron;
 use itertools as it;
 use crate::it::Itertools;
+use crate::it::ExactlyOneError;
 use crate::it::multizip;
 use crate::it::multipeek;
 use crate::it::peek_nth;
@@ -913,3 +914,13 @@ fn tree_fold1() {
         assert_eq!(actual, expected);
     }
 }
+
+#[test]
+fn exactly_one_question_mark_syntax_works() {
+    exactly_one_question_mark_return().unwrap_err();
+}
+
+fn exactly_one_question_mark_return() -> Result<(), ExactlyOneError<std::slice::Iter<'static, ()>>> {
+    [].iter().exactly_one()?;
+    Ok(())
+}