Improve parsing error messages

Nom's custom error type handling isn't great, requiring a lot of
fix_error! macro calls, and even then the complete! macro hardcodes the
error type it expects as u32, so it can't be used and instead the
CompleteStr type needs to be used everywhere.

src/error.rs

@@ -0,0 +1,136 @@
+use std::error;
+use std::fmt;
+use std::io;
+use std::num::ParseIntError;
+use std::path::PathBuf;
+
+use nom::{Context, Err, ErrorKind};
+use regex;
+
+#[derive(Debug)]
+pub enum Error {
+    ParsingIncomplete,
+    /// The first string is the expression parsed, the second is a tag describing the parser that failed.
+    GenericParsingError(String, String),
+    /// The string is the expression parsed.
+    CustomParsingError(String, ParsingError),
+    PeParsingError(PathBuf, Box<error::Error>),
+    IoError(PathBuf, io::Error),
+}
+
+fn escape<I: fmt::Display>(input: I) -> String {
+    input.to_string().replace("\"", "\\\"")
+}
+
+impl<I: fmt::Debug + fmt::Display> From<Err<I, ParsingError>> for Error {
+    fn from(error: Err<I, ParsingError>) -> Self {
+        match error {
+            Err::Incomplete(_) => Error::ParsingIncomplete,
+            Err::Error(Context::Code(i, ErrorKind::Custom(e)))
+            | Err::Failure(Context::Code(i, ErrorKind::Custom(e))) => {
+                Error::CustomParsingError(escape(i), e)
+            }
+            Err::Error(Context::Code(i, e)) | Err::Failure(Context::Code(i, e)) => {
+                Error::GenericParsingError(escape(i), format!("{:?}", e))
+            }
+        }
+    }
+}
+
+impl fmt::Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            Error::ParsingIncomplete => write!(f, "More input was expected by the parser"),
+            Error::GenericParsingError(i, e) => write!(
+                f,
+                "An error was encountered in the parser \"{}\" while parsing the expression \"{}\"",
+                e, i
+            ),
+            Error::CustomParsingError(i, e) => write!(
+                f,
+                "An error was encountered while parsing the expression \"{}\": {}",
+                i, e
+            ),
+            Error::PeParsingError(p, e) => write!(
+                f,
+                "An error was encountered while reading the file version field of \"{}\": {}",
+                p.display(),
+                e
+            ),
+            Error::IoError(p, e) => write!(
+                f,
+                "An error was encountered while accessing the path \"{}\": {}",
+                p.display(),
+                e
+            ),
+        }
+    }
+}
+
+impl error::Error for Error {
+    fn cause(&self) -> Option<&error::Error> {
+        match self {
+            Error::CustomParsingError(_, e) => Some(e),
+            Error::PeParsingError(_, e) => Some(e.as_ref()),
+            Error::IoError(_, e) => Some(e),
+            _ => None,
+        }
+    }
+}
+
+#[derive(Debug)]
+pub enum ParsingError {
+    InvalidRegexSyntax(String),
+    InvalidRegexUnknown,
+    InvalidCrc(ParseIntError),
+    PathEndsInADirectorySeparator(PathBuf),
+    PathIsNotInGameDirectory(PathBuf),
+    Unknown(u32),
+}
+
+impl From<regex::Error> for ParsingError {
+    fn from(error: regex::Error) -> Self {
+        match error {
+            regex::Error::Syntax(s) => ParsingError::InvalidRegexSyntax(s),
+            _ => ParsingError::InvalidRegexUnknown,
+        }
+    }
+}
+
+impl From<ParseIntError> for ParsingError {
+    fn from(error: ParseIntError) -> Self {
+        ParsingError::InvalidCrc(error)
+    }
+}
+
+impl From<u32> for ParsingError {
+    fn from(error: u32) -> Self {
+        ParsingError::Unknown(error)
+    }
+}
+
+impl fmt::Display for ParsingError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            ParsingError::InvalidRegexSyntax(s) => write!(f, "{}", s),
+            ParsingError::InvalidRegexUnknown => write!(f, "Unknown regex parsing error"),
+            ParsingError::InvalidCrc(e) => e.fmt(f),
+            ParsingError::PathEndsInADirectorySeparator(p) => {
+                write!(f, "\"{}\" ends in a directory separator", p.display())
+            }
+            ParsingError::PathIsNotInGameDirectory(p) => {
+                write!(f, "\"{}\" is not in the game directory", p.display())
+            }
+            ParsingError::Unknown(e) => write!(f, "Unknown error code {}", e),
+        }
+    }
+}
+
+impl error::Error for ParsingError {
+    fn cause(&self) -> Option<&error::Error> {
+        match self {
+            ParsingError::InvalidCrc(e) => Some(e),
+            _ => None,
+        }
+    }
+}

src/function/parse.rs

@@ -1,14 +1,16 @@
 use std::path::{Component, Path, PathBuf};
 use std::str;
 
-use nom::{hex_digit, Context, Err, ErrorKind};
+use nom::types::CompleteStr;
+use nom::{hex_digit, Context, Err, ErrorKind, IResult};
 use regex::{Regex, RegexBuilder};
 
-use ParsingResult;
 use super::{ComparisonOperator, Function};
+use ParsingError;
+use ParsingResult;
 
 impl ComparisonOperator {
-    pub fn parse(input: &str) -> ParsingResult<ComparisonOperator> {
+    pub fn parse(input: CompleteStr) -> IResult<CompleteStr, ComparisonOperator> {
         do_parse!(
             input,
             operator:
@@ -27,10 +29,6 @@ impl ComparisonOperator {
 const INVALID_PATH_CHARS: &str = "\":*?<>|\\"; // \ is treated as invalid to distinguish regex strings.
 const INVALID_REGEX_PATH_CHARS: &str = "\"<>";
 
-const PARSE_REGEX_ERROR: ErrorKind = ErrorKind::Custom(1);
-const PARSE_CRC_ERROR: ErrorKind = ErrorKind::Custom(2);
-const PARSE_PATH_ERROR: ErrorKind = ErrorKind::Custom(3);
-
 fn is_in_game_path(path: &Path) -> bool {
     let mut previous_component = Component::CurDir;
     for component in path.components() {
@@ -46,17 +44,31 @@ fn is_in_game_path(path: &Path) -> bool {
     true
 }
 
-fn parse_regex(input: &str) -> ParsingResult<Regex> {
+fn parse_regex(input: CompleteStr) -> ParsingResult<Regex> {
-    RegexBuilder::new(input)
+    RegexBuilder::new(input.as_ref())
         .case_insensitive(true)
         .build()
-        .map(|r| ("", r))
+        .map(|r| (CompleteStr(""), r))
-        .map_err(|_| Err::Failure(Context::Code(input, PARSE_REGEX_ERROR)))
+        .map_err(|e| {
+            Err::Failure(Context::Code(
+                input,
+                ErrorKind::Custom(ParsingError::from(e)),
+            ))
+        })
 }
 
-fn parse_version_args(input: &str) -> ParsingResult<(PathBuf, &str, ComparisonOperator)> {
+fn not_in_game_directory(input: CompleteStr, path: PathBuf) -> Err<CompleteStr, ParsingError> {
+    Err::Failure(Context::Code(
+        input,
+        ErrorKind::Custom(ParsingError::PathIsNotInGameDirectory(path)),
+    ))
+}
+
+fn parse_version_args(input: CompleteStr) -> ParsingResult<(PathBuf, String, ComparisonOperator)> {
     let (remaining_input, (path, version, comparator)) = try_parse!(
         input,
+        fix_error!(
+            ParsingError,
             do_parse!(
                 tag!("\"")
                     >> path: is_not!(INVALID_PATH_CHARS)
@@ -67,109 +79,160 @@ fn parse_version_args(input: &str) -> ParsingResult<(PathBuf, &str, ComparisonOp
                     >> tag!("\"")
                     >> ws!(tag!(","))
                     >> operator: call!(ComparisonOperator::parse)
-                >> (PathBuf::from(path), version, operator)
+                    >> (PathBuf::from(path.as_ref()), version.to_string(), operator)
+            )
         )
     );
 
     if is_in_game_path(&path) {
         Ok((remaining_input, (path, version, comparator)))
     } else {
-        Err(Err::Failure(Context::Code(input, PARSE_PATH_ERROR)))
+        Err(not_in_game_directory(input, path))
     }
 }
 
-fn parse_crc(input: &str) -> ParsingResult<u32> {
+fn parse_crc(input: CompleteStr) -> ParsingResult<u32> {
-    u32::from_str_radix(input, 16)
+    u32::from_str_radix(input.as_ref(), 16)
-        .map(|c| ("", c))
+        .map(|c| (CompleteStr(""), c))
-        .map_err(|_| Err::Failure(Context::Code(input, PARSE_CRC_ERROR)))
+        .map_err(|e| {
+            Err::Failure(Context::Code(
+                input,
+                ErrorKind::Custom(ParsingError::from(e)),
+            ))
+        })
 }
 
-fn parse_checksum_args(input: &str) -> ParsingResult<(PathBuf, u32)> {
+fn parse_checksum_args(input: CompleteStr) -> ParsingResult<(PathBuf, u32)> {
     let (remaining_input, (path, crc)) = try_parse!(
         input,
         do_parse!(
-            tag!("\"")
+            fix_error!(ParsingError, tag!("\""))
-                >> path: is_not!(INVALID_PATH_CHARS)
+                >> path: fix_error!(ParsingError, is_not!(INVALID_PATH_CHARS))
-                >> tag!("\"")
+                >> fix_error!(ParsingError, tag!("\""))
-                >> ws!(tag!(","))
+                >> fix_error!(ParsingError, ws!(tag!(",")))
-                >> crc: flat_map!(call!(hex_digit), parse_crc)
+                >> crc: flat_map!(fix_error!(ParsingError, hex_digit), parse_crc)
-                >> (PathBuf::from(path), crc)
+                >> (PathBuf::from(path.as_ref()), crc)
         )
     );
 
     if is_in_game_path(&path) {
         Ok((remaining_input, (path, crc)))
     } else {
-        Err(Err::Failure(Context::Code(input, PARSE_PATH_ERROR)))
+        Err(not_in_game_directory(input, path))
     }
 }
 
-fn parse_path(input: &str) -> ParsingResult<PathBuf> {
+fn parse_path(input: CompleteStr) -> ParsingResult<PathBuf> {
-    let (remaining_input, path) =
+    let (remaining_input, path) = try_parse!(
-        try_parse!(input, map!(is_not!(INVALID_PATH_CHARS), PathBuf::from));
+        input,
+        fix_error!(
+            ParsingError,
+            map!(is_not!(INVALID_PATH_CHARS), |s| PathBuf::from(s.as_ref()))
+        )
+    );
 
     if is_in_game_path(&path) {
         Ok((remaining_input, path))
     } else {
-        Err(Err::Failure(Context::Code(input, PARSE_PATH_ERROR)))
+        Err(not_in_game_directory(input, path))
     }
 }
 
 /// Parse a string that is a path where the last component is a regex string
 /// that may contain characters that are invalid in paths but valid in regex.
-fn parse_regex_path(input: &str) -> ParsingResult<(PathBuf, Regex)> {
+fn parse_regex_path(input: CompleteStr) -> ParsingResult<(PathBuf, Regex)> {
-    let (remaining_input, string) = try_parse!(input, is_not!(INVALID_REGEX_PATH_CHARS));
+    let (remaining_input, string) = try_parse!(
+        input,
+        fix_error!(ParsingError, is_not!(INVALID_REGEX_PATH_CHARS))
+    );
 
     if string.ends_with('/') {
-        return Err(Err::Failure(Context::Code(input, PARSE_PATH_ERROR)));
+        return Err(Err::Failure(Context::Code(
+            input,
+            ErrorKind::Custom(ParsingError::PathEndsInADirectorySeparator(
+                string.as_ref().into(),
+            )),
+        )));
     }
 
     let (parent_path_slice, regex_slice) = string
         .rfind('/')
         .map(|i| (&string[..i], &string[i + 1..]))
-        .unwrap_or_else(|| (".", string));
+        .unwrap_or_else(|| (".", &string));
 
     let parent_path = PathBuf::from(parent_path_slice);
 
     if !is_in_game_path(&parent_path) {
-        return Err(Err::Failure(Context::Code(input, PARSE_PATH_ERROR)));
+        return Err(not_in_game_directory(input, parent_path));
     }
 
-    let regex = parse_regex(regex_slice)?.1;
+    let regex = parse_regex(CompleteStr(regex_slice))?.1;
 
     Ok((remaining_input, (parent_path, regex)))
 }
 
 impl Function {
-    pub fn parse(input: &str) -> ParsingResult<Function> {
+    pub fn parse(input: CompleteStr) -> ParsingResult<Function> {
         do_parse!(
             input,
             function:
                 alt!(
-                delimited!(tag!("file(\""), call!(parse_path), tag!("\")")) => {
+                delimited!(
+                    fix_error!(ParsingError, tag!("file(\"")),
+                    call!(parse_path),
+                    fix_error!(ParsingError, tag!("\")"))
+                ) => {
                     |path| Function::FilePath(path)
                 } |
-                delimited!(tag!("file(\""), call!(parse_regex_path), tag!("\"")) => {
+                delimited!(
+                    fix_error!(ParsingError, tag!("file(\"")),
+                    call!(parse_regex_path),
+                    fix_error!(ParsingError, tag!("\""))
+                ) => {
                     |(p, r)| Function::FileRegex(p, r)
                 } |
-                delimited!(tag!("active(\""), call!(parse_path), tag!("\")")) => {
+                delimited!(
+                    fix_error!(ParsingError, tag!("active(\"")),
+                    call!(parse_path),
+                    fix_error!(ParsingError, tag!("\")"))
+                ) => {
                     |path| Function::ActivePath(path)
                 } |
-                delimited!(tag!("active(\""), flat_map!(is_not!(INVALID_REGEX_PATH_CHARS), parse_regex), tag!("\"")) => {
+                delimited!(
+                    fix_error!(ParsingError, tag!("active(\"")),
+                    flat_map!(fix_error!(ParsingError, is_not!(INVALID_REGEX_PATH_CHARS)), parse_regex),
+                    fix_error!(ParsingError, tag!("\""))
+                ) => {
                     |r| Function::ActiveRegex(r)
                 } |
-                delimited!(tag!("many(\""), call!(parse_regex_path), tag!("\"")) => {
+                delimited!(
+                    fix_error!(ParsingError, tag!("many(\"")),
+                    call!(parse_regex_path),
+                    fix_error!(ParsingError, tag!("\""))
+                ) => {
                     |(p, r)| Function::Many(p, r)
                 } |
-                delimited!(tag!("many_active(\""), flat_map!(is_not!(INVALID_REGEX_PATH_CHARS), parse_regex), tag!("\"")) => {
+                delimited!(
+                    fix_error!(ParsingError, tag!("many_active(\"")),
+                    flat_map!(fix_error!(ParsingError, is_not!(INVALID_REGEX_PATH_CHARS)), parse_regex),
+                    fix_error!(ParsingError, tag!("\""))
+                ) => {
                     |r| Function::ManyActive(r)
                 } |
-                delimited!(tag!("version("), call!(parse_version_args), tag!(")")) => {
+                delimited!(
-                    |(path, version, comparator): (PathBuf, &str, ComparisonOperator)| {
+                    fix_error!(ParsingError, tag!("version(")),
-                        Function::Version(path, version.to_string(), comparator)
+                    call!(parse_version_args),
+                    fix_error!(ParsingError, tag!(")"))
+                ) => {
+                    |(path, version, comparator)| {
+                        Function::Version(path, version, comparator)
                     }
                 } |
-                delimited!(tag!("checksum("), call!(parse_checksum_args), tag!(")")) => {
+                delimited!(
+                    fix_error!(ParsingError, tag!("checksum(")),
+                    call!(parse_checksum_args),
+                    fix_error!(ParsingError, tag!(")"))
+                ) => {
                     |(path, crc)| Function::Checksum(path, crc)
                 }
             ) >> (function)
@@ -185,14 +248,14 @@ mod tests {
 
     #[test]
     fn parse_regex_should_produce_case_insensitive_regex() {
-        let (_, regex) = parse_regex("cargo.*").unwrap();
+        let (_, regex) = parse_regex("cargo.*".into()).unwrap();
 
         assert!(regex.is_match("Cargo.toml"));
     }
 
     #[test]
     fn function_parse_should_parse_a_file_path_function() {
-        let result = Function::parse("file(\"Cargo.toml\")").unwrap().1;
+        let result = Function::parse("file(\"Cargo.toml\")".into()).unwrap().1;
 
         match result {
             Function::FilePath(f) => assert_eq!(Path::new("Cargo.toml"), f),
@@ -202,12 +265,12 @@ mod tests {
 
     #[test]
     fn function_parse_should_error_if_the_file_path_is_outside_the_game_directory() {
-        assert!(Function::parse("file(\"../../Cargo.toml\")").is_err());
+        assert!(Function::parse("file(\"../../Cargo.toml\")".into()).is_err());
     }
 
     #[test]
     fn function_parse_should_parse_a_file_regex_function_with_no_parent_path() {
-        let result = Function::parse("file(\"Cargo.*\")").unwrap().1;
+        let result = Function::parse("file(\"Cargo.*\")".into()).unwrap().1;
 
         match result {
             Function::FileRegex(p, r) => {
@@ -220,7 +283,9 @@ mod tests {
 
     #[test]
     fn function_parse_should_parse_a_file_regex_function_with_a_parent_path() {
-        let result = Function::parse("file(\"subdir/Cargo.*\")").unwrap().1;
+        let result = Function::parse("file(\"subdir/Cargo.*\")".into())
+            .unwrap()
+            .1;
 
         match result {
             Function::FileRegex(p, r) => {
@@ -233,17 +298,17 @@ mod tests {
 
     #[test]
     fn function_parse_should_error_if_given_a_file_regex_function_ending_in_a_forward_slash() {
-        assert!(Function::parse("file(\"sub\\dir/\")").is_err());
+        assert!(Function::parse("file(\"sub\\dir/\")".into()).is_err());
     }
 
     #[test]
     fn function_parse_should_error_if_the_file_regex_parent_path_is_outside_the_game_directory() {
-        assert!(Function::parse("file(\"../../Cargo.*\")").is_err());
+        assert!(Function::parse("file(\"../../Cargo.*\")".into()).is_err());
     }
 
     #[test]
     fn function_parse_should_parse_an_active_path_function() {
-        let result = Function::parse("active(\"Cargo.toml\")").unwrap().1;
+        let result = Function::parse("active(\"Cargo.toml\")".into()).unwrap().1;
 
         match result {
             Function::ActivePath(f) => assert_eq!(Path::new("Cargo.toml"), f),
@@ -255,12 +320,12 @@ mod tests {
     fn function_parse_should_error_if_the_active_path_is_outside_the_game_directory() {
         // Trying to check if a path that isn't a plugin in the data folder is
         // active is pointless, but it's not worth having a more specific check.
-        assert!(Function::parse("active(\"../../Cargo.toml\")").is_err());
+        assert!(Function::parse("active(\"../../Cargo.toml\")".into()).is_err());
     }
 
     #[test]
     fn function_parse_should_parse_an_active_regex_function() {
-        let result = Function::parse("active(\"Cargo.*\")").unwrap().1;
+        let result = Function::parse("active(\"Cargo.*\")".into()).unwrap().1;
 
         match result {
             Function::ActiveRegex(r) => {
@@ -272,7 +337,7 @@ mod tests {
 
     #[test]
     fn function_parse_should_parse_a_many_function_with_no_parent_path() {
-        let result = Function::parse("many(\"Cargo.*\")").unwrap().1;
+        let result = Function::parse("many(\"Cargo.*\")".into()).unwrap().1;
 
         match result {
             Function::Many(p, r) => {
@@ -285,7 +350,9 @@ mod tests {
 
     #[test]
     fn function_parse_should_parse_a_many_function_with_a_parent_path() {
-        let result = Function::parse("many(\"subdir/Cargo.*\")").unwrap().1;
+        let result = Function::parse("many(\"subdir/Cargo.*\")".into())
+            .unwrap()
+            .1;
 
         match result {
             Function::Many(p, r) => {
@@ -298,17 +365,19 @@ mod tests {
 
     #[test]
     fn function_parse_should_error_if_given_a_many_function_ending_in_a_forward_slash() {
-        assert!(Function::parse("many(\"subdir/\")").is_err());
+        assert!(Function::parse("many(\"subdir/\")".into()).is_err());
     }
 
     #[test]
     fn function_parse_should_error_if_the_many_parent_path_is_outside_the_game_directory() {
-        assert!(Function::parse("file(\"../../Cargo.*\")").is_err());
+        assert!(Function::parse("file(\"../../Cargo.*\")".into()).is_err());
     }
 
     #[test]
     fn function_parse_should_parse_a_many_active_function() {
-        let result = Function::parse("many_active(\"Cargo.*\")").unwrap().1;
+        let result = Function::parse("many_active(\"Cargo.*\")".into())
+            .unwrap()
+            .1;
 
         match result {
             Function::ManyActive(r) => {
@@ -320,7 +389,7 @@ mod tests {
 
     #[test]
     fn function_parse_should_parse_a_checksum_function() {
-        let result = Function::parse("checksum(\"Cargo.toml\", DEADBEEF)")
+        let result = Function::parse("checksum(\"Cargo.toml\", DEADBEEF)".into())
             .unwrap()
             .1;
 
@@ -335,12 +404,12 @@ mod tests {
 
     #[test]
     fn function_parse_should_error_if_the_checksum_path_is_outside_the_game_directory() {
-        assert!(Function::parse("checksum(\"../../Cargo.toml\", DEADBEEF)").is_err());
+        assert!(Function::parse("checksum(\"../../Cargo.toml\", DEADBEEF)".into()).is_err());
     }
 
     #[test]
     fn function_parse_should_parse_a_version_equals_function() {
-        let result = Function::parse("version(\"Cargo.toml\", \"1.2\", ==)")
+        let result = Function::parse("version(\"Cargo.toml\", \"1.2\", ==)".into())
             .unwrap()
             .1;
 
@@ -356,6 +425,6 @@ mod tests {
 
     #[test]
     fn function_parse_should_error_if_the_version_path_is_outside_the_game_directory() {
-        assert!(Function::parse("version(\"../../Cargo.toml\", \"1.2\", ==)").is_err());
+        assert!(Function::parse("version(\"../../Cargo.toml\", \"1.2\", ==)".into()).is_err());
     }
 }

src/lib.rs

@@ -8,72 +8,24 @@ extern crate unicase;
 #[cfg(test)]
 extern crate tempfile;
 
+mod error;
 mod function;
 mod version;
 
 use std::collections::{HashMap, HashSet};
-use std::error;
 use std::ffi::OsStr;
 use std::fmt;
-use std::io;
 use std::path::{Path, PathBuf};
 use std::str;
 use std::sync::RwLock;
 
-use nom::{Err, IResult};
+use nom::types::CompleteStr;
+use nom::IResult;
 
+pub use error::{Error, ParsingError};
 use function::Function;
 
-#[derive(Debug)]
+type ParsingResult<'a, T> = IResult<CompleteStr<'a>, T, ParsingError>;
-pub enum Error {
-    ParsingIncomplete,
-    ParsingError,
-    PeParsingError(PathBuf, Box<error::Error>),
-    IoError(PathBuf, io::Error),
-}
-
-impl<I> From<Err<I>> for Error {
-    fn from(error: Err<I>) -> Self {
-        match error {
-            Err::Incomplete(_) => Error::ParsingIncomplete,
-            _ => Error::ParsingError,
-        }
-    }
-}
-
-impl fmt::Display for Error {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        match self {
-            Error::ParsingIncomplete => write!(f, "More input was expected by the parser"),
-            Error::ParsingError => {
-                write!(f, "An error was encountered while parsing the expression")
-            }
-            Error::PeParsingError(p, e) => write!(
-                f,
-                "An error was encountered while reading the file version field of \"{}\": {}",
-                p.display(),
-                e
-            ),
-            Error::IoError(p, e) => write!(
-                f,
-                "An error was encountered while accessing the path \"{}\": {}",
-                p.display(),
-                e
-            ),
-        }
-    }
-}
-
-impl error::Error for Error {
-    fn cause(&self) -> Option<&error::Error> {
-        match self {
-            Error::IoError(_, e) => Some(e),
-            _ => None,
-        }
-    }
-}
-
-type ParsingResult<'a, T> = IResult<&'a str, T, u32>;
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub enum GameType {
@@ -177,16 +129,20 @@ impl str::FromStr for Expression {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        parse_expression(s)
+        parse_expression(nom::types::CompleteStr(s))
             .map(|(_, expression)| expression)
             .map_err(Error::from)
     }
 }
 
-fn parse_expression(input: &str) -> ParsingResult<Expression> {
+fn parse_expression(input: nom::types::CompleteStr) -> ParsingResult<Expression> {
     do_parse!(
         input,
-        compound_conditions: separated_list_complete!(ws!(tag!("or")), CompoundCondition::parse)
+        compound_conditions:
+            separated_list!(
+                fix_error!(ParsingError, ws!(tag!("or"))),
+                CompoundCondition::parse
+            )
             >> (Expression(compound_conditions))
     )
 }
@@ -212,10 +168,11 @@ impl CompoundCondition {
         Ok(true)
     }
 
-    fn parse(input: &str) -> ParsingResult<CompoundCondition> {
+    fn parse(input: nom::types::CompleteStr) -> ParsingResult<CompoundCondition> {
         do_parse!(
             input,
-            conditions: separated_list_complete!(ws!(tag!("and")), Condition::parse)
+            conditions:
+                separated_list!(fix_error!(ParsingError, ws!(tag!("and"))), Condition::parse)
                 >> (CompoundCondition(conditions))
         )
     }
@@ -244,7 +201,7 @@ impl Condition {
         }
     }
 
-    fn parse(input: &str) -> ParsingResult<Condition> {
+    fn parse(input: nom::types::CompleteStr) -> ParsingResult<Condition> {
         do_parse!(
             input,
             condition:
@@ -252,10 +209,10 @@ impl Condition {
                     call!(Function::parse) => {
                         |f| Condition::Function(f)
                     } |
-                    preceded!(ws!(tag!("not")), call!(Function::parse)) => {
+                    preceded!(fix_error!(ParsingError, ws!(tag!("not"))), call!(Function::parse)) => {
                         |f| Condition::InvertedFunction(f)
                     } |
-                    delimited!(tag!("("), call!(parse_expression), tag!(")")) => {
+                    delimited!(fix_error!(ParsingError, tag!("(")), call!(parse_expression), fix_error!(ParsingError, tag!(")"))) => {
                         |e| Condition::Expression(e)
                     }
             ) >> (condition)
@@ -440,6 +397,56 @@ mod tests {
         assert!(!GameType::Fo4vr.is_plugin_filename(filename));
     }
 
+    #[test]
+    fn expression_from_str_should_error_with_input_on_incomplete_input() {
+        let error = Expression::from_str("file(\"Carg").unwrap_err();
+
+        assert_eq!(
+            "An error was encountered in the parser \"SeparatedList\" while parsing the expression \"file(\\\"Carg\"",
+            error.to_string()
+        );
+    }
+
+    #[test]
+    fn expression_from_str_should_error_with_input_on_invalid_regex() {
+        let error = Expression::from_str("file(\"Carg\\.*(\")").unwrap_err();
+
+        assert_eq!(
+            "An error was encountered while parsing the expression \"Carg\\.*(\": regex parse error:\n    Carg\\.*(\n           ^\nerror: unclosed group",
+            error.to_string()
+        );
+    }
+
+    #[test]
+    fn expression_from_str_should_error_with_input_on_invalid_crc() {
+        let error = Expression::from_str("checksum(\"Cargo.toml\", DEADBEEFDEAD)").unwrap_err();
+
+        assert_eq!(
+            "An error was encountered while parsing the expression \"DEADBEEFDEAD\": number too large to fit in target type",
+            error.to_string()
+        );
+    }
+
+    #[test]
+    fn expression_from_str_should_error_with_input_on_directory_regex() {
+        let error = Expression::from_str("file(\"targ.*et/\")").unwrap_err();
+
+        assert_eq!(
+            "An error was encountered while parsing the expression \"targ.*et/\\\")\": \"targ.*et/\" ends in a directory separator",
+            error.to_string()
+        );
+    }
+
+    #[test]
+    fn expression_from_str_should_error_with_input_on_path_outside_game_directory() {
+        let error = Expression::from_str("file(\"../../Cargo.toml\")").unwrap_err();
+
+        assert_eq!(
+            "An error was encountered while parsing the expression \"../../Cargo.toml\\\")\": \"../../Cargo.toml\" is not in the game directory",
+            error.to_string()
+        );
+    }
+
     #[test]
     fn expression_parse_should_handle_a_single_compound_condition() {
         let result = Expression::from_str("file(\"Cargo.toml\")").unwrap();
@@ -465,7 +472,9 @@ mod tests {
 
     #[test]
     fn compound_condition_parse_should_handle_a_single_condition() {
-        let result = CompoundCondition::parse("file(\"Cargo.toml\")").unwrap().1;
+        let result = CompoundCondition::parse("file(\"Cargo.toml\")".into())
+            .unwrap()
+            .1;
 
         match result.0.as_slice() {
             [Condition::Function(Function::FilePath(f))] => {
@@ -480,7 +489,8 @@ mod tests {
 
     #[test]
     fn compound_condition_parse_should_handle_multiple_conditions() {
-        let result = CompoundCondition::parse("file(\"Cargo.toml\") and file(\"README.md\")")
+        let result =
+            CompoundCondition::parse("file(\"Cargo.toml\") and file(\"README.md\")".into())
                 .unwrap()
                 .1;
 
@@ -499,7 +509,7 @@ mod tests {
 
     #[test]
     fn condition_parse_should_handle_a_function() {
-        let result = Condition::parse("file(\"Cargo.toml\")").unwrap().1;
+        let result = Condition::parse("file(\"Cargo.toml\")".into()).unwrap().1;
 
         match result {
             Condition::Function(Function::FilePath(f)) => {
@@ -514,7 +524,9 @@ mod tests {
 
     #[test]
     fn condition_parse_should_handle_a_inverted_function() {
-        let result = Condition::parse("not file(\"Cargo.toml\")").unwrap().1;
+        let result = Condition::parse("not file(\"Cargo.toml\")".into())
+            .unwrap()
+            .1;
 
         match result {
             Condition::InvertedFunction(Function::FilePath(f)) => {
@@ -529,7 +541,9 @@ mod tests {
 
     #[test]
     fn condition_parse_should_handle_an_expression_in_parentheses() {
-        let result = Condition::parse("(not file(\"Cargo.toml\"))").unwrap().1;
+        let result = Condition::parse("(not file(\"Cargo.toml\"))".into())
+            .unwrap()
+            .1;
 
         match result {
             Condition::Expression(_) => {}

src/version.rs

@@ -5,7 +5,7 @@ use pelite::resources::version_info::VersionInfo;
 use pelite::resources::FindError;
 use pelite::FileMap;
 
-use Error;
+use error::Error;
 
 #[derive(Clone, Debug, PartialEq, PartialOrd)]
 enum Identifier {