Skip to content

Commit

Permalink
Remove dependency on the (deprecated) failure crate
Browse files Browse the repository at this point in the history
* Switch to using thiserror instead of failure_derive
* Switch to using the std `Error` type instead of `Fail`
  • Loading branch information
ipetkov committed Jun 15, 2020
1 parent 687190b commit 5deb6af
Show file tree
Hide file tree
Showing 27 changed files with 158 additions and 179 deletions.
2 changes: 1 addition & 1 deletion conch-runtime-tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ publish = false
async-trait = "0.1"
conch-parser = "*"
conch-runtime = { path = "../conch-runtime" }
failure = "0.1"
futures-core = "0.3"
futures-util = "0.3"
tempfile = "3.1"
thiserror = "1"
tokio = { version = "0.2", features = ["full"] }
void = "1"
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use conch_runtime::STDERR_FILENO;
mod support;
pub use self::support::*;

#[derive(Debug, Fail)]
#[fail(display = "some error message")]
#[derive(Debug, thiserror::Error)]
#[error("some error message")]
struct MockErr;

async fn test_with_perms(perms: Permissions) {
Expand All @@ -18,7 +18,7 @@ async fn test_with_perms(perms: Permissions) {
env.set_file_desc(STDERR_FILENO, pipe.writer, perms);

let reader = env.read_all(pipe.reader);
tokio::spawn(env.report_failure(&MockErr));
tokio::spawn(env.report_error(&MockErr));

let name = env.name().clone();
drop(env);
Expand Down Expand Up @@ -52,5 +52,5 @@ async fn read_write() {
async fn closed_fd() {
let mut env = DefaultEnv::<String>::new().expect("failed to create env");
env.close_file_desc(STDERR_FILENO);
env.report_failure(&MockErr).await;
env.report_error(&MockErr).await;
}
37 changes: 5 additions & 32 deletions conch-runtime-tests/tests/support.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ pub use conch_runtime::eval::*;
pub use conch_runtime::path::*;
pub use conch_runtime::spawn::{self, *};
pub use conch_runtime::{ExitStatus, EXIT_ERROR, EXIT_SUCCESS};
pub use failure::Fail;
pub use futures_core::future::*;
pub use futures_util::future::*;

Expand Down Expand Up @@ -63,23 +62,13 @@ pub fn dev_null<E: ?Sized + FileDescOpener>(env: &mut E) -> E::OpenedFileHandle
.expect("failed to open DEV_NULL")
}

#[derive(Debug, Clone, PartialEq, Eq)]
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
#[error("mock {}fatal error", if self.is_fatal() { "non-" } else { "" })]
pub enum MockErr {
Fatal(bool),
ExpansionError(ExpansionError),
RedirectionError(Arc<RedirectionError>),
CommandError(Arc<CommandError>),
}

impl failure::Fail for MockErr {
fn cause(&self) -> Option<&dyn failure::Fail> {
match *self {
MockErr::Fatal(_) => None,
MockErr::ExpansionError(ref e) => Some(e),
MockErr::RedirectionError(ref e) => Some(&**e),
MockErr::CommandError(ref e) => Some(&**e),
}
}
ExpansionError(#[from] ExpansionError),
RedirectionError(#[source] Arc<RedirectionError>),
CommandError(#[source] Arc<CommandError>),
}

impl conch_runtime::error::IsFatalError for MockErr {
Expand All @@ -93,28 +82,12 @@ impl conch_runtime::error::IsFatalError for MockErr {
}
}

impl ::std::fmt::Display for MockErr {
fn fmt(&self, fmt: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
write!(
fmt,
"mock {}fatal error",
if self.is_fatal() { "non-" } else { "" }
)
}
}

impl From<RuntimeError> for MockErr {
fn from(err: RuntimeError) -> Self {
MockErr::Fatal(err.is_fatal())
}
}

impl From<ExpansionError> for MockErr {
fn from(err: ExpansionError) -> Self {
MockErr::ExpansionError(err)
}
}

impl From<RedirectionError> for MockErr {
fn from(err: RedirectionError) -> Self {
MockErr::RedirectionError(Arc::new(err))
Expand Down
9 changes: 7 additions & 2 deletions conch-runtime-tests/tests/swallow_non_fatal.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
#![deny(rust_2018_idioms)]

use std::error::Error;

#[macro_use]
mod support;
pub use self::support::*;

struct MockEnv;

impl ReportFailureEnvironment for MockEnv {
fn report_failure<'a>(&mut self, fail: &'a dyn Fail) -> BoxFuture<'a, ()> {
impl ReportErrorEnvironment for MockEnv {
fn report_error<'a>(
&mut self,
fail: &'a (dyn Error + Send + Sync + 'static),
) -> BoxFuture<'a, ()> {
Box::pin(async move {
println!("{}", fail);
})
Expand Down
3 changes: 1 addition & 2 deletions conch-runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ default = ["conch-parser"]
async-trait = "0.1"
conch-parser = { version = "0.1", optional = true }
clap = "2"
failure = "0.1"
failure_derive = "0.1"
futures-core = "0.3"
futures-util = "0.3"
glob = "0.3"
lazy_static = "1"
thiserror = "1"
tokio = { version = "0.2", features = ["fs", "io-util", "process"] }
void = "1"

Expand Down
22 changes: 14 additions & 8 deletions conch-runtime/src/env.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
//! This module defines various interfaces and implementations of shell environments.
//! See the documentation around `Env` or `DefaultEnv` to get started.

use failure::Fail;
use futures_core::future::BoxFuture;
use std::error::Error;

mod args;
mod async_io;
Expand Down Expand Up @@ -58,15 +58,21 @@ impl<'a, T: ?Sized + IsInteractiveEnvironment> IsInteractiveEnvironment for &'a
}
}

/// An interface for reporting arbitrary failures.
pub trait ReportFailureEnvironment {
/// Reports any `Fail`ure as appropriate, e.g. print to stderr.
fn report_failure<'a>(&mut self, fail: &'a dyn Fail) -> BoxFuture<'a, ()>;
/// An interface for reporting arbitrary errors.
pub trait ReportErrorEnvironment {
/// Reports any `Error` as appropriate, e.g. print to stderr.
fn report_error<'a>(
&mut self,
fail: &'a (dyn Error + Sync + Send + 'static),
) -> BoxFuture<'a, ()>;
}

impl<'b, T: ?Sized + ReportFailureEnvironment> ReportFailureEnvironment for &'b mut T {
fn report_failure<'a>(&mut self, fail: &'a dyn Fail) -> BoxFuture<'a, ()> {
(**self).report_failure(fail)
impl<'b, T: ?Sized + ReportErrorEnvironment> ReportErrorEnvironment for &'b mut T {
fn report_error<'a>(
&mut self,
fail: &'a (dyn Error + Sync + Send + 'static),
) -> BoxFuture<'a, ()> {
(**self).report_error(fail)
}
}

Expand Down
12 changes: 7 additions & 5 deletions conch-runtime/src/env/env_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ use crate::env::{
ArgsEnv, ArgumentsEnvironment, AsyncIoEnvironment, ChangeWorkingDirectoryEnvironment,
ExecutableData, ExecutableEnvironment, ExportedVariableEnvironment, FileDescEnvironment,
FileDescOpener, FnEnv, FnFrameEnv, FunctionEnvironment, FunctionFrameEnvironment,
IsInteractiveEnvironment, LastStatusEnv, LastStatusEnvironment, Pipe, ReportFailureEnvironment,
IsInteractiveEnvironment, LastStatusEnv, LastStatusEnvironment, Pipe, ReportErrorEnvironment,
SetArgumentsEnvironment, ShiftArgumentsEnvironment, StringWrapper, SubEnvironment,
TokioExecEnv, TokioFileDescManagerEnv, UnsetFunctionEnvironment, UnsetVariableEnvironment,
VarEnv, VariableEnvironment, VirtualWorkingDirEnv, WorkingDirectoryEnvironment,
};
use crate::error::{CommandError, RuntimeError};
use crate::io::Permissions;
use crate::{ExitStatus, Fd, Spawn, IFS_DEFAULT, STDERR_FILENO};
use failure::Fail;
use futures_core::future::BoxFuture;
use std::borrow::{Borrow, Cow};
use std::convert::From;
use std::error::Error;
use std::fmt;
use std::fs::OpenOptions;
use std::hash::Hash;
Expand Down Expand Up @@ -563,8 +563,7 @@ where
}
}

impl<A, FM, L, V, EX, WD, B, N, ERR> ReportFailureEnvironment
for Env<A, FM, L, V, EX, WD, B, N, ERR>
impl<A, FM, L, V, EX, WD, B, N, ERR> ReportErrorEnvironment for Env<A, FM, L, V, EX, WD, B, N, ERR>
where
A: ArgumentsEnvironment,
A::Arg: fmt::Display,
Expand All @@ -573,7 +572,10 @@ where
FM::IoHandle: From<FM::FileHandle>,
N: Hash + Eq,
{
fn report_failure<'a>(&mut self, fail: &'a dyn Fail) -> BoxFuture<'a, ()> {
fn report_error<'a>(
&mut self,
fail: &'a (dyn Error + Sync + Send + 'static),
) -> BoxFuture<'a, ()> {
let fd = match self.file_desc(STDERR_FILENO) {
Some((fdes, perms)) if perms.writable() => fdes.clone(),
_ => return Box::pin(async {}),
Expand Down
53 changes: 18 additions & 35 deletions conch-runtime/src/error.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
//! A module defining the various kinds of errors that may arise
//! while executing commands.
#![allow(unused_qualifications)] // False positives with thiserror derive

use crate::io::Permissions;
use crate::Fd;
use failure::Fail;
use std::convert::From;
use std::error::Error;
use std::fmt::{self, Display, Formatter};
use std::io::Error as IoError;

Expand All @@ -18,7 +19,7 @@ use std::io::Error as IoError;
///
/// Ultimately it is up to the caller to decide how to handle fatal vs non-fatal
/// errors.
pub trait IsFatalError: Fail {
pub trait IsFatalError: 'static + Send + Sync + Error {
/// Checks whether the error should be considered a "fatal" error.
fn is_fatal(&self) -> bool;
}
Expand All @@ -30,19 +31,19 @@ impl IsFatalError for void::Void {
}

/// An error which may arise during parameter expansion.
#[derive(PartialEq, Eq, Clone, Debug, failure_derive::Fail)]
#[derive(PartialEq, Eq, Clone, Debug, thiserror::Error)]
pub enum ExpansionError {
/// Attempted to divide by zero in an arithmetic subsitution.
#[fail(display = "attempted to divide by zero")]
#[error("attempted to divide by zero")]
DivideByZero,
/// Attempted to raise to a negative power in an arithmetic subsitution.
#[fail(display = "attempted to raise to a negative power")]
#[error("attempted to raise to a negative power")]
NegativeExponent,
/// Attempted to assign a special parameter, e.g. `${!:-value}`.
#[fail(display = "{}: cannot assign in this way", _0)]
/// Attempted to assign a special parameter, e.g. `${!:=value}`.
#[error("{0}: cannot assign in this way")]
BadAssig(String),
/// Attempted to evaluate a null or unset parameter, i.e. `${var:?msg}`.
#[fail(display = "{}: {}", _0, _1)]
#[error("{0}: {1}")]
EmptyParameter(String /* var */, String /* msg */),
}

Expand All @@ -59,7 +60,7 @@ impl IsFatalError for ExpansionError {
}

/// An error which may arise during redirection.
#[derive(Debug, failure_derive::Fail)]
#[derive(Debug, thiserror::Error)]
pub enum RedirectionError {
/// A redirect path evaluated to multiple fields.
Ambiguous(Vec<String>),
Expand All @@ -70,7 +71,7 @@ pub enum RedirectionError {
BadFdPerms(Fd, Permissions /* new perms */),
/// Any I/O error returned by the OS during execution and the
/// file that caused the error if applicable.
Io(#[cause] IoError, Option<String>),
Io(#[source] IoError, Option<String>),
}

impl Eq for RedirectionError {}
Expand Down Expand Up @@ -137,15 +138,15 @@ impl IsFatalError for RedirectionError {
}

/// An error which may arise when spawning a command process.
#[derive(Debug, failure_derive::Fail)]
#[derive(Debug, thiserror::Error)]
pub enum CommandError {
/// Unable to find a command/function/builtin to execute.
NotFound(String),
/// Utility or script does not have executable permissions.
NotExecutable(String),
/// Any I/O error returned by the OS during execution and the
/// file that caused the error if applicable.
Io(#[cause] IoError, Option<String>),
Io(#[source] IoError, Option<String>),
}

impl Eq for CommandError {}
Expand Down Expand Up @@ -184,17 +185,17 @@ impl IsFatalError for CommandError {
}

/// An error which may arise while executing commands.
#[derive(Debug, failure_derive::Fail)]
#[derive(Debug, thiserror::Error)]
pub enum RuntimeError {
/// Any I/O error returned by the OS during execution and the
/// file that caused the error if applicable.
Io(#[cause] IoError, Option<String>),
Io(#[source] IoError, Option<String>),
/// Any error that occured during a parameter expansion.
Expansion(#[cause] ExpansionError),
Expansion(#[from] ExpansionError),
/// Any error that occured during a redirection.
Redirection(#[cause] RedirectionError),
Redirection(#[from] RedirectionError),
/// Any error that occured during a command spawning.
Command(#[cause] CommandError),
Command(#[from] CommandError),
/// Runtime feature not currently supported.
Unimplemented(&'static str),
}
Expand Down Expand Up @@ -245,24 +246,6 @@ impl From<IoError> for RuntimeError {
}
}

impl From<ExpansionError> for RuntimeError {
fn from(err: ExpansionError) -> Self {
RuntimeError::Expansion(err)
}
}

impl From<RedirectionError> for RuntimeError {
fn from(err: RedirectionError) -> Self {
RuntimeError::Redirection(err)
}
}

impl From<CommandError> for RuntimeError {
fn from(err: CommandError) -> Self {
RuntimeError::Command(err)
}
}

impl From<void::Void> for RuntimeError {
fn from(err: void::Void) -> Self {
void::unreachable(err)
Expand Down
4 changes: 2 additions & 2 deletions conch-runtime/src/eval/ast_impl/param_subst.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::env::{
AsyncIoEnvironment, FileDescEnvironment, FileDescOpener, IsInteractiveEnvironment,
LastStatusEnvironment, ReportFailureEnvironment, SubEnvironment, VariableEnvironment,
LastStatusEnvironment, ReportErrorEnvironment, SubEnvironment, VariableEnvironment,
};
use crate::error::{ExpansionError, IsFatalError};
use crate::eval::{
Expand Down Expand Up @@ -31,7 +31,7 @@ where
+ FileDescOpener
+ IsInteractiveEnvironment
+ LastStatusEnvironment
+ ReportFailureEnvironment
+ ReportErrorEnvironment
+ SubEnvironment
+ VariableEnvironment<VarName = W::EvalResult, Var = W::EvalResult>,
E::FileHandle: Send + From<E::OpenedFileHandle>,
Expand Down

0 comments on commit 5deb6af

Please sign in to comment.