Skip to content

Commit 047f101

Browse files
hovinenbcopybara-github
authored andcommitted
Fold the Describe trait into the Matcher trait.
This simplifies the implementation of matchers, since only one trait must be implemented. On the downside, testing the describe method directly can be a bit more awkward due to the additional type annotations which are needed. However, this should not be a major problem in practice outside the library itself. This change breaks API compatibility: existing matchers must be modified. In most cases it suffices to move the implementation of the Describe trait into that of the Matcher trait and remove the Describe implementation and references to it. PiperOrigin-RevId: 503969016
1 parent c9e6b57 commit 047f101

39 files changed

+105
-144
lines changed

README.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,7 @@ fn struct_has_expected_values() -> Result<()> {
131131
## Writing matchers
132132

133133
One can extend the library by writing additional matchers. To do so, create a
134-
struct holding the matcher's data and have it implement the traits [`Matcher`]
135-
and [`Describe`]:
134+
struct holding the matcher's data and have it implement the trait [`Matcher`]:
136135

137136
```rust
138137
struct MyEqMatcher<T> {
@@ -143,9 +142,7 @@ impl<T: PartialEq + Debug> Matcher<T> for MyEqMatcher<T> {
143142
fn matches(&self, actual: &A) -> MatcherResult {
144143
if self.expected == *actual { MatcherResult::Matches } else { MatcherResult::DoesNotMatch }
145144
}
146-
}
147145

148-
impl<T: Debug> Describe for MyEqMatcher<T> {
149146
fn describe(&self, matcher_result: MatcherResult) -> String {
150147
match matcher_result {
151148
MatcherResult::Matches => {

googletest/src/matcher.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ 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>: Describe {
20+
pub trait Matcher<T: Debug + ?Sized> {
2121
/// Returns whether the condition matches the datum `actual`.
2222
///
2323
/// The trait implementation defines what it means to "match". Often the
@@ -34,10 +34,7 @@ pub trait Matcher<T: Debug + ?Sized>: Describe {
3434
fn explain_match(&self, actual: &T) -> MatchExplanation {
3535
MatchExplanation::create(format!("which {}", self.describe(self.matches(actual))))
3636
}
37-
}
3837

39-
/// An interface to describe matchers, depending on `matcher_result`.
40-
pub trait Describe {
4138
/// Returns a description of `self` or a negative description if
4239
/// `matcher_result` is `DoesNotMatch`.
4340
///

googletest/src/matchers/all_matcher.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ pub mod internal {
5454
use crate as googletest;
5555
#[cfg(google3)]
5656
use anything_matcher::anything;
57-
use googletest::matcher::{Describe, MatchExplanation, Matcher, MatcherResult};
57+
use googletest::matcher::{MatchExplanation, Matcher, MatcherResult};
5858
#[cfg(not(google3))]
5959
use googletest::matchers::anything;
6060
use std::fmt::Debug;
@@ -107,9 +107,7 @@ pub mod internal {
107107
}
108108
}
109109
}
110-
}
111110

112-
impl<'a, T: Debug + ?Sized, const N: usize> Describe for AllMatcher<'a, T, N> {
113111
fn describe(&self, matcher_result: MatcherResult) -> String {
114112
match N {
115113
0 => anything::<T>().describe(matcher_result),
@@ -130,10 +128,13 @@ mod tests {
130128
use super::*;
131129
#[cfg(not(google3))]
132130
use crate as googletest;
133-
use googletest::matcher::{Describe, MatcherResult};
134131
#[cfg(not(google3))]
135132
use googletest::matchers;
136-
use googletest::{google_test, matcher::Matcher, verify_that, Result};
133+
use googletest::{
134+
google_test,
135+
matcher::{Matcher, MatcherResult},
136+
verify_that, Result,
137+
};
137138
use matchers::{contains_substring, displays_as, ends_with, eq, not, starts_with};
138139

139140
#[google_test]

googletest/src/matchers/anything_matcher.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
#[cfg(not(google3))]
1616
use crate as googletest;
17-
use googletest::matcher::{Describe, Matcher, MatcherResult};
17+
use googletest::matcher::{Matcher, MatcherResult};
1818
use std::fmt::Debug;
1919

2020
/// Matches anything. This matcher always succeeds.
@@ -36,9 +36,7 @@ impl<T: Debug + ?Sized> Matcher<T> for Anything {
3636
fn matches(&self, _: &T) -> MatcherResult {
3737
MatcherResult::Matches
3838
}
39-
}
4039

41-
impl Describe for Anything {
4240
fn describe(&self, matcher_result: MatcherResult) -> String {
4341
match matcher_result {
4442
MatcherResult::Matches => "is anything".to_string(),

googletest/src/matchers/conjunction_matcher.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
#[cfg(not(google3))]
1616
use crate as googletest;
17-
use googletest::matcher::{Describe, MatchExplanation, Matcher, MatcherResult};
17+
use googletest::matcher::{MatchExplanation, Matcher, MatcherResult};
1818
use std::fmt::Debug;
1919

2020
/// Extension trait providing the [`and`][AndMatcherExt::and] method.
@@ -68,9 +68,7 @@ impl<T: Debug, M1: Matcher<T>, M2: Matcher<T>> Matcher<T> for ConjunctionMatcher
6868
),
6969
}
7070
}
71-
}
7271

73-
impl<M1: Describe, M2: Describe> Describe for ConjunctionMatcher<M1, M2> {
7472
fn describe(&self, matcher_result: MatcherResult) -> String {
7573
format!("{}, and {}", self.m1.describe(matcher_result), self.m2.describe(matcher_result))
7674
}

googletest/src/matchers/container_eq_matcher.rs

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
#[cfg(not(google3))]
1616
use crate as googletest;
17-
use googletest::matcher::{Describe, MatchExplanation, Matcher, MatcherResult};
17+
use googletest::matcher::{MatchExplanation, Matcher, MatcherResult};
1818
use std::fmt::Debug;
1919
use std::iter::zip;
2020

@@ -92,6 +92,10 @@ where
9292
fn explain_match(&self, actual: &ContainerT) -> MatchExplanation {
9393
self.explain_match_impl(actual)
9494
}
95+
96+
fn describe(&self, matcher_result: MatcherResult) -> String {
97+
self.describe_impl(matcher_result)
98+
}
9599
}
96100

97101
impl<T: PartialEq + Debug, const N: usize> Matcher<Vec<T>> for ContainerEqMatcher<[T; N]> {
@@ -106,6 +110,10 @@ impl<T: PartialEq + Debug, const N: usize> Matcher<Vec<T>> for ContainerEqMatche
106110
fn explain_match(&self, actual: &Vec<T>) -> MatchExplanation {
107111
self.explain_match_impl(actual)
108112
}
113+
114+
fn describe(&self, matcher_result: MatcherResult) -> String {
115+
self.describe_impl(matcher_result)
116+
}
109117
}
110118

111119
impl<T: PartialEq + Debug, const N: usize> Matcher<[T]> for ContainerEqMatcher<[T; N]> {
@@ -116,6 +124,10 @@ impl<T: PartialEq + Debug, const N: usize> Matcher<[T]> for ContainerEqMatcher<[
116124
fn explain_match(&self, actual: &[T]) -> MatchExplanation {
117125
self.explain_match_impl(actual)
118126
}
127+
128+
fn describe(&self, matcher_result: MatcherResult) -> String {
129+
self.describe_impl(matcher_result)
130+
}
119131
}
120132

121133
impl<const N: usize> Matcher<Vec<String>> for ContainerEqMatcher<[&str; N]> {
@@ -137,6 +149,10 @@ impl<const N: usize> Matcher<Vec<String>> for ContainerEqMatcher<[&str; N]> {
137149
self.get_unexpected_string_items(actual),
138150
)
139151
}
152+
153+
fn describe(&self, matcher_result: MatcherResult) -> String {
154+
self.describe_impl(matcher_result)
155+
}
140156
}
141157

142158
impl<T: PartialEq + Debug, ExpectedT: Debug> ContainerEqMatcher<ExpectedT>
@@ -171,6 +187,15 @@ where
171187
}
172188
}
173189

190+
impl<ExpectedT: Debug> ContainerEqMatcher<ExpectedT> {
191+
fn describe_impl(&self, matcher_result: MatcherResult) -> String {
192+
match matcher_result {
193+
MatcherResult::Matches => format!("is equal to {:?}", self.expected),
194+
MatcherResult::DoesNotMatch => format!("isn't equal to {:?}", self.expected),
195+
}
196+
}
197+
}
198+
174199
fn build_explanation<T: Debug, U: Debug>(missing: Vec<T>, unexpected: Vec<U>) -> MatchExplanation {
175200
match (missing.len(), unexpected.len()) {
176201
// TODO(b/261175849) add more data here (out of order elements, duplicated elements, etc...)
@@ -222,15 +247,6 @@ impl<const N: usize> ContainerEqMatcher<[&str; N]> {
222247
}
223248
}
224249

225-
impl<T: Debug> Describe for ContainerEqMatcher<T> {
226-
fn describe(&self, matcher_result: MatcherResult) -> String {
227-
match matcher_result {
228-
MatcherResult::Matches => format!("is equal to {:?}", self.expected),
229-
MatcherResult::DoesNotMatch => format!("isn't equal to {:?}", self.expected),
230-
}
231-
}
232-
}
233-
234250
#[cfg(test)]
235251
mod tests {
236252
use super::*;

googletest/src/matchers/contains_matcher.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
#[cfg(not(google3))]
1616
use crate as googletest;
17-
use googletest::matcher::{Describe, MatchExplanation, Matcher, MatcherResult};
17+
use googletest::matcher::{MatchExplanation, Matcher, MatcherResult};
1818
use std::fmt::Debug;
1919

2020
/// Matches an iterable type whose elements contain a value matched by `inner`.
@@ -97,9 +97,7 @@ where
9797
(_, None) => MatchExplanation::create("which contains a matching element".to_string()),
9898
}
9999
}
100-
}
101100

102-
impl<InnerMatcherT: Describe> Describe for ContainsMatcher<InnerMatcherT> {
103101
fn describe(&self, matcher_result: MatcherResult) -> String {
104102
match (matcher_result, &self.count) {
105103
(MatcherResult::Matches, Some(count)) => format!(
@@ -146,7 +144,7 @@ mod tests {
146144
use crate as googletest;
147145
#[cfg(not(google3))]
148146
use googletest::matchers;
149-
use googletest::{google_test, verify_that, Result};
147+
use googletest::{google_test, matcher::Matcher, verify_that, Result};
150148
use matchers::{displays_as, eq};
151149

152150
#[google_test]
@@ -226,7 +224,7 @@ mod tests {
226224
let matcher = contains(eq(1));
227225

228226
verify_that!(
229-
matcher.describe(MatcherResult::Matches),
227+
<ContainsMatcher<_> as Matcher<Vec<i32>>>::describe(&matcher, MatcherResult::Matches),
230228
eq("contains at least one element which is equal to 1")
231229
)
232230
}
@@ -236,7 +234,7 @@ mod tests {
236234
let matcher = contains(eq(1)).times(eq(2));
237235

238236
verify_that!(
239-
matcher.describe(MatcherResult::Matches),
237+
<ContainsMatcher<_> as Matcher<Vec<i32>>>::describe(&matcher, MatcherResult::Matches),
240238
eq("contains n elements which is equal to 1\n where n is equal to 2")
241239
)
242240
}

googletest/src/matchers/contains_regex_matcher.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
#[cfg(not(google3))]
1616
use crate as googletest;
17-
use googletest::matcher::{Describe, Matcher, MatcherResult};
17+
use googletest::matcher::{Matcher, MatcherResult};
1818
use regex::Regex;
1919
use std::fmt::Debug;
2020
use std::ops::Deref;
@@ -60,9 +60,7 @@ impl<ActualT: AsRef<str> + Debug + ?Sized> Matcher<ActualT> for ContainsRegexMat
6060
MatcherResult::DoesNotMatch
6161
}
6262
}
63-
}
6463

65-
impl Describe for ContainsRegexMatcher {
6664
fn describe(&self, matcher_result: MatcherResult) -> String {
6765
match matcher_result {
6866
MatcherResult::Matches => {
@@ -82,7 +80,7 @@ mod tests {
8280
use crate as googletest;
8381
#[cfg(not(google3))]
8482
use googletest::matchers;
85-
use googletest::{google_test, verify_that, Result};
83+
use googletest::{google_test, matcher::Matcher, verify_that, Result};
8684
use matchers::eq;
8785

8886
#[google_test]
@@ -131,7 +129,7 @@ mod tests {
131129
let matcher = contains_regex("\n");
132130

133131
verify_that!(
134-
matcher.describe(MatcherResult::Matches),
132+
<ContainsRegexMatcher as Matcher<&str>>::describe(&matcher, MatcherResult::Matches),
135133
eq("contains the regular expression \"\\n\"")
136134
)
137135
}

googletest/src/matchers/contains_substring_matcher.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
#[cfg(not(google3))]
1616
use crate as googletest;
17-
use googletest::matcher::{Describe, Matcher, MatcherResult};
17+
use googletest::matcher::{Matcher, MatcherResult};
1818
use std::fmt::Debug;
1919
use std::ops::Deref;
2020

@@ -60,9 +60,7 @@ where
6060
MatcherResult::DoesNotMatch
6161
}
6262
}
63-
}
6463

65-
impl<SubstringT: Deref<Target = str>> Describe for ContainsSubstringMatcher<SubstringT> {
6664
fn describe(&self, matcher_result: MatcherResult) -> String {
6765
match matcher_result {
6866
MatcherResult::Matches => format!("contains substring {:#?}", self.substring.deref()),
@@ -128,6 +126,12 @@ mod tests {
128126
fn contains_substring_displays_quoted_debug_of_substring() -> Result<()> {
129127
let matcher = contains_substring("\n");
130128

131-
verify_that!(matcher.describe(MatcherResult::Matches), eq("contains substring \"\\n\""))
129+
verify_that!(
130+
<ContainsSubstringMatcher<&str> as Matcher<&str>>::describe(
131+
&matcher,
132+
MatcherResult::Matches
133+
),
134+
eq("contains substring \"\\n\"")
135+
)
132136
}
133137
}

googletest/src/matchers/disjunction_matcher.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
#[cfg(not(google3))]
1616
use crate as googletest;
17-
use googletest::matcher::{Describe, MatchExplanation, Matcher, MatcherResult};
17+
use googletest::matcher::{MatchExplanation, Matcher, MatcherResult};
1818
use std::fmt::Debug;
1919

2020
/// Extension trait providing the [`or`][OrMatcherExt::or] method.
@@ -66,9 +66,7 @@ impl<T: Debug, M1: Matcher<T>, M2: Matcher<T>> Matcher<T> for DisjunctionMatcher
6666
self.m2.explain_match(actual)
6767
))
6868
}
69-
}
7069

71-
impl<M1: Describe, M2: Describe> Describe for DisjunctionMatcher<M1, M2> {
7270
fn describe(&self, matcher_result: MatcherResult) -> String {
7371
format!("{}, or {}", self.m1.describe(matcher_result), self.m2.describe(matcher_result))
7472
}

0 commit comments

Comments
 (0)