greg/mentat
1
0
Fork 0
Code Issues 294 Pull requests Projects Releases 9 Packages Wiki Activity

Review comment: Make or be or_exactly.

Browse source 
I baked the eof checking directly into the parser, rather than using
the skip and eof parsers.  I also took the time to restore some tests
that were mistakenly commented out.
This commit is contained in:
Nick Alexander 2017-04-03 11:48:01 -07:00
parent f8e75d817e
commit 0165a842ef
3 changed files with 126 additions and 128 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

41
parser-utils/src/value_and_span.rs
View file

@ -22,7 +22,6 @@ use combine::{
Parser,
ParseResult,
StreamOnce,
eof,
many,
many1,
parser,
@ -35,10 +34,8 @@ use combine::primitives::{
FastResult,
};
use combine::combinator::{
Eof,
Expected,
FnParser,
Skip,
};
use edn;
@ -165,9 +162,9 @@ impl Item for edn::ValueAndSpan {
}
}
/// `Of` and `of` allow us to express nested parsers naturally.
/// `OfExactly` and `of_exactly` allow us to express nested parsers naturally.
///
/// For example, `vector().of(many(list()))` parses a vector-of-lists, like [(1 2) (:a :b) ("test") ()].
/// For example, `vector().of_exactly(many(list()))` parses a vector-of-lists, like [(1 2) (:a :b) ("test") ()].
///
/// The "outer" parser `P` and the "nested" parser `N` must be compatible: `P` must produce an
/// output `edn::ValueAndSpan` which can itself be turned into a stream of child elements; and `N`
@ -175,9 +172,9 @@ impl Item for edn::ValueAndSpan {
/// nested parser to the outer parser, which is part of what has made parsing `&'a [edn::Value]`
/// difficult.
#[derive(Clone)]
pub struct Of<P, N>(P, N);
pub struct OfExactly<P, N>(P, N);
impl<P, N, O> Parser for Of<P, N>
impl<P, N, O> Parser for OfExactly<P, N>
where P: Parser<Input=Stream, Output=edn::ValueAndSpan>,
N: Parser<Input=Stream, Output=O>,
{
@ -190,7 +187,12 @@ impl<P, N, O> Parser for Of<P, N>
match self.0.parse_lazy(input) {
ConsumedOk((outer_value, outer_input)) => {
match self.1.parse_lazy(outer_value.into_child_stream()) {
ConsumedOk((inner_value, _)) | EmptyOk((inner_value, _)) => ConsumedOk((inner_value, outer_input)),
ConsumedOk((inner_value, mut inner_input)) | EmptyOk((inner_value, mut inner_input)) => {
match inner_input.uncons() {
Err(ref err) if *err == primitives::Error::end_of_input() => ConsumedOk((inner_value, outer_input)),
_ => EmptyErr(ParseError::empty(inner_input.position())),
}
},
// TODO: Improve the error output to reference the nested value (or span) in
// some way. This seems surprisingly difficult to do, so we just surface the
// inner error message right now. See also the comment below.
@ -199,7 +201,12 @@ impl<P, N, O> Parser for Of<P, N>
},
EmptyOk((outer_value, outer_input)) => {
match self.1.parse_lazy(outer_value.into_child_stream()) {
ConsumedOk((inner_value, _)) | EmptyOk((inner_value, _)) => EmptyOk((inner_value, outer_input)),
ConsumedOk((inner_value, mut inner_input)) | EmptyOk((inner_value, mut inner_input)) => {
match inner_input.uncons() {
Err(ref err) if *err == primitives::Error::end_of_input() => EmptyOk((inner_value, outer_input)),
_ => EmptyErr(ParseError::empty(inner_input.position())),
}
},
// TODO: Improve the error output. See the comment above.
EmptyErr(e) | ConsumedErr(e) => EmptyErr(e),
}
@ -215,27 +222,27 @@ impl<P, N, O> Parser for Of<P, N>
}
#[inline(always)]
pub fn of<P, N, O>(p: P, n: N) -> Of<P, Skip<N, Eof<Stream>>>
pub fn of_exactly<P, N, O>(p: P, n: N) -> OfExactly<P, N>
where P: Parser<Input=Stream, Output=edn::ValueAndSpan>,
N: Parser<Input=Stream, Output=O>,
{
Of(p, n.skip(eof()))
OfExactly(p, n)
}
/// We need a trait to define `Parser.of` and have it live outside of the `combine` crate.
pub trait OfParsing: Parser + Sized {
fn of<N, O>(self, n: N) -> Of<Self, Skip<N, Eof<Self::Input>>>
pub trait OfExactlyParsing: Parser + Sized {
fn of_exactly<N, O>(self, n: N) -> OfExactly<Self, N>
where Self: Sized,
N: Parser<Input = Self::Input, Output=O>;
}
impl<P> OfParsing for P
impl<P> OfExactlyParsing for P
where P: Parser<Input=Stream, Output=edn::ValueAndSpan>
{
fn of<N, O>(self, n: N) -> Of<P, Skip<N, Eof<Self::Input>>>
fn of_exactly<N, O>(self, n: N) -> OfExactly<P, N>
where N: Parser<Input = Self::Input, Output=O>
{
of(self, n)
of_exactly(self, n)
}
}
@ -311,7 +318,7 @@ fn keyword_map_(input: Stream) -> ParseResult<edn::ValueAndSpan, Stream>
}
}));
let mut runs = vector().of(many::<Vec<_>, _>(run));
let mut runs = vector().of_exactly(many::<Vec<_>, _>(run));
let (data, input) = try!(runs.parse_lazy(input).into());

34
query-parser/src/parse.rs
View file

@ -26,7 +26,7 @@ use self::mentat_parser_utils::{
use self::mentat_parser_utils::value_and_span::Stream as ValueStream;
use self::mentat_parser_utils::value_and_span::{
Item,
OfParsing,
OfExactlyParsing,
keyword_map,
list,
map,
@ -162,7 +162,7 @@ def_parser!(Where, or_join, edn::ValueAndSpan, {
def_parser!(Where, rule_vars, Vec<Variable>, {
seq()
.of(many1(Query::variable()))
.of_exactly(many1(Query::variable()))
});
def_parser!(Where, or_pattern_clause, OrWhereClause, {
@ -171,7 +171,7 @@ def_parser!(Where, or_pattern_clause, OrWhereClause, {
def_parser!(Where, or_and_clause, OrWhereClause, {
seq()
.of(Where::and()
.of_exactly(Where::and()
.with(many1(Where::clause()))
.map(OrWhereClause::And))
});
@ -182,7 +182,7 @@ def_parser!(Where, or_where_clause, OrWhereClause, {
def_parser!(Where, or_clause, WhereClause, {
seq()
.of(Where::or()
.of_exactly(Where::or()
.with(many1(Where::or_where_clause()))
.map(|clauses| {
WhereClause::OrJoin(
@ -195,7 +195,7 @@ def_parser!(Where, or_clause, WhereClause, {
def_parser!(Where, or_join_clause, WhereClause, {
seq()
.of(Where::or_join()
.of_exactly(Where::or_join()
.with(Where::rule_vars())
.and(many1(Where::or_where_clause()))
.map(|(vars, clauses)| {
@ -212,8 +212,8 @@ def_parser!(Where, pred, WhereClause, {
// Accept either a nested list or a nested vector here:
// `[(foo ?x ?y)]` or `[[foo ?x ?y]]`
vector()
.of(seq()
.of((Query::predicate_fn(), Query::arguments())
.of_exactly(seq()
.of_exactly((Query::predicate_fn(), Query::arguments())
.map(|(f, args)| {
WhereClause::Pred(
Predicate {
@ -225,7 +225,7 @@ def_parser!(Where, pred, WhereClause, {
def_parser!(Where, pattern, WhereClause, {
vector()
.of(
.of_exactly(
// While *technically* Datomic allows you to have a query like:
// [:find … :where [[?x]]]
// We don't -- we require at least e, a.
@ -313,7 +313,7 @@ def_parser!(Find, find_scalar, FindSpec, {
def_parser!(Find, find_coll, FindSpec, {
vector()
.of(Query::variable()
.of_exactly(Query::variable()
.map(|var| FindSpec::FindColl(Element::Variable(var)))
.skip(Find::ellipsis()))
});
@ -328,7 +328,7 @@ def_parser!(Find, find_rel, FindSpec, {
def_parser!(Find, find_tuple, FindSpec, {
vector()
.of(Find::elements().map(FindSpec::FindTuple))
.of_exactly(Find::elements().map(FindSpec::FindTuple))
});
/// Parse a stream values into one of four find specs.
@ -395,16 +395,16 @@ enum FindQueryPart {
/// construct a `FindQuery` from them.
def_parser!(Find, query, FindQuery, {
let p_find_spec = Find::literal_find()
.with(vector().of(Find::spec().map(FindQueryPart::FindSpec)));
.with(vector().of_exactly(Find::spec().map(FindQueryPart::FindSpec)));
let p_with_vars = Find::literal_with()
.with(vector().of(many(Query::variable()).map(FindQueryPart::With)));
.with(vector().of_exactly(many(Query::variable()).map(FindQueryPart::With)));
let p_where_clauses = Find::literal_where()
.with(vector().of(Where::clauses().map(FindQueryPart::WhereClauses))).expected(":where clauses");
.with(vector().of_exactly(Where::clauses().map(FindQueryPart::WhereClauses))).expected(":where clauses");
(or(map(), keyword_map()))
.of(many(choice::<[&mut Parser<Input = ValueStream, Output = FindQueryPart>; 3], _>([
.of_exactly(many(choice::<[&mut Parser<Input = ValueStream, Output = FindQueryPart>; 3], _>([
&mut try(p_find_spec),
&mut try(p_with_vars),
&mut try(p_where_clauses),
@ -619,7 +619,7 @@ mod test {
fn test_find_sp_variable() {
let sym = edn::PlainSymbol::new("?x");
let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(sym.clone())]);
assert_parses_to!(|| vector().of(Query::variable()), input, variable(sym));
assert_parses_to!(|| vector().of_exactly(Query::variable()), input, variable(sym));
}
#[test]
@ -627,7 +627,7 @@ mod test {
let sym = edn::PlainSymbol::new("?x");
let period = edn::PlainSymbol::new(".");
let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(sym.clone()), edn::Value::PlainSymbol(period.clone())]);
assert_parses_to!(|| vector().of(Find::find_scalar()),
assert_parses_to!(|| vector().of_exactly(Find::find_scalar()),
input,
FindSpec::FindScalar(Element::Variable(variable(sym))));
}
@ -648,7 +648,7 @@ mod test {
let vx = edn::PlainSymbol::new("?x");
let vy = edn::PlainSymbol::new("?y");
let input = edn::Value::Vector(vec![edn::Value::PlainSymbol(vx.clone()), edn::Value::PlainSymbol(vy.clone())]);
assert_parses_to!(|| vector().of(Find::find_rel()),
assert_parses_to!(|| vector().of_exactly(Find::find_rel()),
input,
FindSpec::FindRel(vec![Element::Variable(variable(vx)),
Element::Variable(variable(vy))]));

179
tx-parser/src/lib.rs
View file

@ -45,7 +45,7 @@ use mentat_tx::entities::{
use mentat_parser_utils::{ResultParser};
use mentat_parser_utils::value_and_span::{
Item,
OfParsing,
OfExactlyParsing,
integer,
list,
map,
@ -66,8 +66,8 @@ def_parser!(Tx, entid, Entid, {
});
def_parser!(Tx, lookup_ref, LookupRef, {
list()
.of(value(edn::Value::PlainSymbol(PlainSymbol::new("lookup-ref")))
list().of_exactly(
value(edn::Value::PlainSymbol(PlainSymbol::new("lookup-ref")))
.with((Tx::entid(),
Tx::atom()))
.map(|(a, v)| LookupRef { a: a, v: v.without_spans() }))
@ -88,7 +88,7 @@ def_parser!(Tx, atom, edn::ValueAndSpan, {
});
def_parser!(Tx, nested_vector, Vec<AtomOrLookupRefOrVectorOrMapNotation>, {
vector().of(many(Tx::atom_or_lookup_ref_or_vector()))
vector().of_exactly(many(Tx::atom_or_lookup_ref_or_vector()))
});
def_parser!(Tx, atom_or_lookup_ref_or_vector, AtomOrLookupRefOrVectorOrMapNotation, {
@ -116,12 +116,12 @@ def_parser!(Tx, add_or_retract, Entity, {
}
});
vector().of(p)
vector().of_exactly(p)
});
def_parser!(Tx, map_notation, MapNotation, {
map()
.of(many((Tx::entid(), Tx::atom_or_lookup_ref_or_vector())))
.of_exactly(many((Tx::entid(), Tx::atom_or_lookup_ref_or_vector())))
.map(|avs: Vec<(Entid, AtomOrLookupRefOrVectorOrMapNotation)>| -> MapNotation {
avs.into_iter().collect()
})
@ -133,7 +133,7 @@ def_parser!(Tx, entity, Entity, {
});
def_parser!(Tx, entities, Vec<Entity>, {
vector().of(many(Tx::entity()))
vector().of_exactly(many(Tx::entity()))
});
impl Tx {
@ -185,7 +185,9 @@ pub fn remove_db_id(map: &mut MapNotation) -> std::result::Result<Option<EntidOr
#[cfg(test)]
mod tests {
use super::*;
use std::collections::LinkedList;
use std::collections::BTreeMap;
use combine::Parser;
use edn::symbols::NamespacedKeyword;
use edn::{
@ -198,11 +200,9 @@ mod tests {
Entid,
EntidOrLookupRefOrTempId,
Entity,
LookupRef,
OpType,
AtomOrLookupRefOrVectorOrMapNotation,
};
use std::collections::BTreeMap;
fn kw(namespace: &str, name: &str) -> Value {
Value::NamespacedKeyword(NamespacedKeyword::new(namespace, name))
@ -213,9 +213,9 @@ mod tests {
let input = Value::Vector(vec![kw("db", "add"),
kw("test", "entid"),
kw("test", "a"),
Value::Text("v".into())]).with_spans();
Value::Text("v".into())]);
let mut parser = Tx::entity();
let result = parser.parse(input.into_atom_stream()).map(|x| x.0);
let result = parser.parse(input.with_spans().into_atom_stream()).map(|x| x.0);
assert_eq!(result,
Ok(Entity::AddOrRetract {
op: OpType::Add,
@ -226,91 +226,82 @@ mod tests {
}));
}
// #[test]
// fn test_retract() {
// let input = [Value::Vector(vec![kw("db", "retract"),
// Value::Integer(101),
// kw("test", "a"),
// Value::Text("v".into())])];
// let mut parser = Tx::entity();
// let result = parser.parse(&input[..]);
// assert_eq!(result,
// Ok((Entity::AddOrRetract {
// op: OpType::Retract,
// e: EntidOrLookupRefOrTempId::Entid(Entid::Entid(101)),
// a: Entid::Ident(NamespacedKeyword::new("test", "a")),
// v: AtomOrLookupRefOrVectorOrMapNotation::Atom(Value::Text("v".into())),
// },
// &[][..])));
// }
#[test]
fn test_retract() {
let input = Value::Vector(vec![kw("db", "retract"),
Value::Integer(101),
kw("test", "a"),
Value::Text("v".into())]);
let mut parser = Tx::entity();
let result = parser.parse(input.with_spans().into_atom_stream()).map(|x| x.0);
assert_eq!(result,
Ok(Entity::AddOrRetract {
op: OpType::Retract,
e: EntidOrLookupRefOrTempId::Entid(Entid::Entid(101)),
a: Entid::Ident(NamespacedKeyword::new("test", "a")),
v: AtomOrLookupRefOrVectorOrMapNotation::Atom(ValueAndSpan::new(SpannedValue::Text("v".into()), Span(25, 28))),
}));
}
// #[test]
// fn test_lookup_ref() {
// let mut list = LinkedList::new();
// list.push_back(Value::PlainSymbol(PlainSymbol::new("lookup-ref")));
// list.push_back(kw("test", "a1"));
// list.push_back(Value::Text("v1".into()));
#[test]
fn test_lookup_ref() {
let input = Value::Vector(vec![kw("db", "add"),
Value::List(vec![Value::PlainSymbol(PlainSymbol::new("lookup-ref")),
kw("test", "a1"),
Value::Text("v1".into())].into_iter().collect()),
kw("test", "a"),
Value::Text("v".into())]);
let mut parser = Tx::entity();
let result = parser.parse(input.with_spans().into_atom_stream()).map(|x| x.0);
assert_eq!(result,
Ok(Entity::AddOrRetract {
op: OpType::Add,
e: EntidOrLookupRefOrTempId::LookupRef(LookupRef {
a: Entid::Ident(NamespacedKeyword::new("test", "a1")),
v: Value::Text("v1".into()),
}),
a: Entid::Ident(NamespacedKeyword::new("test", "a")),
v: AtomOrLookupRefOrVectorOrMapNotation::Atom(ValueAndSpan::new(SpannedValue::Text("v".into()), Span(44, 47))),
}));
}
// let input = [Value::Vector(vec![kw("db", "add"),
// Value::List(list),
// kw("test", "a"),
// Value::Text("v".into())])];
// let mut parser = Tx::entity();
// let result = parser.parse(&input[..]);
// assert_eq!(result,
// Ok((Entity::AddOrRetract {
// op: OpType::Add,
// e: EntidOrLookupRefOrTempId::LookupRef(LookupRef {
// a: Entid::Ident(NamespacedKeyword::new("test", "a1")),
// v: Value::Text("v1".into()),
// }),
// a: Entid::Ident(NamespacedKeyword::new("test", "a")),
// v: AtomOrLookupRefOrVectorOrMapNotation::Atom(Value::Text("v".into())),
// },
// &[][..])));
// }
#[test]
fn test_nested_vector() {
let input = Value::Vector(vec![kw("db", "add"),
Value::List(vec![Value::PlainSymbol(PlainSymbol::new("lookup-ref")),
kw("test", "a1"),
Value::Text("v1".into())].into_iter().collect()),
kw("test", "a"),
Value::Vector(vec![Value::Text("v1".into()), Value::Text("v2".into())])]);
let mut parser = Tx::entity();
let result = parser.parse(input.with_spans().into_atom_stream()).map(|x| x.0);
assert_eq!(result,
Ok(Entity::AddOrRetract {
op: OpType::Add,
e: EntidOrLookupRefOrTempId::LookupRef(LookupRef {
a: Entid::Ident(NamespacedKeyword::new("test", "a1")),
v: Value::Text("v1".into()),
}),
a: Entid::Ident(NamespacedKeyword::new("test", "a")),
v: AtomOrLookupRefOrVectorOrMapNotation::Vector(vec![AtomOrLookupRefOrVectorOrMapNotation::Atom(ValueAndSpan::new(SpannedValue::Text("v1".into()), Span(45, 49))),
AtomOrLookupRefOrVectorOrMapNotation::Atom(ValueAndSpan::new(SpannedValue::Text("v2".into()), Span(50, 54)))]),
}));
}
// #[test]
// fn test_nested_vector() {
// let mut list = LinkedList::new();
// list.push_back(Value::PlainSymbol(PlainSymbol::new("lookup-ref")));
// list.push_back(kw("test", "a1"));
// list.push_back(Value::Text("v1".into()));
#[test]
fn test_map_notation() {
let mut expected: MapNotation = BTreeMap::default();
expected.insert(Entid::Ident(NamespacedKeyword::new("db", "id")), AtomOrLookupRefOrVectorOrMapNotation::Atom(ValueAndSpan::new(SpannedValue::Text("t".to_string()), Span(8, 11))));
expected.insert(Entid::Ident(NamespacedKeyword::new("db", "ident")), AtomOrLookupRefOrVectorOrMapNotation::Atom(ValueAndSpan::new(SpannedValue::NamespacedKeyword(NamespacedKeyword::new("test", "attribute")), Span(22, 37))));
// let input = [Value::Vector(vec![kw("db", "add"),
// Value::List(list),
// kw("test", "a"),
// Value::Vector(vec![Value::Text("v1".into()), Value::Text("v2".into())])])];
// let mut parser = Tx::entity();
// let result = parser.parse(&input[..]);
// assert_eq!(result,
// Ok((Entity::AddOrRetract {
// op: OpType::Add,
// e: EntidOrLookupRefOrTempId::LookupRef(LookupRef {
// a: Entid::Ident(NamespacedKeyword::new("test", "a1")),
// v: Value::Text("v1".into()),
// }),
// a: Entid::Ident(NamespacedKeyword::new("test", "a")),
// v: AtomOrLookupRefOrVectorOrMapNotation::Vector(vec![AtomOrLookupRefOrVectorOrMapNotation::Atom(Value::Text("v1".into())),
// AtomOrLookupRefOrVectorOrMapNotation::Atom(Value::Text("v2".into()))]),
// },
// &[][..])));
// }
let mut map: BTreeMap<Value, Value> = BTreeMap::default();
map.insert(kw("db", "id"), Value::Text("t".to_string()));
map.insert(kw("db", "ident"), kw("test", "attribute"));
let input = Value::Map(map.clone());
// #[test]
// fn test_map_notation() {
// let mut expected: MapNotation = BTreeMap::default();
// expected.insert(Entid::Ident(NamespacedKeyword::new("db", "id")), AtomOrLookupRefOrVectorOrMapNotation::Atom(Value::Text("t".to_string())));
// expected.insert(Entid::Ident(NamespacedKeyword::new("db", "ident")), AtomOrLookupRefOrVectorOrMapNotation::Atom(kw("test", "attribute")));
// let mut map: BTreeMap<Value, Value> = BTreeMap::default();
// map.insert(kw("db", "id"), Value::Text("t".to_string()));
// map.insert(kw("db", "ident"), kw("test", "attribute"));
// let input = [Value::Map(map.clone())];
// let mut parser = Tx::entity();
// let result = parser.parse(&input[..]);
// assert_eq!(result,
// Ok((Entity::MapNotation(expected),
// &[][..])));
// }
let mut parser = Tx::entity();
let result = parser.parse(input.with_spans().into_atom_stream()).map(|x| x.0);
assert_eq!(result,
Ok(Entity::MapNotation(expected)));
}
}