From af005a7669053a1d0854e906201daee3b501f84a Mon Sep 17 00:00:00 2001 From: Nick Alexander Date: Wed, 27 Jun 2018 11:53:35 -0700 Subject: [PATCH] Convert query-algebrizer/ to AlgebrizerError. --- query-algebrizer/src/clauses/fulltext.rs | 11 ++-- query-algebrizer/src/clauses/ground.rs | 5 +- query-algebrizer/src/clauses/not.rs | 2 +- query-algebrizer/src/clauses/tx_log_api.rs | 17 +++--- query-algebrizer/src/errors.rs | 65 ++++++---------------- query-algebrizer/src/lib.rs | 1 - query-algebrizer/tests/ground.rs | 65 ++++------------------ query-algebrizer/tests/predicate.rs | 26 ++++----- query-algebrizer/tests/utils/mod.rs | 8 +-- 9 files changed, 57 insertions(+), 143 deletions(-) diff --git a/query-algebrizer/src/clauses/fulltext.rs b/query-algebrizer/src/clauses/fulltext.rs index 5e97fb52..6f5159f4 100644 --- a/query-algebrizer/src/clauses/fulltext.rs +++ b/query-algebrizer/src/clauses/fulltext.rs @@ -32,7 +32,6 @@ use clauses::{ use errors::{ AlgebrizerError, BindingError, - InvalidBinding, Result, }; @@ -59,12 +58,12 @@ impl ConjoiningClauses { if where_fn.binding.is_empty() { // The binding must introduce at least one bound variable. - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::NoBoundVariable)); + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable)); } if !where_fn.binding.is_valid() { // The binding must not duplicate bound variables. - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); } // We should have exactly four bindings. Destructure them now. @@ -72,7 +71,7 @@ impl ConjoiningClauses { Binding::BindRel(bindings) => { let bindings_count = bindings.len(); if bindings_count < 1 || bindings_count > 4 { - bail!(InvalidBinding::new(where_fn.operator.clone(), + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::InvalidNumberOfBindings { number: bindings.len(), expected: 4, @@ -83,7 +82,7 @@ impl ConjoiningClauses { }, Binding::BindScalar(_) | Binding::BindTuple(_) | - Binding::BindColl(_) => bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::ExpectedBindRel)), + Binding::BindColl(_) => bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::ExpectedBindRel)), }; let mut bindings = bindings.into_iter(); let b_entity = bindings.next().unwrap(); @@ -246,7 +245,7 @@ impl ConjoiningClauses { // We do not allow the score to be bound. if self.value_bindings.contains_key(var) || self.input_variables.contains(var) { - bail!(InvalidBinding::new(var.name(), BindingError::UnexpectedBinding)); + bail!(AlgebrizerError::InvalidBinding(var.name(), BindingError::UnexpectedBinding)); } // We bind the value ourselves. This handily takes care of substituting into existing uses. diff --git a/query-algebrizer/src/clauses/ground.rs b/query-algebrizer/src/clauses/ground.rs index 18f68d64..2e18ef7e 100644 --- a/query-algebrizer/src/clauses/ground.rs +++ b/query-algebrizer/src/clauses/ground.rs @@ -33,7 +33,6 @@ use clauses::convert::ValueConversion; use errors::{ AlgebrizerError, BindingError, - InvalidBinding, Result, }; @@ -125,12 +124,12 @@ impl ConjoiningClauses { if where_fn.binding.is_empty() { // The binding must introduce at least one bound variable. - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::NoBoundVariable)); + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable)); } if !where_fn.binding.is_valid() { // The binding must not duplicate bound variables. - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); } let schema = known.schema; diff --git a/query-algebrizer/src/clauses/not.rs b/query-algebrizer/src/clauses/not.rs index 05b88330..7330342f 100644 --- a/query-algebrizer/src/clauses/not.rs +++ b/query-algebrizer/src/clauses/not.rs @@ -553,7 +553,7 @@ mod testing { :where (not [?x :foo/knows ?y])]"#; let parsed = parse_find_string(query).expect("parse failed"); let err = algebrize(known, parsed).expect_err("algebrization should have failed"); - match err.downcast().expect("expected AlgebrizerError") { + match err { AlgebrizerError::UnboundVariable(var) => { assert_eq!(var, PlainSymbol("?x".to_string())); }, x => panic!("expected Unbound Variable error, got {:?}", x), } diff --git a/query-algebrizer/src/clauses/tx_log_api.rs b/query-algebrizer/src/clauses/tx_log_api.rs index d7dbb686..9c487be8 100644 --- a/query-algebrizer/src/clauses/tx_log_api.rs +++ b/query-algebrizer/src/clauses/tx_log_api.rs @@ -27,7 +27,6 @@ use clauses::{ use errors::{ AlgebrizerError, BindingError, - InvalidBinding, Result, }; @@ -66,12 +65,12 @@ impl ConjoiningClauses { if where_fn.binding.is_empty() { // The binding must introduce at least one bound variable. - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::NoBoundVariable)); + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable)); } if !where_fn.binding.is_valid() { // The binding must not duplicate bound variables. - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); } // We should have exactly one binding. Destructure it now. @@ -79,7 +78,7 @@ impl ConjoiningClauses { Binding::BindRel(bindings) => { let bindings_count = bindings.len(); if bindings_count != 1 { - bail!(InvalidBinding::new(where_fn.operator.clone(), + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::InvalidNumberOfBindings { number: bindings_count, expected: 1, @@ -93,7 +92,7 @@ impl ConjoiningClauses { Binding::BindColl(v) => v, Binding::BindScalar(_) | Binding::BindTuple(_) => { - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::ExpectedBindRelOrBindColl)) + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::ExpectedBindRelOrBindColl)) }, }; @@ -144,12 +143,12 @@ impl ConjoiningClauses { if where_fn.binding.is_empty() { // The binding must introduce at least one bound variable. - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::NoBoundVariable)); + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::NoBoundVariable)); } if !where_fn.binding.is_valid() { // The binding must not duplicate bound variables. - bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::RepeatedBoundVariable)); } // We should have at most five bindings. Destructure them now. @@ -157,7 +156,7 @@ impl ConjoiningClauses { Binding::BindRel(bindings) => { let bindings_count = bindings.len(); if bindings_count < 1 || bindings_count > 5 { - bail!(InvalidBinding::new(where_fn.operator.clone(), + bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::InvalidNumberOfBindings { number: bindings.len(), expected: 5, @@ -167,7 +166,7 @@ impl ConjoiningClauses { }, Binding::BindScalar(_) | Binding::BindTuple(_) | - Binding::BindColl(_) => bail!(InvalidBinding::new(where_fn.operator.clone(), BindingError::ExpectedBindRel)), + Binding::BindColl(_) => bail!(AlgebrizerError::InvalidBinding(where_fn.operator.clone(), BindingError::ExpectedBindRel)), }; let mut bindings = bindings.into_iter(); let b_e = bindings.next().unwrap_or(VariableOrPlaceholder::Placeholder); diff --git a/query-algebrizer/src/errors.rs b/query-algebrizer/src/errors.rs index bba74686..20f0979a 100644 --- a/query-algebrizer/src/errors.rs +++ b/query-algebrizer/src/errors.rs @@ -11,17 +11,9 @@ extern crate mentat_query; use std; // To refer to std::result::Result. -use std::fmt; -use std::fmt::Display; - -use failure::{ - Backtrace, - Context, - Error, - Fail, -}; use mentat_core::{ + EdnParseError, ValueType, ValueTypeSet, }; @@ -30,7 +22,7 @@ use self::mentat_query::{ PlainSymbol, }; -pub type Result = std::result::Result; +pub type Result = std::result::Result; #[macro_export] macro_rules! bail { @@ -39,44 +31,7 @@ macro_rules! bail { ) } -#[derive(Debug)] -pub struct InvalidBinding { - pub function: PlainSymbol, - pub inner: Context -} - -impl InvalidBinding { - pub fn new(function: PlainSymbol, inner: BindingError) -> InvalidBinding { - InvalidBinding { - function: function, - inner: Context::new(inner) - } - } -} - -impl Fail for InvalidBinding { - fn cause(&self) -> Option<&Fail> { - self.inner.cause() - } - - fn backtrace(&self) -> Option<&Backtrace> { - self.inner.backtrace() - } -} - -impl Display for InvalidBinding { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "invalid binding for {}: {:?}", self.function, self.inner) - } -} - -impl Display for BindingError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "BindingError: {:?}", self) - } -} - -#[derive(Clone, Debug, Eq, PartialEq, Fail)] +#[derive(Clone, Debug, Eq, PartialEq)] pub enum BindingError { NoBoundVariable, UnexpectedBinding, @@ -96,7 +51,7 @@ pub enum BindingError { InvalidNumberOfBindings { number: usize, expected: usize }, } -#[derive(Debug, Fail)] +#[derive(Clone, Debug, Eq, Fail, PartialEq)] pub enum AlgebrizerError { #[fail(display = "{} var {} is duplicated", _0, _1)] DuplicateVariableError(PlainSymbol, &'static str), @@ -144,4 +99,16 @@ pub enum AlgebrizerError { #[fail(display = "non-matching variables in 'not' clause")] NonMatchingVariablesInNotClause, + + #[fail(display = "binding error in {}: {:?}", _0, _1)] + InvalidBinding(PlainSymbol, BindingError), + + #[fail(display = "{}", _0)] + EdnParseError(#[cause] EdnParseError), +} + +impl From for AlgebrizerError { + fn from(error: EdnParseError) -> AlgebrizerError { + AlgebrizerError::EdnParseError(error) + } } diff --git a/query-algebrizer/src/lib.rs b/query-algebrizer/src/lib.rs index 0a137110..09e18bdc 100644 --- a/query-algebrizer/src/lib.rs +++ b/query-algebrizer/src/lib.rs @@ -51,7 +51,6 @@ pub use errors::{ AlgebrizerError, BindingError, Result, - InvalidBinding, }; pub use clauses::{ diff --git a/query-algebrizer/tests/ground.rs b/query-algebrizer/tests/ground.rs index d3e74faf..a78c9a09 100644 --- a/query-algebrizer/tests/ground.rs +++ b/query-algebrizer/tests/ground.rs @@ -33,7 +33,6 @@ use mentat_query_algebrizer::{ AlgebrizerError, BindingError, ComputedTable, - InvalidBinding, Known, QueryInputs, }; @@ -255,14 +254,8 @@ fn test_ground_coll_heterogeneous_types() { let q = r#"[:find ?x :where [?x _ ?v] [(ground [false 8.5]) [?v ...]]]"#; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let e = bails(known, &q); - match e.downcast().expect("proper error") { - AlgebrizerError::InvalidGroundConstant => { - }, - _ => { - panic!(); - }, - } + assert_eq!(bails(known, &q), + AlgebrizerError::InvalidGroundConstant); } #[test] @@ -270,14 +263,8 @@ fn test_ground_rel_heterogeneous_types() { let q = r#"[:find ?x :where [?x _ ?v] [(ground [[false] [5]]) [[?v]]]]"#; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let e = bails(known, &q); - match e.downcast().expect("proper error") { - AlgebrizerError::InvalidGroundConstant => { - }, - _ => { - panic!(); - }, - } + assert_eq!(bails(known, &q), + AlgebrizerError::InvalidGroundConstant); } #[test] @@ -285,15 +272,8 @@ fn test_ground_tuple_duplicate_vars() { let q = r#"[:find ?x :where [?x :foo/age ?v] [(ground [8 10]) [?x ?x]]]"#; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let e: InvalidBinding = bails(known, &q).downcast().expect("proper error"); - assert_eq!(e.function, PlainSymbol::plain("ground")); - match e.inner.get_context() { - &BindingError::RepeatedBoundVariable => { - }, - _ => { - panic!(); - }, - } + assert_eq!(bails(known, &q), + AlgebrizerError::InvalidBinding(PlainSymbol::plain("ground"), BindingError::RepeatedBoundVariable)); } #[test] @@ -301,15 +281,8 @@ fn test_ground_rel_duplicate_vars() { let q = r#"[:find ?x :where [?x :foo/age ?v] [(ground [[8 10]]) [[?x ?x]]]]"#; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let e: InvalidBinding = bails(known, &q).downcast().expect("expected InvalidBinding"); - assert_eq!(e.function, PlainSymbol::plain("ground")); - match e.inner.get_context() { - &BindingError::RepeatedBoundVariable => { - }, - _ => { - panic!(); - }, - } + assert_eq!(bails(known, &q), + AlgebrizerError::InvalidBinding(PlainSymbol::plain("ground"), BindingError::RepeatedBoundVariable)); } #[test] @@ -317,15 +290,8 @@ fn test_ground_nonexistent_variable_invalid() { let q = r#"[:find ?x ?e :where [?e _ ?x] (not [(ground 17) ?v])]"#; let schema = prepopulated_schema(); let known = Known::for_schema(&schema); - let e = bails(known, &q).downcast().expect("proper error"); - match e { - AlgebrizerError::UnboundVariable(PlainSymbol(v)) => { - assert_eq!(v, "?v".to_string()); - }, - _ => { - panic!(); - }, - } + assert_eq!(bails(known, &q), + AlgebrizerError::UnboundVariable(PlainSymbol::plain("?v"))); } #[test] @@ -341,13 +307,6 @@ fn test_unbound_input_variable_invalid() { let i = QueryInputs::new(types, BTreeMap::default()).expect("valid QueryInputs"); - let e = bails_with_inputs(known, &q, i).downcast().expect("proper error"); - match e { - AlgebrizerError::UnboundVariable(v) => { - assert_eq!(v.0, "?x"); - }, - _ => { - panic!(); - }, - } + assert_eq!(bails_with_inputs(known, &q, i), + AlgebrizerError::UnboundVariable(PlainSymbol::plain("?x"))); } diff --git a/query-algebrizer/tests/predicate.rs b/query-algebrizer/tests/predicate.rs index 7a2e4591..8724b145 100644 --- a/query-algebrizer/tests/predicate.rs +++ b/query-algebrizer/tests/predicate.rs @@ -78,27 +78,21 @@ fn test_instant_predicates_require_instants() { :where [?e :foo/date ?t] [(> ?t "2017-06-16T00:56:41.257Z")]]"#; - match bails(known, query).downcast().expect("proper cause") { - AlgebrizerError::InvalidArgumentType(op, why, idx) => { - assert_eq!(op, PlainSymbol::plain(">")); - assert_eq!(why, ValueTypeSet::of_numeric_and_instant_types()); - assert_eq!(idx, 1); - }, - _ => panic!("Expected InvalidArgument."), - } + assert_eq!(bails(known, query), + AlgebrizerError::InvalidArgumentType( + PlainSymbol::plain(">"), + ValueTypeSet::of_numeric_and_instant_types(), + 1)); let query = r#"[:find ?e :where [?e :foo/date ?t] [(> "2017-06-16T00:56:41.257Z", ?t)]]"#; - match bails(known, query).downcast().expect("proper cause") { - AlgebrizerError::InvalidArgumentType(op, why, idx) => { - assert_eq!(op, PlainSymbol::plain(">")); - assert_eq!(why, ValueTypeSet::of_numeric_and_instant_types()); - assert_eq!(idx, 0); // We get this right. - }, - _ => panic!("Expected InvalidArgument."), - } + assert_eq!(bails(known, query), + AlgebrizerError::InvalidArgumentType( + PlainSymbol::plain(">"), + ValueTypeSet::of_numeric_and_instant_types(), + 0)); // We get this right. // You can try using a number, which is valid input to a numeric predicate. // In this store and query, though, that means we expect `?t` to be both diff --git a/query-algebrizer/tests/utils/mod.rs b/query-algebrizer/tests/utils/mod.rs index 88bae48c..fb4e5c4f 100644 --- a/query-algebrizer/tests/utils/mod.rs +++ b/query-algebrizer/tests/utils/mod.rs @@ -13,9 +13,6 @@ // this module will get warnings otherwise). #![allow(dead_code)] -extern crate failure; -use self::failure::Error; - use mentat_core::{ Attribute, Entid, @@ -28,6 +25,7 @@ use mentat_query::{ }; use mentat_query_algebrizer::{ + AlgebrizerError, ConjoiningClauses, Known, QueryInputs, @@ -83,12 +81,12 @@ impl SchemaBuilder { } } -pub fn bails(known: Known, input: &str) -> Error { +pub fn bails(known: Known, input: &str) -> AlgebrizerError { let parsed = parse_find_string(input).expect("query input to have parsed"); algebrize(known, parsed).expect_err("algebrize to have failed") } -pub fn bails_with_inputs(known: Known, input: &str, inputs: QueryInputs) -> Error { +pub fn bails_with_inputs(known: Known, input: &str, inputs: QueryInputs) -> AlgebrizerError { let parsed = parse_find_string(input).expect("query input to have parsed"); algebrize_with_inputs(known, parsed, 0, inputs).expect_err("algebrize to have failed") }