Add failure mode to `write_list`

12 files changed, 296 insertions, 214 deletions

diff --git a/src/expr.rs b/src/expr.rs
index 3f88e45884e..08acd5de67d 100644
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -8,11 +8,13 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+use std::cmp::Ordering;
+
 use rewrite::{Rewrite, RewriteContext};
 use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic};
 use string::{StringFormat, rewrite_string};
 use StructLitStyle;
-use utils::{span_after, make_indent, extra_offset, first_line_width, last_line_width};
+use utils::{span_after, make_indent, extra_offset, first_line_width, last_line_width, wrap_str};
 use visitor::FmtVisitor;
 use config::BlockIndentStyle;
 use comment::{FindUncommented, rewrite_comment, contains_comment};
@@ -134,37 +136,9 @@ impl Rewrite for ast::Expr {
             ast::Expr_::ExprClosure(capture, ref fn_decl, ref body) => {
                 rewrite_closure(capture, fn_decl, body, self.span, context, width, offset)
             }
-            _ => {
-                // We do not format these expressions yet, but they should still
-                // satisfy our width restrictions.
-                let snippet = context.snippet(self.span);
-
-                {
-                    let mut lines = snippet.lines();
-
-                    // The caller of this function has already placed `offset`
-                    // characters on the first line.
-                    let first_line_max_len = try_opt!(context.config.max_width.checked_sub(offset));
-                    if lines.next().unwrap().len() > first_line_max_len {
-                        return None;
-                    }
-
-                    // The other lines must fit within the maximum width.
-                    if lines.find(|line| line.len() > context.config.max_width).is_some() {
-                        return None;
-                    }
-
-                    // `width` is the maximum length of the last line, excluding
-                    // indentation.
-                    // A special check for the last line, since the caller may
-                    // place trailing characters on this line.
-                    if snippet.lines().rev().next().unwrap().len() > offset + width {
-                        return None;
-                    }
-                }
-
-                Some(snippet)
-            }
+            // We do not format these expressions yet, but they should still
+            // satisfy our width restrictions.
+            _ => wrap_str(context.snippet(self.span), context.config.max_width, width, offset),
         }
     }
 }
@@ -202,7 +176,8 @@ fn rewrite_closure(capture: ast::CaptureClause,
                                  body.span.lo);
 
     let fmt = ListFormatting::for_fn(argument_budget, argument_offset);
-    let prefix = format!("{}|{}|", mover, write_list(&arg_items.collect::<Vec<_>>(), &fmt));
+    let list_str = try_opt!(write_list(&arg_items.collect::<Vec<_>>(), &fmt));
+    let prefix = format!("{}|{}|", mover, list_str);
     let closure_indent = closure_indent(context, offset);
 
     // Try to format closure body as a single line expression without braces.
@@ -869,19 +844,58 @@ fn rewrite_call(context: &RewriteContext,
                 -> Option<String> {
     debug!("rewrite_call, width: {}, offset: {}", width, offset);
 
-    // FIXME using byte lens instead of char lens (and probably all over the place too)
     // 2 is for parens
-    let max_callee_width = try_opt!(width.checked_sub(2));
-    let callee_str = try_opt!(callee.rewrite(context, max_callee_width, offset));
-    debug!("rewrite_call, callee_str: `{}`", callee_str);
-
-    if args.is_empty() {
-        return Some(format!("{}()", callee_str));
+    let mut hi = try_opt!(width.checked_sub(2)) * 2;
+    let mut lo = 0;
+    let mut tries = 0;
+
+    // Binary search for the best split between callee and arguments.
+    loop {
+        let middle = (lo + hi) / 2;
+
+        match rewrite_call_inner(context, callee, middle, args, span, width, offset) {
+            _ if tries > 10 => return None,
+            Ok(result) => return Some(result),
+            Err(Ordering::Less) => {
+                lo = middle;
+                tries += 1;
+            }
+            Err(Ordering::Greater) => {
+                hi = middle;
+                tries += 1;
+            }
+            _ => unreachable!(),
+        }
     }
+}
+
+fn rewrite_call_inner(context: &RewriteContext,
+                      callee: &ast::Expr,
+                      max_callee_width: usize,
+                      args: &[ptr::P<ast::Expr>],
+                      span: Span,
+                      width: usize,
+                      offset: usize)
+                      -> Result<String, Ordering> {
+    // FIXME using byte lens instead of char lens (and probably all over the place too)
+    let callee_str = match callee.rewrite(context, max_callee_width, offset) {
+        Some(string) => {
+            if !string.contains('\n') && string.len() > max_callee_width {
+                panic!("{:?} {}", string, max_callee_width);
+            } else {
+                string
+            }
+        }
+        None => return Err(Ordering::Less),
+    };
+    debug!("rewrite_call, callee_str: `{}`", callee_str);
 
     let extra_offset = extra_offset(&callee_str, offset);
     // 2 is for parens.
-    let remaining_width = try_opt!(width.checked_sub(extra_offset + 2));
+    let remaining_width = match width.checked_sub(extra_offset + 2) {
+        Some(str) => str,
+        None => return Err(Ordering::Greater),
+    };
     let offset = offset + extra_offset + 1;
     let inner_indent = expr_indent(context, offset);
     let inner_context = context.overflow_context(inner_indent - context.block_indent);
@@ -900,8 +914,12 @@ fn rewrite_call(context: &RewriteContext,
                              span.hi);
 
     let fmt = ListFormatting::for_fn(remaining_width, offset);
+    let list_str = match write_list(&items.collect::<Vec<_>>(), &fmt) {
+        Some(str) => str,
+        None => return Err(Ordering::Greater),
+    };
 
-    Some(format!("{}({})", callee_str, write_list(&items.collect::<Vec<_>>(), &fmt)))
+    Ok(format!("{}({})", callee_str, list_str))
 }
 
 macro_rules! block_indent_helper {
@@ -1024,7 +1042,7 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext,
         v_width: v_budget,
         ends_with_newline: false,
     };
-    let fields_str = write_list(&items.collect::<Vec<_>>(), &fmt);
+    let fields_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
 
     match context.config.struct_lit_style {
         StructLitStyle::Block if fields_str.contains('\n') => {
@@ -1046,7 +1064,8 @@ fn rewrite_field(context: &RewriteContext,
                  -> Option<String> {
     let name = &field.ident.node.to_string();
     let overhead = name.len() + 2;
-    let expr = field.expr.rewrite(context, try_opt!(width.checked_sub(overhead)), offset + overhead);
+    let expr =
+        field.expr.rewrite(context, try_opt!(width.checked_sub(overhead)), offset + overhead);
     expr.map(|s| format!("{}: {}", name, s))
 }
 
@@ -1080,8 +1099,9 @@ fn rewrite_tuple_lit(context: &RewriteContext,
 
     let budget = try_opt!(width.checked_sub(2));
     let fmt = ListFormatting::for_fn(budget, indent);
+    let list_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
 
-    Some(format!("({})", write_list(&items.collect::<Vec<_>>(), &fmt)))
+    Some(format!("({})", list_str))
 }
 
 fn rewrite_binary_op(context: &RewriteContext,
@@ -1206,12 +1226,13 @@ pub fn rewrite_assign_rhs<S: Into<String>>(context: &RewriteContext,
             let new_offset = offset + context.config.tab_spaces;
             result.push_str(&format!("\n{}", make_indent(new_offset)));
 
+            // FIXME: we probably should related max_width to width instead of config.max_width
+            // where is the 1 coming from anyway?
             let max_width = try_opt!(context.config.max_width.checked_sub(new_offset + 1));
-            let rhs = try_opt!(ex.rewrite(&context.overflow_context(context.config.tab_spaces),
-                                          max_width,
-                                          new_offset));
+            let overflow_context = context.overflow_context(context.config.tab_spaces);
+            let rhs = ex.rewrite(&overflow_context, max_width, new_offset);
 
-            result.push_str(&rhs);
+            result.push_str(&&try_opt!(rhs));
         }
     }
 
diff --git a/src/imports.rs b/src/imports.rs
index cf9e174f9b0..8411b3208df 100644
--- a/src/imports.rs
+++ b/src/imports.rs
@@ -22,13 +22,11 @@ impl Rewrite for ast::ViewPath {
     // Returns an empty string when the ViewPath is empty (like foo::bar::{})
     fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> {
         match self.node {
+            ast::ViewPath_::ViewPathList(_, ref path_list) if path_list.is_empty() => {
+                Some(String::new())
+            }
             ast::ViewPath_::ViewPathList(ref path, ref path_list) => {
-                Some(rewrite_use_list(width,
-                                      offset,
-                                      path,
-                                      path_list,
-                                      self.span,
-                                      context).unwrap_or("".to_owned()))
+                rewrite_use_list(width, offset, path, path_list, self.span, context)
             }
             ast::ViewPath_::ViewPathGlob(_) => {
                 // FIXME convert to list?
@@ -67,8 +65,8 @@ fn rewrite_single_use_list(path_str: String, vpi: ast::PathListItem) -> String {
     }
 }
 
-// Basically just pretty prints a multi-item import.
-// Returns None when the import can be removed.
+// Pretty prints a multi-item import.
+// Assumes that path_list.len() > 0.
 pub fn rewrite_use_list(width: usize,
                         offset: usize,
                         path: &ast::Path,
@@ -80,7 +78,7 @@ pub fn rewrite_use_list(width: usize,
     let path_str = try_opt!(path.rewrite(context, width - 1, offset));
 
     match path_list.len() {
-        0 => return None,
+        0 => unreachable!(),
         1 => return Some(rewrite_single_use_list(path_str, path_list[0])),
         _ => (),
     }
@@ -149,12 +147,12 @@ pub fn rewrite_use_list(width: usize,
         items[1..].sort_by(|a, b| a.item.cmp(&b.item));
     }
 
-    let list = write_list(&items[first_index..], &fmt);
+    let list_str = try_opt!(write_list(&items[first_index..], &fmt));
 
     Some(if path_str.is_empty() {
-            format!("{{{}}}", list)
+            format!("{{{}}}", list_str)
         } else {
-            format!("{}::{{{}}}", path_str, list)
+            format!("{}::{{{}}}", path_str, list_str)
         })
 }
 
diff --git a/src/items.rs b/src/items.rs
index 972866f2638..e2224ce4eff 100644
--- a/src/items.rs
+++ b/src/items.rs
@@ -99,20 +99,20 @@ impl<'a> FmtVisitor<'a> {
                       abi: &abi::Abi,
                       vis: ast::Visibility,
                       span: Span)
-                      -> String {
+                      -> Option<String> {
         let mut newline_brace = self.newline_for_brace(&generics.where_clause);
 
-        let mut result = self.rewrite_fn_base(indent,
-                                              ident,
-                                              fd,
-                                              explicit_self,
-                                              generics,
-                                              unsafety,
-                                              constness,
-                                              abi,
-                                              vis,
-                                              span,
-                                              newline_brace);
+        let mut result = try_opt!(self.rewrite_fn_base(indent,
+                                                       ident,
+                                                       fd,
+                                                       explicit_self,
+                                                       generics,
+                                                       unsafety,
+                                                       constness,
+                                                       abi,
+                                                       vis,
+                                                       span,
+                                                       newline_brace));
 
         if self.config.fn_brace_style != BraceStyle::AlwaysNextLine && !result.contains('\n') {
             newline_brace = false;
@@ -130,7 +130,7 @@ impl<'a> FmtVisitor<'a> {
             result.push(' ');
         }
 
-        result
+        Some(result)
     }
 
     pub fn rewrite_required_fn(&mut self,
@@ -138,26 +138,26 @@ impl<'a> FmtVisitor<'a> {
                                ident: ast::Ident,
                                sig: &ast::MethodSig,
                                span: Span)
-                               -> String {
+                               -> Option<String> {
         // Drop semicolon or it will be interpreted as comment
         let span = codemap::mk_sp(span.lo, span.hi - BytePos(1));
 
-        let mut result = self.rewrite_fn_base(indent,
-                                              ident,
-                                              &sig.decl,
-                                              Some(&sig.explicit_self),
-                                              &sig.generics,
-                                              &sig.unsafety,
-                                              &sig.constness,
-                                              &sig.abi,
-                                              ast::Visibility::Inherited,
-                                              span,
-                                              false);
+        let mut result = try_opt!(self.rewrite_fn_base(indent,
+                                                       ident,
+                                                       &sig.decl,
+                                                       Some(&sig.explicit_self),
+                                                       &sig.generics,
+                                                       &sig.unsafety,
+                                                       &sig.constness,
+                                                       &sig.abi,
+                                                       ast::Visibility::Inherited,
+                                                       span,
+                                                       false));
 
         // Re-attach semicolon
         result.push(';');
 
-        result
+        Some(result)
     }
 
     fn rewrite_fn_base(&mut self,
@@ -172,7 +172,7 @@ impl<'a> FmtVisitor<'a> {
                        vis: ast::Visibility,
                        span: Span,
                        newline_brace: bool)
-                       -> String {
+                       -> Option<String> {
         // FIXME we'll lose any comments in between parts of the function decl, but anyone
         // who comments there probably deserves what they get.
 
@@ -200,11 +200,12 @@ impl<'a> FmtVisitor<'a> {
 
         // Generics.
         let generics_indent = indent + result.len();
-        result.push_str(&self.rewrite_generics(generics,
-                                               indent,
-                                               generics_indent,
-                                               codemap::mk_sp(span.lo,
-                                                              span_for_return(&fd.output).lo)));
+        let generics_span = codemap::mk_sp(span.lo, span_for_return(&fd.output).lo);
+        let generics_str = try_opt!(self.rewrite_generics(generics,
+                                                          indent,
+                                                          generics_indent,
+                                                          generics_span));
+        result.push_str(&generics_str);
 
         let ret_str = self.rewrite_return(&fd.output, indent);
 
@@ -243,13 +244,14 @@ impl<'a> FmtVisitor<'a> {
                                                   "(",
                                                   self.codemap),
                                        span_for_return(&fd.output).lo);
-        result.push_str(&self.rewrite_args(&fd.inputs,
-                                           explicit_self,
-                                           one_line_budget,
-                                           multi_line_budget,
-                                           indent,
-                                           arg_indent,
-                                           args_span));
+        let arg_str = try_opt!(self.rewrite_args(&fd.inputs,
+                                                 explicit_self,
+                                                 one_line_budget,
+                                                 multi_line_budget,
+                                                 indent,
+                                                 arg_indent,
+                                                 args_span));
+        result.push_str(&arg_str);
         if self.config.fn_args_layout == StructLitStyle::Block {
             result.push('\n');
         }
@@ -307,13 +309,14 @@ impl<'a> FmtVisitor<'a> {
         };
 
         // Where clause.
-        result.push_str(&self.rewrite_where_clause(where_clause,
-                                                   self.config,
-                                                   indent,
-                                                   where_density,
-                                                   span.hi));
-
-        result
+        let where_clause_str = try_opt!(self.rewrite_where_clause(where_clause,
+                                                                  self.config,
+                                                                  indent,
+                                                                  where_density,
+                                                                  span.hi));
+        result.push_str(&where_clause_str);
+
+        Some(result)
     }
 
     fn rewrite_args(&self,
@@ -324,7 +327,7 @@ impl<'a> FmtVisitor<'a> {
                     indent: usize,
                     arg_indent: usize,
                     span: Span)
-                    -> String {
+                    -> Option<String> {
         let mut arg_item_strs: Vec<_> = args.iter().map(rewrite_fn_input).collect();
         // Account for sugary self.
         // FIXME: the comment for the self argument is dropped. This is blocked
@@ -461,8 +464,8 @@ impl<'a> FmtVisitor<'a> {
                                                 " {",
                                                 self.block_indent,
                                                 self.block_indent + self.config.tab_spaces,
-                                                codemap::mk_sp(span.lo,
-                                                               body_start));
+                                                codemap::mk_sp(span.lo, body_start))
+                               .unwrap();
         self.buffer.push_str(&generics_str);
 
         self.last_pos = body_start;
@@ -534,7 +537,12 @@ impl<'a> FmtVisitor<'a> {
                         v_width: budget,
                         ends_with_newline: true,
                     };
-                    result.push_str(&write_list(&items.collect::<Vec<_>>(), &fmt));
+                    let list_str = match write_list(&items.collect::<Vec<_>>(), &fmt) {
+                        Some(list_str) => list_str,
+                        None => return,
+                    };
+
+                    result.push_str(&list_str);
                     result.push(')');
                 }
 
@@ -554,13 +562,18 @@ impl<'a> FmtVisitor<'a> {
             }
             ast::VariantKind::StructVariantKind(ref struct_def) => {
                 // TODO Should limit the width, as we have a trailing comma
-                self.format_struct("",
-                                   field.node.name,
-                                   field.node.vis,
-                                   struct_def,
-                                   None,
-                                   field.span,
-                                   self.block_indent)
+                let struct_rewrite = self.format_struct("",
+                                                        field.node.name,
+                                                        field.node.vis,
+                                                        struct_def,
+                                                        None,
+                                                        field.span,
+                                                        self.block_indent);
+
+                match struct_rewrite {
+                    Some(struct_str) => struct_str,
+                    None => return,
+                }
             }
         };
         self.buffer.push_str(&result);
@@ -580,7 +593,7 @@ impl<'a> FmtVisitor<'a> {
                      generics: Option<&ast::Generics>,
                      span: Span,
                      offset: usize)
-                     -> String {
+                     -> Option<String> {
         let mut result = String::with_capacity(1024);
 
         let header_str = self.format_header(item_name, ident, vis);
@@ -588,7 +601,7 @@ impl<'a> FmtVisitor<'a> {
 
         if struct_def.fields.is_empty() {
             result.push(';');
-            return result;
+            return Some(result);
         }
 
         let is_tuple = match struct_def.fields[0].node.kind {
@@ -603,12 +616,14 @@ impl<'a> FmtVisitor<'a> {
         };
 
         let generics_str = match generics {
-            Some(g) => self.format_generics(g,
-                                            opener,
-                                            offset,
-                                            offset + header_str.len(),
-                                            codemap::mk_sp(span.lo,
-                                                           struct_def.fields[0].span.lo)),
+            Some(g) => {
+                try_opt!(self.format_generics(g,
+                                              opener,
+                                              offset,
+                                              offset + header_str.len(),
+                                              codemap::mk_sp(span.lo,
+                                                             struct_def.fields[0].span.lo)))
+            }
             None => opener.to_owned(),
         };
         result.push_str(&generics_str);
@@ -658,8 +673,9 @@ impl<'a> FmtVisitor<'a> {
             v_width: budget,
             ends_with_newline: true,
         };
+        let list_str = write_list(&items.collect::<Vec<_>>(), &fmt).unwrap();
 
-        result.push_str(&write_list(&items.collect::<Vec<_>>(), &fmt));
+        result.push_str(&list_str);
 
         if break_line {
             result.push('\n');
@@ -672,7 +688,7 @@ impl<'a> FmtVisitor<'a> {
             result.push(';');
         }
 
-        result
+        Some(result)
     }
 
     pub fn visit_struct(&mut self,
@@ -688,7 +704,9 @@ impl<'a> FmtVisitor<'a> {
                                         struct_def,
                                         Some(generics),
                                         span,
-                                        indent);
+                                        indent)
+                         .unwrap();
+
         self.buffer.push_str(&result);
         self.last_pos = span.hi;
     }
@@ -703,15 +721,16 @@ impl<'a> FmtVisitor<'a> {
                        offset: usize,
                        generics_offset: usize,
                        span: Span)
-                       -> String {
-        let mut result = self.rewrite_generics(generics, offset, generics_offset, span);
+                       -> Option<String> {
+        let mut result = try_opt!(self.rewrite_generics(generics, offset, generics_offset, span));
 
         if !generics.where_clause.predicates.is_empty() || result.contains('\n') {
-            result.push_str(&self.rewrite_where_clause(&generics.where_clause,
-                                                       self.config,
-                                                       self.block_indent,
-                                                       Density::Tall,
-                                                       span.hi));
+            let where_clause_str = try_opt!(self.rewrite_where_clause(&generics.where_clause,
+                                                                      self.config,
+                                                                      self.block_indent,
+                                                                      Density::Tall,
+                                                                      span.hi));
+            result.push_str(&where_clause_str);
             result.push_str(&make_indent(self.block_indent));
             result.push('\n');
             result.push_str(opener.trim());
@@ -719,7 +738,7 @@ impl<'a> FmtVisitor<'a> {
             result.push_str(opener);
         }
 
-        result
+        Some(result)
     }
 
     // Field of a struct
@@ -761,13 +780,13 @@ impl<'a> FmtVisitor<'a> {
                         offset: usize,
                         generics_offset: usize,
                         span: Span)
-                        -> String {
+                        -> Option<String> {
         // FIXME convert bounds to where clauses where they get too big or if
         // there is a where clause at all.
         let lifetimes: &[_] = &generics.lifetimes;
         let tys: &[_] = &generics.ty_params;
         if lifetimes.is_empty() && tys.is_empty() {
-            return String::new();
+            return Some(String::new());
         }
 
         let offset = match self.config.generics_indent {
@@ -816,8 +835,9 @@ impl<'a> FmtVisitor<'a> {
         }
 
         let fmt = ListFormatting::for_fn(h_budget, offset);
+        let list_str = try_opt!(write_list(&items, &fmt));
 
-        format!("<{}>", write_list(&items, &fmt))
+        Some(format!("<{}>", list_str))
     }
 
     fn rewrite_where_clause(&self,
@@ -826,9 +846,9 @@ impl<'a> FmtVisitor<'a> {
                             indent: usize,
                             density: Density,
                             span_end: BytePos)
-                            -> String {
+                            -> Option<String> {
         if where_clause.predicates.is_empty() {
-            return String::new();
+            return Some(String::new());
         }
 
         let extra_indent = match self.config.where_indent {
@@ -870,16 +890,16 @@ impl<'a> FmtVisitor<'a> {
             v_width: budget,
             ends_with_newline: true,
         };
-        let preds_str = write_list(&items.collect::<Vec<_>>(), &fmt);
+        let preds_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
 
         // 9 = " where ".len() + " {".len()
         if density == Density::Tall || preds_str.contains('\n') ||
            indent + 9 + preds_str.len() > self.config.max_width {
-            format!("\n{}where {}",
-                    make_indent(indent + extra_indent),
-                    preds_str)
+            Some(format!("\n{}where {}",
+                         make_indent(indent + extra_indent),
+                         preds_str))
         } else {
-            format!(" where {}", preds_str)
+            Some(format!(" where {}", preds_str))
         }
     }
 
diff --git a/src/lists.rs b/src/lists.rs
index cc51e1286b9..e21327631d4 100644
--- a/src/lists.rs
+++ b/src/lists.rs
@@ -13,7 +13,7 @@ use std::iter::Peekable;
 
 use syntax::codemap::{self, CodeMap, BytePos};
 
-use utils::{round_up_to_power_of_two, make_indent};
+use utils::{round_up_to_power_of_two, make_indent, wrap_str};
 use comment::{FindUncommented, rewrite_comment, find_comment_end};
 
 #[derive(Eq, PartialEq, Debug, Copy, Clone)]
@@ -95,9 +95,9 @@ impl ListItem {
 // Format a list of commented items into a string.
 // FIXME: this has grown into a monstrosity
 // TODO: add unit tests
-pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> String {
+pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Option<String> {
     if items.is_empty() {
-        return String::new();
+        return Some(String::new());
     }
 
     let mut tactic = formatting.tactic;
@@ -204,7 +204,9 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St
             }
         }
 
-        result.push_str(&item.item);
+        // FIXME: no magic numbers!
+        let item_str = wrap_str(&item.item[..], 100, formatting.v_width, formatting.indent);
+        result.push_str(&&try_opt!(item_str));
 
         // Post-comments
         if tactic != ListTactic::Vertical && item.post_comment.is_some() {
@@ -240,7 +242,7 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St
         }
     }
 
-    result
+    Some(result)
 }
 
 pub struct ListItems<'a, I, F1, F2, F3>
diff --git a/src/types.rs b/src/types.rs
index dfb7f255a98..58bb930510c 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -87,6 +87,12 @@ fn rewrite_path_segments<'a, I>(mut buffer: String,
     let mut first = true;
 
     for segment in iter {
+        if first {
+            first = false;
+        } else {
+            buffer.push_str("::");
+        }
+
         let extra_offset = extra_offset(&buffer, offset);
         let remaining_width = try_opt!(width.checked_sub(extra_offset));
         let new_offset = offset + extra_offset;
@@ -97,12 +103,6 @@ fn rewrite_path_segments<'a, I>(mut buffer: String,
                                                       remaining_width,
                                                       new_offset));
 
-        if first {
-            first = false;
-        } else {
-            buffer.push_str("::");
-        }
-
         buffer.push_str(&segment_string);
     }
 
@@ -218,18 +218,20 @@ fn rewrite_segment(segment: &ast::PathSegment,
                                      ">",
                                      |param| param.get_span().lo,
                                      |param| param.get_span().hi,
+                                     // FIXME: need better params
                                      |seg| {
-                                         seg.rewrite(context, list_width, offset + extra_offset).unwrap()
+                                         seg.rewrite(context, 1000, offset + extra_offset).unwrap()
                                      },
                                      list_lo,
                                      span_hi);
 
             let fmt = ListFormatting::for_fn(list_width, offset + extra_offset);
+            let list_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
 
             // update pos
             *span_lo = next_span_lo;
 
-            format!("{}<{}>", separator, write_list(&items.collect::<Vec<_>>(), &fmt))
+            format!("{}<{}>", separator, list_str)
         }
         ast::PathParameters::ParenthesizedParameters(ref data) => {
             let output = match data.output {
@@ -252,11 +254,12 @@ fn rewrite_segment(segment: &ast::PathSegment,
 
             // 1 for (
             let fmt = ListFormatting::for_fn(budget, offset + 1);
+            let list_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
 
             // update pos
             *span_lo = data.span.hi + BytePos(1);
 
-            format!("({}){}", write_list(&items.collect::<Vec<_>>(), &fmt), output)
+            format!("({}){}", list_str, output)
         }
         _ => String::new(),
     };
@@ -354,7 +357,8 @@ impl Rewrite for ast::TyParamBound {
 
 impl Rewrite for ast::TyParamBounds {
     fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> {
-        let strs: Vec<_> = self.iter().map(|b| b.rewrite(context, width, offset).unwrap()).collect();
+        let strs: Vec<_> =
+            self.iter().map(|b| b.rewrite(context, width, offset).unwrap()).collect();
         Some(strs.join(" + "))
     }
 }
diff --git a/src/utils.rs b/src/utils.rs
index 936a712ca18..07807bf31aa 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -164,6 +164,38 @@ macro_rules! try_opt {
     })
 }
 
+pub fn wrap_str<S: AsRef<str>>(s: S, max_width: usize, width: usize, offset: usize) -> Option<S> {
+    let snippet = s.as_ref();
+
+    if !snippet.contains('\n') && snippet.len() > width {
+        return None;
+    } else {
+        let mut lines = snippet.lines();
+
+        // The caller of this function has already placed `offset`
+        // characters on the first line.
+        let first_line_max_len = try_opt!(max_width.checked_sub(offset));
+        if lines.next().unwrap().len() > first_line_max_len {
+            return None;
+        }
+
+        // The other lines must fit within the maximum width.
+        if lines.find(|line| line.len() > max_width).is_some() {
+            return None;
+        }
+
+        // `width` is the maximum length of the last line, excluding
+        // indentation.
+        // A special check for the last line, since the caller may
+        // place trailing characters on this line.
+        if snippet.lines().rev().next().unwrap().len() > offset + width {
+            return None;
+        }
+    }
+
+    Some(s)
+}
+
 #[test]
 fn power_rounding() {
     assert_eq!(0, round_up_to_power_of_two(0));
diff --git a/src/visitor.rs b/src/visitor.rs
index c2e4013e182..c1c1f2fcbbd 100644
--- a/src/visitor.rs
+++ b/src/visitor.rs
@@ -87,7 +87,7 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
 
         // Check if this block has braces.
         let snippet = self.snippet(b.span);
-        let has_braces = snippet.chars().next().unwrap() == '{' || &snippet[..6] == "unsafe";
+        let has_braces = &snippet[..1] == "{" || &snippet[..6] == "unsafe";
         let brace_compensation = if has_braces {
             BytePos(1)
         } else {
@@ -125,43 +125,45 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
                 b: &'v ast::Block,
                 s: Span,
                 _: ast::NodeId) {
-        self.format_missing_with_indent(s.lo);
-        self.last_pos = s.lo;
-
         let indent = self.block_indent;
-        match fk {
+        let rewrite = match fk {
             visit::FnKind::ItemFn(ident,
-                            ref generics,
-                            ref unsafety,
-                            ref constness,
-                            ref abi,
-                            vis) => {
-                let new_fn = self.rewrite_fn(indent,
-                                             ident,
-                                             fd,
-                                             None,
-                                             generics,
-                                             unsafety,
-                                             constness,
-                                             abi,
-                                             vis,
-                                             codemap::mk_sp(s.lo, b.span.lo));
-                self.buffer.push_str(&new_fn);
+                                  ref generics,
+                                  ref unsafety,
+                                  ref constness,
+                                  ref abi,
+                                  vis) => {
+                self.rewrite_fn(indent,
+                                ident,
+                                fd,
+                                None,
+                                generics,
+                                unsafety,
+                                constness,
+                                abi,
+                                vis,
+                                codemap::mk_sp(s.lo, b.span.lo))
             }
             visit::FnKind::Method(ident, ref sig, vis) => {
-                let new_fn = self.rewrite_fn(indent,
-                                             ident,
-                                             fd,
-                                             Some(&sig.explicit_self),
-                                             &sig.generics,
-                                             &sig.unsafety,
-                                             &sig.constness,
-                                             &sig.abi,
-                                             vis.unwrap_or(ast::Visibility::Inherited),
-                                             codemap::mk_sp(s.lo, b.span.lo));
-                self.buffer.push_str(&new_fn);
+                self.rewrite_fn(indent,
+                                ident,
+                                fd,
+                                Some(&sig.explicit_self),
+                                &sig.generics,
+                                &sig.unsafety,
+                                &sig.constness,
+                                &sig.abi,
+                                vis.unwrap_or(ast::Visibility::Inherited),
+                                codemap::mk_sp(s.lo, b.span.lo))
             }
-            visit::FnKind::Closure => {}
+            visit::FnKind::Closure => None,
+        };
+
+        if let Some(fn_str) = rewrite {
+            self.format_missing_with_indent(s.lo);
+            self.buffer.push_str(&fn_str);
+        } else {
+            self.format_missing(b.span.lo);
         }
 
         self.last_pos = b.span.lo;
@@ -239,8 +241,11 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
                                                   sig,
                                                   ti.span);
 
-            self.buffer.push_str(&new_fn);
-            self.last_pos = ti.span.hi;
+
+            if let Some(fn_str) = new_fn {
+                self.buffer.push_str(&fn_str);
+                self.last_pos = ti.span.hi;
+            }
         }
         // TODO format trait types
 
diff --git a/tests/source/imports.rs b/tests/source/imports.rs
index f759de3e7bb..92380ea51b0 100644
--- a/tests/source/imports.rs
+++ b/tests/source/imports.rs
@@ -2,8 +2,8 @@
 
 // Long import.
 use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl};
-use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA,
-                                                                                                ItemB};
+use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA, ItemB};
+use exceedingly::loooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA, ItemB};
 
 use list::{
     // Some item
diff --git a/tests/source/paths.rs b/tests/source/paths.rs
index d2b4a66622a..9a5513e1589 100644
--- a/tests/source/paths.rs
+++ b/tests/source/paths.rs
@@ -1,6 +1,5 @@
 
 fn main() {
-    // FIXME(#133): the list rewrite should fail and force a different format
    let constellation_chan = Constellation::<layout::layout_task::LayoutTask,  script::script_task::ScriptTask> ::start(
      compositor_proxy,
      resource_task,
diff --git a/tests/target/imports.rs b/tests/target/imports.rs
index bbef3e85e16..07449a51982 100644
--- a/tests/target/imports.rs
+++ b/tests/target/imports.rs
@@ -2,8 +2,9 @@
 
 // Long import.
 use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl};
-use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA,
-                                                                                                ItemB};
+use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA, ItemB};
+use exceedingly::loooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA,
+                                                                                             ItemB};
 
 use list::{// Some item
            SomeItem, // Comment
diff --git a/tests/target/paths.rs b/tests/target/paths.rs
index 4fa3a52fe6c..fd97bb4d71c 100644
--- a/tests/target/paths.rs
+++ b/tests/target/paths.rs
@@ -1,16 +1,16 @@
 
 fn main() {
-    // FIXME(#133): the list rewrite should fail and force a different format
-    let constellation_chan = Constellation::<layout::layout_task::LayoutTask,
-                                             script::script_task::ScriptTask>::start(compositor_proxy,
-                                                                                     resource_task,
-                                                                                     image_cache_task,
-                                                                                     font_cache_task,
-                                                                                     time_profiler_chan,
-                                                                                     mem_profiler_chan,
-                                                                                     devtools_chan,
-                                                                                     storage_task,
-                                                                                     supports_clipboard);
+    let constellation_chan =
+        Constellation::<layout::layout_task::LayoutTask,
+                        script::script_task::ScriptTask>::start(compositor_proxy,
+                                                                resource_task,
+                                                                image_cache_task,
+                                                                font_cache_task,
+                                                                time_profiler_chan,
+                                                                mem_profiler_chan,
+                                                                devtools_chan,
+                                                                storage_task,
+                                                                supports_clipboard);
 
     Quux::<ParamOne /* Comment 1 */, ParamTwo /* Comment 2 */>::some_func();
 
diff --git a/tests/target/tuple.rs b/tests/target/tuple.rs
index a77fae11cb4..a5bcc7f701a 100644
--- a/tests/target/tuple.rs
+++ b/tests/target/tuple.rs
@@ -4,7 +4,7 @@ fn foo() {
     let a = (a, a, a, a, a);
     let aaaaaaaaaaaaaaaa = (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaa, aaaaaaaaaaaaaa);
     let aaaaaaaaaaaaaaaaaaaaaa = (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
-                                  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
+                                  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
                                   aaaaaaaaaaaaaaaaaaaaaaaaa,
                                   aaaa);
     let a = (a,);