Replace deprecated 'error-chain' with 'thiserror' (#1820)

We can't use #[from] on Error::Msg(String) because String does not implement Error. (Which it shouldn't; see e.g. https://internals.rust-lang.org/t/impl-error-for-string/8881.) So we implement From manually for Error::Msg, since our current code was written in that way for error-chain.
2025-01-18 20:11:03 +00:00 · 2021-08-26 13:12:21 +02:00 · 2021-08-26 13:12:21 +02:00 · 19c3e82abf
commit 19c3e82abf
parent f1c0fd7343
12 changed files with 83 additions and 77 deletions
--- a/.github/workflows/CICD.yml
+++ b/.github/workflows/CICD.yml
@ -37,7 +37,7 @@ jobs:
      uses: actions-rs/cargo@v1
      with:
        command: clippy
-        args: --locked --all-targets --all-features
+        args: --locked --all-targets --all-features -- --allow clippy::unknown_clippy_lints
    - name: Run tests
      uses: actions-rs/cargo@v1
      with:
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -31,6 +31,7 @@

 - Deprecate `HighlightingAssets::syntaxes()` and `HighlightingAssets::syntax_for_file_name()`. Use `HighlightingAssets::get_syntaxes()` and `HighlightingAssets::get_syntax_for_file_name()` instead. They return a `Result` which is needed for upcoming lazy-loading work to improve startup performance. They also return what `SyntaxSet` the returned `SyntaxReference` belongs to. See #1747, #1755 and #1776 (@Enselic)
 - Remove `HighlightingAssets::from_files` and `HighlightingAssets::save_to_cache`. Instead of calling the former and then the latter you now make a single call to `bat::assets::build`. See #1802 (@Enselic)
+- Replace  the `error::Error(error::ErrorKind, _)` struct and enum with an `error::Error` enum. `Error(ErrorKind::UnknownSyntax, _)` becomes `Error::UnknownSyntax`, etc. Also remove the `error::ResultExt` trait. These changes stem from replacing `error-chain` with `thiserror`. See #1820 (@Enselic)



--- a/Cargo.lock
+++ b/Cargo.lock
@ -96,7 +96,6 @@ dependencies = [
 "content_inspector",
 "dirs-next",
 "encoding",
- "error-chain",
 "git2",
 "globset",
 "grep-cli",
@ -112,6 +111,7 @@ dependencies = [
 "shell-words",
 "syntect",
 "tempfile",
+ "thiserror",
 "unicode-width",
 "wait-timeout",
 "wild",
@ -376,15 +376,6 @@ version = "0.1.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "a246d82be1c9d791c5dfde9a2bd045fc3cbba3fa2b11ad558f27d01712f00569"

-[[package]]
-name = "error-chain"
-version = "0.12.4"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2d2f06b9cac1506ece98fe3231e3cc9c4410ec3d5b1f24ae1c8946f0742cdefc"
-dependencies = [
- "version_check",
-]
-
 [[package]]
 name = "fancy-regex"
 version = "0.7.1"
@ -1188,6 +1179,26 @@ dependencies = [
 "unicode-width",
 ]

+[[package]]
+name = "thiserror"
+version = "1.0.26"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "93119e4feac1cbe6c798c34d3a53ea0026b0b1de6a120deef895137c0529bfe2"
+dependencies = [
+ "thiserror-impl",
+]
+
+[[package]]
+name = "thiserror-impl"
+version = "1.0.26"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "060d69a0afe7796bf42e9e2ff91f5ee691fb15c53d38b4b62a9a53eb23164745"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
 [[package]]
 name = "tinyvec"
 version = "1.2.0"
@ -1263,12 +1274,6 @@ version = "0.8.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f1bddf1187be692e79c5ffeab891132dfb0f236ed36a43c7ed39f1165ee20191"

-[[package]]
-name = "version_check"
-version = "0.9.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5fecdca9a5291cc2b8dcf7dc02453fee791a280f3743cb0905f8822ae463b3fe"
-
 [[package]]
 name = "wait-timeout"
 version = "0.2.0"
--- a/Cargo.toml
+++ b/Cargo.toml
@ -48,6 +48,7 @@ ansi_colours = "^1.0"
 console = "0.14.1"
 lazy_static = { version = "1.4", optional = true }
 lazycell = "1.0"
+thiserror = "1.0"
 wild = { version = "2.0", optional = true }
 content_inspector = "0.2.4"
 encoding = "0.2"
@ -79,10 +80,6 @@ optional = true
 default-features = false
 features = ["suggestions", "color", "wrap_help"]

-[dependencies.error-chain]
-version = "0.12"
-default-features = false
-
 [dev-dependencies]
 assert_cmd = "1.0.8"
 serial_test = "0.5.1"
--- a/src/assets.rs
+++ b/src/assets.rs
@ -163,7 +163,7 @@ impl HighlightingAssets {
            syntax_set
                .find_syntax_by_token(language)
                .map(|syntax| SyntaxReferenceInSet { syntax, syntax_set })
-                .ok_or_else(|| ErrorKind::UnknownSyntax(language.to_owned()).into())
+                .ok_or_else(|| Error::UnknownSyntax(language.to_owned()))
        } else {
            let line_syntax = self.get_first_line_syntax(&mut input.reader)?;

@ -189,30 +189,27 @@ impl HighlightingAssets {
                    .unwrap_or_else(|| path.to_owned());

                match mapping.get_syntax_for(absolute_path) {
-                    Some(MappingTarget::MapToUnknown) => line_syntax.ok_or_else(|| {
-                        ErrorKind::UndetectedSyntax(path.to_string_lossy().into()).into()
-                    }),
+                    Some(MappingTarget::MapToUnknown) => line_syntax
+                        .ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into())),

                    Some(MappingTarget::MapTo(syntax_name)) => {
                        let syntax_set = self.get_syntax_set()?;
                        syntax_set
                            .find_syntax_by_name(syntax_name)
                            .map(|syntax| SyntaxReferenceInSet { syntax, syntax_set })
-                            .ok_or_else(|| ErrorKind::UnknownSyntax(syntax_name.to_owned()).into())
+                            .ok_or_else(|| Error::UnknownSyntax(syntax_name.to_owned()))
                    }

                    None => {
                        let file_name = path.file_name().unwrap_or_default();
                        self.get_extension_syntax(file_name)?
                            .or(line_syntax)
-                            .ok_or_else(|| {
-                                ErrorKind::UndetectedSyntax(path.to_string_lossy().into()).into()
-                            })
+                            .ok_or_else(|| Error::UndetectedSyntax(path.to_string_lossy().into()))
                    }
                }
            } else {
                // If a path wasn't provided, we fall back to the detect first-line syntax.
-                line_syntax.ok_or_else(|| ErrorKind::UndetectedSyntax("[unknown]".into()).into())
+                line_syntax.ok_or_else(|| Error::UndetectedSyntax("[unknown]".into()))
            }
        }
    }
@ -314,14 +311,14 @@ pub(crate) fn get_integrated_themeset() -> ThemeSet {
 }

 fn asset_from_cache<T: serde::de::DeserializeOwned>(path: &Path, description: &str) -> Result<T> {
-    let contents = fs::read(path).chain_err(|| {
+    let contents = fs::read(path).map_err(|_| {
        format!(
            "Could not load cached {} '{}'",
            description,
            path.to_string_lossy()
        )
    })?;
-    from_reader(&contents[..]).chain_err(|| format!("Could not parse cached {}", description))
+    from_reader(&contents[..]).map_err(|_| format!("Could not parse cached {}", description).into())
 }

 #[cfg(test)]
--- a/src/assets_metadata.rs
+++ b/src/assets_metadata.rs
@ -52,16 +52,15 @@ impl AssetsMetadata {
    pub fn load_from_folder(path: &Path) -> Result<Option<Self>> {
        match Self::try_load_from_folder(path) {
            Ok(metadata) => Ok(Some(metadata)),
-            Err(e) => match e.kind() {
-                ErrorKind::SerdeYamlError(_) => Err(e),
-                _ => {
-                    if path.join("syntaxes.bin").exists() || path.join("themes.bin").exists() {
-                        Ok(Some(Self::default()))
-                    } else {
-                        Ok(None)
-                    }
+            Err(e) => {
+                if let Error::SerdeYamlError(_) = e {
+                    Err(e)
+                } else if path.join("syntaxes.bin").exists() || path.join("themes.bin").exists() {
+                    Ok(Some(Self::default()))
+                } else {
+                    Ok(None)
                }
-            },
+            }
        }
    }

--- a/src/bin/bat/app.rs
+++ b/src/bin/bat/app.rs
@ -62,7 +62,7 @@ impl App {
            // Read arguments from bats config file
            let mut args = get_args_from_env_var()
                .unwrap_or_else(get_args_from_config_file)
-                .chain_err(|| "Could not parse configuration file")?;
+                .map_err(|_| "Could not parse configuration file")?;

            // Put the zero-th CLI argument (program name) first
            args.insert(0, cli_args.next().unwrap());
--- a/src/bin/bat/main.rs
+++ b/src/bin/bat/main.rs
@ -1,6 +1,3 @@
-// `error_chain!` can recurse deeply
-#![recursion_limit = "1024"]
-
 mod app;
 mod assets;
 mod clap_app;
--- a/src/build_assets.rs
+++ b/src/build_assets.rs
@ -267,7 +267,7 @@ impl SyntaxSetDependencyBuilder {

 fn asset_to_cache<T: serde::Serialize>(asset: &T, path: &Path, description: &str) -> Result<()> {
    print!("Writing {} to {} ... ", description, path.to_string_lossy());
-    syntect::dumps::dump_to_file(asset, &path).chain_err(|| {
+    syntect::dumps::dump_to_file(asset, &path).map_err(|_| {
        format!(
            "Could not save {} to {}",
            description,
--- a/src/error.rs
+++ b/src/error.rs
@ -1,42 +1,52 @@
-use error_chain::error_chain;
 use std::io::Write;
+use thiserror::Error;

-error_chain! {
-    foreign_links {
-        Clap(::clap::Error) #[cfg(feature = "minimal-application")];
-        Io(::std::io::Error);
-        SyntectError(::syntect::LoadingError);
-        ParseIntError(::std::num::ParseIntError);
-        GlobParsingError(::globset::Error);
-        SerdeYamlError(::serde_yaml::Error);
-    }
+#[derive(Error, Debug)]
+pub enum Error {
+    #[error(transparent)]
+    Io(#[from] ::std::io::Error),
+    #[error(transparent)]
+    SyntectError(#[from] ::syntect::LoadingError),
+    #[error(transparent)]
+    ParseIntError(#[from] ::std::num::ParseIntError),
+    #[error(transparent)]
+    GlobParsingError(#[from] ::globset::Error),
+    #[error(transparent)]
+    SerdeYamlError(#[from] ::serde_yaml::Error),
+    #[error("unable to detect syntax for {0}")]
+    UndetectedSyntax(String),
+    #[error("unknown syntax: '{0}'")]
+    UnknownSyntax(String),
+    #[error("Unknown style '{0}'")]
+    UnknownStyle(String),
+    #[error("Use of bat as a pager is disallowed in order to avoid infinite recursion problems")]
+    InvalidPagerValueBat,
+    #[error("{0}")]
+    Msg(String),
+}

-    errors {
-        UndetectedSyntax(input: String) {
-            description("unable to detect syntax"),
-            display("unable to detect syntax for {}", input)
-        }
-        UnknownSyntax(name: String) {
-            description("unknown syntax"),
-            display("unknown syntax: '{}'", name)
-        }
-        InvalidPagerValueBat {
-            description("invalid value `bat` for pager property"),
-            display("Use of bat as a pager is disallowed in order to avoid infinite recursion problems")
-        }
+impl From<&'static str> for Error {
+    fn from(s: &'static str) -> Self {
+        Error::Msg(s.to_owned())
    }
 }

+impl From<String> for Error {
+    fn from(s: String) -> Self {
+        Error::Msg(s)
+    }
+}
+
+pub type Result<T> = std::result::Result<T, Error>;
+
 pub fn default_error_handler(error: &Error, output: &mut dyn Write) {
    use ansi_term::Colour::Red;

    match error {
-        Error(ErrorKind::Io(ref io_error), _)
-            if io_error.kind() == ::std::io::ErrorKind::BrokenPipe =>
-        {
+        Error::Io(ref io_error) if io_error.kind() == ::std::io::ErrorKind::BrokenPipe => {
            ::std::process::exit(0);
        }
-        Error(ErrorKind::SerdeYamlError(_), _) => {
+        Error::SerdeYamlError(_) => {
            writeln!(
                output,
                "{}: Error while parsing metadata.yaml file: {}",
--- a/src/output.rs
+++ b/src/output.rs
@ -52,7 +52,7 @@ impl OutputType {
        use std::process::{Command, Stdio};

        let pager_opt =
-            pager::get_pager(pager_from_config).chain_err(|| "Could not parse pager command.")?;
+            pager::get_pager(pager_from_config).map_err(|_| "Could not parse pager command.")?;

        let pager = match pager_opt {
            Some(pager) => pager,
@ -60,7 +60,7 @@ impl OutputType {
        };

        if pager.kind == PagerKind::Bat {
-            return Err(ErrorKind::InvalidPagerValueBat.into());
+            return Err(Error::InvalidPagerValueBat);
        }

        let resolved_path = match grep_cli::resolve_binary(&pager.bin) {
@ -142,7 +142,7 @@ impl OutputType {
            OutputType::Pager(ref mut command) => command
                .stdin
                .as_mut()
-                .chain_err(|| "Could not open stdin for pager")?,
+                .ok_or("Could not open stdin for pager")?,
            OutputType::Stdout(ref mut handle) => handle,
        })
    }
--- a/src/printer.rs
+++ b/src/printer.rs
@ -174,7 +174,7 @@ impl<'a> InteractivePrinter<'a> {
            let syntax_in_set =
                match assets.get_syntax(config.language, input, &config.syntax_mapping) {
                    Ok(syntax_in_set) => syntax_in_set,
-                    Err(Error(ErrorKind::UndetectedSyntax(_), _)) => {
+                    Err(Error::UndetectedSyntax(_)) => {
                        let syntax_set = assets.get_syntax_set()?;
                        let syntax = syntax_set.find_syntax_plain_text();
                        SyntaxReferenceInSet { syntax, syntax_set }