Merge pull request #2480 from topecongiro/issue-2476

Avoid drifting macro body which is unformattable

Author: nrc

rustfmt-bin/build.rs

@@ -27,7 +27,9 @@ fn main() {
 // (git not installed or if this is not a git repository) just return an empty string.
 fn commit_info() -> String {
     match (channel(), commit_hash(), commit_date()) {
-        (channel, Some(hash), Some(date)) => format!("{} ({} {})", channel, hash.trim_right(), date),
+        (channel, Some(hash), Some(date)) => {
+            format!("{} ({} {})", channel, hash.trim_right(), date)
+        }
         _ => String::new(),
     }
 }

rustfmt-core/src/expr.rs

@@ -1908,12 +1908,16 @@ where
         1
     };
     let used_width = extra_offset(callee_str, shape);
-    let one_line_width = shape.width.checked_sub(used_width + 2 * paren_overhead)?;
+    let one_line_width = shape
+        .width
+        .checked_sub(used_width + 2 * paren_overhead)
+        .unwrap_or(0);
 
     // 1 = "(" or ")"
     let one_line_shape = shape
-        .offset_left(last_line_width(callee_str) + 1)?
-        .sub_width(1)?;
+        .offset_left(last_line_width(callee_str) + 1)
+        .and_then(|shape| shape.sub_width(1))
+        .unwrap_or(Shape { width: 0, ..shape });
     let nested_shape = shape_from_indent_style(
         context,
         shape,
@@ -1950,7 +1954,13 @@ where
         );
     }
 
-    let args_shape = shape.sub_width(last_line_width(callee_str))?;
+    let args_shape = Shape {
+        width: shape
+            .width
+            .checked_sub(last_line_width(callee_str))
+            .unwrap_or(0),
+        ..shape
+    };
     Some(format!(
         "{}{}",
         callee_str,
@@ -2317,9 +2327,16 @@ pub fn wrap_args_with_parens(
     shape: Shape,
     nested_shape: Shape,
 ) -> String {
+    let paren_overhead = paren_overhead(context);
+    let fits_one_line = args_str.len() + paren_overhead <= shape.width;
+    let extend_width = if args_str.is_empty() {
+        paren_overhead
+    } else {
+        paren_overhead / 2
+    };
     if !context.use_block_indent()
-        || (context.inside_macro && !args_str.contains('\n')
-            && args_str.len() + paren_overhead(context) <= shape.width) || is_extendable
+        || (context.inside_macro && !args_str.contains('\n') && fits_one_line)
+        || (is_extendable && extend_width <= shape.width)
     {
         let mut result = String::with_capacity(args_str.len() + 4);
         if context.config.spaces_within_parens_and_brackets() && !args_str.is_empty() {
@@ -2338,8 +2355,10 @@ pub fn wrap_args_with_parens(
         let mut result =
             String::with_capacity(args_str.len() + 2 + indent_str.len() + nested_indent_str.len());
         result.push_str("(");
-        result.push_str(&nested_indent_str);
-        result.push_str(args_str);
+        if !args_str.is_empty() {
+            result.push_str(&nested_indent_str);
+            result.push_str(args_str);
+        }
         result.push_str(&indent_str);
         result.push_str(")");
         result

rustfmt-core/src/macros.rs

@@ -38,7 +38,7 @@ use expr::{rewrite_array, rewrite_call_inner};
 use lists::{itemize_list, write_list, ListFormatting};
 use rewrite::{Rewrite, RewriteContext};
 use shape::{Indent, Shape};
-use utils::{format_visibility, mk_sp};
+use utils::{format_visibility, mk_sp, wrap_str};
 
 const FORCED_BRACKET_MACROS: &[&str] = &["vec!"];
 
@@ -810,6 +810,7 @@ impl MacroBranch {
                 None => return None,
             },
         };
+        let new_body = wrap_str(new_body, config.max_width(), shape)?;
 
         // Indent the body since it is in a block.
         let indent_str = body_indent.to_string(&config);

rustfmt-core/src/types.rs

@@ -529,12 +529,9 @@ impl Rewrite for ast::TyParamBound {
             ast::TyParamBound::TraitTyParamBound(ref tref, ast::TraitBoundModifier::None) => {
                 tref.rewrite(context, shape)
             }
-            ast::TyParamBound::TraitTyParamBound(ref tref, ast::TraitBoundModifier::Maybe) => {
-                Some(format!(
-                    "?{}",
-                    tref.rewrite(context, shape.offset_left(1)?)?
-                ))
-            }
+            ast::TyParamBound::TraitTyParamBound(ref tref, ast::TraitBoundModifier::Maybe) => Some(
+                format!("?{}", tref.rewrite(context, shape.offset_left(1)?)?),
+            ),
             ast::TyParamBound::RegionTyParamBound(ref l) => l.rewrite(context, shape),
         }
     }

rustfmt-core/tests/source/macro_rules.rs

@@ -1,3 +1,5 @@
+// rustfmt-error_on_line_overflow: false
+
 macro_rules! m {
 	// a
 	($expr :expr,  $( $func : ident    ) *   ) => {
@@ -50,12 +52,37 @@ macro m2 {
 }
 }
 
-
-// #2438
+// #2438, #2476
+macro_rules! m {
+    () => {
+        fn foo() {
+            this_line_is_98_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
+            );
+        }
+    }
+}
+macro_rules! m {
+    () => {
+        fn foo() {
+            this_line_is_99_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
+);
+        }
+    };
+}
+macro_rules! m {
+    () => {
+        fn foo() {
+            this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
+);
+        }
+    };
+}
 macro_rules! m {
     () => {
-        this_line_is_99_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
-        ); // this line is drifting
+        fn foo() {
+            this_line_is_101_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
+            );
+        }
     };
 }
 

rustfmt-core/tests/target/configs/indent_style/block_call.rs

@@ -13,9 +13,9 @@ fn main() {
         "elit",
     );
     // #1501
-    let hyper = Arc::new(Client::with_connector(HttpsConnector::new(
-        TlsClient::new(),
-    )));
+    let hyper = Arc::new(Client::with_connector(
+        HttpsConnector::new(TlsClient::new()),
+    ));
 
     // chain
     let x = yooooooooooooo

rustfmt-core/tests/target/macro_rules.rs

@@ -1,3 +1,5 @@
+// rustfmt-error_on_line_overflow: false
+
 macro_rules! m {
     // a
     ($expr: expr, $($func: ident)*) => {{
@@ -42,11 +44,36 @@ macro m2 {
     }
 }
 
-// #2438
+// #2438, #2476
 macro_rules! m {
     () => {
-        this_line_is_99_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
-        ); // this line is drifting
+        fn foo() {
+            this_line_is_98_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx();
+        }
+    };
+}
+macro_rules! m {
+    () => {
+        fn foo() {
+            this_line_is_99_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
+            );
+        }
+    };
+}
+macro_rules! m {
+    () => {
+        fn foo() {
+            this_line_is_100_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
+            );
+        }
+    };
+}
+macro_rules! m {
+    () => {
+        fn foo() {
+            this_line_is_101_characters_long_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx(
+            );
+        }
     };
 }