Skip to content

Commit 9f3ab0b

Browse files
marcusklaasmarcusklaas
committed
Format comments in struct literals
1 parent 0ef5db9 commit 9f3ab0b

File tree

5 files changed

+108
-47
lines changed

5 files changed

+108
-47
lines changed

‎src/expr.rs

Lines changed: 67 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@
99
// except according to those terms.
1010

1111
use rewrite::{Rewrite, RewriteContext};
12-
use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic};
12+
use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic};
1313
use string::{StringFormat, rewrite_string};
14+
use utils::span_after;
1415

1516
use syntax::{ast, ptr};
1617
use syntax::codemap::{Pos, Span, BytePos};
@@ -37,11 +38,13 @@ impl Rewrite for ast::Expr {
3738
return rewrite_paren(context, subexpr, width, offset);
3839
}
3940
ast::Expr_::ExprStruct(ref path, ref fields, ref base) => {
40-
return rewrite_struct_lit(context, path,
41-
fields,
42-
base.as_ref().map(|e| &**e),
43-
width,
44-
offset);
41+
return rewrite_struct_lit(context,
42+
path,
43+
fields,
44+
base.as_ref().map(|e| &**e),
45+
self.span,
46+
width,
47+
offset);
4548
}
4649
ast::Expr_::ExprTup(ref items) => {
4750
return rewrite_tuple_lit(context, items, self.span, width, offset);
@@ -107,8 +110,10 @@ fn rewrite_call(context: &RewriteContext,
107110
")",
108111
|item| item.span.lo,
109112
|item| item.span.hi,
113+
// Take old span when rewrite fails.
110114
|item| item.rewrite(context, remaining_width, offset)
111-
.unwrap(), // FIXME: don't unwrap, take span literal
115+
.unwrap_or(context.codemap.span_to_snippet(item.span)
116+
.unwrap()),
112117
callee.span.hi + BytePos(1),
113118
span.hi);
114119

@@ -134,35 +139,68 @@ fn rewrite_paren(context: &RewriteContext, subexpr: &ast::Expr, width: usize, of
134139
subexpr_str.map(|s| format!("({})", s))
135140
}
136141

137-
fn rewrite_struct_lit(context: &RewriteContext,
138-
path: &ast::Path,
139-
fields: &[ast::Field],
140-
base: Option<&ast::Expr>,
141-
width: usize,
142-
offset: usize)
142+
fn rewrite_struct_lit<'a>(context: &RewriteContext,
143+
path: &ast::Path,
144+
fields: &'a [ast::Field],
145+
base: Option<&'a ast::Expr>,
146+
span: Span,
147+
width: usize,
148+
offset: usize)
143149
-> Option<String>
144150
{
145151
debug!("rewrite_struct_lit: width {}, offset {}", width, offset);
146152
assert!(fields.len() > 0 || base.is_some());
147153

154+
enum StructLitField<'a> {
155+
Regular(&'a ast::Field),
156+
Base(&'a ast::Expr)
157+
}
158+
148159
let path_str = pprust::path_to_string(path);
149160
// Foo { a: Foo } - indent is +3, width is -5.
150161
let indent = offset + path_str.len() + 3;
151162
let budget = width - (path_str.len() + 5);
152163

153-
let field_strs: Vec<_> =
154-
try_opt!(fields.iter()
155-
.map(|field| rewrite_field(context, field, budget, indent))
156-
.chain(base.iter()
157-
.map(|expr| expr.rewrite(context,
158-
// 2 = ".."
159-
budget - 2,
160-
indent + 2)
161-
.map(|s| format!("..{}", s))))
162-
.collect());
163-
164-
// FIXME comments
165-
let field_strs: Vec<_> = field_strs.into_iter().map(ListItem::from_str).collect();
164+
let field_iter = fields.into_iter().map(StructLitField::Regular)
165+
.chain(base.into_iter().map(StructLitField::Base));
166+
167+
let items = itemize_list(context.codemap,
168+
Vec::new(),
169+
field_iter,
170+
",",
171+
"}",
172+
|item| {
173+
match *item {
174+
StructLitField::Regular(ref field) => field.span.lo,
175+
// 2 = ..
176+
StructLitField::Base(ref expr) => expr.span.lo - BytePos(2)
177+
}
178+
},
179+
|item| {
180+
match *item {
181+
StructLitField::Regular(ref field) => field.span.hi,
182+
StructLitField::Base(ref expr) => expr.span.hi
183+
}
184+
},
185+
|item| {
186+
match *item {
187+
StructLitField::Regular(ref field) => {
188+
rewrite_field(context, &field, budget, indent)
189+
.unwrap_or(context.codemap.span_to_snippet(field.span)
190+
.unwrap())
191+
},
192+
StructLitField::Base(ref expr) => {
193+
// 2 = ..
194+
expr.rewrite(context, budget - 2, indent + 2)
195+
.map(|s| format!("..{}", s))
196+
.unwrap_or(context.codemap.span_to_snippet(expr.span)
197+
.unwrap())
198+
}
199+
}
200+
},
201+
span_after(span, "{", context.codemap),
202+
span.hi);
203+
166204
let fmt = ListFormatting {
167205
tactic: ListTactic::HorizontalVertical,
168206
separator: ",",
@@ -176,7 +214,7 @@ fn rewrite_struct_lit(context: &RewriteContext,
176214
v_width: budget,
177215
is_expression: true,
178216
};
179-
let fields_str = write_list(&field_strs, &fmt);
217+
let fields_str = write_list(&items, &fmt);
180218
Some(format!("{} {{ {} }}", path_str, fields_str))
181219

182220
// 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,
210248
|item| item.rewrite(context,
211249
context.config.max_width - indent - 2,
212250
indent)
213-
.unwrap(), // FIXME: don't unwrap, take span literal
251+
.unwrap_or(context.codemap.span_to_snippet(item.span)
252+
.unwrap()),
214253
span.lo + BytePos(1), // Remove parens
215254
span.hi - BytePos(1));
216255

‎src/items.rs

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
// Formatting top-level items - functions, structs, enums, traits, impls.
1212

1313
use {ReturnIndent, BraceStyle};
14-
use utils::{format_visibility, make_indent, contains_skip};
14+
use utils::{format_visibility, make_indent, contains_skip, span_after};
1515
use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic};
1616
use comment::FindUncommented;
1717
use visitor::FmtVisitor;
@@ -165,7 +165,7 @@ impl<'a> FmtVisitor<'a> {
165165
one_line_budget,
166166
multi_line_budget,
167167
arg_indent,
168-
codemap::mk_sp(self.span_after(span, "("),
168+
codemap::mk_sp(span_after(span, "(", self.codemap),
169169
span_for_return(&fd.output).lo)));
170170
result.push(')');
171171

@@ -278,7 +278,7 @@ impl<'a> FmtVisitor<'a> {
278278
// You also don't get to put a comment on self, unless it is explicit.
279279
if args.len() >= min_args {
280280
let comment_span_start = if min_args == 2 {
281-
self.span_after(span, ",")
281+
span_after(span, ",", self.codemap)
282282
} else {
283283
span.lo
284284
};
@@ -438,7 +438,7 @@ impl<'a> FmtVisitor<'a> {
438438
|arg| arg.ty.span.lo,
439439
|arg| arg.ty.span.hi,
440440
|arg| pprust::ty_to_string(&arg.ty),
441-
self.span_after(field.span, "("),
441+
span_after(field.span, "(", self.codemap),
442442
next_span_start);
443443

444444
result.push('(');
@@ -549,7 +549,7 @@ impl<'a> FmtVisitor<'a> {
549549
},
550550
|field| field.node.ty.span.hi,
551551
|field| self.format_field(field),
552-
self.span_after(span, opener.trim()),
552+
span_after(span, opener.trim(), self.codemap),
553553
span.hi);
554554

555555
// 2 terminators and a semicolon
@@ -714,7 +714,7 @@ impl<'a> FmtVisitor<'a> {
714714
|sp| sp.lo,
715715
|sp| sp.hi,
716716
|_| String::new(),
717-
self.span_after(span, "<"),
717+
span_after(span, "<", self.codemap),
718718
span.hi);
719719

720720
for (item, ty) in items.iter_mut().zip(lt_strs.chain(ty_strs)) {
@@ -793,12 +793,6 @@ impl<'a> FmtVisitor<'a> {
793793
pprust::pat_to_string(&arg.pat),
794794
pprust::ty_to_string(&arg.ty))
795795
}
796-
797-
fn span_after(&self, original: Span, needle: &str) -> BytePos {
798-
let snippet = self.snippet(original);
799-
800-
original.lo + BytePos(snippet.find_uncommented(needle).unwrap() as u32 + 1)
801-
}
802796
}
803797

804798
fn span_for_return(ret: &ast::FunctionRetTy) -> Span {

‎src/lists.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,11 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St
203203
}
204204

205205
if tactic == ListTactic::Vertical && item.post_comment.is_some() {
206-
let width = formatting.v_width - item_width - 1; // Space between item and comment
206+
// 1 = space between item and comment.
207+
let width = formatting.v_width.checked_sub(item_width + 1).unwrap_or(1);
207208
let offset = formatting.indent + item_width + 1;
208209
let comment = item.post_comment.as_ref().unwrap();
209-
// Use block-style only for the last item or multiline comments
210+
// Use block-style only for the last item or multiline comments.
210211
let block_style = formatting.is_expression && last ||
211212
comment.trim().contains('\n') ||
212213
comment.trim().len() > width;
@@ -241,7 +242,7 @@ pub fn itemize_list<T, I, F1, F2, F3>(codemap: &CodeMap,
241242
terminator: &str,
242243
get_lo: F1,
243244
get_hi: F2,
244-
get_item: F3,
245+
get_item_string: F3,
245246
mut prev_span_end: BytePos,
246247
next_span_start: BytePos)
247248
-> Vec<ListItem>
@@ -306,7 +307,7 @@ pub fn itemize_list<T, I, F1, F2, F3>(codemap: &CodeMap,
306307

307308
result.push(ListItem {
308309
pre_comment: pre_comment,
309-
item: get_item(&item),
310+
item: get_item_string(&item),
310311
post_comment: if post_snippet.len() > 0 {
311312
Some(post_snippet.to_owned())
312313
} else {

‎src/utils.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,19 @@
99
// except according to those terms.
1010

1111
use syntax::ast::{Visibility, Attribute, MetaItem, MetaItem_};
12+
use syntax::codemap::{CodeMap, Span, BytePos};
13+
14+
use comment::FindUncommented;
1215

1316
use SKIP_ANNOTATION;
1417

18+
#[inline]
19+
pub fn span_after(original: Span, needle: &str, codemap: &CodeMap) -> BytePos {
20+
let snippet = codemap.span_to_snippet(original).unwrap();
21+
22+
original.lo + BytePos(snippet.find_uncommented(needle).unwrap() as u32 + 1)
23+
}
24+
1525
#[inline]
1626
pub fn prev_char(s: &str, mut i: usize) -> usize {
1727
if i == 0 { return 0; }

‎tests/target/multiple.rs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,28 @@ fn struct_lits() {
145145
let x = Bar;
146146
// Comment
147147
let y = Foo { a: x };
148-
Foo { a: foo(), b: bar(), ..something };
148+
Foo { a: foo(), // comment
149+
// comment
150+
b: bar(),
151+
..something };
149152
Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: foo(),
150153
b: bar(), };
151-
Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { a: foo(),
152-
b: bar(), };
154+
Fooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo { // Comment
155+
a: foo(), /* C
156+
* o
157+
* m
158+
* m
159+
* e
160+
* n
161+
* t */
162+
// Comment
163+
b: bar(), /* C
164+
* o
165+
* m
166+
* m
167+
* e
168+
* n
169+
* t */ };
153170

154171
Foo { a: Bar, b: foo() };
155172
}

0 commit comments

Comments
 (0)