Review comments: another test, add unreversed().

db/src/db.rs

@@ -2170,6 +2170,27 @@ mod tests {
                         "{\"s\" 65542
                           \"t\" 65543}");
 
+        // Check that we can use the same attribute in both forward and backward form in the same
+        // transaction.
+        let report = assert_transact!(conn, "[[:db/add 888 :test/dangling 889]
+                                              [:db/add 888 :test/_dangling 889]]");
+        assert_matches!(conn.last_transaction(),
+                        "[[888 :test/dangling 889 ?tx true]
+                          [889 :test/dangling 888 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(tempids(&report),
+                        "{}");
+
+        // Check that we can use the same attribute in both forward and backward form in the same
+        // transaction in map notation.
+        let report = assert_transact!(conn, "[{:db/id 998 :test/dangling 999 :test/_dangling 999}]");
+        assert_matches!(conn.last_transaction(),
+                        "[[998 :test/dangling 999 ?tx true]
+                          [999 :test/dangling 998 ?tx true]
+                          [?tx :db/txInstant ?ms ?tx true]]");
+        assert_matches!(tempids(&report),
+                        "{}");
+
         // Verify that we can't explode direct reverse notation with nested value maps.
         assert_transact!(conn,
                          "[[:db/add \"t\" :test/_dangling {:test/many 11}]]",

db/src/tx.rs

@@ -257,13 +257,12 @@ impl<'conn, 'a> Tx<'conn, 'a> {
                 }
             }
 
-            fn entity_a_into_term_a(&mut self, x: entmod::Entid) -> Result<(Entid, &'a Attribute)> {
+            fn entity_a_into_term_a(&mut self, x: entmod::Entid) -> Result<Entid> {
                 let a = match x {
                     entmod::Entid::Entid(ref a) => *a,
                     entmod::Entid::Ident(ref a) => self.schema.require_entid(&a)?,
                 };
-                let attribute: &Attribute = self.schema.require_attribute_for_entid(a)?;
+                Ok(a)
-                Ok((a, attribute))
             }
 
             fn entity_e_into_term_v(&mut self, x: entmod::EntidOrLookupRefOrTempId) -> Result<TypedValueOr<LookupRefOrTempId>> {
@@ -271,49 +270,44 @@ impl<'conn, 'a> Tx<'conn, 'a> {
             }
 
             fn entity_v_into_term_e(&mut self, x: entmod::AtomOrLookupRefOrVectorOrMapNotation, backward_a: &entmod::Entid) -> Result<EntidOr<LookupRefOrTempId>> {
-                let reversed_a = match backward_a {
+                match backward_a.unreversed() {
-                    &entmod::Entid::Entid(a) => {
+                    None => {
-                        // Programmer error.
+                        bail!(ErrorKind::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for forward attribute")));
-                        unreachable!("Expected ident backward attribute but got entid {}", a)
                     },
-                    &entmod::Entid::Ident(ref a) => {
+                    Some(forward_a) => {
-                        entmod::Entid::Ident(a.to_reversed())
+                        let forward_a = self.entity_a_into_term_a(forward_a)?;
-                    },
+                        let forward_attribute = self.schema.require_attribute_for_entid(forward_a)?;
-                };
+                        if forward_attribute.value_type != ValueType::Ref {
-                let (reversed_a, reversed_attribute) = self.entity_a_into_term_a(reversed_a)?;
+                            bail!(ErrorKind::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} that is not :db/valueType :db.type/ref", forward_a)))
+                        }
 
-                if reversed_attribute.value_type != ValueType::Ref {
+                        match x {
-                    bail!(ErrorKind::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} that is not :db/valueType :db.type/ref", reversed_a)))
+                            entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(ref v) => {
-                }
+                                // Here is where we do schema-aware typechecking: we either assert
+                                // that the given value is in the attribute's value set, or (in
+                                // limited cases) coerce the value into the attribute's value set.
+                                if let Some(text) = v.inner.as_text() {
+                                    Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(TempId::External(text.clone())))))
+                                } else {
+                                    if let TypedValue::Ref(entid) = self.schema.to_typed_value(&v.clone().without_spans(), ValueType::Ref)? {
+                                        Ok(Either::Left(entid))
+                                    } else {
+                                        // The given value is expected to be :db.type/ref, so this shouldn't happen.
+                                        bail!(ErrorKind::NotYetImplemented(format!("Cannot use :attr/_reversed notation for attribute {} with value that is not :db.valueType :db.type/ref", forward_a)))
+                                    }
+                                }
+                            },
 
-                match x {
+                            entmod::AtomOrLookupRefOrVectorOrMapNotation::LookupRef(ref lookup_ref) =>
-                    entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(ref v) => {
+                                Ok(Either::Right(LookupRefOrTempId::LookupRef(self.intern_lookup_ref(lookup_ref)?))),
-                        // Here is where we do schema-aware typechecking: we either assert
+
-                        // that the given value is in the attribute's value set, or (in
+                            entmod::AtomOrLookupRefOrVectorOrMapNotation::Vector(_) =>
-                        // limited cases) coerce the value into the attribute's value set.
+                                bail!(ErrorKind::NotYetImplemented(format!("Cannot explode vector value in :attr/_reversed notation for attribute {}", forward_a))),
-                        if let Some(text) = v.inner.as_text() {
+
-                            Ok(Either::Right(LookupRefOrTempId::TempId(self.intern_temp_id(TempId::External(text.clone())))))
+                            entmod::AtomOrLookupRefOrVectorOrMapNotation::MapNotation(_) =>
-                        } else {
+                                bail!(ErrorKind::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for attribute {}", forward_a))),
-                            // Here is where we do schema-aware typechecking: we either assert that
-                            // the given value is in the attribute's value set, or (in limited
-                            // cases) coerce the value into the attribute's value set.
-                            if let TypedValue::Ref(entid) = self.schema.to_typed_value(&v.clone().without_spans(), reversed_attribute.value_type)? {
-                                Ok(Either::Left(entid))
-                            } else {
-                                // reversed_attribute is :db.type/ref, so this shouldn't happen.
-                                unreachable!()
-                            }
                         }
                     },
-
-                    entmod::AtomOrLookupRefOrVectorOrMapNotation::LookupRef(ref lookup_ref) =>
-                        Ok(Either::Right(LookupRefOrTempId::LookupRef(self.intern_lookup_ref(lookup_ref)?))),
-
-                    entmod::AtomOrLookupRefOrVectorOrMapNotation::Vector(_) =>
-                        bail!(ErrorKind::NotYetImplemented(format!("Cannot explode vector value in :attr/_reversed notation for attribute {}", reversed_a))),
-
-                    entmod::AtomOrLookupRefOrVectorOrMapNotation::MapNotation(_) =>
-                        bail!(ErrorKind::NotYetImplemented(format!("Cannot explode map notation value in :attr/_reversed notation for attribute {}", reversed_a))),
                 }
             }
         }
@@ -348,13 +342,14 @@ impl<'conn, 'a> Tx<'conn, 'a> {
                 },
 
                 Entity::AddOrRetract { op, e, a, v } => {
-                    if a.is_backward() {
+                    if let Some(reversed_a) = a.unreversed() {
                         let reversed_e = in_process.entity_v_into_term_e(v, &a)?;
-                        let (reversed_a, _reversed_attribute) = in_process.entity_a_into_term_a(a.to_reversed().unwrap())?;
+                        let reversed_a = in_process.entity_a_into_term_a(reversed_a)?;
                         let reversed_v = in_process.entity_e_into_term_v(e)?;
                         terms.push(Term::AddOrRetract(OpType::Add, reversed_e, reversed_a, reversed_v));
                     } else {
-                        let (a, attribute) = in_process.entity_a_into_term_a(a)?;
+                        let a = in_process.entity_a_into_term_a(a)?;
+                        let attribute = self.schema.require_attribute_for_entid(a)?;
 
                         let v = match v {
                             entmod::AtomOrLookupRefOrVectorOrMapNotation::Atom(v) => {
@@ -426,21 +421,22 @@ impl<'conn, 'a> Tx<'conn, 'a> {
                                 }
 
                                 for (inner_a, inner_v) in map_notation {
-                                    if inner_a.is_backward() {
+                                    if let Some(reversed_a) = inner_a.unreversed() {
                                         // We definitely have a reference.  The reference might be
                                         // dangling (a bare entid, for example), but we don't yet
                                         // support nested maps and reverse notation simultaneously
                                         // (i.e., we don't accept {:reverse/_attribute {:nested map}})
                                         // so we don't need to check that the nested map reference isn't
-                                        // danglign.
+                                        // dangling.
                                         dangling = false;
 
                                         let reversed_e = in_process.entity_v_into_term_e(inner_v, &inner_a)?;
-                                        let (reversed_a, _reversed_attribute) = in_process.entity_a_into_term_a(inner_a.to_reversed().unwrap())?;
+                                        let reversed_a = in_process.entity_a_into_term_a(reversed_a)?;
                                         let reversed_v = in_process.entity_e_into_term_v(db_id.clone())?;
                                         terms.push(Term::AddOrRetract(OpType::Add, reversed_e, reversed_a, reversed_v));
                                     } else {
-                                        let (inner_a, inner_attribute) = in_process.entity_a_into_term_a(inner_a)?;
+                                        let inner_a = in_process.entity_a_into_term_a(inner_a)?;
+                                        let inner_attribute = self.schema.require_attribute_for_entid(inner_a)?;
                                         if inner_attribute.unique == Some(attribute::Unique::Identity) {
                                             dangling = false;
                                         }

edn/src/symbols.rs

@@ -214,6 +214,30 @@ impl NamespacedKeyword {
             namespace: self.namespace.clone(),
         }
     }
+
+    /// If this `NamespacedKeyword` is 'backward' (see `symbols::NamespacedKeyword::is_backward`),
+    /// return `Some('forward name')`; otherwise, return `None`.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// # use edn::symbols::NamespacedKeyword;
+    /// let nsk = NamespacedKeyword::new("foo", "bar");
+    /// assert_eq!(None, nsk.unreversed());
+    ///
+    /// let reversed = nsk.to_reversed();
+    /// assert_eq!(Some(nsk), reversed.unreversed());
+    /// ```
+    pub fn unreversed(&self) -> Option<NamespacedKeyword> {
+        if self.is_backward() {
+            Some(NamespacedKeyword {
+                name: self.name[1..].to_string(),
+                namespace: self.namespace.clone(),
+            })
+        } else {
+            None
+        }
+    }
 }
 
 //

edn/src/types.rs

@@ -594,7 +594,6 @@ mod test {
 
     use chrono::{
         DateTime,
-        TimeZone,
         UTC,
     };
     use num::BigInt;

tx/src/entities.rs

@@ -70,6 +70,13 @@ impl Entid {
             &Entid::Ident(ref a) => Some(Entid::Ident(a.to_reversed())),
         }
     }
+
+    pub fn unreversed(&self) -> Option<Entid> {
+        match self {
+            &Entid::Entid(_) => None,
+            &Entid::Ident(ref a) => a.unreversed().map(Entid::Ident),
+        }
+    }
 }
 
 #[derive(Clone, Debug, Eq, Hash, Ord, PartialOrd, PartialEq)]