Skip to content

Commit 1a6bcf1

Browse files
hovinenbcopybara-github
authored andcommitted
Remove the dependency of elements_are! on HasSize.
HasSize is annoying because it can only be implemented for a priori known types. This change eliminates that dependency by having the matcher check whether the actual and expected values have matching sizes through inspection of the iterator used for matching rather than up front. This introduces a new type ZippedIterator, which is almost identical to std::iter::Zip except that: * It contains an additional field which indicates whether the two input iterators had different sizes, and * It otherwise only implements the bare minimum functionality required by its caller and the Iterator trait. Using this, the matcher can check whether a size mismatch was detected after the full iteration and fail the test in that case. In particular, this makes elements_are! actually work with slices, e.g., from calling as_slice() on a Vec. Previously that would fail because (1) the slice must be dereferenced when passed as an actual value to verify_that! and (2) the resulting dereferenced type cannot implement HasSize. PiperOrigin-RevId: 515585610
1 parent 008b366 commit 1a6bcf1

File tree

3 files changed

+95
-25
lines changed

3 files changed

+95
-25
lines changed

googletest/src/matchers/elements_are_matcher.rs

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,14 @@
2626
/// verify_that!(vec![1, 2, 3], elements_are![eq(1), anything(), gt(0).and(lt(123))])
2727
/// ```
2828
///
29-
/// The actual value must be a container implementing [`IntoIterator`] and
30-
/// [`HasSize`][crate::matchers::has_size::HasSize]. This includes all common
31-
/// containers in the Rust standard library.
29+
/// The actual value must be a container implementing [`IntoIterator`]. This
30+
/// includes standard containers, slices (when dereferenced) and arrays.
31+
///
32+
/// ```
33+
/// let vector = vec![1, 2, 3];
34+
/// let slice = vector.as_slice();
35+
/// verify_that!(*slice, elements_are![eq(1), anything(), gt(0).and(lt(123))])
36+
/// ```
3237
///
3338
/// This matcher does not support matching directly against an [`Iterator`]. To
3439
/// match against an iterator, use [`Iterator::collect`] to build a [`Vec`].
@@ -59,16 +64,16 @@ macro_rules! elements_are {
5964
pub mod internal {
6065
#[cfg(not(google3))]
6166
use crate as googletest;
67+
#[cfg(not(google3))]
68+
use crate::matchers::zipped_iterator::zip;
6269
#[cfg(google3)]
6370
use description::Description;
6471
use googletest::matcher::{MatchExplanation, Matcher, MatcherResult};
6572
#[cfg(not(google3))]
6673
use googletest::matchers::description::Description;
67-
#[cfg(not(google3))]
68-
use googletest::matchers::has_size::HasSize;
69-
#[cfg(google3)]
70-
use has_size::HasSize;
7174
use std::fmt::Debug;
75+
#[cfg(google3)]
76+
use zipped_iterator::zip;
7277

7378
/// This struct is meant to be used only by the macro `elements_are!`.
7479
///
@@ -88,38 +93,47 @@ pub mod internal {
8893
}
8994
}
9095

91-
impl<'a, T: Debug, ContainerT: Debug + HasSize> Matcher<ContainerT> for ElementsAre<'a, T>
96+
impl<'a, T: Debug, ContainerT: Debug + ?Sized> Matcher<ContainerT> for ElementsAre<'a, T>
9297
where
9398
for<'b> &'b ContainerT: IntoIterator<Item = &'b T>,
9499
{
95100
fn matches(&self, actual: &ContainerT) -> MatcherResult {
96-
if actual.size() != self.elements.len() {
97-
return MatcherResult::DoesNotMatch;
98-
}
99-
for (a, e) in actual.into_iter().zip(self.elements) {
101+
let mut zipped_iterator = zip(actual.into_iter(), self.elements.iter());
102+
for (a, e) in zipped_iterator.by_ref() {
100103
if matches!(e.matches(a), MatcherResult::DoesNotMatch) {
101104
return MatcherResult::DoesNotMatch;
102105
}
103106
}
104-
MatcherResult::Matches
107+
if !zipped_iterator.has_size_mismatch() {
108+
MatcherResult::Matches
109+
} else {
110+
MatcherResult::DoesNotMatch
111+
}
105112
}
106113

107114
fn explain_match(&self, actual: &ContainerT) -> MatchExplanation {
108-
if actual.size() != self.elements.len() {
109-
return MatchExplanation::create(format!("whose size is {}", actual.size(),));
115+
let actual_iterator = actual.into_iter();
116+
// TODO(b/271570144): This is a lower bound and not an actual value, so fix it
117+
// to use the real number of elements in actual.
118+
let actual_size = actual_iterator.size_hint().0;
119+
let mut zipped_iterator = zip(actual_iterator, self.elements.iter());
120+
let mut mismatches = Vec::new();
121+
for (idx, (a, e)) in zipped_iterator.by_ref().enumerate() {
122+
if matches!(e.matches(a), MatcherResult::DoesNotMatch) {
123+
mismatches.push(format!("element #{idx} is {a:?}, {}", e.explain_match(a)));
124+
}
110125
}
111-
let mismatches = actual
112-
.into_iter()
113-
.zip(self.elements)
114-
.enumerate()
115-
.filter(|&(_, (a, e))| matches!(e.matches(a), MatcherResult::DoesNotMatch))
116-
.map(|(idx, (a, e))| format!("element #{idx} is {a:?}, {}", e.explain_match(a)))
117-
.collect::<Description>();
118126
if mismatches.is_empty() {
119-
MatchExplanation::create("whose elements all match".to_string())
127+
if !zipped_iterator.has_size_mismatch() {
128+
MatchExplanation::create("whose elements all match".to_string())
129+
} else {
130+
MatchExplanation::create(format!("whose size is {}", actual_size))
131+
}
120132
} else if mismatches.len() == 1 {
133+
let mismatches = mismatches.into_iter().collect::<Description>();
121134
MatchExplanation::create(format!("where {mismatches}"))
122135
} else {
136+
let mismatches = mismatches.into_iter().collect::<Description>();
123137
MatchExplanation::create(format!("where:\n{}", mismatches.bullet_list().indent()))
124138
}
125139
}
@@ -159,8 +173,9 @@ mod tests {
159173

160174
#[google_test]
161175
fn elements_are_matches_slice() -> Result<()> {
162-
let value = &[1, 2, 3];
163-
verify_that!(*value, elements_are![eq(1), eq(2), eq(3)])
176+
let value = vec![1, 2, 3];
177+
let slice = value.as_slice();
178+
verify_that!(*slice, elements_are![eq(1), eq(2), eq(3)])
164179
}
165180

166181
#[google_test]

googletest/src/matchers/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ pub mod superset_of_matcher;
8888
pub mod tuple_matcher;
8989
#[cfg(not(google3))]
9090
pub mod unordered_elements_are_matcher;
91+
#[cfg(not(google3))]
92+
pub mod zipped_iterator;
9193

9294
#[cfg(google3)]
9395
pub use all_matcher::all;
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/// Zips up two iterators into a single iterator of pairs.
2+
///
3+
/// This is identical to [`Iterator::zip`] except that this version allows the
4+
/// caller to determine whether the two iterators had mismatching sizes using
5+
/// the method [`ZippedIterator::has_size_mismatch`].
6+
///
7+
/// [`Iterator::zip`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.zip
8+
pub(crate) fn zip<I1, I2>(left: I1, right: I2) -> ZippedIterator<I1, I2> {
9+
ZippedIterator { left, right, has_size_mismatch: false }
10+
}
11+
12+
/// An iterator over pairs of the elements of two constituent iterators, which
13+
/// keeps track of whether the two iterators have the same size.
14+
///
15+
/// This is identical to [`Zip`] except that it allows the caller to determine
16+
/// whether the two iterators had mismatching sizes using the method
17+
/// [`ZippedIterator::has_size_mismatch`].
18+
///
19+
/// [`Zip`]: https://doc.rust-lang.org/std/iter/struct.Zip.html
20+
pub(crate) struct ZippedIterator<I1, I2> {
21+
left: I1,
22+
right: I2,
23+
has_size_mismatch: bool,
24+
}
25+
26+
impl<I1, I2> ZippedIterator<I1, I2> {
27+
/// Returns whether a mismatch in the two sizes of the two iterators was
28+
/// detected during iteration.
29+
///
30+
/// This returns `true` if and only if, at some previous call to
31+
/// [`Iterator::next`] on this instance, one of the constituent iterators
32+
/// had a next element and the other did not.
33+
///
34+
/// [`Iterator::next`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#tymethod.next
35+
pub(crate) fn has_size_mismatch(&self) -> bool {
36+
self.has_size_mismatch
37+
}
38+
}
39+
40+
impl<I1: Iterator, I2: Iterator> Iterator for ZippedIterator<I1, I2> {
41+
type Item = (I1::Item, I2::Item);
42+
43+
fn next(&mut self) -> Option<(I1::Item, I2::Item)> {
44+
match (self.left.next(), self.right.next()) {
45+
(Some(v1), Some(v2)) => Some((v1, v2)),
46+
(None, None) => None,
47+
_ => {
48+
self.has_size_mismatch = true;
49+
None
50+
}
51+
}
52+
}
53+
}

0 commit comments

Comments
 (0)