Skip to content

Commit

Permalink
Make matches_pattern for braced structs support exhaustiveness with…
Browse files Browse the repository at this point in the history
… generic structs.

Previously, for field exhaustiveness, we emitted an isolated `Foo { a: _, b: _ }` divorced from the `actual` value, but if `Foo`'s `a` field is generic, we're not able to infer it at all. So instead, put the `match` within a `predicate` matcher and match against the `actual` value to be able to infer the field.

Toward #447

PiperOrigin-RevId: 704716108
  • Loading branch information
marcianx authored and copybara-github committed Dec 12, 2024
1 parent 3d65dea commit 213f3c5
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 29 deletions.
60 changes: 60 additions & 0 deletions googletest/src/matchers/matches_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,66 @@ pub mod internal {
}
}
}

/// A matcher that ensures that the passed-in function compiles with the
/// benefit of inference from the value being tested. The passed-in
/// matcher is what is actually invoked for matching.
///
/// It forwards all description responsibilities to the passed-in matcher.
pub fn compile_assert_and_match<T, M>(
must_compile_function: fn(&T),
matcher: M,
) -> CompileAssertAndMatch<T, M> {
CompileAssertAndMatch { must_compile_function, matcher }
}

#[derive(MatcherBase)]
#[doc(hidden)]
pub struct CompileAssertAndMatch<T, M> {
#[allow(dead_code)]
must_compile_function: fn(&T),
matcher: M,
}

impl<'a, T: Debug, M> Matcher<&'a T> for CompileAssertAndMatch<T, M>
where
M: Matcher<&'a T>,
{
fn matches(&self, actual: &'a T) -> crate::matcher::MatcherResult {
self.matcher.matches(actual)
}

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

fn explain_match(&self, actual: &'a T) -> crate::description::Description {
self.matcher.explain_match(actual)
}
}

impl<T: Debug + Copy, M> Matcher<T> for CompileAssertAndMatch<T, M>
where
M: Matcher<T>,
{
fn matches(&self, actual: T) -> crate::matcher::MatcherResult {
self.matcher.matches(actual)
}

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

fn explain_match(&self, actual: T) -> crate::description::Description {
self.matcher.explain_match(actual)
}
}
}

mod compile_fail_tests {
Expand Down
4 changes: 3 additions & 1 deletion googletest/src/matchers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ pub mod __internal_unstable_do_not_depend_on_these {
pub use super::elements_are_matcher::internal::ElementsAre;
pub use super::field_matcher::internal::field_matcher;
pub use super::is_matcher::is;
pub use super::matches_pattern::internal::{__googletest_macro_matches_pattern, pattern_only};
pub use super::matches_pattern::internal::{
__googletest_macro_matches_pattern, compile_assert_and_match, pattern_only,
};
pub use super::pointwise_matcher::internal::PointwiseMatcher;
pub use super::property_matcher::internal::{property_matcher, property_ref_matcher};
pub use super::result_of_matcher::internal::{result_of, result_of_ref};
Expand Down
14 changes: 14 additions & 0 deletions googletest/tests/matches_pattern_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,20 @@ fn matches_struct_containing_non_copy_field_binding_mode() -> Result<()> {
verify_that!(actual, matches_pattern!(AStruct { a_string: eq("123") }))
}

#[test]
fn matches_generic_struct_exhaustively() -> Result<()> {
#[allow(dead_code)]
#[derive(Debug)]
struct AStruct<T> {
a: T,
b: u32,
}
let actual = AStruct { a: 1, b: 3 };

verify_that!(actual, matches_pattern!(&AStruct { a: _, b: _ }))?;
verify_that!(actual, matches_pattern!(AStruct { a: _, b: _ }))
}

#[test]
fn matches_struct_with_interleaved_underscore() -> Result<()> {
#[derive(Debug)]
Expand Down
52 changes: 24 additions & 28 deletions googletest_macro/src/matches_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,13 @@ fn parse_braced_pattern_args(
})
.collect();

let matcher = quote! {
googletest::matchers::__internal_unstable_do_not_depend_on_these::is(
stringify!(#struct_name),
all!(#(#field_patterns),* )
)
};

// Do an exhaustiveness check only if the pattern doesn't end with `..` and has
// any fields in the pattern. This latter part is required because
// `matches_pattern!` also uses the brace notation for tuple structs when
Expand All @@ -268,36 +275,25 @@ fn parse_braced_pattern_args(
// matches_pattern!(foo, Struct { bar(): eq(1) })
// ```
// and we can't emit an exhaustiveness check based on the `matches_pattern!`.
let maybe_assert_exhaustive = if non_exhaustive || field_names.is_empty() {
None
if non_exhaustive || field_names.is_empty() {
Ok(matcher)
} else {
// Note that `struct_name` might be an enum variant (`Enum::Foo`), which is not
// a valid type. So we need to infer the type, produce an instance, and match on
// it. Fortunately, `[_; 0]` can be trivially initialized to `[]` and can
// produce an instance by indexing into it without failing compilation.
Some(quote! {
fn __matches_pattern_ensure_exhastive_match(i: usize) {
let val: [_; 0] = [];
let _ = match val[i] {
#struct_name { #(#field_names: _),* } => (),
// The pattern below is unreachable if the type is a struct (as opposed to an
// enum). Since the macro can't know which it is, we always include it and just
// tell the compiler not to complain.
#[allow(unreachable_patterns)]
_ => (),
};
}
Ok(quote! {
googletest::matchers::__internal_unstable_do_not_depend_on_these::compile_assert_and_match(
|actual| {
// Exhaustively check that all field names are specified.
match actual {
#struct_name { #(#field_names: _),* } => {},
// The pattern below is unreachable if the type is a struct (as opposed to
// an enum). Since the macro can't know which it is, we always include it
// and just tell the compiler not to complain.
#[allow(unreachable_patterns)]
_ => {},
}
},
#matcher)
})
};

Ok(quote! {
googletest::matchers::__internal_unstable_do_not_depend_on_these::is(
stringify!(#struct_name), {
#maybe_assert_exhaustive
all!( #(#field_patterns),* )
}
)
})
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down

0 comments on commit 213f3c5

Please sign in to comment.