From 9f3ab0b5fe31bec1cc27e8748f82f17134d21603 Mon Sep 17 00:00:00 2001 From: Marcus Klaas Date: Wed, 24 Jun 2015 01:11:29 +0200 Subject: [PATCH] Format comments in struct literals --- src/expr.rs | 93 ++++++++++++++++++++++++++++------------ src/items.rs | 18 +++----- src/lists.rs | 9 ++-- src/utils.rs | 10 +++++ tests/target/multiple.rs | 23 ++++++++-- 5 files changed, 107 insertions(+), 46 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index f3509b10da8..56c8772dd9e 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -9,8 +9,9 @@ // except according to those terms. use rewrite::{Rewrite, RewriteContext}; -use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic}; +use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic}; use string::{StringFormat, rewrite_string}; +use utils::span_after; use syntax::{ast, ptr}; use syntax::codemap::{Pos, Span, BytePos}; @@ -37,11 +38,13 @@ impl Rewrite for ast::Expr { return rewrite_paren(context, subexpr, width, offset); } ast::Expr_::ExprStruct(ref path, ref fields, ref base) => { - return rewrite_struct_lit(context, path, - fields, - base.as_ref().map(|e| &**e), - width, - offset); + return rewrite_struct_lit(context, + path, + fields, + base.as_ref().map(|e| &**e), + self.span, + width, + offset); } ast::Expr_::ExprTup(ref items) => { return rewrite_tuple_lit(context, items, self.span, width, offset); @@ -107,8 +110,10 @@ fn rewrite_call(context: &RewriteContext, ")", |item| item.span.lo, |item| item.span.hi, + // Take old span when rewrite fails. |item| item.rewrite(context, remaining_width, offset) - .unwrap(), // FIXME: don't unwrap, take span literal + .unwrap_or(context.codemap.span_to_snippet(item.span) + .unwrap()), callee.span.hi + BytePos(1), span.hi); @@ -134,35 +139,68 @@ fn rewrite_paren(context: &RewriteContext, subexpr: &ast::Expr, width: usize, of subexpr_str.map(|s| format!("({})", s)) } -fn rewrite_struct_lit(context: &RewriteContext, - path: &ast::Path, - fields: &[ast::Field], - base: Option<&ast::Expr>, - width: usize, - offset: usize) +fn rewrite_struct_lit<'a>(context: &RewriteContext, + path: &ast::Path, + fields: &'a [ast::Field], + base: Option<&'a ast::Expr>, + span: Span, + width: usize, + offset: usize) -> Option { debug!("rewrite_struct_lit: width {}, offset {}", width, offset); assert!(fields.len() > 0 || base.is_some()); + enum StructLitField<'a> { + Regular(&'a ast::Field), + Base(&'a ast::Expr) + } + let path_str = pprust::path_to_string(path); // Foo { a: Foo } - indent is +3, width is -5. let indent = offset + path_str.len() + 3; let budget = width - (path_str.len() + 5); - let field_strs: Vec<_> = - try_opt!(fields.iter() - .map(|field| rewrite_field(context, field, budget, indent)) - .chain(base.iter() - .map(|expr| expr.rewrite(context, - // 2 = ".." - budget - 2, - indent + 2) - .map(|s| format!("..{}", s)))) - .collect()); + let field_iter = fields.into_iter().map(StructLitField::Regular) + .chain(base.into_iter().map(StructLitField::Base)); + + let items = itemize_list(context.codemap, + Vec::new(), + field_iter, + ",", + "}", + |item| { + match *item { + StructLitField::Regular(ref field) => field.span.lo, + // 2 = .. + StructLitField::Base(ref expr) => expr.span.lo - BytePos(2) + } + }, + |item| { + match *item { + StructLitField::Regular(ref field) => field.span.hi, + StructLitField::Base(ref expr) => expr.span.hi + } + }, + |item| { + match *item { + StructLitField::Regular(ref field) => { + rewrite_field(context, &field, budget, indent) + .unwrap_or(context.codemap.span_to_snippet(field.span) + .unwrap()) + }, + StructLitField::Base(ref expr) => { + // 2 = .. + expr.rewrite(context, budget - 2, indent + 2) + .map(|s| format!("..{}", s)) + .unwrap_or(context.codemap.span_to_snippet(expr.span) + .unwrap()) + } + } + }, + span_after(span, "{", context.codemap), + span.hi); - // FIXME comments - let field_strs: Vec<_> = field_strs.into_iter().map(ListItem::from_str).collect(); let fmt = ListFormatting { tactic: ListTactic::HorizontalVertical, separator: ",", @@ -176,7 +214,7 @@ fn rewrite_struct_lit(context: &RewriteContext, v_width: budget, is_expression: true, }; - let fields_str = write_list(&field_strs, &fmt); + let fields_str = write_list(&items, &fmt); Some(format!("{} {{ {} }}", path_str, fields_str)) // FIXME if the usual multi-line layout is too wide, we should fall back to @@ -210,7 +248,8 @@ fn rewrite_tuple_lit(context: &RewriteContext, |item| item.rewrite(context, context.config.max_width - indent - 2, indent) - .unwrap(), // FIXME: don't unwrap, take span literal + .unwrap_or(context.codemap.span_to_snippet(item.span) + .unwrap()), span.lo + BytePos(1), // Remove parens span.hi - BytePos(1)); diff --git a/src/items.rs b/src/items.rs index 77b750eb475..10a93c23033 100644 --- a/src/items.rs +++ b/src/items.rs @@ -11,7 +11,7 @@ // Formatting top-level items - functions, structs, enums, traits, impls. use {ReturnIndent, BraceStyle}; -use utils::{format_visibility, make_indent, contains_skip}; +use utils::{format_visibility, make_indent, contains_skip, span_after}; use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic}; use comment::FindUncommented; use visitor::FmtVisitor; @@ -165,7 +165,7 @@ impl<'a> FmtVisitor<'a> { one_line_budget, multi_line_budget, arg_indent, - codemap::mk_sp(self.span_after(span, "("), + codemap::mk_sp(span_after(span, "(", self.codemap), span_for_return(&fd.output).lo))); result.push(')'); @@ -278,7 +278,7 @@ impl<'a> FmtVisitor<'a> { // You also don't get to put a comment on self, unless it is explicit. if args.len() >= min_args { let comment_span_start = if min_args == 2 { - self.span_after(span, ",") + span_after(span, ",", self.codemap) } else { span.lo }; @@ -438,7 +438,7 @@ impl<'a> FmtVisitor<'a> { |arg| arg.ty.span.lo, |arg| arg.ty.span.hi, |arg| pprust::ty_to_string(&arg.ty), - self.span_after(field.span, "("), + span_after(field.span, "(", self.codemap), next_span_start); result.push('('); @@ -549,7 +549,7 @@ impl<'a> FmtVisitor<'a> { }, |field| field.node.ty.span.hi, |field| self.format_field(field), - self.span_after(span, opener.trim()), + span_after(span, opener.trim(), self.codemap), span.hi); // 2 terminators and a semicolon @@ -714,7 +714,7 @@ impl<'a> FmtVisitor<'a> { |sp| sp.lo, |sp| sp.hi, |_| String::new(), - self.span_after(span, "<"), + span_after(span, "<", self.codemap), span.hi); for (item, ty) in items.iter_mut().zip(lt_strs.chain(ty_strs)) { @@ -793,12 +793,6 @@ impl<'a> FmtVisitor<'a> { pprust::pat_to_string(&arg.pat), pprust::ty_to_string(&arg.ty)) } - - fn span_after(&self, original: Span, needle: &str) -> BytePos { - let snippet = self.snippet(original); - - original.lo + BytePos(snippet.find_uncommented(needle).unwrap() as u32 + 1) - } } fn span_for_return(ret: &ast::FunctionRetTy) -> Span { diff --git a/src/lists.rs b/src/lists.rs index d27a1943e2b..aefc93d7c86 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -203,10 +203,11 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St } if tactic == ListTactic::Vertical && item.post_comment.is_some() { - let width = formatting.v_width - item_width - 1; // Space between item and comment + // 1 = space between item and comment. + let width = formatting.v_width.checked_sub(item_width + 1).unwrap_or(1); let offset = formatting.indent + item_width + 1; let comment = item.post_comment.as_ref().unwrap(); - // Use block-style only for the last item or multiline comments + // Use block-style only for the last item or multiline comments. let block_style = formatting.is_expression && last || comment.trim().contains('\n') || comment.trim().len() > width; @@ -241,7 +242,7 @@ pub fn itemize_list(codemap: &CodeMap, terminator: &str, get_lo: F1, get_hi: F2, - get_item: F3, + get_item_string: F3, mut prev_span_end: BytePos, next_span_start: BytePos) -> Vec @@ -306,7 +307,7 @@ pub fn itemize_list(codemap: &CodeMap, result.push(ListItem { pre_comment: pre_comment, - item: get_item(&item), + item: get_item_string(&item), post_comment: if post_snippet.len() > 0 { Some(post_snippet.to_owned()) } else { diff --git a/src/utils.rs b/src/utils.rs index e1a152e42c6..de17f989f7a 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -9,9 +9,19 @@ // except according to those terms. use syntax::ast::{Visibility, Attribute, MetaItem, MetaItem_}; +use syntax::codemap::{CodeMap, Span, BytePos}; + +use comment::FindUncommented; use SKIP_ANNOTATION; +#[inline] +pub fn span_after(original: Span, needle: &str, codemap: &CodeMap) -> BytePos { + let snippet = codemap.span_to_snippet(original).unwrap(); + + original.lo + BytePos(snippet.find_uncommented(needle).unwrap() as u32 + 1) +} + #[inline] pub fn prev_char(s: &str, mut i: usize) -> usize { if i == 0 { return 0; } diff --git a/tests/target/multiple.rs b/tests/target/multiple.rs index 828f1b163a9..ad6e1c085b3 100644 --- a/tests/target/multiple.rs +++ b/tests/target/multiple.rs @@ -145,11 +145,28 @@ fn struct_lits() { let x = Bar; // Comment let y = Foo { a: x }; - Foo { a: foo(), b: bar(), ..something }; + Foo { a: foo(), // comment + // comment + b: bar(), + ..something }; Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: foo(), b: bar(), }; - Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: foo(), - b: bar(), }; + Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { // Comment + a: foo(), /* C + * o + * m + * m + * e + * n + * t */ + // Comment + b: bar(), /* C + * o + * m + * m + * e + * n + * t */ }; Foo { a: Bar, b: foo() }; }