Use DisjunctionMatcher for any![] macro.

bjacotg · bjacotg · commit 2eca8c43f400 · 2024-02-03T11:35:05.000+01:00
diff --git a/googletest/src/description.rs b/googletest/src/description.rs
@@ -90,6 +90,7 @@ pub struct Description {
     elements: List,
     initial_indentation: usize,
     is_conjunction: bool,
+    is_disjunction: bool,
 }
 
 impl Description {
@@ -201,6 +202,11 @@ impl Description {
         self.elements.is_empty()
     }
 
+    pub(crate) fn push_in_last_nested(mut self, inner: Description) -> Self {
+        self.elements.push_at_end(inner.elements);
+        self
+    }
+
     pub(crate) fn conjunction_description(self) -> Self {
         Self { is_conjunction: true, ..self }
     }
@@ -209,9 +215,12 @@ impl Description {
         self.is_conjunction
     }
 
-    pub(crate) fn push_in_last_nested(mut self, inner: Description) -> Self {
-        self.elements.push_at_end(inner.elements);
-        self
+    pub(crate) fn disjunction_description(self) -> Self {
+        Self { is_disjunction: true, ..self }
+    }
+
+    pub(crate) fn is_disjunction_description(&self) -> bool {
+        self.is_disjunction
     }
 }
 
diff --git a/googletest/src/matchers/any_matcher.rs b/googletest/src/matchers/any_matcher.rs
@@ -55,112 +55,31 @@
 #[macro_export]
 #[doc(hidden)]
 macro_rules! __any {
-    ($($matcher:expr),* $(,)?) => {{
-        use $crate::matchers::__internal_unstable_do_not_depend_on_these::AnyMatcher;
-        AnyMatcher::new([$(Box::new($matcher)),*])
+    ($matcher:expr $(,)?) => {{
+        $matcher
+    }};
+    ($head:expr, $head2:expr $(,)?) => {{
+        $crate::matchers::__internal_unstable_do_not_depend_on_these::DisjunctionMatcher::new($head, $head2)
+    }};
+    ($head:expr, $head2:expr, $($tail:expr),+ $(,)?) => {{
+        $crate::__any![
+            $crate::matchers::__internal_unstable_do_not_depend_on_these::DisjunctionMatcher::new($head, $head2),
+            $($tail),+
+        ]
     }}
 }
 
-/// Functionality needed by the [`any`] macro.
-///
-/// For internal use only. API stablility is not guaranteed!
-#[doc(hidden)]
-pub mod internal {
-    use crate::description::Description;
-    use crate::matcher::{Matcher, MatcherResult};
-    use crate::matchers::anything;
-    use std::fmt::Debug;
-
-    /// A matcher which matches an input value matched by all matchers in the
-    /// array `components`.
-    ///
-    /// For internal use only. API stablility is not guaranteed!
-    #[doc(hidden)]
-    pub struct AnyMatcher<'a, T: Debug + ?Sized, const N: usize> {
-        components: [Box<dyn Matcher<ActualT = T> + 'a>; N],
-    }
-
-    impl<'a, T: Debug + ?Sized, const N: usize> AnyMatcher<'a, T, N> {
-        /// Constructs an [`AnyMatcher`] with the given component matchers.
-        ///
-        /// Intended for use only by the [`all`] macro.
-        pub fn new(components: [Box<dyn Matcher<ActualT = T> + 'a>; N]) -> Self {
-            Self { components }
-        }
-    }
-
-    impl<'a, T: Debug + ?Sized, const N: usize> Matcher for AnyMatcher<'a, T, N> {
-        type ActualT = T;
-
-        fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
-            MatcherResult::from(self.components.iter().any(|c| c.matches(actual).is_match()))
-        }
-
-        fn explain_match(&self, actual: &Self::ActualT) -> Description {
-            match N {
-                0 => format!("which {}", anything::<T>().describe(MatcherResult::NoMatch)).into(),
-                1 => self.components[0].explain_match(actual),
-                _ => {
-                    let failures = self
-                        .components
-                        .iter()
-                        .filter(|component| component.matches(actual).is_no_match())
-                        .collect::<Vec<_>>();
-
-                    if failures.len() == 1 {
-                        failures[0].explain_match(actual)
-                    } else {
-                        Description::new()
-                            .collect(
-                                failures
-                                    .into_iter()
-                                    .map(|component| component.explain_match(actual)),
-                            )
-                            .bullet_list()
-                    }
-                }
-            }
-        }
-
-        fn describe(&self, matcher_result: MatcherResult) -> Description {
-            match N {
-                0 => anything::<T>().describe(matcher_result),
-                1 => self.components[0].describe(matcher_result),
-                _ => {
-                    let properties = self
-                        .components
-                        .iter()
-                        .map(|m| m.describe(matcher_result))
-                        .collect::<Description>()
-                        .bullet_list()
-                        .indent();
-                    format!(
-                        "{}:\n{properties}",
-                        if matcher_result.into() {
-                            "has at least one of the following properties"
-                        } else {
-                            "has none of the following properties"
-                        }
-                    )
-                    .into()
-                }
-            }
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
-    use super::internal;
     use crate::matcher::{Matcher, MatcherResult};
     use crate::prelude::*;
     use indoc::indoc;
 
     #[test]
     fn description_shows_more_than_one_matcher() -> Result<()> {
-        let first_matcher = starts_with("A");
+        let first_matcher: StrMatcher<String, &str> = starts_with("A");
         let second_matcher = ends_with("string");
-        let matcher: internal::AnyMatcher<String, 2> = any!(first_matcher, second_matcher);
+        let matcher = any!(first_matcher, second_matcher);
 
         verify_that!(
             matcher.describe(MatcherResult::Match),
@@ -175,8 +94,8 @@ mod tests {
 
     #[test]
     fn description_shows_one_matcher_directly() -> Result<()> {
-        let first_matcher = starts_with("A");
-        let matcher: internal::AnyMatcher<String, 1> = any!(first_matcher);
+        let first_matcher: StrMatcher<String, &str> = starts_with("A");
+        let matcher = any!(first_matcher);
 
         verify_that!(
             matcher.describe(MatcherResult::Match),
@@ -189,7 +108,7 @@ mod tests {
     {
         let first_matcher = starts_with("Another");
         let second_matcher = ends_with("string");
-        let matcher: internal::AnyMatcher<str, 2> = any!(first_matcher, second_matcher);
+        let matcher = any!(first_matcher, second_matcher);
 
         verify_that!(
             matcher.explain_match("A string"),
@@ -200,7 +119,7 @@ mod tests {
     #[test]
     fn mismatch_description_is_simple_when_only_one_constituent() -> Result<()> {
         let first_matcher = starts_with("Another");
-        let matcher: internal::AnyMatcher<str, 1> = any!(first_matcher);
+        let matcher = any!(first_matcher);
 
         verify_that!(
             matcher.explain_match("A string"),
diff --git a/googletest/src/matchers/disjunction_matcher.rs b/googletest/src/matchers/disjunction_matcher.rs
@@ -21,8 +21,15 @@ use crate::{
 };
 use std::fmt::Debug;
 
-/// Matcher created by [`Matcher::or`].
+/// Matcher created by [`Matcher::or`] and [`any!`].
 ///
+/// Both [`Matcher::or`] and [`any!`] nest on m1. In other words,
+/// both `x.or(y).or(z)` and `any![x, y, z]` produce:
+/// ```ignore
+/// DisjunctionMatcher {
+///   m1: DisjunctionMatcher {
+///     m1: x, m2: y}, m2: z}
+/// ```
 /// **For internal use only. API stablility is not guaranteed!**
 #[doc(hidden)]
 pub struct DisjunctionMatcher<M1, M2> {
@@ -31,7 +38,7 @@ pub struct DisjunctionMatcher<M1, M2> {
 }
 
 impl<M1, M2> DisjunctionMatcher<M1, M2> {
-    pub(crate) fn new(m1: M1, m2: M2) -> Self {
+    pub fn new(m1: M1, m2: M2) -> Self {
         Self { m1, m2 }
     }
 }
@@ -50,15 +57,43 @@ where
     }
 
     fn explain_match(&self, actual: &M1::ActualT) -> Description {
-        Description::new()
-            .nested(self.m1.explain_match(actual))
-            .text("and")
-            .nested(self.m2.explain_match(actual))
+        match (self.m1.matches(actual), self.m2.matches(actual)) {
+            (MatcherResult::NoMatch, MatcherResult::Match) => self.m1.explain_match(actual),
+            (MatcherResult::Match, MatcherResult::NoMatch) => self.m2.explain_match(actual),
+            (_, _) => {
+                let m1_description = self.m1.explain_match(actual);
+                if m1_description.is_disjunction_description() {
+                    m1_description.nested(self.m2.explain_match(actual))
+                } else {
+                    Description::new()
+                        .bullet_list()
+                        .collect([self.m1.explain_match(actual), self.m2.explain_match(actual)])
+                        .disjunction_description()
+                }
+            }
+        }
     }
 
     fn describe(&self, matcher_result: MatcherResult) -> Description {
-        format!("{}, or {}", self.m1.describe(matcher_result), self.m2.describe(matcher_result))
-            .into()
+        let m1_description = self.m1.describe(matcher_result);
+        if m1_description.is_disjunction_description() {
+            m1_description.push_in_last_nested(self.m2.describe(matcher_result))
+        } else {
+            let header = if matcher_result.into() {
+                "has at least one of the following properties:"
+            } else {
+                "has all of the following properties:"
+            };
+            Description::new()
+                .text(header)
+                .nested(
+                    Description::new().bullet_list().collect([
+                        self.m1.describe(matcher_result),
+                        self.m2.describe(matcher_result),
+                    ]),
+                )
+                .disjunction_description()
+        }
     }
 }
 
@@ -90,11 +125,12 @@ mod tests {
             err(displays_as(contains_substring(indoc!(
                 "
                 Value of: 1
-                Expected: never matches, or never matches
+                Expected: has at least one of the following properties:
+                  * never matches
+                  * never matches
                 Actual: 1,
-                    which is anything
-                  and
-                    which is anything
+                  * which is anything
+                  * which is anything
                 "
             ))))
         )
diff --git a/googletest/src/matchers/mod.rs b/googletest/src/matchers/mod.rs
@@ -105,7 +105,6 @@ pub use crate::{
 // should only be used through their respective macros.
 #[doc(hidden)]
 pub mod __internal_unstable_do_not_depend_on_these {
-    pub use super::any_matcher::internal::AnyMatcher;
     pub use super::conjunction_matcher::ConjunctionMatcher;
     pub use super::disjunction_matcher::DisjunctionMatcher;
     pub use super::elements_are_matcher::internal::ElementsAre;
diff --git a/googletest/tests/any_matcher_test.rs b/googletest/tests/any_matcher_test.rs
@@ -16,11 +16,6 @@ use googletest::matcher::Matcher;
 use googletest::prelude::*;
 use indoc::indoc;
 
-#[test]
-fn does_not_match_value_when_list_is_empty() -> Result<()> {
-    verify_that!((), not(any!()))
-}
-
 #[test]
 fn matches_value_with_single_matching_component() -> Result<()> {
     verify_that!(123, any!(eq(123)))
@@ -65,11 +60,6 @@ fn mismatch_description_two_failed_matchers() -> Result<()> {
     )
 }
 
-#[test]
-fn mismatch_description_empty_matcher() -> Result<()> {
-    verify_that!(any!().explain_match("Three"), displays_as(eq("which never matches")))
-}
-
 #[test]
 fn all_multiple_failed_assertions() -> Result<()> {
     let result = verify_that!(4, any![eq(1), eq(2), eq(3)]);

Original file line number	Diff line number	Diff line change
`@@ -90,6 +90,7 @@ pub struct Description {`
`90`	`90`	`elements: List,`
`91`	`91`	`initial_indentation: usize,`
`92`	`92`	`is_conjunction: bool,`
	`93`	`+ is_disjunction: bool,`
`93`	`94`	`}`
`94`	`95`
`95`	`96`	`impl Description {`
`@@ -201,6 +202,11 @@ impl Description {`
`201`	`202`	`self.elements.is_empty()`
`202`	`203`	`}`
`203`	`204`
	`205`	`+ pub(crate) fn push_in_last_nested(mut self, inner: Description) -> Self {`
	`206`	`+ self.elements.push_at_end(inner.elements);`
	`207`	`+ self`
	`208`	`+ }`
	`209`	`+`
`204`	`210`	`pub(crate) fn conjunction_description(self) -> Self {`
`205`	`211`	`Self { is_conjunction: true, ..self }`
`206`	`212`	`}`
`@@ -209,9 +215,12 @@ impl Description {`
`209`	`215`	`self.is_conjunction`
`210`	`216`	`}`
`211`	`217`
`212`		`- pub(crate) fn push_in_last_nested(mut self, inner: Description) -> Self {`
`213`		`- self.elements.push_at_end(inner.elements);`
`214`		`- self`
	`218`	`+ pub(crate) fn disjunction_description(self) -> Self {`
	`219`	`+ Self { is_disjunction: true, ..self }`
	`220`	`+ }`
	`221`	`+`
	`222`	`+ pub(crate) fn is_disjunction_description(&self) -> bool {`
	`223`	`+ self.is_disjunction`
`215`	`224`	`}`
`216`	`225`	`}`
`217`	`226`