Merge pull request #827 from vsbogd/separate-debug-and-display

Separate Debug and Display trait implementations for Atom

lib/Cargo.toml

@@ -13,6 +13,7 @@ im = "15.1.0"
 rand = "0.8.5"
 bitset = "0.1.2"
 dyn-fmt = "0.4.0"
+itertools = "0.13.0"
 
 # pkg_mgmt deps
 xxhash-rust = {version="0.8.7", features=["xxh3"], optional=true }

lib/src/atom/mod.rs

@@ -784,7 +784,7 @@ impl Clone for Box<dyn GroundedAtom> {
 
 /// Atoms are main components of the atomspace. There are four meta-types of
 /// atoms: symbol, expression, variable and grounded.
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub enum Atom {
     /// Symbol represents some idea or concept. Two symbols having
     /// the same name are considered equal and representing the same concept.
@@ -1083,12 +1083,6 @@ impl Display for Atom {
     }
 }
 
-impl Debug for Atom {
-    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-        Display::fmt(self, f)
-    }
-}
-
 #[cfg(test)]
 mod test {
     #![allow(non_snake_case)]

lib/src/common/assert.rs

@@ -1,117 +1,114 @@
 use super::collections::{ListMap, Equality, DefaultEquality};
 
-use std::cmp::Ordering;
+use std::fmt::{Debug, Display, Formatter};
+use itertools::Itertools;
 
-pub fn vec_eq_no_order<'a, T, A, B>(left: A, right: B) -> Option<String>
-where
-    T: 'a + PartialEq + std::fmt::Debug,
-    A: Iterator<Item=&'a T>,
-    B: Iterator<Item=&'a T>,
-{
-    compare_vec_no_order(left, right, DefaultEquality{}).as_string()
-}
-
-pub fn compare_vec_no_order<'a, T, A, B, E>(left: A, right: B, _cmp: E) -> VecDiff<'a, T, E>
+pub fn compare_vec_no_order<'a, T, A, B, E>(actual: A, expected: B, _cmp: E) -> VecDiff<'a, T, E>
 where
     A: Iterator<Item=&'a T>,
     B: Iterator<Item=&'a T>,
     E: Equality<&'a T>,
 {
-    let mut left_count: ListMap<&T, usize, E> = ListMap::new();
-    let mut right_count: ListMap<&T, usize, E> = ListMap::new();
-    for i in left {
-        *left_count.entry(&i).or_insert(0) += 1;
+    let mut diff: ListMap<&T, Count, E> = ListMap::new();
+    for i in actual {
+        diff.entry(&i).or_default().actual += 1;
     }
-    for i in right {
-        *right_count.entry(&i).or_insert(0) += 1;
+    for i in expected {
+        diff.entry(&i).or_default().expected += 1;
     }
-    VecDiff{ left_count, right_count }
+    diff = diff.into_iter().filter(|(_v, c)| c.actual != c.expected).collect();
+    VecDiff{ diff }
+}
+
+#[derive(Default)]
+struct Count {
+    actual: usize,
+    expected: usize,
 }
 
 pub struct VecDiff<'a, T, E: Equality<&'a T>> {
-    left_count: ListMap<&'a T, usize, E>,
-    right_count: ListMap<&'a T, usize, E>,
+    diff: ListMap<&'a T, Count, E>,
+}
+
+struct FormatAsDebug<T: Debug>(T);
+impl<T: Debug> Display for FormatAsDebug<T> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        Debug::fmt(&self.0, f)
+    }
 }
 
-trait DiffVisitor<'a, T> {
-    fn diff(&mut self, item: &'a T, left: usize, right: usize) -> bool;
+struct FormatAsDisplay<T: Display>(T);
+impl<T: Display> Display for FormatAsDisplay<T> {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        Display::fmt(&self.0, f)
+    }
 }
 
-impl<'a, T: std::fmt::Debug, E: Equality<&'a T>> VecDiff<'a, T, E> {
+impl<'a, T, E: Equality<&'a T>> VecDiff<'a, T, E> {
     pub fn has_diff(&self) -> bool {
-        #[derive(Default)]
-        struct FindDiff {
-            diff: bool,
-        }
-        impl<T: std::fmt::Debug> DiffVisitor<'_, T> for FindDiff {
-            fn diff(&mut self, _item: &T, left: usize, right: usize) -> bool {
-                if left == right {
-                    false
-                } else {
-                    self.diff = true;
-                    true
-                }
-            }
-        }
-        let mut f = FindDiff::default();
-        self.visit(&mut f);
-        f.diff
+        !self.diff.is_empty()
     }
 
-    pub fn as_string(&self) -> Option<String> {
-        #[derive(Default)]
-        struct StringDiff {
-            diff: Option<String>,
-        }
-        impl<'a, T: std::fmt::Debug> DiffVisitor<'a, T> for StringDiff {
-            fn diff(&mut self, item: &'a T, left: usize, right: usize) -> bool {
-                match left.cmp(&right) {
-                    Ordering::Less => {
-                        self.diff = Some(format!("Missed result: {:?}", item));
-                        true
-                    },
-                    Ordering::Greater => {
-                        self.diff = Some(format!("Excessive result: {:?}", item));
-                        true
-                    },
-                    Ordering::Equal => false,
-                }
-            }
-        }
-        let mut d = StringDiff{ diff: None };
-        self.visit(&mut d);
-        d.diff
+    pub fn as_display(&self) -> Option<String> where T: Display {
+        self.as_string(FormatAsDisplay)
     }
 
-    fn visit<'b, V: DiffVisitor<'b, T>>(&'b self, visitor: &mut V) {
-        for e in self.right_count.iter() {
-            let count = self.left_count.get(e.0).unwrap_or(&0);
-            if visitor.diff(e.0, *count, *e.1) { return }
-        }
-        for e in self.left_count.iter() {
-            let count = self.right_count.get(e.0).unwrap_or(&0);
-            if visitor.diff(e.0, *e.1, *count) { return }
+    pub fn as_debug(&self) -> Option<String> where T: Debug {
+        self.as_string(FormatAsDebug)
+    }
+
+    fn as_string<F, I: Display>(&self, f: F) -> Option<String>
+        where F: Fn(&'a T) -> I
+    {
+        let mut diff = String::new();
+        if self.has_diff() {
+            let mut missed = self.diff.iter()
+                .filter(|(_v, c)| c.actual < c.expected)
+                .flat_map(|(v, c)| std::iter::repeat_n(v, c.expected - c.actual))
+                .map(|v| f(v))
+                .peekable();
+            let mut excessive = self.diff.iter()
+                .filter(|(_v, c)| c.actual > c.expected)
+                .flat_map(|(v, c)| std::iter::repeat_n(v, c.actual - c.expected))
+                .map(|v| f(v))
+                .peekable();
+            if missed.peek().is_some() {
+                diff.push_str(format!("Missed results: {}", missed.format(", ")).as_str());
+            }
+            if excessive.peek().is_some() {
+                if !diff.is_empty() {
+                    diff.push_str("\n");
+                }
+                diff.push_str(format!("Excessive results: {}", excessive.format(", ")).as_str());
+            }
+            Some(diff)
+        } else {
+            None
         }
     }
 }
 
 #[macro_export]
 macro_rules! assert_eq_no_order {
-    ($left:expr, $right:expr) => {
+    ($actual:expr, $expected:expr) => {
         {
-            assert!($crate::common::assert::vec_eq_no_order($left.iter(), $right.iter()) == None,
-                "(left == right some order)\n  left: {:?}\n right: {:?}", $left, $right);
+            let diff = $crate::common::assert::compare_vec_no_order($actual.iter(), $expected.iter(),
+                $crate::common::collections::DefaultEquality{}).as_debug();
+            assert!(diff.is_none(),
+                "(actual != expected)\nActual: {:?}\nExpected: {:?}\n{}",
+                    $actual, $expected, diff.unwrap());
         }
     }
 }
 
-pub fn metta_results_eq<T: PartialEq + std::fmt::Debug>(
-    left: &Result<Vec<Vec<T>>, String>, right: &Result<Vec<Vec<T>>, String>) -> bool
+pub fn metta_results_eq<T: PartialEq>(
+    actual: &Result<Vec<Vec<T>>, String>, expected: &Result<Vec<Vec<T>>, String>) -> bool
 {
-    match (left, right) {
-        (Ok(left), Ok(right)) if left.len() == right.len() => {
-            for (left, right) in left.iter().zip(right.iter()) {
-                if vec_eq_no_order(left.iter(), right.iter()).is_some() {
+    match (actual, expected) {
+        (Ok(actual), Ok(expected)) if actual.len() == expected.len() => {
+            for (actual, expected) in actual.iter().zip(expected.iter()) {
+                let diff = compare_vec_no_order(actual.iter(), expected.iter(), DefaultEquality{});
+                if diff.has_diff() {
                     return false;
                 }
             }
@@ -123,12 +120,12 @@ pub fn metta_results_eq<T: PartialEq + std::fmt::Debug>(
 
 #[macro_export]
 macro_rules! assert_eq_metta_results {
-    ($left:expr, $right:expr) => {
+    ($actual:expr, $expected:expr) => {
         {
-            let left = &$left;
-            let right = &$right;
-            assert!($crate::common::assert::metta_results_eq(left, right),
-                "(left == right)\n  left: {:?}\n right: {:?}", left, right);
+            let actual = &$actual;
+            let expected = &$expected;
+            assert!($crate::common::assert::metta_results_eq(actual, expected),
+                "(actual == expected)\n  actual: {:?}\n expected: {:?}", actual, expected);
         }
     }
 }

lib/src/common/collections.rs

@@ -1,4 +1,5 @@
 use std::fmt::Display;
+use itertools::Itertools;
 
 pub trait Equality<T> {
     fn eq(a: &T, b: &T) -> bool;
@@ -26,6 +27,10 @@ pub enum ListMapEntry<'a, K, V, E: Equality<K>> {
 }
 
 impl<'a, K, V, E: Equality<K>> ListMapEntry<'a, K, V, E> {
+    pub fn or_default(self) -> &'a mut V where V: Default {
+        self.or_insert(Default::default())
+    }
+
     pub fn or_insert(self, default: V) -> &'a mut V {
         match self {
             ListMapEntry::Occupied(key, map) => map.get_mut(&key).unwrap(),
@@ -55,6 +60,8 @@ macro_rules! list_map_iterator {
 list_map_iterator!(ListMapIter, Iter, {});
 list_map_iterator!(ListMapIterMut, IterMut, { mut });
 
+type ListMapIntoIter<K, V> = std::vec::IntoIter<(K, V)>;
+
 macro_rules! list_map_get {
     ($get:ident, {$( $mut_:tt )?}) => {
         pub fn $get(& $( $mut_ )? self, key: &K) -> Option<& $( $mut_ )? V> {
@@ -92,6 +99,10 @@ impl<K, V, E: Equality<K>> ListMap<K, V, E> {
         &mut self.list.last_mut().unwrap().1
     }
 
+    pub fn is_empty(&self) -> bool {
+        self.list.is_empty()
+    }
+
     pub fn clear(&mut self) {
         self.list.clear()
     }
@@ -103,11 +114,15 @@ impl<K, V, E: Equality<K>> ListMap<K, V, E> {
     pub fn iter_mut(&mut self) -> ListMapIterMut<'_, K, V> {
         ListMapIterMut{ delegate: self.list.iter_mut() }
     }
+
+    pub fn into_iter(self) -> ListMapIntoIter<K, V> {
+        self.list.into_iter()
+    }
 }
 
-impl<K: PartialEq, V: PartialEq> PartialEq for ListMap<K, V> {
+impl<K, V: PartialEq, E: Equality<K>> PartialEq for ListMap<K, V, E> {
     fn eq(&self, other: &Self) -> bool {
-        fn left_includes_right<K: PartialEq, V: PartialEq>(left: &ListMap<K, V>, right: &ListMap<K, V>) -> bool {
+        fn left_includes_right<K, V: PartialEq, E: Equality<K>>(left: &ListMap<K, V, E>, right: &ListMap<K, V, E>) -> bool {
             for e in right.iter() {
                 if left.get(e.0) != Some(e.1) {
                     return false;
@@ -119,7 +134,7 @@ impl<K: PartialEq, V: PartialEq> PartialEq for ListMap<K, V> {
     }
 }
 
-impl<K: PartialEq, V: PartialEq> From<Vec<(K, V)>> for ListMap<K, V> {
+impl<K: PartialEq, V> From<Vec<(K, V)>> for ListMap<K, V> {
     fn from(list: Vec<(K, V)>) -> Self {
         let mut map = ListMap::new();
         for (k, v) in list.into_iter() {
@@ -129,6 +144,14 @@ impl<K: PartialEq, V: PartialEq> From<Vec<(K, V)>> for ListMap<K, V> {
     }
 }
 
+impl<K, V, E: Equality<K>> FromIterator<(K, V)> for ListMap<K, V, E> {
+    fn from_iter<T>(iter: T) -> Self where T: IntoIterator<Item = (K, V)> {
+        let mut map = ListMap::new();
+        iter.into_iter().for_each(|(k, v)| map.insert(k, v));
+        map
+    }
+}
+
 #[derive(Debug, Clone)]
 pub enum ImmutableString {
     Allocated(String),
@@ -266,6 +289,14 @@ impl<'a, T> Into<&'a [T]> for &'a CowArray<T> {
     }
 }
 
+pub struct VecDisplay<'a, T: 'a + Display>(pub &'a Vec<T>);
+
+impl<'a, T: 'a + Display> Display for VecDisplay<'a, T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "[{}]", self.0.iter().format(", "))
+    }
+}
+
 
 #[cfg(test)]
 mod test {