Merge pull request #2730 from topecongiro/issue-2704

Disallow combining a method call with prefix or suffix

Author: nrc

src/expr.rs

@@ -2173,3 +2173,15 @@ impl ToExpr for ast::GenericParam {
         false
     }
 }
+
+pub fn is_method_call(expr: &ast::Expr) -> bool {
+    match expr.node {
+        ast::ExprKind::MethodCall(..) => true,
+        ast::ExprKind::AddrOf(_, ref expr)
+        | ast::ExprKind::Box(ref expr)
+        | ast::ExprKind::Cast(ref expr, _)
+        | ast::ExprKind::Try(ref expr)
+        | ast::ExprKind::Unary(_, ref expr) => is_method_call(expr),
+        _ => false,
+    }
+}

src/items.rs

@@ -1380,9 +1380,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);
 

src/overflow.rs

@@ -18,7 +18,7 @@ use syntax::parse::token::DelimToken;
 
 use closures;
 use codemap::SpanUtils;
-use expr::{is_every_expr_simple, is_nested_call, maybe_get_args_offset, ToExpr};
+use expr::{is_every_expr_simple, is_method_call, is_nested_call, maybe_get_args_offset, ToExpr};
 use lists::{definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator};
 use rewrite::{Rewrite, RewriteContext};
 use shape::Shape;
@@ -231,8 +231,8 @@ impl<'a, T: 'a + Rewrite + ToExpr + Spanned> Context<'a, T> {
         let placeholder = if overflow_last {
             let old_value = *self.context.force_one_line_chain.borrow();
             if !combine_arg_with_callee {
-                if let Some(expr) = self.last_item().and_then(|item| item.to_expr()) {
-                    if let ast::ExprKind::MethodCall(..) = expr.node {
+                if let Some(ref expr) = self.last_item().and_then(|item| item.to_expr()) {
+                    if is_method_call(expr) {
                         self.context.force_one_line_chain.replace(true);
                     }
                 }

tests/source/expr.rs

@@ -398,3 +398,41 @@ fn foo() {
     let my_var =
         Mutex::new(RpcClientType::connect(server_iddd).chain_err(|| "Unable to create RPC client")?);
 }
+
+// #2704
+// Method call with prefix and suffix.
+fn issue2704() {
+    // We should not combine the callee with a multi-lined method call.
+    let requires = requires.set(&requires0
+                                .concat(&requires1)
+                                .concat(&requires2)
+                                .distinct_total());
+    let requires = requires.set(box requires0
+                                .concat(&requires1)
+                                .concat(&requires2)
+                                .distinct_total());
+    let requires = requires.set(requires0
+                                .concat(&requires1)
+                                .concat(&requires2)
+                                .distinct_total() as u32);
+    let requires = requires.set(requires0
+                                .concat(&requires1)
+                                .concat(&requires2)
+                                .distinct_total()?);
+    let requires = requires.set(!requires0
+                                .concat(&requires1)
+                                .concat(&requires2)
+                                .distinct_total());
+    // We should combine a small callee with an argument.
+    bar(vec![22]
+        .into_iter()
+        .map(|x| x * 2)
+        .filter(|_| true)
+        .collect());
+    // But we should not combine a long callee with an argument.
+    barrrr(vec![22]
+        .into_iter()
+        .map(|x| x * 2)
+        .filter(|_| true)
+        .collect());
+}

tests/target/expr.rs

@@ -424,6 +424,56 @@ fn dots() {
 // A function call with a large single argument.
 fn foo() {
     let my_var = Mutex::new(
-        RpcClientType::connect(server_iddd).chain_err(|| "Unable to create RPC client")?
+        RpcClientType::connect(server_iddd).chain_err(|| "Unable to create RPC client")?,
+    );
+}
+
+// #2704
+// Method call with prefix and suffix.
+fn issue2704() {
+    // We should not combine the callee with a multi-lined method call.
+    let requires = requires.set(
+        &requires0
+            .concat(&requires1)
+            .concat(&requires2)
+            .distinct_total(),
+    );
+    let requires = requires.set(
+        box requires0
+            .concat(&requires1)
+            .concat(&requires2)
+            .distinct_total(),
+    );
+    let requires = requires.set(
+        requires0
+            .concat(&requires1)
+            .concat(&requires2)
+            .distinct_total() as u32,
+    );
+    let requires = requires.set(
+        requires0
+            .concat(&requires1)
+            .concat(&requires2)
+            .distinct_total()?,
+    );
+    let requires = requires.set(
+        !requires0
+            .concat(&requires1)
+            .concat(&requires2)
+            .distinct_total(),
+    );
+    // We should combine a small callee with an argument.
+    bar(vec![22]
+        .into_iter()
+        .map(|x| x * 2)
+        .filter(|_| true)
+        .collect());
+    // But we should not combine a long callee with an argument.
+    barrrr(
+        vec![22]
+            .into_iter()
+            .map(|x| x * 2)
+            .filter(|_| true)
+            .collect(),
     );
 }