Create io::util::IteratorExtension to provide fail_on_error() and update io iterators to produce IoResults

Most IO related functions return an IoResult so that the caller can handle failure
in whatever way is appropriate. However, the `lines`, `bytes`, and `chars` iterators all
supress errors. This means that code that needs to handle errors can't use any of these
iterators. All three of these iterators were updated to produce IoResults.

The problem with returning IoResults is that the caller now *has* to handle error conditions
since the produced value is now wrapped in an IoResult. This complicates simple example
code or one-off programs. In order to mitigate that problem, a new extension trait,
io::util::IteratorExtension, is created that provides a new method for all iterators over
IoResults - fail_on_error(). This method wraps the iterator in a FailOnError iterator that
unwraps all results before producing them, but fails if a non-EndOfFile error is
encountered. This allows for simple programs to remain almost as simple as before.

With these changes, the existing behavior of supressing all errors for these iterators is
effectively no longer available.

Fixes rust-lang#12368

Author: Palmer Cox

src/compiletest/errors.rs

@@ -9,6 +9,7 @@
 // except according to those terms.
 
 use std::io::{BufferedReader, File};
+use std::io::util::IteratorExtensions;
 
 pub struct ExpectedError { line: uint, kind: ~str, msg: ~str }
 
@@ -18,7 +19,7 @@ pub fn load_errors(testfile: &Path) -> ~[ExpectedError] {
     let mut error_patterns = ~[];
     let mut rdr = BufferedReader::new(File::open(testfile).unwrap());
     let mut line_num = 1u;
-    for ln in rdr.lines() {
+    for ln in rdr.lines().fail_on_error() {
         error_patterns.push_all_move(parse_expected(line_num, ln));
         line_num += 1u;
     }

src/compiletest/header.rs

@@ -134,9 +134,10 @@ pub fn is_test_ignored(config: &config, testfile: &Path) -> bool {
 
 fn iter_header(testfile: &Path, it: |&str| -> bool) -> bool {
     use std::io::{BufferedReader, File};
+    use std::io::util::IteratorExtensions;
 
     let mut rdr = BufferedReader::new(File::open(testfile).unwrap());
-    for ln in rdr.lines() {
+    for ln in rdr.lines().fail_on_error() {
         // Assume that any directives will be found before the first
         // module or function. This doesn't seem to be an optimization
         // with a warm page cache. Maybe with a cold one.

src/libstd/io/buffered.rs

@@ -537,9 +537,9 @@ mod test {
         let in_buf = MemReader::new(bytes!("a\nb\nc").to_owned());
         let mut reader = BufferedReader::with_capacity(2, in_buf);
         let mut it = reader.lines();
-        assert_eq!(it.next(), Some(~"a\n"));
-        assert_eq!(it.next(), Some(~"b\n"));
-        assert_eq!(it.next(), Some(~"c"));
+        assert_eq!(it.next(), Some(Ok(~"a\n")));
+        assert_eq!(it.next(), Some(Ok(~"b\n")));
+        assert_eq!(it.next(), Some(Ok(~"c")));
         assert_eq!(it.next(), None);
     }
 
@@ -569,8 +569,8 @@ mod test {
         let buf = [195u8, 159u8, 'a' as u8];
         let mut reader = BufferedReader::with_capacity(1, BufReader::new(buf));
         let mut it = reader.chars();
-        assert_eq!(it.next(), Some('ß'));
-        assert_eq!(it.next(), Some('a'));
+        assert_eq!(it.next(), Some(Ok('ß')));
+        assert_eq!(it.next(), Some(Ok('a')));
         assert_eq!(it.next(), None);
     }
 

src/libstd/io/extensions.rs

@@ -15,27 +15,28 @@
 // FIXME: Not sure how this should be structured
 // FIXME: Iteration should probably be considered separately
 
+use prelude::*;
 use container::Container;
 use iter::Iterator;
 use option::Option;
-use io::Reader;
+use io;
+use io::{IoError, IoResult, Reader};
 use vec::{OwnedVector, ImmutableVector};
 use ptr::RawPtr;
 
 /// An iterator that reads a single byte on each iteration,
-/// until `.read_byte()` returns `None`.
+/// until `.read_byte()` returns `EndOfFile`.
 ///
 /// # Notes about the Iteration Protocol
 ///
 /// The `Bytes` may yield `None` and thus terminate
 /// an iteration, but continue to yield elements if iteration
 /// is attempted again.
 ///
-/// # Failure
+/// # Error
 ///
-/// Raises the same conditions as the `read` method, for
-/// each call to its `.next()` method.
-/// Yields `None` if the condition is handled.
+/// Any error other than EndOfFile that is produced by the underlying Reader
+/// is returned by the iterator and should be handled by the caller.
 pub struct Bytes<'r, T> {
     priv reader: &'r mut T,
 }
@@ -46,10 +47,14 @@ impl<'r, R: Reader> Bytes<'r, R> {
     }
 }
 
-impl<'r, R: Reader> Iterator<u8> for Bytes<'r, R> {
+impl<'r, R: Reader> Iterator<IoResult<u8>> for Bytes<'r, R> {
     #[inline]
-    fn next(&mut self) -> Option<u8> {
-        self.reader.read_byte().ok()
+    fn next(&mut self) -> Option<IoResult<u8>> {
+        match self.reader.read_byte() {
+            Ok(x) => Some(Ok(x)),
+            Err(IoError { kind: io::EndOfFile, .. }) => None,
+            Err(e) => Some(Err(e))
+        }
     }
 }
 
@@ -257,7 +262,7 @@ mod test {
             count: 0,
         };
         let byte = reader.bytes().next();
-        assert!(byte == Some(10));
+        assert!(byte == Some(Ok(10)));
     }
 
     #[test]
@@ -272,7 +277,7 @@ mod test {
         let mut reader = ErroringReader;
         let mut it = reader.bytes();
         let byte = it.next();
-        assert!(byte.is_none());
+        assert!(byte.unwrap().is_err());
     }
 
     #[test]

src/libstd/io/mod.rs

@@ -29,8 +29,9 @@ Some examples of obvious things you might want to do
 
     ```rust
     use std::io;
+    use std::io::util::IteratorExtensions;
 
-    for line in io::stdin().lines() {
+    for line in io::stdin().lines().fail_on_error() {
         print!("{}", line);
     }
     ```
@@ -60,23 +61,29 @@ Some examples of obvious things you might want to do
     ```rust
     use std::io::BufferedReader;
     use std::io::File;
+    use std::io::util::IteratorExtensions;
 
     let path = Path::new("message.txt");
+    # drop(File::create(&path).unwrap());
     let mut file = BufferedReader::new(File::open(&path));
-    for line in file.lines() {
+    for line in file.lines().fail_on_error() {
         print!("{}", line);
     }
+    # let _ = ::std::io::fs::unlink(&path);
     ```
 
 * Pull the lines of a file into a vector of strings
 
     ```rust
     use std::io::BufferedReader;
     use std::io::File;
+    use std::io::util::IteratorExtensions;
 
     let path = Path::new("message.txt");
+    # drop(File::create(&path).unwrap());
     let mut file = BufferedReader::new(File::open(&path));
-    let lines: ~[~str] = file.lines().collect();
+    let lines: ~[~str] = file.lines().fail_on_error().collect();
+    # let _ = ::std::io::fs::unlink(&path);
     ```
 
 * Make a simple TCP client connection and request
@@ -432,10 +439,8 @@ pub trait Reader {
     ///
     /// # Error
     ///
-    /// The iterator protocol causes all specifics about errors encountered to
-    /// be swallowed. All errors will be signified by returning `None` from the
-    /// iterator. If this is undesirable, it is recommended to use the
-    /// `read_byte` method.
+    /// Any error other than EndOfFile that is produced by the underlying Reader
+    /// is returned by the iterator and should be handled by the caller.
     fn bytes<'r>(&'r mut self) -> extensions::Bytes<'r, Self> {
         extensions::Bytes::new(self)
     }
@@ -952,7 +957,7 @@ pub trait Stream: Reader + Writer { }
 impl<T: Reader + Writer> Stream for T {}
 
 /// An iterator that reads a line on each iteration,
-/// until `.read_line()` returns `None`.
+/// until `.read_line()` encounters EndOfFile.
 ///
 /// # Notes about the Iteration Protocol
 ///
@@ -962,21 +967,24 @@ impl<T: Reader + Writer> Stream for T {}
 ///
 /// # Error
 ///
-/// This iterator will swallow all I/O errors, transforming `Err` values to
-/// `None`. If errors need to be handled, it is recommended to use the
-/// `read_line` method directly.
+/// Any error other than EndOfFile that is produced by the underlying Reader
+/// is returned by the iterator and should be handled by the caller.
 pub struct Lines<'r, T> {
     priv buffer: &'r mut T,
 }
 
-impl<'r, T: Buffer> Iterator<~str> for Lines<'r, T> {
-    fn next(&mut self) -> Option<~str> {
-        self.buffer.read_line().ok()
+impl<'r, T: Buffer> Iterator<IoResult<~str>> for Lines<'r, T> {
+    fn next(&mut self) -> Option<IoResult<~str>> {
+        match self.buffer.read_line() {
+            Ok(x) => Some(Ok(x)),
+            Err(IoError { kind: EndOfFile, ..}) => None,
+            Err(y) => Some(Err(y))
+        }
     }
 }
 
 /// An iterator that reads a utf8-encoded character on each iteration,
-/// until `.read_char()` returns `None`.
+/// until `.read_char()` encounters EndOfFile.
 ///
 /// # Notes about the Iteration Protocol
 ///
@@ -986,16 +994,19 @@ impl<'r, T: Buffer> Iterator<~str> for Lines<'r, T> {
 ///
 /// # Error
 ///
-/// This iterator will swallow all I/O errors, transforming `Err` values to
-/// `None`. If errors need to be handled, it is recommended to use the
-/// `read_char` method directly.
+/// Any error other than EndOfFile that is produced by the underlying Reader
+/// is returned by the iterator and should be handled by the caller.
 pub struct Chars<'r, T> {
     priv buffer: &'r mut T
 }
 
-impl<'r, T: Buffer> Iterator<char> for Chars<'r, T> {
-    fn next(&mut self) -> Option<char> {
-        self.buffer.read_char().ok()
+impl<'r, T: Buffer> Iterator<IoResult<char>> for Chars<'r, T> {
+    fn next(&mut self) -> Option<IoResult<char>> {
+        match self.buffer.read_char() {
+            Ok(x) => Some(Ok(x)),
+            Err(IoError { kind: EndOfFile, ..}) => None,
+            Err(y) => Some(Err(y))
+        }
     }
 }
 
@@ -1061,9 +1072,8 @@ pub trait Buffer: Reader {
     ///
     /// # Error
     ///
-    /// This iterator will transform all error values to `None`, discarding the
-    /// cause of the error. If this is undesirable, it is recommended to call
-    /// `read_line` directly.
+    /// Any error other than EndOfFile that is produced by the underlying Reader
+    /// is returned by the iterator and should be handled by the caller.
     fn lines<'r>(&'r mut self) -> Lines<'r, Self> {
         Lines { buffer: self }
     }
@@ -1149,9 +1159,8 @@ pub trait Buffer: Reader {
     ///
     /// # Error
     ///
-    /// This iterator will transform all error values to `None`, discarding the
-    /// cause of the error. If this is undesirable, it is recommended to call
-    /// `read_char` directly.
+    /// Any error other than EndOfFile that is produced by the underlying Reader
+    /// is returned by the iterator and should be handled by the caller.
     fn chars<'r>(&'r mut self) -> Chars<'r, Self> {
         Chars { buffer: self }
     }

src/libstd/io/util.rs

@@ -195,6 +195,49 @@ pub fn copy<R: Reader, W: Writer>(r: &mut R, w: &mut W) -> io::IoResult<()> {
     }
 }
 
+/// An extension trait that adds utility methods to Iterators over IoResults
+pub trait IteratorExtensions {
+    /// Produce unwrapped elements until an error is encountered. If the error
+    /// is EndOfFile, None is produced to end iteration. If any other error is
+    /// encountered, the iterator fail!()s.
+    ///
+    /// This method is useful for programs that do not want to concern themselves
+    /// with error handling, such as simple example code or one time use programs.
+    /// More robust programs should avoid using this method unless task failure is
+    /// the desired behavior when an IO error occurs.
+    fn fail_on_error(self) -> FailOnError<Self> {
+        FailOnError {
+            wrapped: self
+        }
+    }
+}
+
+impl <T, I: Iterator<io::IoResult<T>>> IteratorExtensions for I {}
+
+/// An iterator which produces unwrapped values from the wrapped iterator until
+/// EndOfFile is encountered. The iterator fail!()s if any other error is
+/// encountered.
+pub struct FailOnError<I> {
+    priv wrapped: I
+}
+
+impl <T, I: Iterator<io::IoResult<T>>> Iterator<T> for FailOnError<I> {
+    fn next(&mut self) -> Option<T> {
+        // All existing IO iterators already produce None when they encounter
+        // EndOfFile. However, in an effort to be more general, this iterator also
+        // checks for EndOfFile and returns None in that case as well.
+        match self.wrapped.next() {
+            Some(x) => match x {
+                Ok(y) => Some(y),
+                Err(io::IoError { kind: io::EndOfFile, .. }) => None,
+                Err(e) => fail!(e.to_str())
+            },
+            None => None
+        }
+    }
+}
+
+
 #[cfg(test)]
 mod test {
     use io;
@@ -307,4 +350,52 @@ mod test {
         copy(&mut r, &mut w).unwrap();
         assert_eq!(~[0, 1, 2, 3, 4], w.unwrap());
     }
+
+    // This tests the usual case - using IteratorExtensions' fail_on_error()
+    // with an Iterator that already produces None when it encounters EndOfFile.
+    #[test]
+    fn test_fail_on_error_with_lines() {
+        let mut reader = MemReader::new(bytes!("a\nb\nc").to_owned());
+        let mut it = reader.lines().fail_on_error();
+        assert_eq!(it.next(), Some(~"a\n"));
+        assert_eq!(it.next(), Some(~"b\n"));
+        assert_eq!(it.next(), Some(~"c"));
+        assert_eq!(it.next(), None);
+    }
+
+    struct CountDown {
+        count: uint,
+        end: io::IoErrorKind
+    }
+
+    impl Iterator<io::IoResult<uint>> for CountDown {
+        fn next(&mut self) -> Option<io::IoResult<uint>> {
+            if self.count > 0 {
+                self.count -= 1;
+                Some(Ok(self.count + 1))
+            } else {
+                Some(Err(io::IoError { kind: self.end, desc: "", detail: None } ))
+            }
+        }
+    }
+
+    // This tests that fail_on_error() handles EndOfFile correctly for iterators
+    // that don't return None when they encounter EndOfFile.
+    #[test]
+    fn test_fail_on_error_eof() {
+        let iter = CountDown { count: 1, end: io::EndOfFile };
+        let mut iter = iter.fail_on_error();
+        assert!(iter.next().unwrap() == 1);
+        assert!(iter.next().is_none());
+    }
+
+    // This test checks that fail_on_error() does indeed fail!() if an iterator
+    // produces an Err other than EndOfFile.
+    #[test]
+    #[should_fail]
+    fn test_fail_on_error_failing() {
+        let iter = CountDown { count: 0, end: io::OtherIoError };
+        let mut iter = iter.fail_on_error();
+        iter.next();
+    }
 }

src/test/bench/shootout-k-nucleotide-pipes.rs

@@ -23,6 +23,7 @@ use std::mem::replace;
 use std::option;
 use std::os;
 use std::io;
+use std::io::util::IteratorExtensions;
 use std::str;
 use std::task;
 use std::vec;
@@ -181,7 +182,7 @@ fn main() {
    // reading the sequence of interest
    let mut proc_mode = false;
 
-   for line in rdr.lines() {
+   for line in rdr.lines().fail_on_error() {
        let line = line.trim().to_owned();
 
        if line.len() == 0u { continue; }