Merge pull request #2306 from dtwood/assert-eq-on-one-line

Add assert_eq! to special-cased macros

Author: nrc

src/codemap.rs

@@ -86,11 +86,9 @@ impl LineRangeUtils for CodeMap {
         let hi = self.lookup_char_pos(span.hi());
 
         assert_eq!(
-            lo.file.name,
-            hi.file.name,
+            lo.file.name, hi.file.name,
             "span crossed file boundary: lo: {:?}, hi: {:?}",
-            lo,
-            hi
+            lo, hi
         );
 
         LineRange {

src/expr.rs

@@ -1811,25 +1811,40 @@ fn rewrite_string_lit(context: &RewriteContext, span: Span, shape: Shape) -> Opt
     )
 }
 
-const FORMAT_LIKE_WHITELIST: &[&str] = &[
+/// A list of `format!`-like macros, that take a long format string and a list of arguments to
+/// format.
+///
+/// Organized as a list of `(&str, usize)` tuples, giving the name of the macro and the number of
+/// arguments before the format string (none for `format!("format", ...)`, one for `assert!(result,
+/// "format", ...)`, two for `assert_eq!(left, right, "format", ...)`).
+const SPECIAL_MACRO_WHITELIST: &[(&str, usize)] = &[
+    // format! like macros
     // From the Rust Standard Library.
-    "eprint!",
-    "eprintln!",
-    "format!",
-    "format_args!",
-    "print!",
-    "println!",
-    "panic!",
-    "unreachable!",
+    ("eprint!", 0),
+    ("eprintln!", 0),
+    ("format!", 0),
+    ("format_args!", 0),
+    ("print!", 0),
+    ("println!", 0),
+    ("panic!", 0),
+    ("unreachable!", 0),
     // From the `log` crate.
-    "debug!",
-    "error!",
-    "info!",
-    "warn!",
+    ("debug!", 0),
+    ("error!", 0),
+    ("info!", 0),
+    ("warn!", 0),
+    // write! like macros
+    ("assert!", 1),
+    ("debug_assert!", 1),
+    ("write!", 1),
+    ("writeln!", 1),
+    // assert_eq! like macros
+    ("assert_eq!", 2),
+    ("assert_ne!", 2),
+    ("debug_assert_eq!", 2),
+    ("debug_assert_ne!", 2),
 ];
 
-const WRITE_LIKE_WHITELIST: &[&str] = &["assert!", "write!", "writeln!"];
-
 pub fn rewrite_call(
     context: &RewriteContext,
     callee: &str,
@@ -2066,24 +2081,26 @@ where
             } else {
                 tactic = default_tactic();
 
-                // For special-case macros, we may want to use different tactics.
-                let maybe_args_offset = maybe_get_args_offset(callee_str, args);
-
-                if tactic == DefinitiveListTactic::Vertical && maybe_args_offset.is_some() {
-                    let args_offset = maybe_args_offset.unwrap();
-                    let args_tactic = definitive_tactic(
-                        &item_vec[args_offset..],
-                        ListTactic::HorizontalVertical,
-                        Separator::Comma,
-                        nested_shape.width,
-                    );
-
-                    // Every argument is simple and fits on a single line.
-                    if args_tactic == DefinitiveListTactic::Horizontal {
-                        tactic = if args_offset == 1 {
-                            DefinitiveListTactic::FormatCall
-                        } else {
-                            DefinitiveListTactic::WriteCall
+                if tactic == DefinitiveListTactic::Vertical {
+                    if let Some((all_simple, num_args_before)) =
+                        maybe_get_args_offset(callee_str, args)
+                    {
+                        let one_line = all_simple
+                            && definitive_tactic(
+                                &item_vec[..num_args_before],
+                                ListTactic::HorizontalVertical,
+                                Separator::Comma,
+                                nested_shape.width,
+                            ) == DefinitiveListTactic::Horizontal
+                            && definitive_tactic(
+                                &item_vec[num_args_before + 1..],
+                                ListTactic::HorizontalVertical,
+                                Separator::Comma,
+                                nested_shape.width,
+                            ) == DefinitiveListTactic::Horizontal;
+
+                        if one_line {
+                            tactic = DefinitiveListTactic::SpecialMacro(num_args_before);
                         };
                     }
                 }
@@ -2120,15 +2137,14 @@ fn is_every_args_simple<T: ToExpr>(lists: &[&T]) -> bool {
 }
 
 /// In case special-case style is required, returns an offset from which we start horizontal layout.
-fn maybe_get_args_offset<T: ToExpr>(callee_str: &str, args: &[&T]) -> Option<usize> {
-    if FORMAT_LIKE_WHITELIST.iter().any(|s| *s == callee_str) && args.len() >= 1
-        && is_every_args_simple(args)
-    {
-        Some(1)
-    } else if WRITE_LIKE_WHITELIST.iter().any(|s| *s == callee_str) && args.len() >= 2
-        && is_every_args_simple(args)
+fn maybe_get_args_offset<T: ToExpr>(callee_str: &str, args: &[&T]) -> Option<(bool, usize)> {
+    if let Some(&(_, num_args_before)) = SPECIAL_MACRO_WHITELIST
+        .iter()
+        .find(|&&(s, _)| s == callee_str)
     {
-        Some(2)
+        let all_simple = args.len() >= num_args_before && is_every_args_simple(args);
+
+        Some((all_simple, num_args_before))
     } else {
         None
     }

src/lists.rs

@@ -160,10 +160,8 @@ pub enum DefinitiveListTactic {
     Vertical,
     Horizontal,
     Mixed,
-    // Special case tactic for `format!()` variants.
-    FormatCall,
-    // Special case tactic for `write!()` varianta.
-    WriteCall,
+    /// Special case tactic for `format!()`, `write!()` style macros.
+    SpecialMacro(usize),
 }
 
 impl DefinitiveListTactic {
@@ -271,7 +269,7 @@ where
     I: IntoIterator<Item = T> + Clone,
     T: AsRef<ListItem>,
 {
-    let mut tactic = formatting.tactic;
+    let tactic = formatting.tactic;
     let sep_len = formatting.separator.len();
 
     // Now that we know how we will layout, we can decide for sure if there
@@ -313,26 +311,16 @@ where
             DefinitiveListTactic::Horizontal if !first => {
                 result.push(' ');
             }
-            DefinitiveListTactic::FormatCall if !first => {
-                result.push('\n');
-                result.push_str(indent_str);
-                tactic = DefinitiveListTactic::Horizontal;
-            }
-            DefinitiveListTactic::WriteCall => {
-                let second = i == 1;
-                let third = i == 2;
-
-                if first {
+            DefinitiveListTactic::SpecialMacro(num_args_before) => {
+                if i == 0 {
                     // Nothing
-                } else if second {
-                    result.push('\n');
-                    result.push_str(indent_str);
-                } else if third {
+                } else if i < num_args_before {
+                    result.push(' ');
+                } else if i <= num_args_before + 1 {
                     result.push('\n');
                     result.push_str(indent_str);
-                    tactic = DefinitiveListTactic::Horizontal;
                 } else {
-                    unreachable!();
+                    result.push(' ');
                 }
             }
             DefinitiveListTactic::Vertical if !first => {

tests/source/macros.rs

@@ -266,9 +266,15 @@ fn special_case_macros() {
     warn!("{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15);
     warn!("{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26);
 
-    assert!(result, "Ahoy there, {}!", target);
-    assert!(result, "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')", result, input, expected);
-    assert!(result, "{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26);
+    assert!(result == 42, "Ahoy there, {}!", target);
+    assert!(result == 42, "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')", result, input, expected);
+    assert!(result == 42, "{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26);
+
+    assert_eq!(left, right, "Ahoy there, {}!", target);
+    assert_eq!(left, right, "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')", result, input, expected);
+    assert_eq!(first_realllllllllllly_long_variable_that_doesnt_fit_one_one_line, second_reallllllllllly_long_variable_that_doesnt_fit_one_one_line, "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')", result, input, expected);
+    assert_eq!(left + 42, right, "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')", result, input, expected);
+    assert_eq!(left, right, "{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}", 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26);
 
     write!(&mut s, "Ahoy there, {}!", target);
     write!(&mut s, "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')", result, input, expected);

tests/system.rs

@@ -192,8 +192,7 @@ fn self_tests() {
     }
 
     assert_eq!(
-        warnings,
-        0,
+        warnings, 0,
         "Rustfmt's code generated {} warnings",
         warnings
     );

tests/target/macros.rs

@@ -691,14 +691,70 @@ fn special_case_macros() {
         26
     );
 
-    assert!(result, "Ahoy there, {}!", target);
+    assert!(result == 42, "Ahoy there, {}!", target);
     assert!(
+        result == 42,
+        "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')",
         result,
+        input,
+        expected
+    );
+    assert!(
+        result == 42,
+        "{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}",
+        1,
+        2,
+        3,
+        4,
+        5,
+        6,
+        7,
+        8,
+        9,
+        10,
+        11,
+        12,
+        13,
+        14,
+        15,
+        16,
+        17,
+        18,
+        19,
+        20,
+        21,
+        22,
+        23,
+        24,
+        25,
+        26
+    );
+
+    assert_eq!(left, right, "Ahoy there, {}!", target);
+    assert_eq!(
+        left, right,
         "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')",
         result, input, expected
     );
-    assert!(
+    assert_eq!(
+        first_realllllllllllly_long_variable_that_doesnt_fit_one_one_line,
+        second_reallllllllllly_long_variable_that_doesnt_fit_one_one_line,
+        "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')",
+        result,
+        input,
+        expected
+    );
+    assert_eq!(
+        left + 42,
+        right,
+        "Arr! While plunderin' the hold, we got '{}' when given '{}' (we expected '{}')",
         result,
+        input,
+        expected
+    );
+    assert_eq!(
+        left,
+        right,
         "{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}{}",
         1,
         2,