Skip to content

Commit

Permalink
Parameterize Matcher::matches and Matcher::explain_match.
Browse files Browse the repository at this point in the history
This allows the use of smart pointers for the actual value when invoking
`Matcher::matches` and `Matcher::explain_match`. The Protobuf
implementation namely uses a
[kind of smart pointer](https://github.com/protocolbuffers/protobuf/blob/62d5d9bf67d27d3b0084dabcf67bff2f8238162b/rust/proxied.rs#L113).
The lifetime on that smart pointer has no a priori relation to that of
the parameter `actual`, causing the borrow checker to complain whenever
invoking an inner matcher on the output of `as_view`.

This commit addresses the problem by allowing the `ViewProxy` -- rather
than a reference to it -- to be passed directly to `Matcher::matches`
and `Matcher::explain_match`. It does this by making those two methods
generic with respect to the input type. They must only implement
`Deref<Target = Self::ActualT>`.

The new test `googletest/tests/arena_test.rs` illustrates this with a
simplified example.

Introducing these generics unfortunately makes `Matcher` no longer
object-safe. Since several matchers construct trait objts on `Matcher`,
this also introduces an internal-only wrapper trait `ObjectSafeMatcher`
which every `Matcher` auto-implements.

While this makes some changes to how `Matcher` is defined and
implemented, it should have no effect on how it is _used_, other than
to fix the aforementioned problem. So it should not affect compatibility
for most users of the crate.

Fixes #323.
  • Loading branch information
hovinen committed Mar 28, 2024
1 parent 799ff7c commit 2b43bd4
Show file tree
Hide file tree
Showing 43 changed files with 694 additions and 226 deletions.
21 changes: 15 additions & 6 deletions googletest/crate_docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ a struct holding the matcher's data and have it implement the trait

```no_run
use googletest::{description::Description, matcher::{Matcher, MatcherResult}};
use std::fmt::Debug;
use std::{fmt::Debug, ops::Deref};
struct MyEqMatcher<T> {
expected: T,
Expand All @@ -223,7 +223,10 @@ struct MyEqMatcher<T> {
impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
type ActualT = T;
fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
actual: ActualRefT,
) -> MatcherResult {
if self.expected == *actual {
MatcherResult::Match
} else {
Expand All @@ -248,7 +251,7 @@ impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {

```no_run
# use googletest::{description::Description, matcher::{Matcher, MatcherResult}};
# use std::fmt::Debug;
# use std::{fmt::Debug, ops::Deref};
#
# struct MyEqMatcher<T> {
# expected: T,
Expand All @@ -257,7 +260,10 @@ impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
# impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
# type ActualT = T;
#
# fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
# fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
# &self,
# actual: ActualRefT,
# ) -> MatcherResult {
# if self.expected == *actual {
# MatcherResult::Match
# } else {
Expand Down Expand Up @@ -287,7 +293,7 @@ impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
```
# use googletest::prelude::*;
# use googletest::{description::Description, matcher::{Matcher, MatcherResult}};
# use std::fmt::Debug;
# use std::{fmt::Debug, ops::Deref};
#
# struct MyEqMatcher<T> {
# expected: T,
Expand All @@ -296,7 +302,10 @@ impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
# impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
# type ActualT = T;
#
# fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
# fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
# &self,
# actual: ActualRefT,
# ) -> MatcherResult {
# if self.expected == *actual {
# MatcherResult::Match
# } else {
Expand Down
71 changes: 64 additions & 7 deletions googletest/src/matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use crate::internal::test_outcome::TestAssertionFailure;
use crate::matchers::__internal_unstable_do_not_depend_on_these::ConjunctionMatcher;
use crate::matchers::__internal_unstable_do_not_depend_on_these::DisjunctionMatcher;
use std::fmt::Debug;
use std::ops::Deref;

/// An interface for checking an arbitrary condition on a datum.
///
Expand All @@ -36,7 +37,10 @@ pub trait Matcher {
/// matching condition is based on data stored in the matcher. For example,
/// `eq` matches when its stored expected value is equal (in the sense of
/// the `==` operator) to the value `actual`.
fn matches(&self, actual: &Self::ActualT) -> MatcherResult;
fn matches<ActualRefT: Deref<Target = Self::ActualT> + Clone>(
&self,
actual: ActualRefT,
) -> MatcherResult;

/// Returns a description of `self` or a negative description if
/// `matcher_result` is `DoesNotMatch`.
Expand Down Expand Up @@ -137,7 +141,10 @@ pub trait Matcher {
/// .nested(self.expected.explain_match(actual.deref()))
/// }
/// ```
fn explain_match(&self, actual: &Self::ActualT) -> Description {
fn explain_match<ActualRefT: Deref<Target = Self::ActualT> + Clone>(
&self,
actual: ActualRefT,
) -> Description {
format!("which {}", self.describe(self.matches(actual))).into()
}

Expand Down Expand Up @@ -205,6 +212,46 @@ pub trait Matcher {
}
}

/// Functionality used by macros within this crate.
///
/// For internal use only. API stablility is not guaranteed!
#[doc(hidden)]
pub mod __internal_unstable_do_not_depend_on_these {
use super::{Matcher, MatcherResult};
use crate::description::Description;
use std::fmt::Debug;

/// A variant of [`Matcher`] which is object-safe.
///
/// This is used in contexts where a `dyn Matcher` is required. It supplies all methods of
/// [`Matcher`] but without any generics on the methods.
pub trait ObjectSafeMatcher {
type ActualT: Debug + ?Sized;

fn obj_matches(&self, actual: &Self::ActualT) -> MatcherResult;

fn obj_describe(&self, matcher_result: MatcherResult) -> Description;

fn obj_explain_match(&self, actual: &Self::ActualT) -> Description;
}

impl<MatcherT: Matcher> ObjectSafeMatcher for MatcherT {
type ActualT = <Self as Matcher>::ActualT;

fn obj_matches(&self, actual: &Self::ActualT) -> MatcherResult {
Matcher::matches(self, actual)
}

fn obj_describe(&self, matcher_result: MatcherResult) -> Description {
Matcher::describe(self, matcher_result)
}

fn obj_explain_match(&self, actual: &Self::ActualT) -> Description {
Matcher::explain_match(self, actual)
}
}
}

/// Any actual value whose debug length is greater than this value will be
/// pretty-printed. Otherwise, it will have normal debug output formatting.
const PRETTY_PRINT_LENGTH_THRESHOLD: usize = 60;
Expand Down Expand Up @@ -249,7 +296,11 @@ pub enum MatcherResult {

impl From<bool> for MatcherResult {
fn from(b: bool) -> Self {
if b { MatcherResult::Match } else { MatcherResult::NoMatch }
if b {
MatcherResult::Match
} else {
MatcherResult::NoMatch
}
}
}

Expand All @@ -276,16 +327,22 @@ impl MatcherResult {
impl<M: Matcher> Matcher for &M {
type ActualT = M::ActualT;

fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
(*self).matches(actual)
fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
actual: ActualRefT,
) -> MatcherResult {
(*self).matches(actual.deref())
}

fn describe(&self, matcher_result: MatcherResult) -> Description {
(*self).describe(matcher_result)
}

fn explain_match(&self, actual: &Self::ActualT) -> Description {
(*self).explain_match(actual)
fn explain_match<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
actual: ActualRefT,
) -> Description {
(*self).explain_match(actual.deref())
}
}

Expand Down
4 changes: 2 additions & 2 deletions googletest/src/matchers/anything_matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
description::Description,
matcher::{Matcher, MatcherResult},
};
use std::{fmt::Debug, marker::PhantomData};
use std::{fmt::Debug, marker::PhantomData, ops::Deref};

/// Matches anything. This matcher always succeeds.
///
Expand All @@ -41,7 +41,7 @@ struct Anything<T: ?Sized>(PhantomData<T>);
impl<T: Debug + ?Sized> Matcher for Anything<T> {
type ActualT = T;

fn matches(&self, _: &T) -> MatcherResult {
fn matches<ActualRefT: Deref<Target = Self::ActualT>>(&self, _: ActualRefT) -> MatcherResult {
MatcherResult::Match
}

Expand Down
23 changes: 18 additions & 5 deletions googletest/src/matchers/char_count_matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use crate::{
description::Description,
matcher::{Matcher, MatcherResult},
};
use std::{fmt::Debug, marker::PhantomData};
use std::{fmt::Debug, marker::PhantomData, ops::Deref};

/// Matches a string whose number of Unicode scalars matches `expected`.
///
Expand Down Expand Up @@ -70,7 +70,10 @@ struct CharLenMatcher<T: ?Sized, E> {
impl<T: Debug + ?Sized + AsRef<str>, E: Matcher<ActualT = usize>> Matcher for CharLenMatcher<T, E> {
type ActualT = T;

fn matches(&self, actual: &T) -> MatcherResult {
fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
actual: ActualRefT,
) -> MatcherResult {
self.expected.matches(&actual.as_ref().chars().count())
}

Expand All @@ -89,7 +92,10 @@ impl<T: Debug + ?Sized + AsRef<str>, E: Matcher<ActualT = usize>> Matcher for Ch
}
}

fn explain_match(&self, actual: &T) -> Description {
fn explain_match<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
actual: ActualRefT,
) -> Description {
let actual_size = actual.as_ref().chars().count();
format!(
"which has character count {}, {}",
Expand All @@ -109,6 +115,7 @@ mod tests {
use indoc::indoc;
use std::fmt::Debug;
use std::marker::PhantomData;
use std::ops::Deref;

#[test]
fn char_count_matches_string_slice() -> Result<()> {
Expand All @@ -134,15 +141,21 @@ mod tests {
impl<T: Debug> Matcher for TestMatcher<T> {
type ActualT = T;

fn matches(&self, _: &T) -> MatcherResult {
fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
_: ActualRefT,
) -> MatcherResult {
false.into()
}

fn describe(&self, _: MatcherResult) -> Description {
"called described".into()
}

fn explain_match(&self, _: &T) -> Description {
fn explain_match<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
_: ActualRefT,
) -> Description {
"called explain_match".into()
}
}
Expand Down
26 changes: 16 additions & 10 deletions googletest/src/matchers/conjunction_matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
description::Description,
matcher::{Matcher, MatcherResult},
};
use std::fmt::Debug;
use std::{fmt::Debug, ops::Deref};

/// Matcher created by [`Matcher::and`] and [`all!`].
///
Expand Down Expand Up @@ -58,25 +58,31 @@ where
{
type ActualT = M1::ActualT;

fn matches(&self, actual: &M1::ActualT) -> MatcherResult {
match (self.m1.matches(actual), self.m2.matches(actual)) {
fn matches<ActualRefT: Deref<Target = Self::ActualT> + Clone>(
&self,
actual: ActualRefT,
) -> MatcherResult {
match (self.m1.matches(actual.clone()), self.m2.matches(actual.clone())) {
(MatcherResult::Match, MatcherResult::Match) => MatcherResult::Match,
_ => MatcherResult::NoMatch,
}
}

fn explain_match(&self, actual: &M1::ActualT) -> Description {
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),
fn explain_match<ActualRefT: Deref<Target = Self::ActualT> + Clone>(
&self,
actual: ActualRefT,
) -> Description {
match (self.m1.matches(actual.clone()), self.m2.matches(actual.clone())) {
(MatcherResult::NoMatch, MatcherResult::Match) => self.m1.explain_match(actual.clone()),
(MatcherResult::Match, MatcherResult::NoMatch) => self.m2.explain_match(actual.clone()),
(_, _) => {
let m1_description = self.m1.explain_match(actual);
let m1_description = self.m1.explain_match(actual.clone());
if m1_description.is_conjunction_description() {
m1_description.nested(self.m2.explain_match(actual))
m1_description.nested(self.m2.explain_match(actual.clone()))
} else {
Description::new()
.bullet_list()
.collect([m1_description, self.m2.explain_match(actual)])
.collect([m1_description, self.m2.explain_match(actual.clone())])
.conjunction_description()
}
}
Expand Down
25 changes: 18 additions & 7 deletions googletest/src/matchers/container_eq_matcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use crate::description::Description;
use crate::matcher::{Matcher, MatcherResult};
use std::fmt::Debug;
use std::marker::PhantomData;
use std::ops::Deref;

/// Matches a container equal (in the sense of `==`) to `expected`.
///
Expand Down Expand Up @@ -116,12 +117,22 @@ where
{
type ActualT = ActualContainerT;

fn matches(&self, actual: &ActualContainerT) -> MatcherResult {
fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
actual: ActualRefT,
) -> MatcherResult {
(*actual == self.expected).into()
}

fn explain_match(&self, actual: &ActualContainerT) -> Description {
build_explanation(self.get_missing_items(actual), self.get_unexpected_items(actual)).into()
fn explain_match<ActualRefT: Deref<Target = Self::ActualT>>(
&self,
actual: ActualRefT,
) -> Description {
build_explanation(
self.get_missing_items(actual.deref()),
self.get_unexpected_items(actual.deref()),
)
.into()
}

fn describe(&self, matcher_result: MatcherResult) -> Description {
Expand Down Expand Up @@ -271,15 +282,15 @@ mod tests {
}

#[test]
fn container_eq_matches_owned_vec_of_owned_strings_with_slice_of_string_references()
-> Result<()> {
fn container_eq_matches_owned_vec_of_owned_strings_with_slice_of_string_references(
) -> Result<()> {
let vector = vec!["A string".to_string(), "Another string".to_string()];
verify_that!(vector, container_eq(["A string", "Another string"]))
}

#[test]
fn container_eq_matches_owned_vec_of_owned_strings_with_shorter_slice_of_string_references()
-> Result<()> {
fn container_eq_matches_owned_vec_of_owned_strings_with_shorter_slice_of_string_references(
) -> Result<()> {
let actual = vec!["A string".to_string(), "Another string".to_string()];
let matcher = container_eq(["A string"]);

Expand Down
Loading

0 comments on commit 2b43bd4

Please sign in to comment.