Merge pull request #1032 from marcusklaas/else-if-let-overflow

Fix constraints on pattern formatting of else arms

src/comment.rs

@@ -29,29 +29,30 @@ pub fn rewrite_comment(orig: &str,
     let s = orig.trim();
 
     // Edge case: block comments. Let's not trim their lines (for now).
-    let (opener, closer, line_start) = if block_style {
-        ("/* ", " */", " * ")
-    } else if !config.normalize_comments {
-        if orig.starts_with("/**") {
-            ("/** ", " **/", " ** ")
-        } else if orig.starts_with("/*!") {
-            ("/*! ", " */", " * ")
-        } else if orig.starts_with("/*") {
+    let (opener, closer, line_start) =
+        if block_style {
             ("/* ", " */", " * ")
-        } else if orig.starts_with("///") {
+        } else if !config.normalize_comments {
+            if orig.starts_with("/**") {
+                ("/** ", " **/", " ** ")
+            } else if orig.starts_with("/*!") {
+                ("/*! ", " */", " * ")
+            } else if orig.starts_with("/*") {
+                ("/* ", " */", " * ")
+            } else if orig.starts_with("///") {
+                ("/// ", "", "/// ")
+            } else if orig.starts_with("//!") {
+                ("//! ", "", "//! ")
+            } else {
+                ("// ", "", "// ")
+            }
+        } else if orig.starts_with("///") || orig.starts_with("/**") {
             ("/// ", "", "/// ")
-        } else if orig.starts_with("//!") {
+        } else if orig.starts_with("//!") || orig.starts_with("/*!") {
             ("//! ", "", "//! ")
         } else {
             ("// ", "", "// ")
-        }
-    } else if orig.starts_with("///") || orig.starts_with("/**") {
-        ("/// ", "", "/// ")
-    } else if orig.starts_with("//!") || orig.starts_with("/*!") {
-        ("//! ", "", "//! ")
-    } else {
-        ("// ", "", "// ")
-    };
+        };
 
     let max_chars = width.checked_sub(closer.len() + opener.len()).unwrap_or(1);
 
@@ -127,11 +128,12 @@ fn left_trim_comment_line(line: &str) -> &str {
        line.starts_with("/** ") {
         &line[4..]
     } else if line.starts_with("/* ") || line.starts_with("// ") || line.starts_with("//!") ||
-       line.starts_with("///") || line.starts_with("** ") || line.starts_with("/*!") ||
-       line.starts_with("/**") {
+              line.starts_with("///") ||
+              line.starts_with("** ") || line.starts_with("/*!") ||
+              line.starts_with("/**") {
         &line[3..]
     } else if line.starts_with("/*") || line.starts_with("* ") || line.starts_with("//") ||
-       line.starts_with("**") {
+              line.starts_with("**") {
         &line[2..]
     } else if line.starts_with("*") {
         &line[1..]

src/expr.rs

@@ -725,6 +725,14 @@ fn rewrite_if_else(context: &RewriteContext,
                    offset: Indent,
                    allow_single_line: bool)
                    -> Option<String> {
+    let (budget, indent) = if !allow_single_line {
+        // We are part of an if-elseif-else chain. Our constraints are tightened.
+        // 7 = "} else" .len()
+        (try_opt!(width.checked_sub(7)), offset + 7)
+    } else {
+        (width, offset)
+    };
+
     // 3 = "if ", 2 = " {"
     let pat_penalty = match context.config.else_if_brace_style {
         ElseIfBraceStyle::AlwaysNextLine => 3,
@@ -735,8 +743,8 @@ fn rewrite_if_else(context: &RewriteContext,
                                                     cond,
                                                     "let ",
                                                     " =",
-                                                    try_opt!(width.checked_sub(pat_penalty)),
-                                                    offset + 3));
+                                                    try_opt!(budget.checked_sub(pat_penalty)),
+                                                    indent + 3));
 
     // Try to format if-else on single line.
     if expr_type == ExprType::SubExpression && allow_single_line &&
@@ -778,6 +786,8 @@ fn rewrite_if_else(context: &RewriteContext,
         let rewrite = match else_block.node {
             // If the else expression is another if-else expression, prevent it
             // from being formatted on a single line.
+            // Note how we're passing the original width and offset, as the
+            // cost of "else" should not cascade.
             ast::ExprKind::IfLet(ref pat, ref cond, ref if_block, ref next_else_block) => {
                 rewrite_if_else(context,
                                 cond,

tests/source/expr.rs

@@ -272,3 +272,16 @@ fn if_else() {
            -1.0
        };
 }
+
+fn complex_if_else() {
+    if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx {
+    } else if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx {
+        ha();
+    } else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxx {
+        yo();
+    } else if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx {
+        ha();
+    } else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxxx {
+        yo();
+    }
+}

tests/target/expr.rs

@@ -275,3 +275,18 @@ fn if_else() {
 
     let cx = tp1.x + any * radius * if anticlockwise { 1.0 } else { -1.0 };
 }
+
+fn complex_if_else() {
+    if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx {
+    } else if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx {
+        ha();
+    } else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxx {
+        yo();
+    } else if let Some(x) =
+                  xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx {
+        ha();
+    } else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +
+              xxxxxxxxx {
+        yo();
+    }
+}