Skip to content

Commit 45fc121

Browse files
marcianxcopybara-github
authored andcommitted
In matches_pattern!, ensure that even when no fields are matched, like in NonExistentStruct { .. } or NonExistentStruct {}, we still get a compilation failure.
I regressed this behavior when implementing #447. PiperOrigin-RevId: 707565682
1 parent cf535b0 commit 45fc121

File tree

3 files changed

+71
-36
lines changed

3 files changed

+71
-36
lines changed

googletest/src/matchers/matches_pattern.rs

+18
Original file line numberDiff line numberDiff line change
@@ -490,4 +490,22 @@ mod compile_fail_tests {
490490
/// verify_that!(actual, matches_pattern!(Foo(eq(&1), eq(&2) )));
491491
/// ```
492492
fn _unexhaustive_tuple_struct_field_check_requires_dot_dot() {}
493+
494+
/// ```compile_fail
495+
/// use ::googletest::prelude::*;
496+
/// verify_that!(1, matches_pattern!(UndefinedSymbol { a: 1 }));
497+
/// ```
498+
fn _should_fail_to_compile_unknown_struct_with_field() {}
499+
500+
/// ```compile_fail
501+
/// use ::googletest::prelude::*;
502+
/// verify_that!(1, matches_pattern!(UndefinedSymbol { .. }));
503+
/// ```
504+
fn _should_fail_to_compile_unknown_struct_with_dot_dot() {}
505+
506+
/// ```compile_fail
507+
/// use ::googletest::prelude::*;
508+
/// verify_that!(1, matches_pattern!(UndefinedSymbol( .. )));
509+
/// ```
510+
fn _should_fail_to_compile_unknown_tuple_struct_with_dot_dot() {}
493511
}

googletest/tests/matches_pattern_test.rs

+8
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,14 @@ fn matches_struct_non_exhaustive() -> Result<()> {
5151
verify_that!(actual, matches_pattern!(&AStruct { a_field: eq(123), .. }))
5252
}
5353

54+
#[test]
55+
fn matches_braced_struct_with_no_fields() -> Result<()> {
56+
#[derive(Debug)]
57+
struct AStruct {}
58+
let actual = AStruct {};
59+
verify_that!(actual, matches_pattern!(AStruct {}))
60+
}
61+
5462
#[test]
5563
#[rustfmt::skip]// Otherwise fmt strips the trailing comma
5664
fn supports_trailing_comma_with_one_field() -> Result<()> {

googletest_macro/src/matches_pattern.rs

+45-36
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,12 @@
1313
// limitations under the License.
1414

1515
use proc_macro2::{Delimiter, Group, Ident, Span, TokenStream, TokenTree};
16-
use quote::quote;
16+
use quote::{quote, ToTokens};
1717
use syn::{
1818
parse::{Parse, ParseStream, Parser as _},
19-
parse_macro_input, Expr, ExprCall, Pat, Token,
19+
parse_macro_input,
20+
token::DotDot,
21+
Expr, ExprCall, Pat, Token,
2022
};
2123

2224
/// This is an implementation detail of `googletest::matches_pattern!`. It
@@ -162,7 +164,7 @@ fn parse_tuple_pattern_args(
162164
struct_name: TokenStream,
163165
group_content: TokenStream,
164166
) -> syn::Result<TokenStream> {
165-
let (patterns, non_exhaustive) =
167+
let (patterns, dot_dot) =
166168
parse_list_terminated_pattern::<MaybeTupleFieldPattern>.parse2(group_content)?;
167169
let field_count = patterns.len();
168170
let field_patterns = patterns
@@ -181,27 +183,27 @@ fn parse_tuple_pattern_args(
181183
)
182184
};
183185

184-
// Do an exhaustiveness check only if the pattern doesn't end with `..`.
185-
if non_exhaustive {
186-
Ok(matcher)
187-
} else {
188-
let empty_fields = std::iter::repeat(quote! { _ }).take(field_count);
189-
Ok(quote! {
190-
googletest::matchers::__internal_unstable_do_not_depend_on_these::compile_assert_and_match(
191-
|actual| {
192-
// Exhaustively check that all field names are specified.
193-
match actual {
194-
#struct_name ( #(#empty_fields),* ) => (),
195-
// The pattern below is unreachable if the type is a struct (as opposed to
196-
// an enum). Since the macro can't know which it is, we always include it
197-
// and just tell the compiler not to complain.
198-
#[allow(unreachable_patterns)]
199-
_ => {},
200-
}
201-
},
202-
#matcher)
203-
})
204-
}
186+
// Do a match to ensure;
187+
// - Fields are exhaustively listed unless the pattern ended with `..`.
188+
// - `UNDEFINED_SYMBOL(..)` fails to compile.
189+
let empty_fields = std::iter::repeat(quote! { _ })
190+
.take(field_count)
191+
.chain(dot_dot.map(ToTokens::into_token_stream));
192+
Ok(quote! {
193+
googletest::matchers::__internal_unstable_do_not_depend_on_these::compile_assert_and_match(
194+
|actual| {
195+
// Exhaustively check that all field names are specified.
196+
match actual {
197+
#struct_name ( #(#empty_fields),* ) => (),
198+
// The pattern below is unreachable if the type is a struct (as opposed to
199+
// an enum). Since the macro can't know which it is, we always include it
200+
// and just tell the compiler not to complain.
201+
#[allow(unreachable_patterns)]
202+
_ => {},
203+
}
204+
},
205+
#matcher)
206+
})
205207
}
206208

207209
////////////////////////////////////////////////////////////////////////////////
@@ -260,7 +262,7 @@ fn parse_braced_pattern_args(
260262
struct_name: TokenStream,
261263
group_content: TokenStream,
262264
) -> syn::Result<TokenStream> {
263-
let (patterns, non_exhaustive) = parse_list_terminated_pattern.parse2(group_content)?;
265+
let (patterns, dot_dot) = parse_list_terminated_pattern.parse2(group_content)?;
264266
let mut field_names = vec![];
265267
let field_patterns: Vec<TokenStream> = patterns
266268
.into_iter()
@@ -286,26 +288,30 @@ fn parse_braced_pattern_args(
286288
)
287289
};
288290

289-
// Do an exhaustiveness check only if the pattern doesn't end with `..` and has
290-
// any fields in the pattern. This latter part is required because
291+
// Do a match to ensure;
292+
// - Fields are exhaustively listed unless the pattern ended with `..` and has
293+
// any fields in the pattern.
294+
// - `UNDEFINED_SYMBOL { .. }` fails to compile.
295+
//
296+
// The requisite that some fields are in the pattern is there because
291297
// `matches_pattern!` also uses the brace notation for tuple structs when
292-
// asserting on method calls. i.e.
298+
// asserting on method calls on tuple structs. i.e.
293299
//
294300
// ```
295301
// struct Struct(u32);
296302
// ...
297303
// matches_pattern!(foo, Struct { bar(): eq(1) })
298304
// ```
299305
// and we can't emit an exhaustiveness check based on the `matches_pattern!`.
300-
if non_exhaustive || field_names.is_empty() {
306+
if field_names.is_empty() && dot_dot.is_none() {
301307
Ok(matcher)
302308
} else {
303309
Ok(quote! {
304310
googletest::matchers::__internal_unstable_do_not_depend_on_these::compile_assert_and_match(
305311
|actual| {
306312
// Exhaustively check that all field names are specified.
307313
match actual {
308-
#struct_name { #(#field_names: _),* } => {},
314+
#struct_name { #(#field_names: _,)* #dot_dot } => {},
309315
// The pattern below is unreachable if the type is a struct (as opposed to
310316
// an enum). Since the macro can't know which it is, we always include it
311317
// and just tell the compiler not to complain.
@@ -321,19 +327,22 @@ fn parse_braced_pattern_args(
321327
////////////////////////////////////////////////////////////////////////////////
322328
// General-purpose helpers
323329

324-
/// Returns the parsed struct pattern body along with a boolean that indicates
325-
/// whether the body ended with `..`.
330+
/// Returns the parsed struct pattern body along with a `..` if it appears at
331+
/// the end of the body.
326332
///
327333
/// This is like `Punctuated::parse_terminated`, but additionally allows for an
328334
/// optional `..`, which cannot be followed by a comma.
329-
fn parse_list_terminated_pattern<T: Parse>(input: ParseStream<'_>) -> syn::Result<(Vec<T>, bool)> {
335+
fn parse_list_terminated_pattern<T: Parse>(
336+
input: ParseStream<'_>,
337+
) -> syn::Result<(Vec<T>, Option<DotDot>)> {
330338
let mut patterns = vec![];
331339
while !input.is_empty() {
332340
// Check for trailing `..`.
333-
if input.parse::<Option<Token![..]>>()?.is_some() {
341+
let dot_dot = input.parse::<Option<Token![..]>>()?;
342+
if dot_dot.is_some() {
334343
// Must be at the end of the group content.
335344
return if input.is_empty() {
336-
Ok((patterns, true))
345+
Ok((patterns, dot_dot))
337346
} else {
338347
compile_err(input.span(), "`..` must be at the end of the struct pattern")
339348
};
@@ -346,7 +355,7 @@ fn parse_list_terminated_pattern<T: Parse>(input: ParseStream<'_>) -> syn::Resul
346355
}
347356
input.parse::<Token![,]>()?;
348357
}
349-
Ok((patterns, false))
358+
Ok((patterns, None))
350359
}
351360

352361
fn compile_err<T>(span: Span, message: &str) -> syn::Result<T> {

0 commit comments

Comments
 (0)