diff --git a/googletest/src/description.rs b/googletest/src/description.rs index 9605559b..aaab7eeb 100644 --- a/googletest/src/description.rs +++ b/googletest/src/description.rs @@ -89,6 +89,7 @@ use crate::internal::description_renderer::{List, INDENTATION_SIZE}; pub struct Description { elements: List, initial_indentation: usize, + is_conjunction: bool, } impl Description { @@ -199,6 +200,19 @@ impl Description { pub fn is_empty(&self) -> bool { self.elements.is_empty() } + + pub(crate) fn conjunction_description(self) -> Self { + Self { is_conjunction: true, ..self } + } + + pub(crate) fn is_conjunction_description(&self) -> bool { + self.is_conjunction + } + + pub(crate) fn push_in_last_nested(mut self, inner: Description) -> Self { + self.elements.push_at_end(inner.elements); + self + } } impl Display for Description { diff --git a/googletest/src/internal/description_renderer.rs b/googletest/src/internal/description_renderer.rs index 937f0d59..e84fabe9 100644 --- a/googletest/src/internal/description_renderer.rs +++ b/googletest/src/internal/description_renderer.rs @@ -83,6 +83,17 @@ impl List { self.0.is_empty() } + /// Append a new [`List`] in the last element which must be a + /// [`Block::Nested`]. Panic if `self` is empty or the last element is + /// not [`Block::Nested`]. + pub(crate) fn push_at_end(&mut self, list: List) { + if let Some(Block::Nested(self_list)) = self.0.last_mut() { + self_list.push_nested(list); + } else { + panic!("pushing elements at the end of {self:#?} which last element is not Nested") + } + } + fn render_with_prefix( &self, f: &mut dyn Write, diff --git a/googletest/src/matchers/all_matcher.rs b/googletest/src/matchers/all_matcher.rs index f2e6d069..2557c3bc 100644 --- a/googletest/src/matchers/all_matcher.rs +++ b/googletest/src/matchers/all_matcher.rs @@ -53,114 +53,34 @@ #[macro_export] #[doc(hidden)] macro_rules! __all { - ($($matcher:expr),* $(,)?) => {{ - use $crate::matchers::__internal_unstable_do_not_depend_on_these::AllMatcher; - AllMatcher::new([$(Box::new($matcher)),*]) + ($(,)?) => {{ + $crate::matchers::anything() + }} ; + ($matcher:expr $(,)?) => {{ + $matcher + }}; + ($head:expr, $head2:expr $(,)?) => {{ + $crate::matchers::__internal_unstable_do_not_depend_on_these::ConjunctionMatcher::new($head, $head2) + }}; + ($head:expr, $head2:expr, $($tail:expr),+ $(,)?) => {{ + $crate::__all![ + $crate::matchers::__internal_unstable_do_not_depend_on_these::ConjunctionMatcher::new($head, $head2), + $($tail),+ + ] }} } -/// Functionality needed by the [`all`] 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 AllMatcher<'a, T: Debug + ?Sized, const N: usize> { - components: [Box + 'a>; N], - } - - impl<'a, T: Debug + ?Sized, const N: usize> AllMatcher<'a, T, N> { - /// Constructs an [`AllMatcher`] 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 AllMatcher<'a, T, N> { - type ActualT = T; - - fn matches(&self, actual: &Self::ActualT) -> MatcherResult { - for component in &self.components { - match component.matches(actual) { - MatcherResult::NoMatch => { - return MatcherResult::NoMatch; - } - MatcherResult::Match => {} - } - } - MatcherResult::Match - } - - fn explain_match(&self, actual: &Self::ActualT) -> Description { - match N { - 0 => anything::().explain_match(actual), - 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 header = if matcher_result.into() { - "has all the following properties:" - } else { - "has at least one of the following properties:" - }; - Description::new().text(header).nested( - Description::new() - .bullet_list() - .collect(self.components.iter().map(|m| m.describe(matcher_result))), - ) - } - } - } - } -} - #[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::AllMatcher = all!(first_matcher, second_matcher); + let matcher = all!(first_matcher, second_matcher); verify_that!( matcher.describe(MatcherResult::Match), @@ -175,8 +95,8 @@ mod tests { #[test] fn description_shows_one_matcher_directly() -> Result<()> { - let first_matcher = starts_with("A"); - let matcher: internal::AllMatcher = all!(first_matcher); + let first_matcher: StrMatcher = starts_with("A"); + let matcher = all!(first_matcher); verify_that!( matcher.describe(MatcherResult::Match), @@ -187,9 +107,9 @@ mod tests { #[test] fn mismatch_description_shows_which_matcher_failed_if_more_than_one_constituent() -> Result<()> { - let first_matcher = starts_with("Another"); + let first_matcher: StrMatcher = starts_with("Another"); let second_matcher = ends_with("string"); - let matcher: internal::AllMatcher = all!(first_matcher, second_matcher); + let matcher = all!(first_matcher, second_matcher); verify_that!( matcher.explain_match("A string"), @@ -200,7 +120,7 @@ mod tests { #[test] fn mismatch_description_is_simple_when_only_one_consistuent() -> Result<()> { let first_matcher = starts_with("Another"); - let matcher: internal::AllMatcher = all!(first_matcher); + let matcher = all!(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 dc50833b..de37dab6 100644 --- a/googletest/src/matchers/conjunction_matcher.rs +++ b/googletest/src/matchers/conjunction_matcher.rs @@ -21,7 +21,23 @@ use crate::{ }; use std::fmt::Debug; -/// Matcher created by [`Matcher::and`]. +/// Matcher created by [`Matcher::and`] and [`all!`]. +/// +/// Both [`Matcher::and`] and [`all!`] nest on m1. In other words, +/// both `x.and(y).and(z)` and `all![x, y, z]` produce: +/// ```ignore +/// ConjunctionMatcher { +/// m1: ConjunctionMatcher { +/// m1: x, +/// m2: y +/// }, +/// m2: z +/// } +/// ``` +/// +/// This behavior must be respected +/// to ensure that [`Matcher::explain_match`] and [`Matcher::describe`] produce +/// useful descriptions. /// /// **For internal use only. API stablility is not guaranteed!** #[doc(hidden)] @@ -31,7 +47,7 @@ pub struct ConjunctionMatcher { } impl ConjunctionMatcher { - pub(crate) fn new(m1: M1, m2: M2) -> Self { + pub fn new(m1: M1, m2: M2) -> Self { Self { m1, m2 } } } @@ -51,22 +67,42 @@ where fn explain_match(&self, actual: &M1::ActualT) -> Description { match (self.m1.matches(actual), self.m2.matches(actual)) { - (MatcherResult::Match, MatcherResult::Match) => Description::new() - .nested(self.m1.explain_match(actual)) - .text("and") - .nested(self.m2.explain_match(actual)), (MatcherResult::NoMatch, MatcherResult::Match) => self.m1.explain_match(actual), (MatcherResult::Match, MatcherResult::NoMatch) => self.m2.explain_match(actual), - (MatcherResult::NoMatch, MatcherResult::NoMatch) => Description::new() - .nested(self.m1.explain_match(actual)) - .text("and") - .nested(self.m2.explain_match(actual)), + (_, _) => { + let m1_description = self.m1.explain_match(actual); + if m1_description.is_conjunction_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)]) + .conjunction_description() + } + } } } fn describe(&self, matcher_result: MatcherResult) -> Description { - format!("{}, and {}", self.m1.describe(matcher_result), self.m2.describe(matcher_result)) - .into() + let m1_description = self.m1.describe(matcher_result); + if m1_description.is_conjunction_description() { + m1_description.push_in_last_nested(self.m2.describe(matcher_result)) + } else { + let header = if matcher_result.into() { + "has all the following properties:" + } else { + "has at least one of the following properties:" + }; + Description::new() + .text(header) + .nested( + Description::new().bullet_list().collect([ + self.m1.describe(matcher_result), + self.m2.describe(matcher_result), + ]), + ) + .conjunction_description() + } } } @@ -88,7 +124,9 @@ mod tests { err(displays_as(contains_substring(indoc!( " Value of: 1 - Expected: is anything, and never matches + Expected: has all the following properties: + * is anything + * never matches Actual: 1, which is anything " @@ -104,7 +142,9 @@ mod tests { err(displays_as(contains_substring(indoc!( " Value of: 1 - Expected: never matches, and is anything + Expected: has all the following properties: + * never matches + * is anything Actual: 1, which is anything " @@ -120,11 +160,37 @@ mod tests { err(displays_as(contains_substring(indoc!( " Value of: 1 - Expected: never matches, and never matches + Expected: has all the following properties: + * never matches + * never matches + Actual: 1, + * which is anything + * which is anything + " + )))) + ) + } + + #[test] + fn and_long_chain_of_matchers() -> Result<()> { + let result = verify_that!( + 1, + anything().and(not(anything())).and(anything()).and(not(anything())).and(anything()) + ); + verify_that!( + result, + err(displays_as(contains_substring(indoc!( + " + Value of: 1 + Expected: has all the following properties: + * is anything + * never matches + * is anything + * never matches + * is anything 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 1e028b97..48124a95 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::all_matcher::internal::AllMatcher; pub use super::any_matcher::internal::AnyMatcher; pub use super::conjunction_matcher::ConjunctionMatcher; pub use super::disjunction_matcher::DisjunctionMatcher;