Refactor verify_pred into a proc macro helps with subsequent improvements to its functionality.

Googler · copybara-github · commit f605d5470ea0 · 2024-10-25T22:13:08.000-07:00
Also supports function/method arguments that don't implement `Debug` using inherent method specialization.

PiperOrigin-RevId: 690021288
diff --git a/googletest/src/assertions.rs b/googletest/src/assertions.rs
@@ -183,14 +183,10 @@ macro_rules! verify_that {
 /// ```text
 /// equals_modulo(a, b(b(2)), 2 + 3) was false with
 ///   a = 1,
-///   b(v) = 7,
-///   2 + 3 = 5
+///   b(b(2)) = 7,
+///   2 + 3 = 5,
 /// ```
 ///
-/// The function passed to this macro must return `bool`. Each of the arguments
-/// must evaluate to a type implementing [`std::fmt::Debug`]. The debug output
-/// is used to construct the failure message.
-///
 /// The predicate can also be a method on a struct, e.g.:
 ///
 /// ```ignore
@@ -204,46 +200,12 @@ macro_rules! verify_that {
 /// ```
 ///
 /// **Warning:** This macro assumes that the arguments passed to the predicate
-/// are pure so that two subsequent invocations to any of the expresssions
-/// passed as arguments result in different values, then the output message of a
-/// test failure will deviate from the values actually passed to the predicate.
-/// For this reason, *always assign the outputs of non-pure functions to
-/// variables before using them in this macro. For example:
-///
-/// ```ignore
-/// let output = generate_random_number();  // Assigned outside of verify_pred.
-/// verify_pred!(is_sufficiently_random(output))?;
-/// ```
+/// cause no mutations, or else the output message of a test failure will
+/// deviate from the values actually passed to the predicate.
 #[macro_export]
 macro_rules! verify_pred {
-    (@internal [$($predicate:tt)+] $(,)?) => {
-        if !$($predicate)* {
-            $crate::assertions::internal::report_failed_predicate(
-                stringify!($($predicate)*),
-                vec![],
-            )
-        } else {
-            Ok(())
-        }
-    };
-
-    (@internal [$($predicate:tt)+]($($arg:expr),* $(,)?)) => {
-        if !$($predicate)*($($arg),*) {
-            $crate::assertions::internal::report_failed_predicate(
-                concat!(stringify!($($predicate)*), stringify!(($($arg),*))),
-                vec![$((format!(concat!(stringify!($arg), " = {:?}"), $arg))),*],
-            )
-        } else {
-            Ok(())
-        }
-    };
-
-    (@internal [$($predicate:tt)+] $first:tt $($rest:tt)*) => {
-        $crate::verify_pred!(@internal [$($predicate)* $first] $($rest)*)
-    };
-
-    ($first:tt $($rest:tt)*) => {
-        $crate::verify_pred!(@internal [$first] $($rest)*)
+    ($expr:expr $(,)?) => {
+        $crate::assertions::internal::__googletest_macro_verify_pred!($expr)
     };
 }
 
@@ -1414,6 +1376,8 @@ pub mod internal {
     };
     use std::fmt::Debug;
 
+    pub use ::googletest_macro::__googletest_macro_verify_pred;
+
     /// Extension trait to perform autoref through method lookup in the
     /// assertion macros. With this trait, the subject can be either a value
     /// or a reference. For example, this trait makes the following code
@@ -1453,25 +1417,6 @@ pub mod internal {
 
     impl<T: Copy + Debug> Subject for T {}
 
-    /// Constructs a `Result::Err(TestAssertionFailure)` for a predicate failure
-    /// as produced by the macro [`crate::verify_pred`].
-    ///
-    /// This intended only for use by the macro [`crate::verify_pred`].
-    ///
-    /// **For internal use only. API stablility is not guaranteed!**
-    #[track_caller]
-    #[must_use = "The assertion result must be evaluated to affect the test result."]
-    pub fn report_failed_predicate(
-        actual_expr: &'static str,
-        formatted_arguments: Vec<String>,
-    ) -> Result<(), TestAssertionFailure> {
-        Err(TestAssertionFailure::create(format!(
-            "{} was false with\n  {}",
-            actual_expr,
-            formatted_arguments.join(",\n  ")
-        )))
-    }
-
     /// Creates a failure at specified location.
     ///
     /// **For internal use only. API stability is not guaranteed!**
@@ -1480,4 +1425,45 @@ pub mod internal {
     pub fn create_fail_result(message: String) -> crate::Result<()> {
         Err(crate::internal::test_outcome::TestAssertionFailure::create(message))
     }
+
+    /// Trait that defaults to not rendering values. Used for autoref
+    /// specialization to conditionally render only values that implement
+    /// `Debug`. See also [`FormatDebug`].
+    pub trait FormatNonDebug {
+        fn __googletest_format_as_line(&self, output: &mut String, expr_label: &str) {
+            use ::std::fmt::Write as _;
+            write!(output, "\n  {} does not implement Debug,", expr_label)
+                .expect("Formatting to String should never fail");
+        }
+    }
+
+    impl<T> FormatNonDebug for &T {}
+
+    /// Trait to render values that implement `Debug` to a format string. Used
+    /// for autoref specialization to conditionally render only values that
+    /// implement `Debug`. See also [`FormatNonDebug`].
+    pub trait FormatDebug {
+        fn __googletest_format_as_line(&self, output: &mut String, expr_label: &str);
+    }
+
+    impl<T: Debug> FormatDebug for T {
+        #[track_caller]
+        fn __googletest_format_as_line(&self, output: &mut String, expr_label: &str) {
+            use ::std::fmt::Write as _;
+            write!(output, "\n  {} = {:?},", expr_label, self)
+                .expect("Formatting to String should never fail");
+        }
+    }
+
+    #[macro_export]
+    macro_rules! __googletest__format_as_line(
+        ($output_str:expr, $expr_str:expr, $value:expr $(,)?) => {
+            {
+                use $crate::assertions::internal::FormatNonDebug as _;
+                use $crate::assertions::internal::FormatDebug as _;
+                (&$value).__googletest_format_as_line($output_str, $expr_str)
+            }
+        }
+    );
+    pub use __googletest__format_as_line;
 }
diff --git a/googletest/src/lib.rs b/googletest/src/lib.rs
@@ -30,6 +30,8 @@ pub mod matcher;
 pub mod matcher_support;
 pub mod matchers;
 
+pub use googletest_macro::__googletest_macro_verify_pred;
+
 /// Re-exports of the symbols in this crate which are most likely to be used.
 ///
 /// This includes:
diff --git a/googletest/tests/assertions_test.rs b/googletest/tests/assertions_test.rs
@@ -3,29 +3,28 @@ mod verify_pred {
     use indoc::indoc;
 
     #[test]
-    fn supports_function_call() -> Result<()> {
-        fn f(_a: u32, _b: u32, _c: u32) -> bool {
+    fn supports_function_call_with_non_debug_types() -> Result<()> {
+        // Non-Debug - cannot be printed.
+        struct Apple;
+        fn f(_a: &Apple, _b: u32, _c: u32) -> bool {
             false
         }
         fn g(_a: u32) -> u32 {
             5
         }
 
-        let a = 1;
+        let a = &Apple;
         let res = verify_pred!(f(a, g(g(3)), 1 + 2));
         verify_that!(
             res,
-            err(displays_as(contains_substring(indoc! {
-                "
+            err(displays_as(contains_substring(indoc! {"
                 f(a, g(g(3)), 1 + 2) was false with
-                  a = 1,
+                  a does not implement Debug,
                   g(g(3)) = 5,
-                  1 + 2 = 3
-                "
+                  1 + 2 = 3,
+                  at"
             })))
-        )?;
-
-        Ok(())
+        )
     }
 
     #[test]
@@ -40,20 +39,32 @@ mod verify_pred {
     }
 
     #[test]
-    fn supports_method_calls() -> Result<()> {
-        struct Foo {
-            b: Bar,
+    fn supports_method_calls_with_non_debug_types() -> Result<()> {
+        struct Apple {
+            b: Banana,
         }
-        struct Bar;
-        impl Bar {
-            fn c(&self) -> bool {
+        struct Banana;
+        impl Banana {
+            fn c(&self, _c: &Cherry, _d: u32) -> bool {
                 false
             }
         }
+        // Non-Debug - cannot be printed.
+        struct Cherry;
 
-        let a = Foo { b: Bar };
-        let res = verify_pred!(a.b.c());
-        verify_that!(res, err(displays_as(contains_substring("a.b.c() was false"))))
+        let a = Apple { b: Banana };
+        let c = &Cherry;
+        let d = 3;
+        let res = verify_pred!(a.b.c(c, d));
+        verify_that!(
+            res,
+            err(displays_as(contains_substring(indoc! {"
+                a.b.c(c, d) was false with
+                  c does not implement Debug,
+                  d = 3,
+                  at"
+            })))
+        )
     }
 
     #[test]
@@ -73,8 +84,12 @@ mod verify_pred {
 
         let a = Foo;
         let res = verify_pred!(a.b().c());
-        verify_that!(res, err(displays_as(contains_substring("a.b().c() was false"))))?;
-
-        Ok(())
+        verify_that!(
+            res,
+            err(displays_as(contains_substring(indoc! {"
+                a.b().c() was false with
+                  at"
+            })))
+        )
     }
 }
diff --git a/googletest_macro/src/lib.rs b/googletest_macro/src/lib.rs
@@ -256,7 +256,11 @@ impl Fixture {
 
     fn parameter(&self) -> proc_macro2::TokenStream {
         let Self { identifier, mutability, consumable, .. } = self;
-        if *consumable { quote!(#identifier) } else { quote!(& #mutability #identifier) }
+        if *consumable {
+            quote!(#identifier)
+        } else {
+            quote!(& #mutability #identifier)
+        }
     }
 
     fn wrap_call(&self, inner_call: proc_macro2::TokenStream) -> proc_macro2::TokenStream {
@@ -327,3 +331,14 @@ pub fn derive(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
     }
     .into()
 }
+
+mod verify_pred;
+
+/// This is an implementation detail of `verify_pred!`.
+///
+/// It's not intended to be used directly.
+#[doc(hidden)]
+#[proc_macro]
+pub fn __googletest_macro_verify_pred(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
+    verify_pred::verify_pred_impl(input)
+}
diff --git a/googletest_macro/src/verify_pred.rs b/googletest_macro/src/verify_pred.rs
@@ -0,0 +1,108 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+use quote::quote;
+use syn::{parse_macro_input, punctuated::Punctuated, token::Comma, Expr, Ident};
+
+struct AccumulatePartsState {
+    error_message_ident: Ident,
+    formats: Vec<proc_macro2::TokenStream>,
+}
+
+fn expr_to_string(expr: &Expr) -> String {
+    quote!(#expr).to_string()
+}
+
+impl AccumulatePartsState {
+    fn new() -> Self {
+        Self {
+            error_message_ident: Ident::new(
+                "__googletest__verify_pred__error_message",
+                ::proc_macro2::Span::call_site(),
+            ),
+            formats: vec![],
+        }
+    }
+
+    /// Accumulates error message formating parts for various parts of the
+    /// expression.
+    fn accumulate_parts(&mut self, expr: &Expr) {
+        let expr_string = expr_to_string(expr);
+        match expr {
+            Expr::Group(group) => {
+                // This is an invisible group added for correct precedence in the AST. Just pass
+                // through without having a separate printing result.
+                return self.accumulate_parts(&group.expr);
+            }
+            Expr::Call(call) => {
+                // Format the args into the error message.
+                self.format_args(&call.args);
+            }
+            Expr::MethodCall(method_call) => {
+                // Format the args into the error message.
+                self.format_args(&method_call.args);
+            }
+            _ => {}
+        }
+        let error_message_ident = &self.error_message_ident;
+        self.formats.push(quote! {
+            ::googletest::fmt::internal::__googletest__write_expr_value!(
+                &mut #error_message_ident,
+                #expr_string,
+                #expr,
+            );
+        });
+    }
+
+    // Formats each argument expression into the error message.
+    fn format_args(&mut self, args: &Punctuated<Expr, Comma>) {
+        for pair in args.pairs() {
+            let error_message_ident = &self.error_message_ident;
+            let expr_string = expr_to_string(pair.value());
+            let expr = pair.value();
+            self.formats.push(quote! {
+                ::googletest::fmt::internal::__googletest__write_expr_value!(
+                    &mut #error_message_ident,
+                    #expr_string,
+                    #expr,
+                );
+            });
+        }
+    }
+}
+
+pub fn verify_pred_impl(input: proc_macro::TokenStream) -> proc_macro::TokenStream {
+    let parsed = parse_macro_input!(input as Expr);
+    let error_message = quote!(#parsed).to_string() + " was false with";
+
+    let mut state = AccumulatePartsState::new();
+    state.accumulate_parts(&parsed);
+    let AccumulatePartsState { error_message_ident, mut formats, .. } = state;
+
+    let _ = formats.pop(); // The last one is the full expression itself.
+    quote! {
+        {
+            if (#parsed) {
+                Ok(())
+            } else {
+                let mut #error_message_ident = #error_message.to_string();
+                #(#formats)*
+                ::core::result::Result::Err(
+                    ::googletest::internal::test_outcome::TestAssertionFailure::create(
+                        #error_message_ident))
+            }
+        }
+    }
+    .into()
+}
diff --git a/integration_tests/src/integration_tests.rs b/integration_tests/src/integration_tests.rs
