fix: Make the positions of slice streams harder to misuse

Previously these streams had a `usize` which was really a pointer offset
as their position which easily led to user errors. By wrapping the
offset in a newtype we get custom formatting which should make it
clearer that something strange is going on. As well as a simple way of
converting the offset into a position.

Fixes #104

BREAKING CHANGE

Change `usize` to `PointerOffset` where applicable.
Change `translate_position` calls into the same method on `PointerOffset`

README.md

@@ -37,7 +37,7 @@ If you end up trying it I welcome any feedback from your experience with it. I a
 
 ### Why does my errors contain inscrutable positions?
 
-Since `combine` aims to crate parsers with little to no overhead streams over `&str` and `&[T]` do not carry any extra position information but instead only rely on comparing the pointer of the buffer to check which `Stream` is further ahead than another `Stream`. To retrieve a better position, either call `translate_position` on the `ParseError` or wrap your stream with `State`.
+Since `combine` aims to crate parsers with little to no overhead streams over `&str` and `&[T]` do not carry any extra position information but instead only rely on comparing the pointer of the buffer to check which `Stream` is further ahead than another `Stream`. To retrieve a better position, either call `translate_position` on the `PointerOffset` which represents the position or wrap your stream with `State`.
 
 ## Extra
 

benches/http.rs

@@ -4,7 +4,7 @@ extern crate combine;
 
 use combine::*;
 use combine::primitives::Error;
-use combine::range::{take_while1, range};
+use combine::range::{range, take_while1};
 
 #[derive(Debug)]
 struct Request<'a> {

benches/json.rs

@@ -7,10 +7,10 @@ use std::io::Read;
 use std::fs::File;
 use std::path::Path;
 
-use pc::primitives::{Consumed, Parser, ParseError, ParseResult, State, Stream, BufferedStream};
-use pc::combinator::{any, between, choice, many, many1, optional, parser, satisfy, sep_by,
-                     Expected, FnParser, Skip};
-use pc::char::{char, digit, spaces, Spaces, string};
+use pc::primitives::{BufferedStream, Consumed, ParseError, ParseResult, Parser, State, Stream};
+use pc::combinator::{any, between, choice, many, optional, parser, satisfy, sep_by, Expected,
+                     FnParser, Skip, many1};
+use pc::char::{char, digit, spaces, string, Spaces};
 use pc::from_iter;
 
 #[derive(PartialEq, Debug)]
@@ -116,12 +116,10 @@ where
         });
         match c {
             '\\' => input.combine(|input| back_slash_char.parse_stream(input)),
-            '"' => {
-                Err(Consumed::Empty(ParseError::from_errors(
-                    input.into_inner().position(),
-                    Vec::new(),
-                )))
-            }
+            '"' => Err(Consumed::Empty(ParseError::from_errors(
+                input.into_inner().position(),
+                Vec::new(),
+            ))),
             _ => Ok((c, input)),
         }
     }

benches/mp4.rs

@@ -5,7 +5,7 @@ extern crate test;
 extern crate combine;
 extern crate byteorder;
 
-use test::{Bencher, black_box};
+use test::{black_box, Bencher};
 
 use std::str::from_utf8;
 

src/byte.rs

@@ -4,8 +4,8 @@ use std::marker::PhantomData;
 
 use self::ascii::AsciiChar;
 
-use combinator::{Expected, satisfy, Satisfy, skip_many, SkipMany, token, Token, tokens, With};
-use primitives::{ConsumedResult, Info, Parser, StreamError, Stream};
+use combinator::{satisfy, skip_many, token, tokens, Expected, Satisfy, SkipMany, Token, With};
+use primitives::{ConsumedResult, Info, Parser, Stream, StreamError};
 
 /// Parses a character and succeeds if the character is equal to `c`.
 ///

src/char.rs

@@ -1,5 +1,5 @@
-use primitives::{Parser, StreamError, ConsumedResult, Stream};
-use combinator::{Expected, satisfy, Satisfy, skip_many, SkipMany, token, Token, tokens, With};
+use primitives::{ConsumedResult, Parser, Stream, StreamError};
+use combinator::{satisfy, skip_many, token, tokens, Expected, Satisfy, SkipMany, Token, With};
 use std::marker::PhantomData;
 
 /// Parses a character and succeeds if the character is equal to `c`.

src/combinator.rs

@@ -1,7 +1,7 @@
 use std::iter::FromIterator;
 use std::marker::PhantomData;
-use primitives::{Info, Parser, ParseResult, ConsumedResult, ParseError, StreamError, Stream,
-                 StreamOnce, Error, Consumed};
+use primitives::{Consumed, ConsumedResult, Error, Info, ParseError, ParseResult, Parser, Stream,
+                 StreamError, StreamOnce};
 use primitives::FastResult::*;
 
 macro_rules! impl_parser {
@@ -88,12 +88,10 @@ where
 {
     let position = input.position();
     match input.uncons() {
-        Ok(c) => {
-            match predicate(c.clone()) {
-                Some(c) => ConsumedOk((c, input)),
-                None => EmptyErr(ParseError::empty(position)),
-            }
-        }
+        Ok(c) => match predicate(c.clone()) {
+            Some(c) => ConsumedOk((c, input)),
+            None => EmptyErr(ParseError::empty(position)),
+        },
         Err(err) => EmptyErr(ParseError::new(position, err)),
     }
 }
@@ -420,12 +418,10 @@ where
             }
         }
         EmptyErr(match empty_err {
-            None => {
-                ParseError::new(
-                    input.position(),
-                    Error::Message("parser choice is empty".into()),
-                )
-            }
+            None => ParseError::new(
+                input.position(),
+                Error::Message("parser choice is empty".into()),
+            ),
             Some(err) => err,
         })
     }
@@ -851,13 +847,11 @@ impl<P: Parser> Iter<P> {
 
     fn into_result_fast<O>(self, value: O) -> ConsumedResult<O, P::Input> {
         match self.state {
-            State::Ok | State::EmptyErr => {
-                if self.consumed {
-                    ConsumedOk((value, self.input))
-                } else {
-                    EmptyOk((value, self.input))
-                }
-            }
+            State::Ok | State::EmptyErr => if self.consumed {
+                ConsumedOk((value, self.input))
+            } else {
+                EmptyOk((value, self.input))
+            },
             State::ConsumedErr(e) => ConsumedErr(e),
         }
     }
@@ -867,27 +861,25 @@ impl<P: Parser> Iterator for Iter<P> {
     type Item = P::Output;
     fn next(&mut self) -> Option<P::Output> {
         match self.state {
-            State::Ok => {
-                match self.parser.parse_lazy(self.input.clone()) {
-                    EmptyOk((v, input)) => {
-                        self.input = input;
-                        Some(v)
-                    }
-                    ConsumedOk((v, input)) => {
-                        self.input = input;
-                        self.consumed = true;
-                        Some(v)
-                    }
-                    EmptyErr(_) => {
-                        self.state = State::EmptyErr;
-                        None
-                    }
-                    ConsumedErr(e) => {
-                        self.state = State::ConsumedErr(e);
-                        None
-                    }
+            State::Ok => match self.parser.parse_lazy(self.input.clone()) {
+                EmptyOk((v, input)) => {
+                    self.input = input;
+                    Some(v)
                 }
-            }
+                ConsumedOk((v, input)) => {
+                    self.input = input;
+                    self.consumed = true;
+                    Some(v)
+                }
+                EmptyErr(_) => {
+                    self.state = State::EmptyErr;
+                    None
+                }
+                ConsumedErr(e) => {
+                    self.state = State::ConsumedErr(e);
+                    None
+                }
+            },
             State::ConsumedErr(_) | State::EmptyErr => None,
         }
     }
@@ -1477,7 +1469,8 @@ where
         loop {
             match (&mut self.1, &mut self.0)
                 .parse_lazy(input.clone().into_inner())
-                .into() {
+                .into()
+            {
                 Ok(((op, r), rest)) => {
                     l = op(l, r);
                     input = input.merge(rest);
@@ -1803,14 +1796,12 @@ where
             ConsumedOk(x) => ConsumedOk(x),
             EmptyOk(x) => EmptyOk(x),
             ConsumedErr(err) => ConsumedErr(err),
-            EmptyErr(error1) => {
-                match self.1.parse_lazy(input) {
-                    ConsumedOk(x) => ConsumedOk(x),
-                    EmptyOk(x) => EmptyOk(x),
-                    ConsumedErr(err) => ConsumedErr(err),
-                    EmptyErr(error2) => EmptyErr(error1.merge(error2)),
-                }
-            }
+            EmptyErr(error1) => match self.1.parse_lazy(input) {
+                ConsumedOk(x) => ConsumedOk(x),
+                EmptyOk(x) => EmptyOk(x),
+                ConsumedErr(err) => ConsumedErr(err),
+                EmptyErr(error2) => EmptyErr(error1.merge(error2)),
+            },
         }
     }
     fn add_error(&mut self, errors: &mut StreamError<Self::Input>) {
@@ -1883,18 +1874,14 @@ where
     #[inline]
     fn parse_lazy(&mut self, input: I) -> ConsumedResult<B, I> {
         match self.0.parse_lazy(input) {
-            EmptyOk((o, input)) => {
-                match (self.1)(o) {
-                    Ok(x) => EmptyOk((x, input)),
-                    Err(err) => EmptyErr(err),
-                }
-            }
-            ConsumedOk((o, input)) => {
-                match (self.1)(o) {
-                    Ok(x) => ConsumedOk((x, input)),
-                    Err(err) => ConsumedErr(err),
-                }
-            }
+            EmptyOk((o, input)) => match (self.1)(o) {
+                Ok(x) => EmptyOk((x, input)),
+                Err(err) => EmptyErr(err),
+            },
+            ConsumedOk((o, input)) => match (self.1)(o) {
+                Ok(x) => ConsumedOk((x, input)),
+                Err(err) => ConsumedErr(err),
+            },
             EmptyErr(err) => EmptyErr(err),
             ConsumedErr(err) => ConsumedErr(err),
         }
@@ -2027,18 +2014,14 @@ where
     #[inline]
     fn parse_lazy(&mut self, input: Self::Input) -> ConsumedResult<O, Self::Input> {
         match self.0.parse_lazy(input) {
-            EmptyOk((o, input)) => {
-                match (self.1)(o) {
-                    Ok(o) => EmptyOk((o, input)),
-                    Err(err) => EmptyErr(ParseError::new(input.position(), err.into())),
-                }
-            }
-            ConsumedOk((o, input)) => {
-                match (self.1)(o) {
-                    Ok(o) => ConsumedOk((o, input)),
-                    Err(err) => ConsumedErr(ParseError::new(input.position(), err.into())),
-                }
-            }
+            EmptyOk((o, input)) => match (self.1)(o) {
+                Ok(o) => EmptyOk((o, input)),
+                Err(err) => EmptyErr(ParseError::new(input.position(), err.into())),
+            },
+            ConsumedOk((o, input)) => match (self.1)(o) {
+                Ok(o) => ConsumedOk((o, input)),
+                Err(err) => ConsumedErr(ParseError::new(input.position(), err.into())),
+            },
             EmptyErr(err) => EmptyErr(err),
             ConsumedErr(err) => ConsumedErr(err),
         }
@@ -2212,7 +2195,7 @@ where
 #[cfg(test)]
 mod tests {
     use super::*;
-    use primitives::{Error, ParseError, Positioner, Parser, SourcePosition, State};
+    use primitives::{Error, ParseError, Parser, Positioner, SourcePosition, State};
     use char::{char, digit, letter};
 
     #[test]
@@ -2224,7 +2207,7 @@ mod tests {
     #[test]
     fn sep_by_consumed_error() {
         let mut parser2 = sep_by((letter(), letter()), token(','));
-        let result_err: Result<(Vec<(char, char)>, &str), ParseError<usize, char, &str>> =
+        let result_err: Result<(Vec<(char, char)>, &str), ParseError<_, char, &str>> =
             parser2.parse("a,bc");
         assert!(result_err.is_err());
     }
@@ -2283,8 +2266,7 @@ mod tests {
         let input = &[
             CloneOnly { s: "x".to_string() },
             CloneOnly { s: "y".to_string() },
-        ]
-            [..];
+        ][..];
         let result = token(CloneOnly { s: "x".to_string() }).parse(input);
         assert_eq!(
             result,

src/lib.rs

@@ -151,12 +151,11 @@
 //! [`RangeStream`]: primitives/trait.RangeStream.html
 //! [`Parser`]: primitives/trait.Parser.html
 //! [fn parser]: combinator/fn.parser.html
-
 // inline(always) is only used on trivial functions returning parsers
 #![cfg_attr(feature = "cargo-clippy", allow(inline_always))]
 
 #[doc(inline)]
-pub use primitives::{Parser, ParseError, StreamError, ConsumedResult, ParseResult, State, Stream,
+pub use primitives::{ConsumedResult, ParseError, ParseResult, Parser, State, Stream, StreamError,
                      StreamOnce};
 
 // import this one separately, so we can set the allow(deprecated) for just this item
@@ -166,10 +165,10 @@ pub use primitives::{Parser, ParseError, StreamError, ConsumedResult, ParseResul
 pub use primitives::from_iter;
 
 #[doc(inline)]
-pub use combinator::{any, between, chainl1, chainr1, choice, count, eof, env_parser, many, many1,
-                     none_of, one_of, optional, parser, position, satisfy, satisfy_map, sep_by,
-                     sep_by1, sep_end_by, sep_end_by1, skip_many, skip_many1, token, tokens, try,
-                     look_ahead, value, unexpected, not_followed_by};
+pub use combinator::{any, between, choice, count, env_parser, eof, look_ahead, many, none_of,
+                     not_followed_by, one_of, optional, parser, position, satisfy, satisfy_map,
+                     sep_by, sep_end_by, skip_many, token, tokens, try, unexpected, value,
+                     chainl1, chainr1, many1, sep_by1, sep_end_by1, skip_many1};
 
 macro_rules! static_fn {
     (($($arg: pat, $arg_ty: ty),*) -> $ret: ty { $body: expr }) => { {
@@ -217,7 +216,7 @@ pub mod char;
 #[cfg(test)]
 mod tests {
     use super::*;
-    use super::primitives::{SourcePosition, Error, Consumed};
+    use super::primitives::{Consumed, Error, SourcePosition};
     use char::{alpha_num, char, digit, letter, spaces, string};
 
 
@@ -375,14 +374,12 @@ mod tests {
 
     fn follow(input: State<&str>) -> ParseResult<(), State<&str>> {
         match input.clone().uncons() {
-            Ok(c) => {
-                if c.is_alphanumeric() {
-                    let e = Error::Unexpected(c.into());
-                    Err(Consumed::Empty(ParseError::new(input.position(), e)))
-                } else {
-                    Ok(((), Consumed::Empty(input)))
-                }
-            }
+            Ok(c) => if c.is_alphanumeric() {
+                let e = Error::Unexpected(c.into());
+                Err(Consumed::Empty(ParseError::new(input.position(), e)))
+            } else {
+                Ok(((), Consumed::Empty(input)))
+            },
             Err(_) => Ok(((), Consumed::Empty(input))),
         }
     }
@@ -484,7 +481,7 @@ mod tests {
                 "error"
             }
         }
-        let result: Result<((), _), ParseError<usize, char, &str>> =
+        let result: Result<((), _), ParseError<_, char, &str>> =
             string("abc").and_then(|_| Err(Error)).parse("abc");
         assert!(result.is_err());
         // Test that ParseError can be coerced to a StdError
@@ -542,7 +539,7 @@ mod tests {
         let input = &[CloneOnly("x".to_string()), CloneOnly("y".to_string())][..];
         let result = token(CloneOnly("z".to_string()))
             .parse(input)
-            .map_err(|e| e.translate_position(input))
+            .map_err(|e| e.map_position(|p| p.translate_position(input)))
             .map_err(|e| {
                 ExtractedError(
                     e.position,