Skip to content

Commit 1b464f9

Browse files
committed
leverage itemize_list and write_list to recover trait bound comments
Fixes 2055 Now when using `version::Two`, rustfmt will permit comments within trait bounds and actually reformat them instead of refusing to rewrite the entire trait definition because there are comments in the bounds.
1 parent 202fa22 commit 1b464f9

File tree

5 files changed

+121
-21
lines changed

5 files changed

+121
-21
lines changed

src/items.rs

+86-18
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ use regex::Regex;
77
use rustc_ast::visit;
88
use rustc_ast::{ast, ptr};
99
use rustc_span::{symbol, BytePos, Span, DUMMY_SP};
10+
use unicode_width::UnicodeWidthStr;
1011

1112
use crate::attr::filter_inline_attrs;
1213
use crate::comment::{
@@ -1132,6 +1133,56 @@ fn format_struct(
11321133
}
11331134
}
11341135

1136+
fn rewrite_bounds(
1137+
result: &mut String,
1138+
context: &RewriteContext<'_>,
1139+
bounds: &ast::GenericBounds,
1140+
terminator: &str,
1141+
shape: Shape,
1142+
span: Span,
1143+
) -> Option<()> {
1144+
let indented = shape.block_indent(context.config.tab_spaces());
1145+
let items = itemize_list(
1146+
context.snippet_provider,
1147+
bounds.iter(),
1148+
terminator,
1149+
"+",
1150+
|bound| bound.span().lo(),
1151+
|bound| bound.span().hi(),
1152+
|bound| bound.rewrite(context, indented),
1153+
span.lo(),
1154+
span.hi(),
1155+
false,
1156+
)
1157+
.collect::<Vec<_>>();
1158+
1159+
let tactic = definitive_tactic(
1160+
&items,
1161+
ListTactic::LimitedHorizontalVertical(shape.width),
1162+
Separator::Plus,
1163+
context.config.max_width(),
1164+
);
1165+
1166+
let fmt = ListFormatting::new(indented, context.config)
1167+
.tactic(tactic)
1168+
.trailing_separator(SeparatorTactic::Never)
1169+
.separator("+")
1170+
.separator_place(SeparatorPlace::Front)
1171+
.align_comments(false);
1172+
1173+
let item_str = write_list(items, &fmt)?;
1174+
1175+
let space = if tactic == DefinitiveListTactic::Horizontal {
1176+
Cow::from(" ")
1177+
} else {
1178+
indented.indent.to_string_with_newline(&context.config)
1179+
};
1180+
result.push(':');
1181+
result.push_str(&space);
1182+
result.push_str(&item_str);
1183+
Some(())
1184+
}
1185+
11351186
pub(crate) fn format_trait(
11361187
context: &RewriteContext<'_>,
11371188
item: &ast::Item,
@@ -1164,25 +1215,40 @@ pub(crate) fn format_trait(
11641215
rewrite_generics(context, rewrite_ident(context, item.ident), generics, shape)?;
11651216
result.push_str(&generics_str);
11661217

1167-
// FIXME(#2055): rustfmt fails to format when there are comments between trait bounds.
11681218
if !bounds.is_empty() {
1169-
// Retrieve *unnormalized* ident (See #6069)
1170-
let source_ident = context.snippet(item.ident.span);
1171-
let ident_hi = context.snippet_provider.span_after(item.span, source_ident);
1172-
let bound_hi = bounds.last().unwrap().span().hi();
1173-
let snippet = context.snippet(mk_sp(ident_hi, bound_hi));
1174-
if contains_comment(snippet) {
1175-
return None;
1176-
}
1219+
if context.config.version() == Version::Two {
1220+
let after_colon = context
1221+
.snippet_provider
1222+
.span_after(item.span.with_lo(item.ident.span.hi()), ":");
11771223

1178-
result = rewrite_assign_rhs_with(
1179-
context,
1180-
result + ":",
1181-
bounds,
1182-
shape,
1183-
&RhsAssignKind::Bounds,
1184-
RhsTactics::ForceNextLineWithoutIndent,
1185-
)?;
1224+
let span = mk_sp(after_colon, body_lo);
1225+
let shape = if result.contains('\n') {
1226+
shape
1227+
} else {
1228+
// `offset_left` takes into account what we've rewritten already + 1 for `:`
1229+
// `sub_width` take into account the trailing `{`
1230+
shape.offset_left(header.width() + 1)?.sub_width(1)?
1231+
};
1232+
rewrite_bounds(&mut result, context, bounds, "{", shape, span)?;
1233+
} else {
1234+
// Retrieve *unnormalized* ident (See #6069)
1235+
let source_ident = context.snippet(item.ident.span);
1236+
let ident_hi = context.snippet_provider.span_after(item.span, source_ident);
1237+
let bound_hi = bounds.last().unwrap().span().hi();
1238+
let snippet = context.snippet(mk_sp(ident_hi, bound_hi));
1239+
if contains_comment(snippet) {
1240+
return None;
1241+
}
1242+
1243+
result = rewrite_assign_rhs_with(
1244+
context,
1245+
result + ":",
1246+
bounds,
1247+
shape,
1248+
&RhsAssignKind::Bounds,
1249+
RhsTactics::ForceNextLineWithoutIndent,
1250+
)?;
1251+
}
11861252
}
11871253

11881254
// Rewrite where-clause.
@@ -1219,7 +1285,9 @@ pub(crate) fn format_trait(
12191285
result.push_str(&where_indent.to_string_with_newline(context.config));
12201286
}
12211287
result.push_str(&where_clause_str);
1222-
} else {
1288+
}
1289+
1290+
if generics.where_clause.predicates.is_empty() && bounds.is_empty() {
12231291
let item_snippet = context.snippet(item.span);
12241292
if let Some(lo) = item_snippet.find('/') {
12251293
// 1 = `{`

src/lists.rs

+4
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,8 @@ impl ListItem {
206206
pub(crate) enum Separator {
207207
Comma,
208208
VerticalBar,
209+
// Used to format trait bounds
210+
Plus,
209211
}
210212

211213
impl Separator {
@@ -215,6 +217,8 @@ impl Separator {
215217
Separator::Comma => 2,
216218
// 3 = ` | `
217219
Separator::VerticalBar => 3,
220+
// 3 = ` + `
221+
Separator::Plus => 3,
218222
}
219223
}
220224
}

tests/source/issue_2055.rs

+12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// rustfmt-version: Two
2+
3+
pub trait A {}
4+
pub trait B {}
5+
pub trait C {}
6+
7+
pub trait Foo:
8+
// A and C
9+
A + C
10+
// and B
11+
+ B
12+
{}

tests/target/issue-4689/two.rs

+6-3
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,8 @@ pub trait PrettyPrinter<'tcx>:
2323
Type = Self,
2424
DynExistential = Self,
2525
Const = Self,
26-
> + fmt::Write
26+
>
27+
+ fmt::Write
2728
{
2829
//
2930
}
@@ -36,7 +37,8 @@ pub trait PrettyPrinter<'tcx>:
3637
Type = Self,
3738
DynExistential = Self,
3839
Const = Self,
39-
> + fmt::Write1
40+
>
41+
+ fmt::Write1
4042
+ fmt::Write2
4143
{
4244
//
@@ -65,7 +67,8 @@ pub trait PrettyPrinter<'tcx>:
6567
Type = Self,
6668
DynExistential = Self,
6769
Const = Self,
68-
> + Printer2<
70+
>
71+
+ Printer2<
6972
'tcx,
7073
Error = fmt::Error,
7174
Path = Self,

tests/target/issue_2055.rs

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// rustfmt-version: Two
2+
3+
pub trait A {}
4+
pub trait B {}
5+
pub trait C {}
6+
7+
pub trait Foo:
8+
// A and C
9+
A
10+
+ C // and B
11+
+ B
12+
{
13+
}

0 commit comments

Comments
 (0)