]> git.lizzy.rs Git - rust.git/commitdiff
Make listItem contain option
authorMarcus Klaas <mail@marcusklaas.nl>
Sun, 4 Oct 2015 18:20:15 +0000 (20:20 +0200)
committerMarcus Klaas <mail@marcusklaas.nl>
Sun, 4 Oct 2015 18:40:14 +0000 (20:40 +0200)
src/config.rs
src/expr.rs
src/imports.rs
src/items.rs
src/lists.rs
src/types.rs

index 14d46a76834527c971d066f2414c49a428b7c6eb..1688b4b363ea8f7e3c75c7cb62f07ef9fe8c5ec3 100644 (file)
@@ -90,7 +90,6 @@ pub fn to_list_tactic(self) -> ListTactic {
     FileLicense,
 }
 
-// TODO: this is not necessary any more
 configuration_option_enum! { MultilineStyle:
     // Use horizontal layout if it fits in one line, fall back to vertical
     PreferSingle,
index d51bd256caa5a7c3353161e8cab9a877f33037fc..53e24b70984eb806d156269db9f9bdff242fc800 100644 (file)
@@ -218,18 +218,16 @@ pub fn rewrite_array<'a, I>(expr_iter: I,
                              |item| item.span.lo,
                              |item| item.span.hi,
                              // 1 = [
-                             // FIXME(#133): itemize_list doesn't support
-                             // rewrite failure. This may not be its
-                             // responsibility, but that of write_list.
-                             |item| {
-                                 item.rewrite(context, max_item_width, offset + 1)
-                                     .unwrap_or_else(|| context.snippet(item.span))
-                             },
+                             |item| item.rewrite(context, max_item_width, offset + 1),
                              span_after(span, "[", context.codemap),
                              span.hi)
                     .collect::<Vec<_>>();
 
-    let tactic = if items.iter().any(|li| li.item.len() > 10 || li.is_multiline()) {
+    let has_long_item = try_opt!(items.iter()
+                                      .map(|li| li.item.as_ref().map(|s| s.len() > 10))
+                                      .fold(Some(false),
+                                            |acc, x| acc.and_then(|y| x.map(|x| (x || y)))));
+    let tactic = if has_long_item || items.iter().any(|li| li.is_multiline()) {
         definitive_tactic(&items, ListTactic::HorizontalVertical, max_item_width)
     } else {
         DefinitiveListTactic::Mixed
@@ -280,15 +278,7 @@ fn rewrite_closure(capture: ast::CaptureClause,
                                  "|",
                                  |arg| span_lo_for_arg(arg),
                                  |arg| span_hi_for_arg(arg),
-                                 |arg| {
-                                     // FIXME: we should just escalate failure
-                                     // here, but itemize_list doesn't allow it.
-                                     arg.rewrite(context, budget, argument_offset)
-                                        .unwrap_or_else(|| {
-                                            context.snippet(mk_sp(span_lo_for_arg(arg),
-                                                                  span_hi_for_arg(arg)))
-                                        })
-                                 },
+                                 |arg| arg.rewrite(context, budget, argument_offset),
                                  span_after(span, "|", context.codemap),
                                  body.span.lo);
     let item_vec = arg_items.collect::<Vec<_>>();
@@ -1151,11 +1141,7 @@ fn rewrite_call_inner<R>(context: &RewriteContext,
                              ")",
                              |item| item.span.lo,
                              |item| item.span.hi,
-                             // Take old span when rewrite fails.
-                             |item| {
-                                 item.rewrite(&inner_context, remaining_width, offset)
-                                     .unwrap_or(context.snippet(item.span))
-                             },
+                             |item| item.rewrite(&inner_context, remaining_width, offset),
                              span.lo,
                              span.hi);
 
@@ -1251,18 +1237,13 @@ enum StructLitField<'a> {
                                  match *item {
                                      StructLitField::Regular(ref field) => {
                                          rewrite_field(inner_context, &field, v_budget, indent)
-                                             .unwrap_or(context.snippet(field.span))
                                      }
                                      StructLitField::Base(ref expr) => {
                                          // 2 = ..
-                                         format!("..{}",
-                                                 v_budget.checked_sub(2)
-                                                         .and_then(|v_budget| {
-                                                             expr.rewrite(inner_context,
-                                                                          v_budget,
-                                                                          indent + 2)
-                                                         })
-                                                         .unwrap_or(context.snippet(expr.span)))
+                                         expr.rewrite(inner_context,
+                                                      try_opt!(v_budget.checked_sub(2)),
+                                                      indent + 2)
+                                             .map(|s| format!("..{}", s))
                                      }
                                  }
                              },
@@ -1365,7 +1346,6 @@ fn rewrite_tuple_lit(context: &RewriteContext,
                              |item| {
                                  let inner_width = context.config.max_width - indent.width() - 1;
                                  item.rewrite(context, inner_width, indent)
-                                     .unwrap_or(context.snippet(item.span))
                              },
                              span.lo + BytePos(1), // Remove parens
                              span.hi - BytePos(1));
index 9c29ece623f9639260df0cdb8db2529ceb1ce46c..2c518378b36a52f223137dd619fcb5a537af4367 100644 (file)
@@ -71,7 +71,7 @@ fn rewrite_single_use_list(path_str: String, vpi: &ast::PathListItem) -> String
     append_alias(path_item_str, vpi)
 }
 
-fn rewrite_path_item(vpi: &&ast::PathListItem) -> String {
+fn rewrite_path_item(vpi: &&ast::PathListItem) -> Option<String> {
     let path_item_str = match vpi.node {
         ast::PathListItem_::PathListIdent{ name, .. } => {
             name.to_string()
@@ -81,7 +81,7 @@ fn rewrite_path_item(vpi: &&ast::PathListItem) -> String {
         }
     };
 
-    append_alias(path_item_str, vpi)
+    Some(append_alias(path_item_str, vpi))
 }
 
 fn append_alias(path_item_str: String, vpi: &ast::PathListItem) -> String {
@@ -142,8 +142,6 @@ pub fn rewrite_use_list(width: usize,
     // We prefixed the item list with a dummy value so that we can
     // potentially move "self" to the front of the vector without touching
     // the rest of the items.
-    // FIXME: Make more efficient by using a linked list? That would require
-    // changes to the signatures of write_list.
     let has_self = move_self_to_front(&mut items);
     let first_index = if has_self {
         0
@@ -181,7 +179,7 @@ pub fn rewrite_use_list(width: usize,
 
 // Returns true when self item was found.
 fn move_self_to_front(items: &mut Vec<ListItem>) -> bool {
-    match items.iter().position(|item| item.item == "self") {
+    match items.iter().position(|item| item.item.as_ref().map(|x| &x[..]) == Some("self")) {
         Some(pos) => {
             items[0] = items.remove(pos);
             true
index c2f547162beaca0045e797c92244dd8dcbe43433..e2f59f0fb6a3b197b45133c46f2daa9dcc533d0c 100644 (file)
@@ -481,7 +481,7 @@ fn rewrite_args(&self,
                                           ")",
                                           |arg| span_lo_for_arg(arg),
                                           |arg| arg.ty.span.hi,
-                                          |_| String::new(),
+                                          |_| None,
                                           comment_span_start,
                                           span.hi);
 
@@ -491,7 +491,7 @@ fn rewrite_args(&self,
         assert_eq!(arg_item_strs.len(), arg_items.len());
 
         for (item, arg) in arg_items.iter_mut().zip(arg_item_strs) {
-            item.item = arg;
+            item.item = Some(arg);
         }
 
         let indent = match self.config.fn_arg_indent {
@@ -630,11 +630,9 @@ fn visit_variant(&mut self, field: &ast::Variant, last_field: bool, next_span_st
                                              |arg| arg.ty.span.hi,
                                              |arg| {
                                                  // FIXME silly width, indent
-                                                 arg.ty
-                                                    .rewrite(&self.get_context(),
-                                                             1000,
-                                                             Indent::empty())
-                                                    .unwrap()
+                                                 arg.ty.rewrite(&self.get_context(),
+                                                                1000,
+                                                                Indent::empty())
                                              },
                                              span_after(field.span, "(", self.codemap),
                                              next_span_start);
@@ -863,9 +861,14 @@ fn format_generics(&self,
     }
 
     // Field of a struct
-    fn format_field(&self, field: &ast::StructField) -> String {
+    fn format_field(&self, field: &ast::StructField) -> Option<String> {
         if contains_skip(&field.node.attrs) {
-            return self.snippet(codemap::mk_sp(field.node.attrs[0].span.lo, field.span.hi));
+            // FIXME: silly width, indent
+            return wrap_str(self.snippet(codemap::mk_sp(field.node.attrs[0].span.lo,
+                                                        field.span.hi)),
+                            self.config.max_width,
+                            1000,
+                            Indent::empty());
         }
 
         let name = match field.node.kind {
@@ -877,24 +880,23 @@ fn format_field(&self, field: &ast::StructField) -> String {
             ast::StructFieldKind::UnnamedField(vis) => format_visibility(vis),
         };
         // FIXME silly width, indent
-        let typ = field.node.ty.rewrite(&self.get_context(), 1000, Indent::empty()).unwrap();
+        let typ = try_opt!(field.node.ty.rewrite(&self.get_context(), 1000, Indent::empty()));
 
         let indent = self.block_indent.block_indent(self.config);
-        let mut attr_str = field.node
-                                .attrs
-                                .rewrite(&self.get_context(),
-                                         self.config.max_width - indent.width(),
-                                         indent)
-                                .unwrap();
+        let mut attr_str = try_opt!(field.node
+                                         .attrs
+                                         .rewrite(&self.get_context(),
+                                                  self.config.max_width - indent.width(),
+                                                  indent));
         if !attr_str.is_empty() {
             attr_str.push('\n');
             attr_str.push_str(&indent.to_string(self.config));
         }
 
-        match name {
+        Some(match name {
             Some(name) => format!("{}{}{}: {}", attr_str, vis, name, typ),
             None => format!("{}{}{}", attr_str, vis, typ),
-        }
+        })
     }
 
     fn rewrite_generics(&self,
@@ -923,10 +925,8 @@ fn rewrite_generics(&self,
 
         // Strings for the generics.
         let context = self.get_context();
-        // FIXME: don't unwrap
-        let lt_strs = lifetimes.iter().map(|lt| lt.rewrite(&context, h_budget, offset).unwrap());
-        let ty_strs = tys.iter()
-                         .map(|ty_param| ty_param.rewrite(&context, h_budget, offset).unwrap());
+        let lt_strs = lifetimes.iter().map(|lt| lt.rewrite(&context, h_budget, offset));
+        let ty_strs = tys.iter().map(|ty_param| ty_param.rewrite(&context, h_budget, offset));
 
         // Extract comments between generics.
         let lt_spans = lifetimes.iter().map(|l| {
@@ -988,10 +988,7 @@ fn rewrite_where_clause(&self,
                                  "{",
                                  |pred| span_for_where_pred(pred).lo,
                                  |pred| span_for_where_pred(pred).hi,
-                                 // FIXME: we should handle failure better
-                                 // this will be taken care of when write_list
-                                 // takes Rewrite object: see issue #133
-                                 |pred| pred.rewrite(&context, budget, offset).unwrap(),
+                                 |pred| pred.rewrite(&context, budget, offset),
                                  span_start,
                                  span_end);
         let item_vec = items.collect::<Vec<_>>();
index 296df76e688e6c4fb0ceedb5571254c7f4810ce7..514acd13f538d6633164ae33ea1ff1f7db2e6b5d 100644 (file)
@@ -43,8 +43,6 @@ pub enum SeparatorTactic {
 
 impl_enum_decodable!(SeparatorTactic, Always, Never, Vertical);
 
-// TODO having some helpful ctors for ListFormatting would be nice.
-// FIXME: this should have only 1 width param
 pub struct ListFormatting<'a> {
     pub tactic: DefinitiveListTactic,
     pub separator: &'a str,
@@ -111,9 +109,11 @@ fn as_ref(&self) -> &ListItem {
 }
 
 pub struct ListItem {
+    // None for comments mean that they are not present.
     pub pre_comment: Option<String>,
-    // Item should include attributes and doc comments.
-    pub item: String,
+    // Item should include attributes and doc comments. None indicates failed
+    // rewrite.
+    pub item: Option<String>,
     pub post_comment: Option<String>,
     // Whether there is extra whitespace before this item.
     pub new_lines: bool,
@@ -121,7 +121,9 @@ pub struct ListItem {
 
 impl ListItem {
     pub fn is_multiline(&self) -> bool {
-        self.item.contains('\n') || self.pre_comment.is_some() ||
+        // FIXME: fail earlier!
+        self.item.as_ref().map(|s| s.contains('\n')).unwrap_or(false) ||
+        self.pre_comment.is_some() ||
         self.post_comment.as_ref().map(|s| s.contains('\n')).unwrap_or(false)
     }
 
@@ -132,7 +134,7 @@ pub fn has_line_pre_comment(&self) -> bool {
     pub fn from_str<S: Into<String>>(s: S) -> ListItem {
         ListItem {
             pre_comment: None,
-            item: s.into(),
+            item: Some(s.into()),
             post_comment: None,
             new_lines: false,
         }
@@ -198,6 +200,7 @@ pub fn write_list<'b, I, T>(items: I, formatting: &ListFormatting<'b>) -> Option
     let indent_str = &formatting.indent.to_string(formatting.config);
     while let Some((i, item)) = iter.next() {
         let item = item.as_ref();
+        let inner_item = try_opt!(item.item.as_ref());
         let first = i == 0;
         let last = iter.peek().is_none();
         let separate = !last || trailing_separator;
@@ -206,7 +209,7 @@ pub fn write_list<'b, I, T>(items: I, formatting: &ListFormatting<'b>) -> Option
         } else {
             0
         };
-        let item_width = item.item.len() + item_sep_len;
+        let item_width = inner_item.len() + item_sep_len;
 
         match tactic {
             DefinitiveListTactic::Horizontal if !first => {
@@ -256,7 +259,8 @@ pub fn write_list<'b, I, T>(items: I, formatting: &ListFormatting<'b>) -> Option
             }
         }
 
-        let item_str = try_opt!(wrap_str(&item.item[..],
+        // Make sure that string actually fits.
+        let item_str = try_opt!(wrap_str(&inner_item[..],
                                          formatting.config.max_width,
                                          formatting.width,
                                          formatting.indent));
@@ -325,7 +329,7 @@ impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3>
     where I: Iterator<Item = T>,
           F1: Fn(&T) -> BytePos,
           F2: Fn(&T) -> BytePos,
-          F3: Fn(&T) -> String
+          F3: Fn(&T) -> Option<String>
 {
     type Item = ListItem;
 
@@ -449,7 +453,7 @@ pub fn itemize_list<'a, T, I, F1, F2, F3>(codemap: &'a CodeMap,
     where I: Iterator<Item = T>,
           F1: Fn(&T) -> BytePos,
           F2: Fn(&T) -> BytePos,
-          F3: Fn(&T) -> String
+          F3: Fn(&T) -> Option<String>
 {
     ListItems {
         codemap: codemap,
@@ -484,8 +488,11 @@ fn calculate_width<'li, I, T>(items: I) -> (usize, usize)
 }
 
 fn total_item_width(item: &ListItem) -> usize {
+    // FIXME: If the item has a `None` item, it may be better to fail earlier
+    // rather than later.
     comment_len(item.pre_comment.as_ref().map(|x| &(*x)[..])) +
-    comment_len(item.post_comment.as_ref().map(|x| &(*x)[..])) + item.item.len()
+    comment_len(item.post_comment.as_ref().map(|x| &(*x)[..])) +
+    item.item.as_ref().map(|str| str.len()).unwrap_or(0)
 }
 
 fn comment_len(comment: Option<&str>) -> usize {
index 9fe837fced4515db17091c7500efd0365b733aa0..6f6afbfc42ed43f05edba0b1c1be9781c04f7bb9 100644 (file)
@@ -219,14 +219,10 @@ fn rewrite_segment(segment: &ast::PathSegment,
                                      ">",
                                      |param| param.get_span().lo,
                                      |param| param.get_span().hi,
-                                     // FIXME(#133): write_list should call
-                                     // rewrite itself, because it has a better
-                                     // context.
                                      |seg| {
                                          seg.rewrite(context,
                                                      context.config.max_width,
                                                      offset + extra_offset)
-                                            .unwrap()
                                      },
                                      list_lo,
                                      span_hi);
@@ -259,7 +255,7 @@ fn rewrite_segment(segment: &ast::PathSegment,
                                      ")",
                                      |ty| ty.span.lo,
                                      |ty| ty.span.hi,
-                                     |ty| ty.rewrite(context, budget, offset).unwrap(),
+                                     |ty| ty.rewrite(context, budget, offset),
                                      list_lo,
                                      span_hi);
             let list_str = try_opt!(::lists::format_fn_args(items, budget, offset, context.config));
@@ -284,40 +280,39 @@ fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Opt
                 let type_str = try_opt!(bounded_ty.rewrite(context, width, offset));
 
                 if !bound_lifetimes.is_empty() {
-                    let lifetime_str = bound_lifetimes.iter()
-                                                      .map(|lt| {
-                                                          lt.rewrite(context, width, offset)
-                                                            .unwrap()
-                                                      })
-                                                      .collect::<Vec<_>>()
-                                                      .join(", ");
+                    let lifetime_str = try_opt!(bound_lifetimes.iter()
+                                                               .map(|lt| {
+                                                                   lt.rewrite(context,
+                                                                              width,
+                                                                              offset)
+                                                               })
+                                                               .collect::<Option<Vec<_>>>())
+                                           .join(", ");
                     // 8 = "for<> : ".len()
                     let used_width = lifetime_str.len() + type_str.len() + 8;
                     let budget = try_opt!(width.checked_sub(used_width));
-                    let bounds_str = bounds.iter()
-                                           .map(|ty_bound| {
-                                               ty_bound.rewrite(context,
-                                                                budget,
-                                                                offset + used_width)
-                                                       .unwrap()
-                                           })
-                                           .collect::<Vec<_>>()
-                                           .join(" + ");
+                    let bounds_str = try_opt!(bounds.iter()
+                                                    .map(|ty_bound| {
+                                                        ty_bound.rewrite(context,
+                                                                         budget,
+                                                                         offset + used_width)
+                                                    })
+                                                    .collect::<Option<Vec<_>>>())
+                                         .join(" + ");
 
                     format!("for<{}> {}: {}", lifetime_str, type_str, bounds_str)
                 } else {
                     // 2 = ": ".len()
                     let used_width = type_str.len() + 2;
                     let budget = try_opt!(width.checked_sub(used_width));
-                    let bounds_str = bounds.iter()
-                                           .map(|ty_bound| {
-                                               ty_bound.rewrite(context,
-                                                                budget,
-                                                                offset + used_width)
-                                                       .unwrap()
-                                           })
-                                           .collect::<Vec<_>>()
-                                           .join(" + ");
+                    let bounds_str = try_opt!(bounds.iter()
+                                                    .map(|ty_bound| {
+                                                        ty_bound.rewrite(context,
+                                                                         budget,
+                                                                         offset + used_width)
+                                                    })
+                                                    .collect::<Option<Vec<_>>>())
+                                         .join(" + ");
 
                     format!("{}: {}", type_str, bounds_str)
                 }
@@ -380,9 +375,9 @@ fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Opt
 
 impl Rewrite for ast::TyParamBounds {
     fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option<String> {
-        let strs: Vec<_> = self.iter()
-                               .map(|b| b.rewrite(context, width, offset).unwrap())
-                               .collect();
+        let strs: Vec<_> = try_opt!(self.iter()
+                                        .map(|b| b.rewrite(context, width, offset))
+                                        .collect());
         Some(strs.join(" + "))
     }
 }
@@ -395,10 +390,10 @@ fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Opt
         if !self.bounds.is_empty() {
             result.push_str(": ");
 
-            let bounds = self.bounds
-                             .iter()
-                             .map(|ty_bound| ty_bound.rewrite(context, width, offset).unwrap())
-                             .collect::<Vec<_>>()
+            let bounds = try_opt!(self.bounds
+                                      .iter()
+                                      .map(|ty_bound| ty_bound.rewrite(context, width, offset))
+                                      .collect::<Option<Vec<_>>>())
                              .join(" + ");
 
             result.push_str(&bounds);
@@ -418,10 +413,10 @@ fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Opt
 impl Rewrite for ast::PolyTraitRef {
     fn rewrite(&self, context: &RewriteContext, width: usize, offset: Indent) -> Option<String> {
         if !self.bound_lifetimes.is_empty() {
-            let lifetime_str = self.bound_lifetimes
-                                   .iter()
-                                   .map(|lt| lt.rewrite(context, width, offset).unwrap())
-                                   .collect::<Vec<_>>()
+            let lifetime_str = try_opt!(self.bound_lifetimes
+                                            .iter()
+                                            .map(|lt| lt.rewrite(context, width, offset))
+                                            .collect::<Option<Vec<_>>>())
                                    .join(", ");
             // 6 is "for<> ".len()
             let extra_offset = lifetime_str.len() + 6;