refac: made the error object simpler

migra-cli/src/commands/list.rs

@@ -1,6 +1,6 @@
 use crate::config::Config;
 use crate::databases::*;
-use crate::error::{ErrorKind, StdResult};
+use crate::error::{Error, StdResult};
 use crate::migration::{
     filter_pending_migrations, DatabaseMigrationManager, Migration, MigrationManager,
 };
@@ -20,9 +20,9 @@ pub(crate) fn print_migration_lists(config: Config) -> StdResult<()> {
 
             applied_migration_names
         }
-        Err(e) if *e.kind() == ErrorKind::MissedEnvVar(String::new()) => {
-            println!("{}", e.kind());
-            println!("No connection to database");
+        Err(e) if e == Error::MissedEnvVar(String::new()) => {
+            eprintln!("WARNING: {}", e);
+            eprintln!("WARNING: No connection to database");
 
             Vec::new()
         }

migra-cli/src/config.rs

@@ -1,4 +1,4 @@
-use crate::error::{Error, ErrorKind};
+use crate::error::{Error, MigraResult};
 use crate::migration::Migration;
 use crate::path::PathBuilder;
 use serde::{Deserialize, Serialize};
@@ -31,18 +31,17 @@ pub(crate) struct DatabaseConfig {
 }
 
 impl DatabaseConfig {
-    pub fn client(&self) -> crate::error::Result<SupportedDatabaseClient> {
+    pub fn client(&self) -> MigraResult<SupportedDatabaseClient> {
         Ok(SupportedDatabaseClient::Postgres)
     }
 
-    pub fn connection_string(&self) -> crate::error::Result<String> {
+    pub fn connection_string(&self) -> MigraResult<String> {
         let connection = self
             .connection
             .clone()
             .unwrap_or_else(|| String::from(DEFAULT_DATABASE_CONNECTION_ENV));
         if let Some(connection_env) = connection.strip_prefix("$") {
-            env::var(connection_env)
-                .map_err(|e| Error::new(ErrorKind::MissedEnvVar(connection_env.to_string()), e))
+            env::var(connection_env).map_err(|_| Error::MissedEnvVar(connection_env.to_string()))
         } else {
             Ok(connection)
         }
@@ -62,35 +61,32 @@ impl Default for Config {
     }
 }
 
-fn recursive_find_config_file() -> io::Result<PathBuf> {
+fn search_for_directory_containing_file(path: &Path, file_name: &str) -> MigraResult<PathBuf> {
+    let file_path = path.join(file_name);
+    if file_path.is_file() {
+        Ok(path.to_owned())
+    } else {
+        path.parent()
+            .ok_or(Error::RootNotFound)
+            .and_then(|p| search_for_directory_containing_file(p, file_name))
+    }
+}
+
+fn recursive_find_project_root() -> MigraResult<PathBuf> {
     let current_dir = std::env::current_dir()?;
 
-    let mut read_dir = Some(current_dir.as_path());
-
-    loop {
-        if let Some(dir) = read_dir {
-            let migra_file_path = PathBuilder::from(dir).append(MIGRA_TOML_FILENAME).build();
-            if !migra_file_path.exists() {
-                read_dir = dir.parent();
-                continue;
-            }
-
-            return Ok(migra_file_path);
-        } else {
-            return Err(io::Error::from(io::ErrorKind::NotFound));
-        }
-    }
+    search_for_directory_containing_file(&current_dir, MIGRA_TOML_FILENAME)
 }
 
 impl Config {
-    pub fn read(config_path: Option<PathBuf>) -> io::Result<Config> {
+    pub fn read(config_path: Option<PathBuf>) -> MigraResult<Config> {
         let config_path = match config_path {
             Some(mut config_path) if config_path.is_dir() => {
                 config_path.push(MIGRA_TOML_FILENAME);
                 Some(config_path)
             }
             Some(config_path) => Some(config_path),
-            None => recursive_find_config_file().ok(),
+            None => recursive_find_project_root().ok(),
         };
 
         match config_path {
@@ -123,7 +119,7 @@ impl Config {
             .build()
     }
 
-    pub fn migrations(&self) -> io::Result<Vec<Migration>> {
+    pub fn migrations(&self) -> MigraResult<Vec<Migration>> {
         let mut entries = match self.migration_dir_path().read_dir() {
             Err(e) if e.kind() == io::ErrorKind::NotFound => return Ok(Vec::new()),
             entries => entries?

migra-cli/src/error.rs

@@ -1,102 +1,43 @@
-use std::error::Error as StdError;
+use std::error;
 use std::fmt;
+use std::io;
 use std::mem;
 use std::result;
 
 pub type StdResult<T> = result::Result<T, Box<dyn std::error::Error>>;
-pub type Result<T> = result::Result<T, Error>;
+pub type MigraResult<T> = result::Result<T, Error>;
 
 #[derive(Debug)]
-pub struct Error {
-    repr: Repr,
-}
-
-enum Repr {
-    Simple(ErrorKind),
-    Custom(Box<Custom>),
-}
-
-#[derive(Debug)]
-struct Custom {
-    kind: ErrorKind,
-    error: Box<dyn StdError + Send + Sync>,
-}
-
-#[derive(Debug, Clone)]
-pub enum ErrorKind {
+pub enum Error {
+    RootNotFound,
     MissedEnvVar(String),
+
+    IoError(io::Error),
 }
 
-impl fmt::Display for ErrorKind {
+impl fmt::Display for Error {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
         match *self {
-            ErrorKind::MissedEnvVar(ref name) => {
+            Error::RootNotFound => fmt.write_str("Cannot find root directory"),
+            Error::MissedEnvVar(ref name) => {
                 write!(fmt, r#"Missed "{}" environment variable"#, name)
             }
+            Error::IoError(ref error) => write!(fmt, "{}", error),
         }
     }
 }
 
-impl PartialEq for ErrorKind {
+impl error::Error for Error {}
+
+impl PartialEq for Error {
     fn eq(&self, other: &Self) -> bool {
         mem::discriminant(self) == mem::discriminant(other)
     }
 }
 
-impl From<ErrorKind> for Error {
+impl From<io::Error> for Error {
     #[inline]
-    fn from(kind: ErrorKind) -> Error {
-        Error {
-            repr: Repr::Simple(kind),
-        }
-    }
-}
-
-impl Error {
-    pub fn new<E>(kind: ErrorKind, error: E) -> Error
-    where
-        E: Into<Box<dyn StdError + Send + Sync>>,
-    {
-        Self::_new(kind, error.into())
-    }
-
-    fn _new(kind: ErrorKind, error: Box<dyn StdError + Send + Sync>) -> Error {
-        Error {
-            repr: Repr::Custom(Box::new(Custom { kind, error })),
-        }
-    }
-
-    pub fn kind(&self) -> &ErrorKind {
-        match &self.repr {
-            Repr::Custom(ref c) => &c.kind,
-            Repr::Simple(kind) => &kind,
-        }
-    }
-}
-
-impl fmt::Debug for Repr {
-    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Repr::Custom(ref c) => fmt::Debug::fmt(&c, fmt),
-            Repr::Simple(kind) => fmt.debug_tuple("Kind").field(&kind).finish(),
-        }
-    }
-}
-
-impl fmt::Display for Error {
-    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match &self.repr {
-            Repr::Custom(ref c) => c.error.fmt(fmt),
-            Repr::Simple(kind) => write!(fmt, "{}", kind),
-        }
-    }
-}
-
-impl StdError for Error {
-    fn source(&self) -> Option<&(dyn StdError + 'static)> {
-        match self.repr {
-            Repr::Simple(..) => None,
-            Repr::Custom(ref c) => c.error.source(),
-        }
+    fn from(err: io::Error) -> Error {
+        Error::IoError(err)
     }
 }

migra-cli/tests/commands.rs

@@ -98,10 +98,12 @@ mod list {
             .arg("ls")
             .assert()
             .success()
+            .stderr(contains(
+                r#"WARNING: Missed "DATABASE_URL" environment variable
+WARNING: No connection to database"#,
+            ))
             .stdout(contains(
-                r#"Missed "DATABASE_URL" environment variable
-No connection to database
-
+                r#"
 Pending migrations:
 —"#,
             ));