Skip to content

Commit ab303ae

Browse files
hovinenbcopybara-github
authored andcommitted
Replace the type parameter on Matcher with an associated type.
This has the immediate effect that the extension traits `AndMatcherExt` and `OrMatcherExt` now have no trouble working with strings. Namely, this change forces the matcher struct itself always to reference the actual type against which it matcher, either via parameterisation or by only matching against a concrete type. Because of this, the matcher declaration always carries the necessary type information so that the compiler can perform type inference. This change introduces unit tests which demonstrate this. This same property has the drawback that matchers must frequently carry a `PhantomData` of the type against which they match. This is annoying, but not fatal. This is of course an API-incompatible change. Downstream targets which build their own matchers will have to change the type parameter to be an associated type, and may need to make additional adjustments as outlined above. This commit shows several such examples. This also has the effect that it is no longer possible to define a single matcher class and implement `Matcher` against several actual types with it. Only one matcher in GoogleTest -- `ContainerEqMatcher` -- did this, and a previous change modified it to have only one `Matcher` implementation with no loss of functionality. Existing tests did not have to be changed, except for tests of some internals of matchers due to direct reference to the `Matcher` type. This suggests that the change will not break downstream _uses_ of matchers. In the future, this should also make it possible to associate a lifetime with the actual value in such a way as to allow the property matcher to work with extractors which return data referencing the containing struct. Most likely that would require modifying the associated type to have a lifetime parameter. Currently that unfortunately does not work for us since some matchers use trait objects and generic associated types are not yet object-safe (see https://blog.rust-lang.org/2022/10/28/gats-stabilization.html#traits-with-gats-are-not-object-safe). This may change in the future. PiperOrigin-RevId: 527884316
1 parent 1d99a84 commit ab303ae

40 files changed

+607
-328
lines changed

README.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,10 @@ struct MyEqMatcher<T> {
138138
expected: T,
139139
}
140140

141-
impl<T: PartialEq + Debug> Matcher<T> for MyEqMatcher<T> {
142-
fn matches(&self, actual: &A) -> MatcherResult {
141+
impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
142+
type ActualT = T;
143+
144+
fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
143145
(self.expected == *actual).into()
144146
}
145147

@@ -159,7 +161,7 @@ impl<T: PartialEq + Debug> Matcher<T> for MyEqMatcher<T> {
159161
It is recommended to expose a function which constructs the matcher:
160162

161163
```rust
162-
pub fn eq_my_way<T: PartialEq + Debug>(expected: T) -> impl Matcher<T> {
164+
pub fn eq_my_way<T: PartialEq + Debug>(expected: T) -> impl Matcher<ActualT = T> {
163165
MyEqMatcher { expected }
164166
}
165167
```

googletest/src/assertions.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ pub mod internal {
332332
#[must_use = "The assertion result must be evaluated to affect the test result."]
333333
pub fn check_matcher<T: Debug + ?Sized>(
334334
actual: &T,
335-
expected: impl Matcher<T>,
335+
expected: impl Matcher<ActualT = T>,
336336
actual_expr: &'static str,
337337
source_location: SourceLocation,
338338
) -> Result<(), TestAssertionFailure> {

googletest/src/lib.rs

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,10 @@
190190
//! expected: T,
191191
//! }
192192
//!
193-
//! impl<T: PartialEq + Debug> Matcher<T> for MyEqMatcher<T> {
194-
//! fn matches(&self, actual: &T) -> MatcherResult {
193+
//! impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
194+
//! type ActualT = T;
195+
//!
196+
//! fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
195197
//! if self.expected == *actual {
196198
//! MatcherResult::Matches
197199
//! } else {
@@ -222,8 +224,10 @@
222224
//! # expected: T,
223225
//! # }
224226
//! #
225-
//! # impl<T: PartialEq + Debug> Matcher<T> for MyEqMatcher<T> {
226-
//! # fn matches(&self, actual: &T) -> MatcherResult {
227+
//! # impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
228+
//! # type ActualT = T;
229+
//! #
230+
//! # fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
227231
//! # if self.expected == *actual {
228232
//! # MatcherResult::Matches
229233
//! # } else {
@@ -243,7 +247,7 @@
243247
//! # }
244248
//! # }
245249
//! #
246-
//! pub fn eq_my_way<T: PartialEq + Debug>(expected: T) -> impl Matcher<T> {
250+
//! pub fn eq_my_way<T: PartialEq + Debug>(expected: T) -> impl Matcher<ActualT = T> {
247251
//! MyEqMatcher { expected }
248252
//! }
249253
//! ```
@@ -258,8 +262,10 @@
258262
//! # expected: T,
259263
//! # }
260264
//! #
261-
//! # impl<T: PartialEq + Debug> Matcher<T> for MyEqMatcher<T> {
262-
//! # fn matches(&self, actual: &T) -> MatcherResult {
265+
//! # impl<T: PartialEq + Debug> Matcher for MyEqMatcher<T> {
266+
//! # type ActualT = T;
267+
//! #
268+
//! # fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
263269
//! # if self.expected == *actual {
264270
//! # MatcherResult::Matches
265271
//! # } else {
@@ -279,7 +285,7 @@
279285
//! # }
280286
//! # }
281287
//! #
282-
//! # pub fn eq_my_way<T: PartialEq + Debug>(expected: T) -> impl Matcher<T> {
288+
//! # pub fn eq_my_way<T: PartialEq + Debug>(expected: T) -> impl Matcher<ActualT = T> {
283289
//! # MyEqMatcher { expected }
284290
//! # }
285291
//! # /* The attribute macro would prevent the function from being compiled in a doctest.

googletest/src/matcher.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,17 @@ use crate::internal::test_outcome::TestAssertionFailure;
1717
use std::fmt::{Debug, Display, Formatter, Result};
1818

1919
/// An interface for checking an arbitrary condition on a datum.
20-
pub trait Matcher<T: Debug + ?Sized> {
20+
pub trait Matcher {
21+
/// The type against which this matcher matches.
22+
type ActualT: Debug + ?Sized;
23+
2124
/// Returns whether the condition matches the datum `actual`.
2225
///
2326
/// The trait implementation defines what it means to "match". Often the
2427
/// matching condition is based on data stored in the matcher. For example,
2528
/// `eq` matches when its stored expected value is equal (in the sense of
2629
/// the `==` operator) to the value `actual`.
27-
fn matches(&self, actual: &T) -> MatcherResult;
30+
fn matches(&self, actual: &Self::ActualT) -> MatcherResult;
2831

2932
/// Returns a description of `self` or a negative description if
3033
/// `matcher_result` is `DoesNotMatch`.
@@ -107,7 +110,7 @@ pub trait Matcher<T: Debug + ?Sized> {
107110
/// inner matcher and appears as follows:
108111
///
109112
/// ```ignore
110-
/// fn explain_match(&self, actual: &ActualT) -> MatchExplanation {
113+
/// fn explain_match(&self, actual: &Self::ActualT) -> MatchExplanation {
111114
/// self.expected.explain_match(actual.deref())
112115
/// }
113116
/// ```
@@ -117,14 +120,14 @@ pub trait Matcher<T: Debug + ?Sized> {
117120
/// inner matcher at a point where a relative clause would fit. For example:
118121
///
119122
/// ```ignore
120-
/// fn explain_match(&self, actual: &ActualT) -> MatchExplanation {
123+
/// fn explain_match(&self, actual: &Self::ActualT) -> MatchExplanation {
121124
/// MatchExplanation::create(
122125
/// format!("which points to a value {}", self.expected.explain_match(actual.deref()))
123126
/// // ^^^^^^^^^^^^^^^^^^^^ Expands to "points to a value which ..."
124127
/// )
125128
/// }
126129
/// ```
127-
fn explain_match(&self, actual: &T) -> MatchExplanation {
130+
fn explain_match(&self, actual: &Self::ActualT) -> MatchExplanation {
128131
MatchExplanation::create(format!("which {}", self.describe(self.matches(actual))))
129132
}
130133
}
@@ -135,7 +138,7 @@ pub trait Matcher<T: Debug + ?Sized> {
135138
/// The parameter `actual_expr` contains the expression which was evaluated to
136139
/// obtain `actual`.
137140
pub(crate) fn create_assertion_failure<T: Debug + ?Sized>(
138-
matcher: &impl Matcher<T>,
141+
matcher: &impl Matcher<ActualT = T>,
139142
actual: &T,
140143
actual_expr: &'static str,
141144
source_location: SourceLocation,

googletest/src/matchers/all_matcher.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,20 +87,22 @@ pub mod internal {
8787
/// For internal use only. API stablility is not guaranteed!
8888
#[doc(hidden)]
8989
pub struct AllMatcher<'a, T: Debug + ?Sized, const N: usize> {
90-
components: [&'a dyn Matcher<T>; N],
90+
components: [&'a dyn Matcher<ActualT = T>; N],
9191
}
9292

9393
impl<'a, T: Debug + ?Sized, const N: usize> AllMatcher<'a, T, N> {
9494
/// Constructs an [`AllMatcher`] with the given component matchers.
9595
///
9696
/// Intended for use only by the [`all`] macro.
97-
pub fn new(components: [&'a dyn Matcher<T>; N]) -> Self {
97+
pub fn new(components: [&'a dyn Matcher<ActualT = T>; N]) -> Self {
9898
Self { components }
9999
}
100100
}
101101

102-
impl<'a, T: Debug + ?Sized, const N: usize> Matcher<T> for AllMatcher<'a, T, N> {
103-
fn matches(&self, actual: &T) -> MatcherResult {
102+
impl<'a, T: Debug + ?Sized, const N: usize> Matcher for AllMatcher<'a, T, N> {
103+
type ActualT = T;
104+
105+
fn matches(&self, actual: &Self::ActualT) -> MatcherResult {
104106
for component in self.components {
105107
match component.matches(actual) {
106108
MatcherResult::DoesNotMatch => {
@@ -112,7 +114,7 @@ pub mod internal {
112114
MatcherResult::Matches
113115
}
114116

115-
fn explain_match(&self, actual: &T) -> MatchExplanation {
117+
fn explain_match(&self, actual: &Self::ActualT) -> MatchExplanation {
116118
match N {
117119
0 => anything::<T>().explain_match(actual),
118120
1 => self.components[0].explain_match(actual),

googletest/src/matchers/anything_matcher.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
use crate::matcher::{Matcher, MatcherResult};
1616
#[cfg(google3)]
1717
use googletest::*;
18-
use std::fmt::Debug;
18+
use std::{fmt::Debug, marker::PhantomData};
1919

2020
/// Matches anything. This matcher always succeeds.
2121
///
@@ -31,13 +31,15 @@ use std::fmt::Debug;
3131
/// # }
3232
/// # should_pass().unwrap();
3333
/// ```
34-
pub fn anything<T: Debug + ?Sized>() -> impl Matcher<T> {
35-
Anything {}
34+
pub fn anything<T: Debug + ?Sized>() -> impl Matcher<ActualT = T> {
35+
Anything::<T>(Default::default())
3636
}
3737

38-
struct Anything {}
38+
struct Anything<T: ?Sized>(PhantomData<T>);
39+
40+
impl<T: Debug + ?Sized> Matcher for Anything<T> {
41+
type ActualT = T;
3942

40-
impl<T: Debug + ?Sized> Matcher<T> for Anything {
4143
fn matches(&self, _: &T) -> MatcherResult {
4244
MatcherResult::Matches
4345
}

googletest/src/matchers/conjunction_matcher.rs

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@
1515
use crate::matcher::{MatchExplanation, Matcher, MatcherResult};
1616
#[cfg(google3)]
1717
use googletest::*;
18-
use std::fmt::Debug;
18+
use std::{fmt::Debug, marker::PhantomData};
1919

2020
/// Extension trait providing the [`and`][AndMatcherExt::and] method.
21-
pub trait AndMatcherExt<T: Debug>: Matcher<T> {
21+
pub trait AndMatcherExt<T: Debug>: Matcher<ActualT = T> {
2222
/// Constructs a matcher that matches both `self` and `right`.
2323
///
2424
/// ```
@@ -34,26 +34,31 @@ pub trait AndMatcherExt<T: Debug>: Matcher<T> {
3434
// TODO(b/264518763): Replace the return type with impl Matcher and reduce
3535
// visibility of ConjunctionMatcher once impl in return position in trait
3636
// methods is stable.
37-
fn and<Right: Matcher<T>>(self, right: Right) -> ConjunctionMatcher<Self, Right>
37+
fn and<Right: Matcher<ActualT = T>>(self, right: Right) -> ConjunctionMatcher<T, Self, Right>
3838
where
3939
Self: Sized,
4040
{
41-
ConjunctionMatcher { m1: self, m2: right }
41+
ConjunctionMatcher { m1: self, m2: right, phantom: Default::default() }
4242
}
4343
}
4444

45-
impl<T: Debug, M> AndMatcherExt<T> for M where M: Matcher<T> {}
45+
impl<T: Debug, M> AndMatcherExt<T> for M where M: Matcher<ActualT = T> {}
4646

4747
/// Matcher created by [`AndMatcherExt::and`].
4848
///
4949
/// **For internal use only. API stablility is not guaranteed!**
5050
#[doc(hidden)]
51-
pub struct ConjunctionMatcher<M1, M2> {
51+
pub struct ConjunctionMatcher<T, M1, M2> {
5252
m1: M1,
5353
m2: M2,
54+
phantom: PhantomData<T>,
5455
}
5556

56-
impl<T: Debug, M1: Matcher<T>, M2: Matcher<T>> Matcher<T> for ConjunctionMatcher<M1, M2> {
57+
impl<T: Debug, M1: Matcher<ActualT = T>, M2: Matcher<ActualT = T>> Matcher
58+
for ConjunctionMatcher<T, M1, M2>
59+
{
60+
type ActualT = T;
61+
5762
fn matches(&self, actual: &T) -> MatcherResult {
5863
match (self.m1.matches(actual), self.m2.matches(actual)) {
5964
(MatcherResult::Matches, MatcherResult::Matches) => MatcherResult::Matches,
@@ -89,7 +94,9 @@ mod tests {
8994
use crate::{verify_that, Result};
9095
#[cfg(google3)]
9196
use matchers::field;
92-
use matchers::{anything, contains_substring, displays_as, eq, err, not};
97+
use matchers::{
98+
anything, contains_substring, displays_as, ends_with, eq, err, not, starts_with,
99+
};
93100

94101
#[test]
95102
fn and_true_true_matches() -> Result<()> {
@@ -149,4 +156,14 @@ mod tests {
149156
field!(Struct.a, eq(1)).and(field!(Struct.b, eq(2))).and(field!(Struct.c, eq(3)))
150157
)
151158
}
159+
160+
#[test]
161+
fn works_with_str_slices() -> Result<()> {
162+
verify_that!("A string", starts_with("A").and(ends_with("string")))
163+
}
164+
165+
#[test]
166+
fn works_with_owned_strings() -> Result<()> {
167+
verify_that!("A string".to_string(), starts_with("A").and(ends_with("string")))
168+
}
152169
}

googletest/src/matchers/container_eq_matcher.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,8 +100,8 @@ pub struct ContainerEqMatcher<ActualContainerT: ?Sized, ExpectedContainerT> {
100100
phantom: PhantomData<ActualContainerT>,
101101
}
102102

103-
impl<ActualElementT, ActualContainerT, ExpectedElementT, ExpectedContainerT>
104-
Matcher<ActualContainerT> for ContainerEqMatcher<ActualContainerT, ExpectedContainerT>
103+
impl<ActualElementT, ActualContainerT, ExpectedElementT, ExpectedContainerT> Matcher
104+
for ContainerEqMatcher<ActualContainerT, ExpectedContainerT>
105105
where
106106
ActualElementT: PartialEq<ExpectedElementT> + Debug + ?Sized,
107107
ActualContainerT: PartialEq<ExpectedContainerT> + Debug + ?Sized,
@@ -110,6 +110,8 @@ where
110110
for<'a> &'a ActualContainerT: IntoIterator<Item = &'a ActualElementT>,
111111
for<'a> &'a ExpectedContainerT: IntoIterator<Item = &'a ExpectedElementT>,
112112
{
113+
type ActualT = ActualContainerT;
114+
113115
fn matches(&self, actual: &ActualContainerT) -> MatcherResult {
114116
(*actual == self.expected).into()
115117
}
@@ -265,7 +267,7 @@ Actual: [
265267
}
266268

267269
#[test]
268-
fn container_eq_matches_owned_vec_with_slice() -> Result<()> {
270+
fn container_eq_matches_owned_vec_with_array() -> Result<()> {
269271
let vector = vec![123, 234];
270272
verify_that!(vector, container_eq([123, 234]))
271273
}

0 commit comments

Comments
 (0)