From 2ef3ca44cb8f149fa06cc798e0846d1b63208200 Mon Sep 17 00:00:00 2001 From: Bastien Jacot-Guillarmod Date: Sat, 3 Feb 2024 11:29:43 +0100 Subject: [PATCH] Use DisjunctionMatcher for any![] macro. --- googletest/src/description.rs | 15 ++- googletest/src/matchers/any_matcher.rs | 118 +++--------------- .../src/matchers/conjunction_matcher.rs | 9 +- .../src/matchers/disjunction_matcher.rs | 62 +++++++-- googletest/src/matchers/mod.rs | 1 - googletest/tests/any_matcher_test.rs | 10 -- 6 files changed, 86 insertions(+), 129 deletions(-) diff --git a/googletest/src/description.rs b/googletest/src/description.rs index aaab7eeb..cb4201db 100644 --- 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 index 95d53fe5..5cf41f99 100644 --- a/googletest/src/matchers/any_matcher.rs +++ b/googletest/src/matchers/any_matcher.rs @@ -55,112 +55,34 @@ #[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)),*]) + ($(,)?) => {{ + std::compile_error!("any![...] expects at least one argument"); + }} ; + ($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 + '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 + '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::().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::>(); - - 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::().describe(matcher_result), - 1 => self.components[0].describe(matcher_result), - _ => { - let properties = self - .components - .iter() - .map(|m| m.describe(matcher_result)) - .collect::() - .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 = starts_with("A"); let second_matcher = ends_with("string"); - let matcher: internal::AnyMatcher = any!(first_matcher, second_matcher); + let matcher = any!(first_matcher, second_matcher); verify_that!( matcher.describe(MatcherResult::Match), @@ -175,8 +97,8 @@ mod tests { #[test] fn description_shows_one_matcher_directly() -> Result<()> { - let first_matcher = starts_with("A"); - let matcher: internal::AnyMatcher = any!(first_matcher); + let first_matcher: StrMatcher = starts_with("A"); + let matcher = any!(first_matcher); verify_that!( matcher.describe(MatcherResult::Match), @@ -189,7 +111,7 @@ mod tests { { let first_matcher = starts_with("Another"); let second_matcher = ends_with("string"); - let matcher: internal::AnyMatcher = any!(first_matcher, second_matcher); + let matcher = any!(first_matcher, second_matcher); verify_that!( matcher.explain_match("A string"), @@ -200,7 +122,7 @@ mod tests { #[test] fn mismatch_description_is_simple_when_only_one_constituent() -> Result<()> { let first_matcher = starts_with("Another"); - let matcher: internal::AnyMatcher = any!(first_matcher); + let matcher = any!(first_matcher); verify_that!( matcher.explain_match("A string"), diff --git a/googletest/src/matchers/conjunction_matcher.rs b/googletest/src/matchers/conjunction_matcher.rs index de37dab6..0f3c606d 100644 --- a/googletest/src/matchers/conjunction_matcher.rs +++ b/googletest/src/matchers/conjunction_matcher.rs @@ -76,7 +76,7 @@ where } else { Description::new() .bullet_list() - .collect([self.m1.explain_match(actual), self.m2.explain_match(actual)]) + .collect([m1_description, self.m2.explain_match(actual)]) .conjunction_description() } } @@ -96,10 +96,9 @@ where Description::new() .text(header) .nested( - Description::new().bullet_list().collect([ - self.m1.describe(matcher_result), - self.m2.describe(matcher_result), - ]), + Description::new() + .bullet_list() + .collect([m1_description, self.m2.describe(matcher_result)]), ) .conjunction_description() } diff --git a/googletest/src/matchers/disjunction_matcher.rs b/googletest/src/matchers/disjunction_matcher.rs index dd56be27..5e0f130d 100644 --- a/googletest/src/matchers/disjunction_matcher.rs +++ b/googletest/src/matchers/disjunction_matcher.rs @@ -21,8 +21,18 @@ 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 { @@ -31,7 +41,7 @@ pub struct DisjunctionMatcher { } impl DisjunctionMatcher { - pub(crate) fn new(m1: M1, m2: M2) -> Self { + pub fn new(m1: M1, m2: M2) -> Self { Self { m1, m2 } } } @@ -50,15 +60,42 @@ 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([m1_description, 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([m1_description, self.m2.describe(matcher_result)]), + ) + .disjunction_description() + } } } @@ -90,11 +127,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 index 48124a95..147c8ab1 100644 --- 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 index 82ed046f..e8fdfa4f 100644 --- 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)]);