Skip to content

Commit 1e0b74e

Browse files
committed
Parameterize Matcher::matches and Matcher::explain_match.
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.
1 parent e808aad commit 1e0b74e

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+580
-244
lines changed

googletest/crate_docs.md

+15-6
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ a struct holding the matcher's data and have it implement the trait
214214

215215
```no_run
216216
use googletest::{description::Description, matcher::{Matcher, MatcherResult}};
217-
use std::fmt::Debug;
217+
use std::{fmt::Debug, ops::Deref};
218218
219219
struct MyEqMatcher<T> {
220220
expected: T,
@@ -223,7 +223,10 @@ struct MyEqMatcher<T> {
223223
impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
224224
type ActualT = T;
225225
226-
fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
226+
fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
227+
&self,
228+
actual: ActualRefT,
229+
) -> MatcherResult {
227230
if self.expected == *actual {
228231
MatcherResult::Match
229232
} else {
@@ -248,7 +251,7 @@ impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
248251

249252
```no_run
250253
# use googletest::{description::Description, matcher::{Matcher, MatcherResult}};
251-
# use std::fmt::Debug;
254+
# use std::{fmt::Debug, ops::Deref};
252255
#
253256
# struct MyEqMatcher<T> {
254257
# expected: T,
@@ -257,7 +260,10 @@ impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
257260
# impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
258261
# type ActualT = T;
259262
#
260-
# fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
263+
# fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
264+
# &self,
265+
# actual: ActualRefT,
266+
# ) -> MatcherResult {
261267
# if self.expected == *actual {
262268
# MatcherResult::Match
263269
# } else {
@@ -287,7 +293,7 @@ impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
287293
```
288294
# use googletest::prelude::*;
289295
# use googletest::{description::Description, matcher::{Matcher, MatcherResult}};
290-
# use std::fmt::Debug;
296+
# use std::{fmt::Debug, ops::Deref};
291297
#
292298
# struct MyEqMatcher<T> {
293299
# expected: T,
@@ -296,7 +302,10 @@ impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
296302
# impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
297303
# type ActualT = T;
298304
#
299-
# fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
305+
# fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
306+
# &self,
307+
# actual: ActualRefT,
308+
# ) -> MatcherResult {
300309
# if self.expected == *actual {
301310
# MatcherResult::Match
302311
# } else {

googletest/src/matcher.rs

+54-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use crate::internal::test_outcome::TestAssertionFailure;
2020
use crate::matchers::__internal_unstable_do_not_depend_on_these::ConjunctionMatcher;
2121
use crate::matchers::__internal_unstable_do_not_depend_on_these::DisjunctionMatcher;
2222
use std::fmt::Debug;
23+
use std::ops::Deref;
2324

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

3741
/// Returns a description of `self` or a negative description if
3842
/// `matcher_result` is `DoesNotMatch`.
@@ -133,7 +137,10 @@ pub trait Matcher {
133137
/// .nested(self.expected.explain_match(actual.deref()))
134138
/// }
135139
/// ```
136-
fn explain_match(&self, actual: &Self::ActualT) -> Description {
140+
fn explain_match<ActualRefT: Deref<Target = Self::ActualT> + Clone>(
141+
&self,
142+
actual: ActualRefT,
143+
) -> Description {
137144
format!("which {}", self.describe(self.matches(actual))).into()
138145
}
139146

@@ -201,6 +208,46 @@ pub trait Matcher {
201208
}
202209
}
203210

211+
/// Functionality used by macros within this crate.
212+
///
213+
/// For internal use only. API stablility is not guaranteed!
214+
#[doc(hidden)]
215+
pub mod __internal_unstable_do_not_depend_on_these {
216+
use super::{Matcher, MatcherResult};
217+
use crate::description::Description;
218+
use std::fmt::Debug;
219+
220+
/// A variant of [`Matcher`] which is object-safe.
221+
///
222+
/// This is used in contexts where a `dyn Matcher` is required. It supplies all methods of
223+
/// [`Matcher`] but without any generics on the methods.
224+
pub trait ObjectSafeMatcher {
225+
type ActualT: Debug + ?Sized;
226+
227+
fn obj_matches(&self, actual: &Self::ActualT) -> MatcherResult;
228+
229+
fn obj_describe(&self, matcher_result: MatcherResult) -> Description;
230+
231+
fn obj_explain_match(&self, actual: &Self::ActualT) -> Description;
232+
}
233+
234+
impl<MatcherT: Matcher> ObjectSafeMatcher for MatcherT {
235+
type ActualT = <Self as Matcher>::ActualT;
236+
237+
fn obj_matches(&self, actual: &Self::ActualT) -> MatcherResult {
238+
Matcher::matches(self, actual)
239+
}
240+
241+
fn obj_describe(&self, matcher_result: MatcherResult) -> Description {
242+
Matcher::describe(self, matcher_result)
243+
}
244+
245+
fn obj_explain_match(&self, actual: &Self::ActualT) -> Description {
246+
Matcher::explain_match(self, actual)
247+
}
248+
}
249+
}
250+
204251
/// Any actual value whose debug length is greater than this value will be
205252
/// pretty-printed. Otherwise, it will have normal debug output formatting.
206253
const PRETTY_PRINT_LENGTH_THRESHOLD: usize = 60;
@@ -245,7 +292,11 @@ pub enum MatcherResult {
245292

246293
impl From<bool> for MatcherResult {
247294
fn from(b: bool) -> Self {
248-
if b { MatcherResult::Match } else { MatcherResult::NoMatch }
295+
if b {
296+
MatcherResult::Match
297+
} else {
298+
MatcherResult::NoMatch
299+
}
249300
}
250301
}
251302

googletest/src/matchers/all_matcher.rs

+23-14
Original file line numberDiff line numberDiff line change
@@ -65,34 +65,40 @@ macro_rules! __all {
6565
#[doc(hidden)]
6666
pub mod internal {
6767
use crate::description::Description;
68-
use crate::matcher::{Matcher, MatcherResult};
68+
use crate::matcher::{
69+
Matcher, MatcherResult, __internal_unstable_do_not_depend_on_these::ObjectSafeMatcher,
70+
};
6971
use crate::matchers::anything;
7072
use std::fmt::Debug;
73+
use std::ops::Deref;
7174

7275
/// A matcher which matches an input value matched by all matchers in the
7376
/// array `components`.
7477
///
7578
/// For internal use only. API stablility is not guaranteed!
7679
#[doc(hidden)]
7780
pub struct AllMatcher<'a, T: Debug + ?Sized, const N: usize> {
78-
components: [Box<dyn Matcher<ActualT = T> + 'a>; N],
81+
components: [Box<dyn ObjectSafeMatcher<ActualT = T> + 'a>; N],
7982
}
8083

8184
impl<'a, T: Debug + ?Sized, const N: usize> AllMatcher<'a, T, N> {
8285
/// Constructs an [`AllMatcher`] with the given component matchers.
8386
///
8487
/// Intended for use only by the [`all`] macro.
85-
pub fn new(components: [Box<dyn Matcher<ActualT = T> + 'a>; N]) -> Self {
88+
pub fn new(components: [Box<dyn ObjectSafeMatcher<ActualT = T> + 'a>; N]) -> Self {
8689
Self { components }
8790
}
8891
}
8992

9093
impl<'a, T: Debug + ?Sized, const N: usize> Matcher for AllMatcher<'a, T, N> {
9194
type ActualT = T;
9295

93-
fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
96+
fn matches<ActualRefT: Deref<Target = Self::ActualT> + Clone>(
97+
&self,
98+
actual: ActualRefT,
99+
) -> MatcherResult {
94100
for component in &self.components {
95-
match component.matches(actual) {
101+
match component.obj_matches(actual.deref()) {
96102
MatcherResult::NoMatch => {
97103
return MatcherResult::NoMatch;
98104
}
@@ -102,25 +108,28 @@ pub mod internal {
102108
MatcherResult::Match
103109
}
104110

105-
fn explain_match(&self, actual: &Self::ActualT) -> Description {
111+
fn explain_match<ActualRefT: Deref<Target = Self::ActualT> + Clone>(
112+
&self,
113+
actual: ActualRefT,
114+
) -> Description {
106115
match N {
107116
0 => anything::<T>().explain_match(actual),
108-
1 => self.components[0].explain_match(actual),
117+
1 => self.components[0].obj_explain_match(actual.deref()),
109118
_ => {
110119
let failures = self
111120
.components
112121
.iter()
113-
.filter(|component| component.matches(actual).is_no_match())
122+
.filter(|component| component.obj_matches(actual.deref()).is_no_match())
114123
.collect::<Vec<_>>();
115124

116125
if failures.len() == 1 {
117-
failures[0].explain_match(actual)
126+
failures[0].obj_explain_match(actual.deref())
118127
} else {
119128
Description::new()
120129
.collect(
121130
failures
122131
.into_iter()
123-
.map(|component| component.explain_match(actual)),
132+
.map(|component| component.obj_explain_match(actual.deref())),
124133
)
125134
.bullet_list()
126135
}
@@ -131,17 +140,17 @@ pub mod internal {
131140
fn describe(&self, matcher_result: MatcherResult) -> Description {
132141
match N {
133142
0 => anything::<T>().describe(matcher_result),
134-
1 => self.components[0].describe(matcher_result),
143+
1 => self.components[0].obj_describe(matcher_result),
135144
_ => {
136145
let header = if matcher_result.into() {
137146
"has all the following properties:"
138147
} else {
139148
"has at least one of the following properties:"
140149
};
141150
Description::new().text(header).nested(
142-
Description::new()
143-
.bullet_list()
144-
.collect(self.components.iter().map(|m| m.describe(matcher_result))),
151+
Description::new().bullet_list().collect(
152+
self.components.iter().map(|m| m.obj_describe(matcher_result)),
153+
),
145154
)
146155
}
147156
}

googletest/src/matchers/any_matcher.rs

+23-12
Original file line numberDiff line numberDiff line change
@@ -67,54 +67,65 @@ macro_rules! __any {
6767
#[doc(hidden)]
6868
pub mod internal {
6969
use crate::description::Description;
70-
use crate::matcher::{Matcher, MatcherResult};
70+
use crate::matcher::{
71+
Matcher, MatcherResult, __internal_unstable_do_not_depend_on_these::ObjectSafeMatcher,
72+
};
7173
use crate::matchers::anything;
7274
use std::fmt::Debug;
75+
use std::ops::Deref;
7376

7477
/// A matcher which matches an input value matched by all matchers in the
7578
/// array `components`.
7679
///
7780
/// For internal use only. API stablility is not guaranteed!
7881
#[doc(hidden)]
7982
pub struct AnyMatcher<'a, T: Debug + ?Sized, const N: usize> {
80-
components: [Box<dyn Matcher<ActualT = T> + 'a>; N],
83+
components: [Box<dyn ObjectSafeMatcher<ActualT = T> + 'a>; N],
8184
}
8285

8386
impl<'a, T: Debug + ?Sized, const N: usize> AnyMatcher<'a, T, N> {
8487
/// Constructs an [`AnyMatcher`] with the given component matchers.
8588
///
8689
/// Intended for use only by the [`all`] macro.
87-
pub fn new(components: [Box<dyn Matcher<ActualT = T> + 'a>; N]) -> Self {
90+
pub fn new(components: [Box<dyn ObjectSafeMatcher<ActualT = T> + 'a>; N]) -> Self {
8891
Self { components }
8992
}
9093
}
9194

9295
impl<'a, T: Debug + ?Sized, const N: usize> Matcher for AnyMatcher<'a, T, N> {
9396
type ActualT = T;
9497

95-
fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
96-
MatcherResult::from(self.components.iter().any(|c| c.matches(actual).is_match()))
98+
fn matches<ActualRefT: Deref<Target = Self::ActualT>>(
99+
&self,
100+
actual: ActualRefT,
101+
) -> MatcherResult {
102+
MatcherResult::from(
103+
self.components.iter().any(|c| c.obj_matches(actual.deref()).is_match()),
104+
)
97105
}
98106

99-
fn explain_match(&self, actual: &Self::ActualT) -> Description {
107+
fn explain_match<ActualRefT: Deref<Target = Self::ActualT>>(
108+
&self,
109+
actual: ActualRefT,
110+
) -> Description {
100111
match N {
101112
0 => format!("which {}", anything::<T>().describe(MatcherResult::NoMatch)).into(),
102-
1 => self.components[0].explain_match(actual),
113+
1 => self.components[0].obj_explain_match(actual.deref()),
103114
_ => {
104115
let failures = self
105116
.components
106117
.iter()
107-
.filter(|component| component.matches(actual).is_no_match())
118+
.filter(|component| component.obj_matches(actual.deref()).is_no_match())
108119
.collect::<Vec<_>>();
109120

110121
if failures.len() == 1 {
111-
failures[0].explain_match(actual)
122+
failures[0].obj_explain_match(actual.deref())
112123
} else {
113124
Description::new()
114125
.collect(
115126
failures
116127
.into_iter()
117-
.map(|component| component.explain_match(actual)),
128+
.map(|component| component.obj_explain_match(actual.deref())),
118129
)
119130
.bullet_list()
120131
}
@@ -125,12 +136,12 @@ pub mod internal {
125136
fn describe(&self, matcher_result: MatcherResult) -> Description {
126137
match N {
127138
0 => anything::<T>().describe(matcher_result),
128-
1 => self.components[0].describe(matcher_result),
139+
1 => self.components[0].obj_describe(matcher_result),
129140
_ => {
130141
let properties = self
131142
.components
132143
.iter()
133-
.map(|m| m.describe(matcher_result))
144+
.map(|m| m.obj_describe(matcher_result))
134145
.collect::<Description>()
135146
.bullet_list()
136147
.indent();

googletest/src/matchers/anything_matcher.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::{
1616
description::Description,
1717
matcher::{Matcher, MatcherResult},
1818
};
19-
use std::{fmt::Debug, marker::PhantomData};
19+
use std::{fmt::Debug, marker::PhantomData, ops::Deref};
2020

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

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

0 commit comments

Comments
 (0)