Merge pull request rust-lang#3101 from nrc/pair-newline

Simplify multi-lining binop exprs

Author: nrc

src/expr.rs

@@ -898,10 +898,11 @@ impl<'a> ControlFlow<'a> {
             let trial = self.rewrite_single_line(&pat_expr_string, context, shape.width);
 
             if let Some(cond_str) = trial {
-                if cond_str.len() <= context
-                    .config
-                    .width_heuristics()
-                    .single_line_if_else_max_width
+                if cond_str.len()
+                    <= context
+                        .config
+                        .width_heuristics()
+                        .single_line_if_else_max_width
                 {
                     return Some((cond_str, 0));
                 }

src/lists.rs

@@ -146,10 +146,12 @@ impl ListItem {
     }
 
     pub fn is_different_group(&self) -> bool {
-        self.inner_as_ref().contains('\n') || self.pre_comment.is_some() || self
-            .post_comment
-            .as_ref()
-            .map_or(false, |s| s.contains('\n'))
+        self.inner_as_ref().contains('\n')
+            || self.pre_comment.is_some()
+            || self
+                .post_comment
+                .as_ref()
+                .map_or(false, |s| s.contains('\n'))
     }
 
     pub fn is_multiline(&self) -> bool {

src/macros.rs

@@ -1168,22 +1168,24 @@ fn indent_macro_snippet(
         .min()?;
 
     Some(
-        first_line + "\n" + &trimmed_lines
-            .iter()
-            .map(
-                |&(trimmed, ref line, prefix_space_width)| match prefix_space_width {
-                    _ if !trimmed => line.to_owned(),
-                    Some(original_indent_width) => {
-                        let new_indent_width = indent.width() + original_indent_width
-                            .saturating_sub(min_prefix_space_width);
-                        let new_indent = Indent::from_width(context.config, new_indent_width);
-                        format!("{}{}", new_indent.to_string(context.config), line)
-                    }
-                    None => String::new(),
-                },
-            )
-            .collect::<Vec<_>>()
-            .join("\n"),
+        first_line
+            + "\n"
+            + &trimmed_lines
+                .iter()
+                .map(
+                    |&(trimmed, ref line, prefix_space_width)| match prefix_space_width {
+                        _ if !trimmed => line.to_owned(),
+                        Some(original_indent_width) => {
+                            let new_indent_width = indent.width()
+                                + original_indent_width.saturating_sub(min_prefix_space_width);
+                            let new_indent = Indent::from_width(context.config, new_indent_width);
+                            format!("{}{}", new_indent.to_string(context.config), line)
+                        }
+                        None => String::new(),
+                    },
+                )
+                .collect::<Vec<_>>()
+                .join("\n"),
     )
 }
 

src/pairs.rs

@@ -43,7 +43,7 @@ pub(crate) fn rewrite_all_pairs(
     context: &RewriteContext,
 ) -> Option<String> {
     // First we try formatting on one line.
-    if let Some(list) = expr.flatten(context, false) {
+    if let Some(list) = expr.flatten(false) {
         if let Some(r) = rewrite_pairs_one_line(&list, shape, context) {
             return Some(r);
         }
@@ -53,7 +53,7 @@ pub(crate) fn rewrite_all_pairs(
     // to only flatten pairs with the same operator, that way we don't
     // necessarily need one line per sub-expression, but we don't do anything
     // too funny wrt precedence.
-    expr.flatten(context, true)
+    expr.flatten(true)
         .and_then(|list| rewrite_pairs_multiline(list, shape, context))
 }
 
@@ -83,33 +83,22 @@ fn rewrite_pairs_one_line<T: Rewrite>(
         result.push(' ');
     }
 
+    let prefix_len = result.len();
     let last = list.list.last().unwrap();
     let cur_shape = base_shape.offset_left(last_line_width(&result))?;
-    let rewrite = last.rewrite(context, cur_shape)?;
-    result.push_str(&rewrite);
+    let last_rewrite = last.rewrite(context, cur_shape)?;
+    result.push_str(&last_rewrite);
 
     if first_line_width(&result) > shape.width {
         return None;
     }
 
-    // Check the last expression in the list. We let this expression go over
-    // multiple lines, but we check that if this is necessary, then we can't
-    // do better using multi-line formatting.
-    if !is_single_line(&result) {
-        let multiline_shape = shape.offset_left(list.separators.last().unwrap().len() + 1)?;
-        let multiline_list: PairList<T> = PairList {
-            list: vec![last],
-            separators: vec![],
-            separator_place: list.separator_place,
-        };
-        // Format as if we were multi-line.
-        if let Some(rewrite) = rewrite_pairs_multiline(multiline_list, multiline_shape, context) {
-            // Also, don't let expressions surrounded by parens go multi-line,
-            // this looks really bad.
-            if rewrite.starts_with('(') || is_single_line(&rewrite) {
-                return None;
-            }
-        }
+    // Check the last expression in the list. We sometimes let this expression
+    // go over multiple lines, but we check for some ugly conditions.
+    if !(is_single_line(&result) || last_rewrite.starts_with('{'))
+        && (last_rewrite.starts_with('(') || prefix_len > context.config.tab_spaces())
+    {
+        return None;
     }
 
     wrap_str(result, context.config.max_width(), shape)
@@ -215,11 +204,12 @@ where
         // If the length of the lhs is equal to or shorter than the tab width or
         // the rhs looks like block expression, we put the rhs on the same
         // line with the lhs even if the rhs is multi-lined.
-        let allow_same_line = lhs_result.len() <= tab_spaces || rhs_result
-            .lines()
-            .next()
-            .map(|first_line| first_line.ends_with('{'))
-            .unwrap_or(false);
+        let allow_same_line = lhs_result.len() <= tab_spaces
+            || rhs_result
+                .lines()
+                .next()
+                .map(|first_line| first_line.ends_with('{'))
+                .unwrap_or(false);
         if !rhs_result.contains('\n') || allow_same_line {
             let one_line_width = last_line_width(&lhs_result)
                 + pp.infix.len()
@@ -272,19 +262,18 @@ trait FlattenPair: Rewrite + Sized {
     // operator into the list. E.g,, if the source is `a * b + c`, if `_same_op`
     // is true, we make `[(a * b), c]` if `_same_op` is false, we make
     // `[a, b, c]`
-    fn flatten(&self, _context: &RewriteContext, _same_op: bool) -> Option<PairList<Self>> {
+    fn flatten(&self, _same_op: bool) -> Option<PairList<Self>> {
         None
     }
 }
 
 struct PairList<'a, 'b, T: Rewrite + 'b> {
     list: Vec<&'b T>,
     separators: Vec<&'a str>,
-    separator_place: SeparatorPlace,
 }
 
 impl FlattenPair for ast::Expr {
-    fn flatten(&self, context: &RewriteContext, same_op: bool) -> Option<PairList<ast::Expr>> {
+    fn flatten(&self, same_op: bool) -> Option<PairList<ast::Expr>> {
         let top_op = match self.node {
             ast::ExprKind::Binary(op, _, _) => op.node,
             _ => return None,
@@ -320,11 +309,7 @@ impl FlattenPair for ast::Expr {
         }
 
         assert_eq!(list.len() - 1, separators.len());
-        Some(PairList {
-            list,
-            separators,
-            separator_place: context.config.binop_separator(),
-        })
+        Some(PairList { list, separators })
     }
 }
 

src/string.rs

@@ -291,11 +291,12 @@ fn break_string(max_chars: usize, trim_end: bool, line_end: &str, input: &[&str]
         return break_at(max_chars - 1);
     }
     if let Some(url_index_end) = detect_url(input, max_chars) {
-        let index_plus_ws = url_index_end + input[url_index_end..]
-            .iter()
-            .skip(1)
-            .position(|grapheme| not_whitespace_except_line_feed(grapheme))
-            .unwrap_or(0);
+        let index_plus_ws = url_index_end
+            + input[url_index_end..]
+                .iter()
+                .skip(1)
+                .position(|grapheme| not_whitespace_except_line_feed(grapheme))
+                .unwrap_or(0);
         return if trim_end {
             SnippetState::LineEnd(
                 input[..=url_index_end].join("").to_string(),

src/utils.rs

@@ -326,10 +326,11 @@ pub fn mk_sp(lo: BytePos, hi: BytePos) -> Span {
 // Return true if the given span does not intersect with file lines.
 macro_rules! out_of_file_lines_range {
     ($self:ident, $span:expr) => {
-        !$self.config.file_lines().is_all() && !$self
-            .config
-            .file_lines()
-            .intersects(&$self.source_map.lookup_line_range($span))
+        !$self.config.file_lines().is_all()
+            && !$self
+                .config
+                .file_lines()
+                .intersects(&$self.source_map.lookup_line_range($span))
     };
 }
 

tests/source/chains.rs

@@ -259,3 +259,8 @@ fn issue_2773() {
             None
         });
 }
+
+fn issue_3034() {
+    disallowed_headers.iter().any(|header| *header == name) ||
+        disallowed_header_prefixes.iter().any(|prefix| name.starts_with(prefix))
+}

tests/target/chains.rs

@@ -299,3 +299,8 @@ fn issue_2773() {
         None
     });
 }
+
+fn issue_3034() {
+    disallowed_headers.iter().any(|header| *header == name)
+        || disallowed_header_prefixes.iter().any(|prefix| name.starts_with(prefix))
+}