From 4e0a8da447e57c1b9bc045828b77dd787b1e449e Mon Sep 17 00:00:00 2001 From: Marcus Klaas Date: Wed, 19 Aug 2015 18:12:05 +0200 Subject: [PATCH] Refactor itemize list so that it produces an iterator --- src/expr.rs | 14 +--- src/imports.rs | 31 ++++---- src/items.rs | 51 ++++++------- src/lists.rs | 194 ++++++++++++++++++++++++++++--------------------- src/types.rs | 8 +- 5 files changed, 156 insertions(+), 142 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 4cfa5a0b879..1dfff001475 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -407,9 +407,7 @@ fn rewrite_call(context: &RewriteContext, let inner_context = &RewriteContext { block_indent: block_indent, ..*context }; let items = itemize_list(context.codemap, - Vec::new(), args.iter(), - ",", ")", |item| item.span.lo, |item| item.span.hi, @@ -430,7 +428,7 @@ fn rewrite_call(context: &RewriteContext, ends_with_newline: false, }; - Some(format!("{}({})", callee_str, write_list(&items, &fmt))) + Some(format!("{}({})", callee_str, write_list(&items.collect::>(), &fmt))) } fn expr_block_indent(context: &RewriteContext, offset: usize) -> usize { @@ -494,9 +492,7 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext, let inner_context = &RewriteContext { block_indent: indent, ..*context }; let items = itemize_list(context.codemap, - Vec::new(), field_iter, - ",", "}", |item| { match *item { @@ -543,7 +539,7 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext, v_width: v_budget, ends_with_newline: false, }; - let fields_str = write_list(&items, &fmt); + let fields_str = write_list(&items.collect::>(), &fmt); match context.config.struct_lit_style { StructLitStyle::BlockIndent if fields_str.contains('\n') => { @@ -584,9 +580,7 @@ fn rewrite_tuple_lit(context: &RewriteContext, } let items = itemize_list(context.codemap, - Vec::new(), - items.into_iter(), - ",", + items.iter(), ")", |item| item.span.lo, |item| item.span.hi, @@ -608,7 +602,7 @@ fn rewrite_tuple_lit(context: &RewriteContext, ends_with_newline: false, }; - Some(format!("({})", write_list(&items, &fmt))) + Some(format!("({})", write_list(&items.collect::>(), &fmt))) } fn rewrite_binary_op(context: &RewriteContext, diff --git a/src/imports.rs b/src/imports.rs index e3629410c6c..392e6fb838a 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -108,30 +108,33 @@ pub fn rewrite_use_list(width: usize, ends_with_newline: false, }; - let mut items = itemize_list(context.codemap, - vec![ListItem::from_str("")], /* Dummy value, explanation - * below */ - path_list.iter(), - ",", - "}", - |vpi| vpi.span.lo, - |vpi| vpi.span.hi, - |vpi| match vpi.node { + let mut items = { + // Dummy value, see explanation below. + let mut items = vec![ListItem::from_str("")]; + let iter = itemize_list(context.codemap, + path_list.iter(), + "}", + |vpi| vpi.span.lo, + |vpi| vpi.span.hi, + |vpi| match vpi.node { ast::PathListItem_::PathListIdent{ name, .. } => { name.to_string() } ast::PathListItem_::PathListMod{ .. } => { "self".to_owned() } - }, - span_after(span, "{", context.codemap), - span.hi); + }, + span_after(span, "{", context.codemap), + span.hi); + items.extend(iter); + items + }; // 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 itemize_list and write_list. + // 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 diff --git a/src/items.rs b/src/items.rs index bbe80bf1231..458a7b0b336 100644 --- a/src/items.rs +++ b/src/items.rs @@ -259,16 +259,16 @@ impl<'a> FmtVisitor<'a> { span.lo }; - arg_items = itemize_list(self.codemap, - arg_items, - args[min_args-1..].iter().cloned(), - ",", - ")", - span_lo_for_arg, - |arg| arg.ty.span.hi, - |_| String::new(), - comment_span_start, - span.hi); + let more_items = itemize_list(self.codemap, + args[min_args-1..].iter(), + ")", + |arg| span_lo_for_arg(arg), + |arg| arg.ty.span.hi, + |_| String::new(), + comment_span_start, + span.hi); + + arg_items.extend(more_items); } assert_eq!(arg_item_strs.len(), arg_items.len()); @@ -401,9 +401,7 @@ impl<'a> FmtVisitor<'a> { if types.len() > 0 { let items = itemize_list(self.codemap, - Vec::new(), types.iter(), - ",", ")", |arg| arg.ty.span.lo, |arg| arg.ty.span.hi, @@ -434,7 +432,7 @@ impl<'a> FmtVisitor<'a> { v_width: budget, ends_with_newline: true, }; - result.push_str(&write_list(&items, &fmt)); + result.push_str(&write_list(&items.collect::>(), &fmt)); result.push(')'); } @@ -513,9 +511,7 @@ impl<'a> FmtVisitor<'a> { result.push_str(&generics_str); let items = itemize_list(self.codemap, - Vec::new(), struct_def.fields.iter(), - ",", terminator, |field| { // Include attributes and doc comments, @@ -563,7 +559,7 @@ impl<'a> FmtVisitor<'a> { ends_with_newline: true, }; - result.push_str(&write_list(&items, &fmt)); + result.push_str(&write_list(&items.collect::>(), &fmt)); if break_line { result.push('\n'); @@ -689,16 +685,15 @@ impl<'a> FmtVisitor<'a> { }); let ty_spans = tys.iter().map(span_for_ty_param); - let mut items = itemize_list(self.codemap, - Vec::new(), - lt_spans.chain(ty_spans), - ",", - ">", - |sp| sp.lo, - |sp| sp.hi, - |_| String::new(), - span_after(span, "<", self.codemap), - span.hi); + let items = itemize_list(self.codemap, + lt_spans.chain(ty_spans), + ">", + |sp| sp.lo, + |sp| sp.hi, + |_| String::new(), + span_after(span, "<", self.codemap), + span.hi); + let mut items = items.collect::>(); for (item, ty) in items.iter_mut().zip(lt_strs.chain(ty_strs)) { item.item = ty; @@ -741,9 +736,7 @@ impl<'a> FmtVisitor<'a> { let budget = self.config.ideal_width + self.config.leeway - offset; let span_start = span_for_where_pred(&where_clause.predicates[0]).lo; let items = itemize_list(self.codemap, - Vec::new(), where_clause.predicates.iter(), - ",", "{", |pred| span_for_where_pred(pred).lo, |pred| span_for_where_pred(pred).hi, @@ -763,7 +756,7 @@ impl<'a> FmtVisitor<'a> { v_width: budget, ends_with_newline: false, }; - result.push_str(&write_list(&items, &fmt)); + result.push_str(&write_list(&items.collect::>(), &fmt)); result } diff --git a/src/lists.rs b/src/lists.rs index 6398c19c9ea..8f1d8a743c2 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -9,6 +9,7 @@ // except according to those terms. use std::cmp; +use std::iter::Peekable; use syntax::codemap::{self, CodeMap, BytePos}; @@ -224,107 +225,134 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St result } -// Turns a list into a vector of items with associated comments. -// TODO: we probably do not want to take a terminator any more. Instead, we -// should demand a proper span end. -pub fn itemize_list(codemap: &CodeMap, - prefix: Vec, - it: I, - separator: &str, - terminator: &str, - get_lo: F1, - get_hi: F2, - get_item_string: F3, - mut prev_span_end: BytePos, - next_span_start: BytePos) - -> Vec +pub struct ListItems<'a, I, F1, F2, F3> + where I: Iterator +{ + codemap: &'a CodeMap, + inner: Peekable, + get_lo: F1, + get_hi: F2, + get_item_string: F3, + prev_span_end: BytePos, + next_span_start: BytePos, + terminator: &'a str, +} + +impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3> where I: Iterator, F1: Fn(&T) -> BytePos, F2: Fn(&T) -> BytePos, F3: Fn(&T) -> String { - let mut result = prefix; - result.reserve(it.size_hint().0); + type Item = ListItem; - let mut new_it = it.peekable(); - let white_space: &[_] = &[' ', '\t']; + fn next(&mut self) -> Option { + let white_space: &[_] = &[' ', '\t']; - while let Some(item) = new_it.next() { - // Pre-comment - let pre_snippet = codemap.span_to_snippet(codemap::mk_sp(prev_span_end, - get_lo(&item))) - .unwrap(); - let pre_snippet = pre_snippet.trim(); - let pre_comment = if pre_snippet.len() > 0 { - Some(pre_snippet.to_owned()) - } else { - None - }; + self.inner.next().map(|item| { + // Pre-comment + let pre_snippet = self.codemap.span_to_snippet(codemap::mk_sp(self.prev_span_end, + (self.get_lo)(&item))) + .unwrap(); + let pre_snippet = pre_snippet.trim(); + let pre_comment = if pre_snippet.len() > 0 { + Some(pre_snippet.to_owned()) + } else { + None + }; - // Post-comment - let next_start = match new_it.peek() { - Some(ref next_item) => get_lo(next_item), - None => next_span_start - }; - let post_snippet = codemap.span_to_snippet(codemap::mk_sp(get_hi(&item), - next_start)) - .unwrap(); + // Post-comment + let next_start = match self.inner.peek() { + Some(ref next_item) => (self.get_lo)(next_item), + None => self.next_span_start + }; + let post_snippet = self.codemap.span_to_snippet(codemap::mk_sp((self.get_hi)(&item), + next_start)) + .unwrap(); - let comment_end = match new_it.peek() { - Some(..) => { - let block_open_index = post_snippet.find("/*"); - let newline_index = post_snippet.find('\n'); - let separator_index = post_snippet.find_uncommented(separator).unwrap(); + let comment_end = match self.inner.peek() { + Some(..) => { + let block_open_index = post_snippet.find("/*"); + let newline_index = post_snippet.find('\n'); + let separator_index = post_snippet.find_uncommented(",").unwrap(); - match (block_open_index, newline_index) { - // Separator before comment, with the next item on same line. - // Comment belongs to next item. - (Some(i), None) if i > separator_index => { separator_index + separator.len() } - // Block-style post-comment before the separator. - (Some(i), None) => { - cmp::max(find_comment_end(&post_snippet[i..]).unwrap() + i, - separator_index + separator.len()) + match (block_open_index, newline_index) { + // Separator before comment, with the next item on same line. + // Comment belongs to next item. + (Some(i), None) if i > separator_index => { + separator_index + 1 + } + // Block-style post-comment before the separator. + (Some(i), None) => { + cmp::max(find_comment_end(&post_snippet[i..]).unwrap() + i, + separator_index + 1) + } + // Block-style post-comment. Either before or after the separator. + (Some(i), Some(j)) if i < j => { + cmp::max(find_comment_end(&post_snippet[i..]).unwrap() + i, + separator_index + 1) + } + // Potential *single* line comment. + (_, Some(j)) => { j + 1 } + _ => post_snippet.len() } - // Block-style post-comment. Either before or after the separator. - (Some(i), Some(j)) if i < j => { - cmp::max(find_comment_end(&post_snippet[i..]).unwrap() + i, - separator_index + separator.len()) - } - // Potential *single* line comment. - (_, Some(j)) => { j + 1 } - _ => post_snippet.len() + }, + None => { + post_snippet.find_uncommented(self.terminator) + .unwrap_or(post_snippet.len()) } - }, - None => { - post_snippet.find_uncommented(terminator) - .unwrap_or(post_snippet.len()) + }; + + // Cleanup post-comment: strip separators and whitespace. + self.prev_span_end = (self.get_hi)(&item) + BytePos(comment_end as u32); + let mut post_snippet = post_snippet[..comment_end].trim(); + + if post_snippet.starts_with(',') { + post_snippet = post_snippet[1..].trim_matches(white_space); + } else if post_snippet.ends_with(",") { + post_snippet = post_snippet[..(post_snippet.len() - 1)].trim_matches(white_space); } - }; - // Cleanup post-comment: strip separators and whitespace. - prev_span_end = get_hi(&item) + BytePos(comment_end as u32); - let mut post_snippet = post_snippet[..comment_end].trim(); - - if post_snippet.starts_with(separator) { - post_snippet = post_snippet[separator.len()..] - .trim_matches(white_space); - } else if post_snippet.ends_with(separator) { - post_snippet = post_snippet[..post_snippet.len()-separator.len()] - .trim_matches(white_space); - } - - result.push(ListItem { - pre_comment: pre_comment, - item: get_item_string(&item), - post_comment: if post_snippet.len() > 0 { + let post_comment = if post_snippet.len() > 0 { Some(post_snippet.to_owned()) } else { None - } - }); - } + }; - result + ListItem { + pre_comment: pre_comment, + item: (self.get_item_string)(&item), + post_comment: post_comment, + } + }) + } +} + +// Creates an iterator over a list's items with associated comments. +pub fn itemize_list<'a, T, I, F1, F2, F3>(codemap: &'a CodeMap, + inner: I, + terminator: &'a str, + get_lo: F1, + get_hi: F2, + get_item_string: F3, + prev_span_end: BytePos, + next_span_start: BytePos) + -> ListItems<'a, I, F1, F2, F3> + where I: Iterator, + F1: Fn(&T) -> BytePos, + F2: Fn(&T) -> BytePos, + F3: Fn(&T) -> String +{ + ListItems { + codemap: codemap, + inner: inner.peekable(), + get_lo: get_lo, + get_hi: get_hi, + get_item_string: get_item_string, + prev_span_end: prev_span_end, + next_span_start: next_span_start, + terminator: terminator, + } } fn needs_trailing_separator(separator_tactic: SeparatorTactic, list_tactic: ListTactic) -> bool { diff --git a/src/types.rs b/src/types.rs index 4917d6ac3c9..ea8787bd846 100644 --- a/src/types.rs +++ b/src/types.rs @@ -204,9 +204,7 @@ fn rewrite_segment(segment: &ast::PathSegment, let separator = get_path_separator(context.codemap, *span_lo, list_lo); let items = itemize_list(context.codemap, - Vec::new(), param_list.into_iter(), - ",", ">", |param| param.get_span().lo, |param| param.get_span().hi, @@ -232,7 +230,7 @@ fn rewrite_segment(segment: &ast::PathSegment, // update pos *span_lo = next_span_lo; - format!("{}<{}>", separator, write_list(&items, &fmt)) + format!("{}<{}>", separator, write_list(&items.collect::>(), &fmt)) } ast::PathParameters::ParenthesizedParameters(ref data) => { let output = match data.output { @@ -242,9 +240,7 @@ fn rewrite_segment(segment: &ast::PathSegment, let list_lo = span_after(codemap::mk_sp(*span_lo, span_hi), "(", context.codemap); let items = itemize_list(context.codemap, - Vec::new(), data.inputs.iter(), - ",", ")", |ty| ty.span.lo, |ty| ty.span.hi, @@ -269,7 +265,7 @@ fn rewrite_segment(segment: &ast::PathSegment, // update pos *span_lo = data.inputs.last().unwrap().span.hi + BytePos(1); - format!("({}){}", write_list(&items, &fmt), output) + format!("({}){}", write_list(&items.collect::>(), &fmt), output) } _ => String::new() };