Review comment: Extract and use def_matches_* macros.

parser-utils/src/value_and_span.rs

@@ -352,6 +352,59 @@ pub fn keyword_map() -> Expected<FnParser<Stream, fn(Stream) -> ParseResult<edn:
     parser(keyword_map_ as fn(Stream) -> ParseResult<edn::ValueAndSpan, Stream>).expected("keyword map")
 }
 
+/// Generate a `satisfy` expression that matches a `PlainSymbol` value with the given name.
+///
+/// We do this rather than using `combine::token` so that we don't need to allocate a new `String`
+/// inside a `PlainSymbol` inside a `SpannedValue` inside a `ValueAndSpan` just to match input.
+#[macro_export]
+macro_rules! def_matches_plain_symbol {
+    ( $parser: ident, $name: ident, $input: expr ) => {
+        def_parser!($parser, $name, edn::ValueAndSpan, {
+            satisfy(|v: edn::ValueAndSpan| {
+                match v.inner {
+                    edn::SpannedValue::PlainSymbol(ref s) => s.0.as_str() == $input,
+                    _ => false,
+                }
+            })
+        });
+    }
+}
+
+/// Generate a `satisfy` expression that matches a `Keyword` value with the given name.
+///
+/// We do this rather than using `combine::token` to save allocations.
+#[macro_export]
+macro_rules! def_matches_keyword {
+    ( $parser: ident, $name: ident, $input: expr ) => {
+        def_parser!($parser, $name, edn::ValueAndSpan, {
+            satisfy(|v: edn::ValueAndSpan| {
+                match v.inner {
+                    edn::SpannedValue::Keyword(ref s) => s.0.as_str() == $input,
+                    _ => false,
+                }
+            })
+        });
+    }
+}
+
+/// Generate a `satisfy` expression that matches a `NamespacedKeyword` value with the given
+/// namespace and name.
+///
+/// We do this rather than using `combine::token` to save allocations.
+#[macro_export]
+macro_rules! def_matches_namespaced_keyword {
+    ( $parser: ident, $name: ident, $input_namespace: expr, $input_name: expr ) => {
+        def_parser!($parser, $name, edn::ValueAndSpan, {
+            satisfy(|v: edn::ValueAndSpan| {
+                match v.inner {
+                    edn::SpannedValue::NamespacedKeyword(ref s) => s.namespace.as_str() == $input_namespace && s.name.as_str() == $input_name,
+                    _ => false,
+                }
+            })
+        });
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use combine::{eof};

query-parser/src/parse.rs

@@ -283,26 +283,9 @@ def_parser!(Where, clauses, Vec<WhereClause>, {
 
 pub struct Find;
 
-/// TODO: extract macro for matching these `PlainSymbol` instances.
+def_matches_plain_symbol!(Find, period, ".");
-def_parser!(Find, period, edn::ValueAndSpan, {
-    satisfy(|v: edn::ValueAndSpan| {
-        if let edn::SpannedValue::PlainSymbol(ref s) = v.inner {
-            s.0.as_str() == "."
-        } else {
-            false
-        }
-    })
-});
 
-def_parser!(Find, ellipsis, edn::ValueAndSpan, {
+def_matches_plain_symbol!(Find, ellipsis, "...");
-    satisfy(|v: edn::ValueAndSpan| {
-        if let edn::SpannedValue::PlainSymbol(ref s) = v.inner {
-            s.0.as_str() == "..."
-        } else {
-            false
-        }
-    })
-});
 
 def_parser!(Find, find_scalar, FindSpec, {
     Query::variable()
@@ -352,36 +335,11 @@ def_parser!(Find, spec, FindSpec, {
           &mut try(Find::find_rel())])
 });
 
-/// TODO: extract macro for matching these `Keyword` instances.
+def_matches_keyword!(Find, literal_find, "find");
-def_parser!(Find, literal_find, edn::ValueAndSpan, {
-    satisfy(|v: edn::ValueAndSpan| {
-        if let edn::SpannedValue::Keyword(ref s) = v.inner {
-            s.0.as_str() == "find"
-        } else {
-            false
-        }
-    })
-});
 
-def_parser!(Find, literal_with, edn::ValueAndSpan, {
+def_matches_keyword!(Find, literal_with, "with");
-    satisfy(|v: edn::ValueAndSpan| {
-        if let edn::SpannedValue::Keyword(ref s) = v.inner {
-            s.0.as_str() == "with"
-        } else {
-            false
-        }
-    })
-});
 
-def_parser!(Find, literal_where, edn::ValueAndSpan, {
+def_matches_keyword!(Find, literal_where, "where");
-    satisfy(|v: edn::ValueAndSpan| {
-        if let edn::SpannedValue::Keyword(ref s) = v.inner {
-            s.0.as_str() == "where"
-        } else {
-            false
-        }
-    })
-});
 
 /// Express something close to a builder pattern for a `FindQuery`.
 enum FindQueryPart {

tx-parser/src/lib.rs

@@ -24,14 +24,11 @@ use combine::{
     eof,
     many,
     parser,
+    satisfy,
     satisfy_map,
     Parser,
     ParseResult,
 };
-use edn::symbols::{
-    NamespacedKeyword,
-    PlainSymbol,
-};
 use mentat_tx::entities::{
     AtomOrLookupRefOrVectorOrMapNotation,
     Entid,
@@ -50,7 +47,6 @@ use mentat_parser_utils::value_and_span::{
     list,
     map,
     namespaced_keyword,
-    value,
     vector,
 };
 
@@ -65,9 +61,11 @@ def_parser!(Tx, entid, Entid, {
         .or(namespaced_keyword().map(|x| Entid::Ident(x)))
 });
 
+def_matches_plain_symbol!(Tx, literal_lookup_ref, "lookup-ref");
+
 def_parser!(Tx, lookup_ref, LookupRef, {
     list().of_exactly(
-        value(edn::Value::PlainSymbol(PlainSymbol::new("lookup-ref")))
+        Tx::literal_lookup_ref()
             .with((Tx::entid(),
                    Tx::atom()))
             .map(|(a, v)| LookupRef { a: a, v: v.without_spans() }))
@@ -84,7 +82,7 @@ def_parser!(Tx, temp_id, TempId, {
 });
 
 def_parser!(Tx, atom, edn::ValueAndSpan, {
-    satisfy_map(|x: edn::ValueAndSpan| x.into_atom().map(|v| v))
+    satisfy_map(|x: edn::ValueAndSpan| x.into_atom())
 });
 
 def_parser!(Tx, nested_vector, Vec<AtomOrLookupRefOrVectorOrMapNotation>, {
@@ -98,12 +96,13 @@ def_parser!(Tx, atom_or_lookup_ref_or_vector, AtomOrLookupRefOrVectorOrMapNotati
         .or(Tx::atom().map(AtomOrLookupRefOrVectorOrMapNotation::Atom))
 });
 
+def_matches_namespaced_keyword!(Tx, literal_db_add, "db", "add");
+
+def_matches_namespaced_keyword!(Tx, literal_db_retract, "db", "retract");
+
 def_parser!(Tx, add_or_retract, Entity, {
-    let add = value(edn::Value::NamespacedKeyword(NamespacedKeyword::new("db", "add")))
+    vector().of_exactly(
-        .map(|_| OpType::Add);
+        (Tx::literal_db_add().map(|_| OpType::Add).or(Tx::literal_db_retract().map(|_| OpType::Retract)),
-    let retract = value(edn::Value::NamespacedKeyword(NamespacedKeyword::new("db", "retract")))
-        .map(|_| OpType::Retract);
-    let p = (add.or(retract),
              Tx::entid_or_lookup_ref_or_temp_id(),
              Tx::entid(),
              Tx::atom_or_lookup_ref_or_vector())
@@ -114,9 +113,7 @@ def_parser!(Tx, add_or_retract, Entity, {
                 a: a,
                 v: v,
             }
-        });
+        }))
-
-    vector().of_exactly(p)
 });
 
 def_parser!(Tx, map_notation, MapNotation, {
@@ -189,8 +186,9 @@ mod tests {
     use std::collections::BTreeMap;
 
     use combine::Parser;
-    use edn::symbols::NamespacedKeyword;
     use edn::{
+        NamespacedKeyword,
+        PlainSymbol,
         Span,
         SpannedValue,
         Value,