Handle trailing commas

This was implemented for some macros but not others (possibly this was my mistake when simpliyfing the macros...). This centralizes their handling in two central macros. FYI I'm 80% confident I've handled all cases -- not sure how else they could get through, but any reports of misses are welcome...
narpfel · Mar 4, 2024 · 9dc95b4 · 9dc95b4
1 parent 3cb9934
commit 9dc95b4
Show file tree

Hide file tree

Showing 9 changed files with 91 additions and 13 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 All notable changes to insta and cargo-insta are documented here.
 
+## 1.37.0
+
+- All macros should now handle trailing commas.
+
 ## 1.36.1
 
 - Fix an ownership issue introduced in 1.36 with snapshot assertions.  #453

diff --git a/src/macros.rs b/src/macros.rs
@@ -219,6 +219,7 @@ macro_rules! assert_compact_json_snapshot {
     };
 }
 
+// This macro is expected to handle optional trailing commas.
 #[doc(hidden)]
 #[macro_export]
 macro_rules! _assert_serialized_snapshot {
@@ -227,19 +228,19 @@ macro_rules! _assert_serialized_snapshot {
     //
     // Note that if we could unify the Inline & File represenations of snapshots
     // redactions we could unify some of these branches.
-    (format=$format:ident, $value:expr, $(match ..)? {$($k:expr => $v:expr),*$(,)?}, @$snapshot:literal) => {{
+    (format=$format:ident, $value:expr, $(match ..)? {$($k:expr => $v:expr),* $(,)?}, @$snapshot:literal $(,)?) => {{
         let transform = |value| {
             let (_, value) = $crate::_prepare_snapshot_for_redaction!(value, {$($k => $v),*}, $format, Inline);
             value
         };
         $crate::_assert_snapshot_base!(transform=transform, $value, @$snapshot);
     }};
     // If there are redaction expressions and no name, add a auto-generated name, call self
-    (format=$format:ident, $value:expr, $(match ..)? {$($k:expr => $v:expr),*$(,)?}) => {{
+    (format=$format:ident, $value:expr, $(match ..)? {$($k:expr => $v:expr),* $(,)?} $(,)?) => {{
         $crate::_assert_serialized_snapshot!(format=$format, $crate::_macro_support::AutoName, $value, {$($k => $v),*});
     }};
     // If there are redaction expressions, capture and pass to `_assert_snapshot_base`
-    (format=$format:ident, $name:expr, $value:expr, $(match ..)? {$($k:expr => $v:expr),*$(,)?}) => {{
+    (format=$format:ident, $name:expr, $value:expr, $(match ..)? {$($k:expr => $v:expr),* $(,)?} $(,)?) => {{
         let transform = |value| {
             let (_, value) = $crate::_prepare_snapshot_for_redaction!(value, {$($k => $v),*}, $format, File);
             value
@@ -257,7 +258,7 @@ macro_rules! _assert_serialized_snapshot {
         $crate::_assert_snapshot_base!(transform = transform, $($arg),*, @$snapshot);
     }};
     // Capture serialization function and pass to `_assert_snapshot_base`,
-    // specifing `File`
+    // specifying `File`
     (format=$format:ident, $($arg:expr),* $(,)?) => {{
         let transform = |value| {$crate::_macro_support::serialize_value(
             &value,
@@ -272,7 +273,7 @@ macro_rules! _assert_serialized_snapshot {
 #[doc(hidden)]
 #[macro_export]
 macro_rules! _prepare_snapshot_for_redaction {
-    ($value:expr, {$($k:expr => $v:expr),*$(,)?}, $format:ident, $location:ident) => {
+    ($value:expr, {$($k:expr => $v:expr),*}, $format:ident, $location:ident) => {
         {
             let vec = vec![
                 $((
@@ -295,7 +296,7 @@ macro_rules! _prepare_snapshot_for_redaction {
 #[doc(hidden)]
 #[macro_export]
 macro_rules! _prepare_snapshot_for_redaction {
-    ($value:expr, {$($k:expr => $v:expr),*$(,)?}, $format:ident, $location:ident) => {
+    ($value:expr, {$($k:expr => $v:expr),*}, $format:ident, $location:ident) => {
         compile_error!("insta was compiled without redaction support.");
     };
 }
@@ -315,13 +316,16 @@ macro_rules! assert_debug_snapshot {
 }
 
 // A helper macro which takes a closure as `transform`, and runs the closure on
-// the value. This allows us to implement other macros with a small wrapper.
+// the value. This allows us to implement other macros with a small wrapper. All
+// snapshot macros eventually call this macro.
+//
+// This macro is expected to handle trailing commas.
 #[doc(hidden)]
 #[macro_export]
 macro_rules! _assert_snapshot_base {
     // If there's an inline literal value, wrap the literal in a
     // `ReferenceValue::Inline`, call self.
-    (transform=$transform:expr, $($arg:expr),*, @$snapshot:literal) => {
+    (transform=$transform:expr, $($arg:expr),*, @$snapshot:literal $(,)?) => {
         $crate::_assert_snapshot_base!(
             transform = $transform,
             #[allow(clippy::needless_raw_string_hashes)]
@@ -330,20 +334,20 @@ macro_rules! _assert_snapshot_base {
         )
     };
     // If there's no debug_expr, use the stringified value, call self.
-    (transform=$transform:expr, $name:expr, $value:expr) => {
+    (transform=$transform:expr, $name:expr, $value:expr $(,)?) => {
         $crate::_assert_snapshot_base!(transform = $transform, $name, $value, stringify!($value))
     };
     // If there's no name (and necessarily no debug expr), auto generate the
     // name, call self.
-    (transform=$transform:expr, $value:expr) => {
+    (transform=$transform:expr, $value:expr $(,)?) => {
         $crate::_assert_snapshot_base!(
             transform = $transform,
             $crate::_macro_support::AutoName,
             $value
         )
     };
     // The main macro body — every call to this macro should end up here.
-    (transform=$transform:expr, $name:expr, $value:expr, $debug_expr:expr) => {
+    (transform=$transform:expr, $name:expr, $value:expr, $debug_expr:expr $(,)?) => {
         $crate::_macro_support::assert_snapshot(
             $name.into(),
             #[allow(clippy::redundant_closure_call)]

diff --git a/tests/snapshots/test_basic__Testing.snap b/tests/snapshots/test_basic__Testing.snap
@@ -0,0 +1,5 @@
+---
+source: tests/test_basic.rs
+expression: expr
+---
+name
diff --git a/tests/snapshots/test_basic__trailing_commas.snap b/tests/snapshots/test_basic__trailing_commas.snap
@@ -0,0 +1,5 @@
+---
+source: tests/test_basic.rs
+expression: "\"Testing\""
+---
+Testing
diff --git a/tests/snapshots/test_redaction__with_random_value_and_match_comma-2.snap b/tests/snapshots/test_redaction__with_random_value_and_match_comma-2.snap
@@ -0,0 +1,8 @@
+---
+source: tests/test_redaction.rs
+expression: "&User {\n        id: 11,\n        username: \"john_doe\".to_string(),\n        email: Email(\"john@example.com\".to_string()),\n        extra: \"\".to_string(),\n    }"
+---
+id: "[id]"
+username: john_doe
+email: john@example.com
+extra: ""
diff --git a/tests/snapshots/test_redaction__with_random_value_and_match_comma.snap b/tests/snapshots/test_redaction__with_random_value_and_match_comma.snap
@@ -0,0 +1,8 @@
+---
+source: tests/test_redaction.rs
+expression: "&User {\n        id: 11,\n        username: \"john_doe\".to_string(),\n        email: Email(\"john@example.com\".to_string()),\n        extra: \"\".to_string(),\n    }"
+---
+id: "[id]"
+username: john_doe
+email: john@example.com
+extra: ""
diff --git a/tests/test_basic.rs b/tests/test_basic.rs
@@ -3,7 +3,7 @@ use insta::assert_json_snapshot;
 #[cfg(feature = "yaml")]
 use insta::assert_yaml_snapshot;
 #[allow(deprecated)]
-use insta::{assert_debug_snapshot, assert_display_snapshot};
+use insta::{assert_debug_snapshot, assert_display_snapshot, assert_snapshot};
 use std::fmt;
 
 #[test]
@@ -63,6 +63,13 @@ mod nested {
     }
 }
 
+#[test]
+fn test_trailing_commas() {
+    assert_snapshot!("Testing",);
+    assert_snapshot!("Testing", "name",);
+    assert_snapshot!("Testing", "name", "expr",);
+}
+
 struct TestDisplay;
 
 impl fmt::Display for TestDisplay {

diff --git a/tests/test_inline.rs b/tests/test_inline.rs
@@ -29,6 +29,14 @@ fn test_single_line() {
     assert_snapshot!("Testing", @"Testing");
 }
 
+#[test]
+fn test_trailing_commas() {
+    assert_snapshot!(
+        "Testing",
+        @"Testing",
+    );
+}
+
 #[test]
 fn test_unnamed_single_line() {
     assert_snapshot!("Testing");

diff --git a/tests/test_redaction.rs b/tests/test_redaction.rs
@@ -88,7 +88,7 @@ fn test_with_random_value_and_trailing_comma() {
 
 #[cfg(feature = "yaml")]
 #[test]
-fn test_with_random_value_and_trailing_comma_match() {
+fn test_with_random_value_and_match_comma() {
     assert_yaml_snapshot!(
         &User {
             id: 11,
@@ -100,6 +100,35 @@ fn test_with_random_value_and_trailing_comma_match() {
             ".id" => "[id]",
         }
     );
+    assert_yaml_snapshot!(
+        &User {
+            id: 11,
+            username: "john_doe".to_string(),
+            email: Email("john@example.com".to_string()),
+            extra: "".to_string(),
+        },
+        match .. {
+            ".id" => "[id]",
+        }, // comma here
+    );
+    assert_yaml_snapshot!(
+        &User {
+            id: 11,
+            username: "john_doe".to_string(),
+            email: Email("john@example.com".to_string()),
+            extra: "".to_string(),
+        },
+        match .. {
+            ".id" => "[id]",
+        },
+        @r###"
+        ---
+        id: "[id]"
+        username: john_doe
+        email: john@example.com
+        extra: ""
+        "###, // comma here
+    );
 }
 
 #[cfg(feature = "csv")]