Merge pull request #2350 from topecongiro/issue-2324

Issue 2324

Author: nrc

src/expr.rs

@@ -31,9 +31,10 @@ use shape::{Indent, Shape};
 use spanned::Spanned;
 use string::{rewrite_string, StringFormat};
 use types::{can_be_overflowed_type, rewrite_path, PathContext};
-use utils::{colon_spaces, contains_skip, extra_offset, first_line_width, inner_attributes,
-            last_line_extendable, last_line_width, mk_sp, outer_attributes, paren_overhead,
-            ptr_vec_to_ref_vec, semicolon_for_stmt, trimmed_last_line_width, wrap_str};
+use utils::{colon_spaces, contains_skip, count_newlines, extra_offset, first_line_width,
+            inner_attributes, last_line_extendable, last_line_width, mk_sp, outer_attributes,
+            paren_overhead, ptr_vec_to_ref_vec, semicolon_for_stmt, trimmed_last_line_width,
+            wrap_str};
 use vertical::rewrite_with_alignment;
 use visitor::FmtVisitor;
 
@@ -2053,6 +2054,26 @@ where
     // Replace the stub with the full overflowing last argument if the rewrite
     // succeeded and its first line fits with the other arguments.
     match (overflow_last, tactic, placeholder) {
+        (true, DefinitiveListTactic::Horizontal, Some(ref overflowed)) if args.len() == 1 => {
+            // When we are rewriting a nested function call, we restrict the
+            // bugdet for the inner function to avoid them being deeply nested.
+            // However, when the inner function has a prefix or a suffix
+            // (e.g. `foo() as u32`), this budget reduction may produce poorly
+            // formatted code, where a prefix or a suffix being left on its own
+            // line. Here we explicitlly check those cases.
+            if count_newlines(overflowed) == 1 {
+                let rw = args.last()
+                    .and_then(|last_arg| last_arg.rewrite(context, nested_shape));
+                let no_newline = rw.as_ref().map_or(false, |s| !s.contains('\n'));
+                if no_newline {
+                    item_vec[args.len() - 1].item = rw;
+                } else {
+                    item_vec[args.len() - 1].item = Some(overflowed.to_owned());
+                }
+            } else {
+                item_vec[args.len() - 1].item = Some(overflowed.to_owned());
+            }
+        }
         (true, DefinitiveListTactic::Horizontal, placeholder @ Some(..)) => {
             item_vec[args.len() - 1].item = placeholder;
         }
@@ -2824,7 +2845,6 @@ pub fn choose_rhs<R: Rewrite>(
 }
 
 fn prefer_next_line(orig_rhs: &str, next_line_rhs: &str) -> bool {
-    use utils::count_newlines;
     !next_line_rhs.contains('\n') || count_newlines(orig_rhs) > count_newlines(next_line_rhs) + 1
 }
 

src/items.rs

@@ -1343,8 +1343,9 @@ fn format_tuple_struct(
         // We need to put the where clause on a new line, but we didn't
         // know that earlier, so the where clause will not be indented properly.
         result.push('\n');
-        result.push_str(&(offset.block_only() + (context.config.tab_spaces() - 1))
-            .to_string(context.config));
+        result
+            .push_str(&(offset.block_only() + (context.config.tab_spaces() - 1))
+                .to_string(context.config));
     }
     result.push_str(&where_clause_str);
 

tests/system.rs

@@ -164,7 +164,7 @@ fn assert_output(source: &Path, expected_filename: &Path) {
     if !compare.is_empty() {
         let mut failures = HashMap::new();
         failures.insert(source.to_owned(), compare);
-        print_mismatches_default_message(failures, source.display());
+        print_mismatches_default_message(failures);
         assert!(false, "Text does not match expected output");
     }
 }
@@ -279,7 +279,7 @@ fn check_files(files: Vec<PathBuf>) -> (Vec<FormatReport>, u32, u32) {
             Ok(report) => reports.push(report),
             Err(err) => {
                 if let IdempotentCheckError::Mismatch(msg) = err {
-                    print_mismatches_default_message(msg, file_name.display());
+                    print_mismatches_default_message(msg);
                 }
                 fails += 1;
             }
@@ -291,13 +291,15 @@ fn check_files(files: Vec<PathBuf>) -> (Vec<FormatReport>, u32, u32) {
     (reports, count, fails)
 }
 
-fn print_mismatches_default_message(
-    result: HashMap<PathBuf, Vec<Mismatch>>,
-    file_name: std::path::Display,
-) {
-    print_mismatches(result, |line_num| {
-        format!("\nMismatch at {}:{}:", file_name, line_num)
-    });
+fn print_mismatches_default_message(result: HashMap<PathBuf, Vec<Mismatch>>) {
+    let mut t = term::stdout().unwrap();
+    for (file_name, diff) in result {
+        let mismatch_msg_formatter =
+            |line_num| format!("\nMismatch at {}:{}:", file_name.display(), line_num);
+        print_diff(diff, &mismatch_msg_formatter, Color::Auto);
+    }
+
+    t.reset().unwrap();
 }
 
 fn print_mismatches<T: Fn(u32) -> String>(

tests/target/issue-2324.rs

@@ -0,0 +1,7 @@
+// nested function calls with cast.
+fn main() {
+    self.ptr
+        .set(intrinsics::arith_offset(self.ptr.get() as *mut u8, 1) as *mut T);
+    self.ptr
+        .set(intrinsics::arith_offset(self.ptr.get(), mem::size_of::<T>() as isize) as *mut u8);
+}