Implement Error and Display for all error enums by using thiserror

Also do some minor tweaks to the way errors are represented, and add basic
integration tests for errors, to give a feel for how the errors behave.

Fixes trishume#94

Cargo.toml

@@ -33,6 +33,7 @@ serde = "1.0"
 serde_derive = "1.0"
 serde_json = "1.0"
 once_cell = "1.8"
+thiserror = "1.0"
 
 [dev-dependencies]
 criterion = { version = "0.3", features = [ "html_reports" ] }

src/highlighting/settings.rs

@@ -13,9 +13,10 @@ pub trait ParseSettings: Sized {
 }
 
 /// An error parsing a settings file
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error)]
 pub enum SettingsError {
     /// Incorrect Plist syntax
+    #[error("Incorrect Plist syntax: {0}")]
     Plist(PlistError),
 }
 

src/highlighting/theme_load.rs

@@ -11,27 +11,34 @@ use crate::parsing::ParseScopeError;
 
 use self::ParseThemeError::*;
 
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error)]
 pub enum ParseThemeError {
+    #[error("Incorrect underline option")]
     IncorrectUnderlineOption,
+    #[error("Incorrect font style: {0}")]
     IncorrectFontStyle(String),
+    #[error("Incorrect color")]
     IncorrectColor,
+    #[error("Incorrect syntax")]
     IncorrectSyntax,
+    #[error("Incorrect settings")]
     IncorrectSettings,
+    #[error("Undefined settings")]
     UndefinedSettings,
+    #[error("Undefined scope settings: {0}")]
     UndefinedScopeSettings(String),
+    #[error("Color sheme scope is not object")]
     ColorShemeScopeIsNotObject,
+    #[error("Color sheme settings is not object")]
     ColorShemeSettingsIsNotObject,
+    #[error("Scope selector is not string: {0}")]
     ScopeSelectorIsNotString(String),
+    #[error("Duplicate settings")]
     DuplicateSettings,
-    ScopeParse(ParseScopeError),
+    #[error("Scope parse error: {0}")]
+    ScopeParse(#[from] ParseScopeError),
 }
 
-impl From<ParseScopeError> for ParseThemeError {
-    fn from(error: ParseScopeError) -> ParseThemeError {
-        ScopeParse(error)
-    }
-}
 
 impl FromStr for UnderlineOption {
     type Err = ParseThemeError;

src/lib.rs

@@ -43,108 +43,37 @@ pub mod util;
 mod utils;
 
 use std::io::Error as IoError;
-use std::error::Error;
-use std::fmt;
 
-#[cfg(feature = "metadata")]
-use serde_json::Error as JsonError;
-#[cfg(all(feature = "yaml-load", feature = "parsing"))]
-use crate::parsing::ParseSyntaxError;
 #[cfg(feature = "plist-load")]
 use crate::highlighting::{ParseThemeError, SettingsError};
 
 /// Common error type used by syntax and theme loading
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error)]
 pub enum LoadingError {
     /// error finding all the files in a directory
-    WalkDir(walkdir::Error),
+    #[error("error finding all the files in a directory: {0}")]
+    WalkDir(#[from] walkdir::Error),
     /// error reading a file
-    Io(IoError),
+    #[error("error reading a file: {0}")]
+    Io(#[from] IoError),
     /// a syntax file was invalid in some way
-    #[cfg(feature = "yaml-load")]
-    ParseSyntax(ParseSyntaxError, Option<String>),
+    #[cfg(all(feature = "yaml-load", feature = "parsing"))]
+    #[error("{1}: {0}")]
+    ParseSyntax(crate::parsing::ParseSyntaxError, String),
     /// a metadata file was invalid in some way
     #[cfg(feature = "metadata")]
-    ParseMetadata(JsonError),
+    #[error("Failed to parse JSON")]
+    ParseMetadata(#[from] serde_json::Error),
     /// a theme file was invalid in some way
     #[cfg(feature = "plist-load")]
-    ParseTheme(ParseThemeError),
+    #[error("Invalid syntax theme")]
+    ParseTheme(#[from] ParseThemeError),
     /// a theme's Plist syntax was invalid in some way
     #[cfg(feature = "plist-load")]
-    ReadSettings(SettingsError),
+    #[error("Invalid syntax theme settings")]
+    ReadSettings(#[from] SettingsError),
     /// A path given to a method was invalid.
     /// Possibly because it didn't reference a file or wasn't UTF-8.
+    #[error("Invalid path")]
     BadPath,
 }
-
-#[cfg(feature = "plist-load")]
-impl From<SettingsError> for LoadingError {
-    fn from(error: SettingsError) -> LoadingError {
-        LoadingError::ReadSettings(error)
-    }
-}
-
-impl From<IoError> for LoadingError {
-    fn from(error: IoError) -> LoadingError {
-        LoadingError::Io(error)
-    }
-}
-
-#[cfg(feature = "plist-load")]
-impl From<ParseThemeError> for LoadingError {
-    fn from(error: ParseThemeError) -> LoadingError {
-        LoadingError::ParseTheme(error)
-    }
-}
-
-#[cfg(feature = "metadata")]
-impl From<JsonError> for LoadingError {
-    fn from(src: JsonError) -> LoadingError {
-        LoadingError::ParseMetadata(src)
-    }
-}
-
-#[cfg(all(feature = "yaml-load", feature = "parsing"))]
-impl From<ParseSyntaxError> for LoadingError {
-    fn from(error: ParseSyntaxError) -> LoadingError {
-        LoadingError::ParseSyntax(error, None)
-    }
-}
-
-impl fmt::Display for LoadingError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        use crate::LoadingError::*;
-
-        match *self {
-            WalkDir(ref error) => error.fmt(f),
-            Io(ref error) => error.fmt(f),
-            #[cfg(feature = "yaml-load")]
-            ParseSyntax(ref error, ref filename) => {
-                if let Some(ref file) = filename {
-                    write!(f, "{}: {}", file, error)
-                } else {
-                    error.fmt(f)
-                }
-            },
-            #[cfg(feature = "metadata")]
-            ParseMetadata(_) => write!(f, "Failed to parse JSON"),
-            #[cfg(feature = "plist-load")]
-            ParseTheme(_) => write!(f, "Invalid syntax theme"),
-            #[cfg(feature = "plist-load")]
-            ReadSettings(_) => write!(f, "Invalid syntax theme settings"),
-            BadPath => write!(f, "Invalid path"),
-        }
-    }
-}
-
-impl Error for LoadingError {
-    fn cause(&self) -> Option<&dyn Error> {
-        use crate::LoadingError::*;
-
-        match *self {
-            WalkDir(ref error) => Some(error),
-            Io(ref error) => Some(error),
-            _ => None,
-        }
-    }
-}

src/parsing/scope.rs

@@ -48,13 +48,15 @@ pub struct Scope {
 }
 
 /// Not all strings are valid scopes
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error)]
 pub enum ParseScopeError {
     /// Due to a limitation of the current optimized internal representation
     /// scopes can be at most 8 atoms long
+    #[error("Too long scope. Scopes can be at most 8 atoms long.")]
     TooLong,
     /// The internal representation uses 16 bits per atom, so if all scopes ever
     /// used by the program have more than 2^16-2 atoms, things break
+    #[error("Too many atoms. Max 2^16-2 atoms allowed.")]
     TooManyAtoms,
 }
 

src/parsing/syntax_set.rs

@@ -102,7 +102,7 @@ fn load_syntax_file(p: &Path,
         lines_include_newline,
         p.file_stem().and_then(|x| x.to_str()),
     )
-    .map_err(|e| LoadingError::ParseSyntax(e, Some(format!("{}", p.display()))))
+    .map_err(|e| LoadingError::ParseSyntax(e, format!("{}", p.display())))
 }
 
 impl Clone for SyntaxSet {

src/parsing/yaml_load.rs

@@ -5,63 +5,39 @@ use yaml_rust::{YamlLoader, Yaml, ScanError};
 use yaml_rust::yaml::Hash;
 use std::collections::HashMap;
 use std::error::Error;
-use std::fmt;
 use std::path::Path;
 use std::ops::DerefMut;
 
-#[derive(Debug)]
+#[derive(Debug, thiserror::Error)]
 pub enum ParseSyntaxError {
     /// Invalid YAML file syntax, or at least something yaml_rust can't handle
-    InvalidYaml(ScanError),
+    #[error("Invalid YAML file syntax: {0}")]
+    InvalidYaml(#[from] ScanError),
     /// The file must contain at least one YAML document
+    #[error("The file must contain at least one YAML document")]
     EmptyFile,
     /// Some keys are required for something to be a valid `.sublime-syntax`
+    #[error("Missing mandatory key in YAML file: {0}")]
     MissingMandatoryKey(&'static str),
     /// Invalid regex
-    RegexCompileError(String, Box<dyn Error + Send + Sync + 'static>),
+    #[error("Error while compiling regex '{0}': {1}")]
+    RegexCompileError(String, #[source] Box<dyn Error + Send + Sync + 'static>),
     /// A scope that syntect's scope implementation can't handle
+    #[error("Invalid scope: {0}")]
     InvalidScope(ParseScopeError),
     /// A reference to another file that is invalid
+    #[error("Invalid file reference")]
     BadFileRef,
     /// Syntaxes must have a context named "main"
+    #[error("Context 'main' is missing")]
     MainMissing,
     /// Some part of the YAML file is the wrong type (e.g a string but should be a list)
     /// Sorry this doesn't give you any way to narrow down where this is.
     /// Maybe use Sublime Text to figure it out.
+    #[error("Type mismatch")]
     TypeMismatch,
 }
 
-impl fmt::Display for ParseSyntaxError {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        use crate::ParseSyntaxError::*;
-
-        match *self {
-            InvalidYaml(ref err) => write!(f, "Invalid YAML file syntax: {}", err),
-            EmptyFile => write!(f, "Empty file"),
-            MissingMandatoryKey(key) => write!(f, "Missing mandatory key in YAML file: {}", key),
-            RegexCompileError(ref regex, ref error) => {
-                write!(f, "Error while compiling regex '{}': {}", regex, error)
-            }
-            InvalidScope(_) => write!(f, "Invalid scope"),
-            BadFileRef => write!(f, "Invalid file reference"),
-            MainMissing => write!(f, "Context 'main' is missing"),
-            TypeMismatch => write!(f, "Type mismatch"),
-        }
-    }
-}
-
-impl Error for ParseSyntaxError {
-    fn cause(&self) -> Option<&dyn Error> {
-        use crate::ParseSyntaxError::*;
-
-        match self {
-            InvalidYaml(ref error) => Some(error),
-            RegexCompileError(_, error) => Some(error.as_ref()),
-            _ => None,
-        }
-    }
-}
-
 fn get_key<'a, R, F: FnOnce(&'a Yaml) -> Option<R>>(map: &'a Hash,
                                                     key: &'static str,
                                                     f: F)

tests/error_handling.rs

@@ -0,0 +1,50 @@
+use std::{error::Error, fmt::Display};
+
+use syntect::{
+    parsing::{ParseScopeError, ParseSyntaxError},
+    LoadingError,
+};
+
+#[test]
+fn loading_error_display() {
+    assert_display(LoadingError::BadPath, "Invalid path");
+
+    assert_display(
+        LoadingError::ParseSyntax(
+            ParseSyntaxError::MissingMandatoryKey("main"),
+            String::from("file.sublime-syntax"),
+        ),
+        "file.sublime-syntax: Missing mandatory key in YAML file: main",
+    );
+
+    assert_display(
+        LoadingError::ParseSyntax(
+            ParseSyntaxError::RegexCompileError("[a-Z]".to_owned(), LoadingError::BadPath.into()),
+            String::from("another-file.sublime-syntax"),
+        ),
+        "another-file.sublime-syntax: Error while compiling regex '[a-Z]': Invalid path",
+    );
+}
+
+#[test]
+fn parse_scope_error_display() {
+    assert_display(
+        ParseScopeError::TooLong,
+        "Too long scope. Scopes can be at most 8 atoms long.",
+    )
+}
+
+#[test]
+fn loading_error_source() {
+    let source = std::io::Error::new(std::io::ErrorKind::Other, "this is an error string");
+    assert_display(
+        LoadingError::Io(source).source().unwrap(),
+        "this is an error string",
+    );
+}
+
+/// Helper to assert that a given implementation of [Display] generates the
+/// expected string.
+fn assert_display(display: impl Display, expected_display: &str) {
+    assert_eq!(format!("{}", display), String::from(expected_display));
+}