Skip to content

Commit

Permalink
Similar to native match pattern for tuple structs, make `matches_patt…
Browse files Browse the repository at this point in the history
…ern` enforce exhaustive field checks for tuple structs by default, to be suppressed by explicit `..` at the end of the pattern.

This change is not backward-compatible since it enforces field exhaustiveness by default in the absence of a trailing `..` in the pattern.

Fixes #447

PiperOrigin-RevId: 704273542
  • Loading branch information
marcianx authored and copybara-github committed Dec 10, 2024
1 parent 3d65dea commit 914bbfb
Show file tree
Hide file tree
Showing 4 changed files with 181 additions and 38 deletions.
78 changes: 78 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 and the attached
/// matcher invokes as well.
///
/// 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 All @@ -393,6 +453,15 @@ mod compile_fail_tests {
/// ```
fn _dot_dot_supported_only_at_end_of_struct_pattern() {}

/// ```compile_fail
/// use ::googletest::prelude::*;
/// #[derive(Debug)]
/// struct Foo(u32, u32);
/// let actual = Foo(1, 2);
/// verify_that!(actual, matches_pattern!(Foo(eq(&1), .., )));
/// ```
fn _dot_dot_supported_only_at_end_of_tuple_struct_pattern() {}

/// ```compile_fail
/// use ::googletest::prelude::*;
/// #[derive(Debug)]
Expand All @@ -412,4 +481,13 @@ mod compile_fail_tests {
/// verify_that!(actual, matches_pattern!(Foo::Bar { a: eq(&1) }));
/// ```
fn _unexhaustive_enum_struct_field_check_requires_dot_dot() {}

/// ```compile_fail
/// use ::googletest::prelude::*;
/// #[derive(Debug)]
/// struct Foo(u32, u32, u32);
/// let actual = Foo(1, 2, 3);
/// verify_that!(actual, matches_pattern!(Foo(eq(&1), eq(&2) )));
/// ```
fn _unexhaustive_tuple_struct_field_check_requires_dot_dot() {}
}
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
45 changes: 45 additions & 0 deletions googletest/tests/matches_pattern_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,22 @@ 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(Copy, Clone, Debug)]
struct AStruct<T> {
a: T,
b: u32,
}
let actual = AStruct { a: 1, b: 3 };

// Need to use `&actual` to match the pattern since otherwise the generic
// argument can't be correctly deduced.
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 Expand Up @@ -481,6 +497,35 @@ fn matches_tuple_struct_with_interleaved_underscore() -> Result<()> {
verify_that!(actual, matches_pattern!(AStruct(eq(&1), _, eq(&3))))
}

#[test]
fn matches_tuple_struct_non_exhaustive() -> Result<()> {
#[allow(dead_code)]
#[derive(Copy, Clone, Debug)]
struct AStruct<T>(T, u32);
let actual = AStruct(1, 3);

// Need to use `&actual` to match the pattern since otherwise the generic
// argument can't be correctly deduced.
verify_that!(&actual, matches_pattern!(&AStruct(_, ..)))?;
verify_that!(actual, matches_pattern!(AStruct(_, ..)))
}

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

// Need to use `&actual` to match the pattern since otherwise the generic
// argument can't be correctly deduced.
verify_that!(&actual, matches_pattern!(&AStruct { a: _, b: _ }))?;
verify_that!(actual, matches_pattern!(AStruct { a: _, b: _ }))
}

#[test]
fn matches_enum_without_field() -> Result<()> {
#[derive(Debug)]
Expand Down
92 changes: 55 additions & 37 deletions googletest_macro/src/matches_pattern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ use proc_macro2::{Delimiter, Group, Ident, Span, TokenStream, TokenTree};
use quote::quote;
use syn::{
parse::{Parse, ParseStream, Parser as _},
parse_macro_input,
punctuated::Punctuated,
Expr, ExprCall, Pat, Token,
parse_macro_input, Expr, ExprCall, Pat, Token,
};

/// This is an implementation detail of `googletest::matches_pattern!`. It
Expand Down Expand Up @@ -164,22 +162,46 @@ fn parse_tuple_pattern_args(
struct_name: TokenStream,
group_content: TokenStream,
) -> syn::Result<TokenStream> {
let parser = Punctuated::<MaybeTupleFieldPattern, Token![,]>::parse_terminated;
let fields = parser
.parse2(group_content)?
let (patterns, non_exhaustive) =
parse_list_terminated_pattern::<MaybeTupleFieldPattern>.parse2(group_content)?;
let field_count = patterns.len();
let field_patterns = patterns
.into_iter()
.enumerate()
.filter_map(|(index, maybe_pattern)| maybe_pattern.0.map(|pattern| (index, pattern)))
.map(|(index, TupleFieldPattern { ref_token, matcher })| {
let index = syn::Index::from(index);
quote! { googletest::matchers::field!(#struct_name.#index, #ref_token #matcher) }
});
Ok(quote! {

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

// Do an exhaustiveness check only if the pattern doesn't end with `..` and has.
if non_exhaustive {
Ok(matcher)
} else {
let empty_fields = std::iter::repeat(quote! { _ }).take(field_count);
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 ( #(#empty_fields),* ) => (),
// 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)
})
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -257,6 +279,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 +297,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 914bbfb

Please sign in to comment.