Skip to content

Commit 51af195

Browse files
topecongirotopecongiro
authored
Merge pull request #3415 from rchaser53/issue-3198
fix the comment for self are swallowed
2 parents 5177809 + dec3902 commit 51af195

File tree

4 files changed

+213
-24
lines changed

4 files changed

+213
-24
lines changed

‎src/items.rs

+46-23
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ use crate::expr::{
2020
ExprType, RhsTactics,
2121
};
2222
use crate::lists::{
23-
definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator,
23+
definitive_tactic, extract_post_comment, extract_pre_comment, get_comment_end,
24+
has_extra_newline, itemize_list, write_list, ListFormatting, ListItem, Separator,
2425
};
2526
use crate::macros::{rewrite_macro, MacroPosition};
2627
use crate::overflow;
@@ -2280,6 +2281,10 @@ fn rewrite_args(
22802281
variadic: bool,
22812282
generics_str_contains_newline: bool,
22822283
) -> Option<String> {
2284+
let terminator = ")";
2285+
let separator = ",";
2286+
let next_span_start = span.hi();
2287+
22832288
let mut arg_item_strs = args
22842289
.iter()
22852290
.map(|arg| {
@@ -2289,11 +2294,20 @@ fn rewrite_args(
22892294
.collect::<Vec<_>>();
22902295

22912296
// Account for sugary self.
2292-
// FIXME: the comment for the self argument is dropped. This is blocked
2293-
// on rust issue #27522.
2297+
let mut pre_comment_str = "";
2298+
let mut post_comment_str = "";
22942299
let min_args = explicit_self
22952300
.and_then(|explicit_self| rewrite_explicit_self(explicit_self, args, context))
22962301
.map_or(1, |self_str| {
2302+
pre_comment_str = context.snippet(mk_sp(span.lo(), args[0].pat.span.lo()));
2303+
2304+
let next_start = if args.len() > 1 {
2305+
args[1].pat.span().lo()
2306+
} else {
2307+
span.hi()
2308+
};
2309+
post_comment_str = context.snippet(mk_sp(args[0].ty.span.hi(), next_start));
2310+
22972311
arg_item_strs[0] = self_str;
22982312
2
22992313
});
@@ -2310,14 +2324,18 @@ fn rewrite_args(
23102324
// it is explicit.
23112325
if args.len() >= min_args || variadic {
23122326
let comment_span_start = if min_args == 2 {
2313-
let second_arg_start = if arg_has_pattern(&args[1]) {
2314-
args[1].pat.span.lo()
2327+
let remove_comma_byte_pos = context
2328+
.snippet_provider
2329+
.span_after(mk_sp(args[0].ty.span.hi(), args[1].pat.span.lo()), ",");
2330+
let first_post_and_second_pre_span =
2331+
mk_sp(remove_comma_byte_pos, args[1].pat.span.lo());
2332+
if count_newlines(context.snippet(first_post_and_second_pre_span)) > 0 {
2333+
context
2334+
.snippet_provider
2335+
.span_after(first_post_and_second_pre_span, "\n")
23152336
} else {
2316-
args[1].ty.span.lo()
2317-
};
2318-
let reduced_span = mk_sp(span.lo(), second_arg_start);
2319-
2320-
context.snippet_provider.span_after_last(reduced_span, ",")
2337+
remove_comma_byte_pos
2338+
}
23212339
} else {
23222340
span.lo()
23232341
};
@@ -2342,8 +2360,8 @@ fn rewrite_args(
23422360
.iter()
23432361
.map(ArgumentKind::Regular)
23442362
.chain(variadic_arg),
2345-
")",
2346-
",",
2363+
terminator,
2364+
separator,
23472365
|arg| match *arg {
23482366
ArgumentKind::Regular(arg) => span_lo_for_arg(arg),
23492367
ArgumentKind::Variadic(start) => start,
@@ -2357,18 +2375,31 @@ fn rewrite_args(
23572375
ArgumentKind::Variadic(..) => Some("...".to_owned()),
23582376
},
23592377
comment_span_start,
2360-
span.hi(),
2378+
next_span_start,
23612379
false,
23622380
);
23632381

23642382
arg_items.extend(more_items);
23652383
}
23662384

2385+
let arg_items_len = arg_items.len();
23672386
let fits_in_one_line = !generics_str_contains_newline
23682387
&& (arg_items.is_empty()
2369-
|| arg_items.len() == 1 && arg_item_strs[0].len() <= one_line_budget);
2388+
|| arg_items_len == 1 && arg_item_strs[0].len() <= one_line_budget);
23702389

2371-
for (item, arg) in arg_items.iter_mut().zip(arg_item_strs) {
2390+
for (index, (item, arg)) in arg_items.iter_mut().zip(arg_item_strs).enumerate() {
2391+
// add pre comment and post comment for first arg(self)
2392+
if index == 0 && explicit_self.is_some() {
2393+
let (pre_comment, pre_comment_style) = extract_pre_comment(pre_comment_str);
2394+
item.pre_comment = pre_comment;
2395+
item.pre_comment_style = pre_comment_style;
2396+
2397+
let comment_end =
2398+
get_comment_end(post_comment_str, separator, terminator, arg_items_len == 1);
2399+
2400+
item.new_lines = has_extra_newline(post_comment_str, comment_end);
2401+
item.post_comment = extract_post_comment(post_comment_str, comment_end, separator);
2402+
}
23722403
item.item = Some(arg);
23732404
}
23742405

@@ -2418,14 +2449,6 @@ fn rewrite_args(
24182449
write_list(&arg_items, &fmt)
24192450
}
24202451

2421-
fn arg_has_pattern(arg: &ast::Arg) -> bool {
2422-
if let ast::PatKind::Ident(_, ident, _) = arg.pat.node {
2423-
ident != symbol::keywords::Invalid.ident()
2424-
} else {
2425-
true
2426-
}
2427-
}
2428-
24292452
fn compute_budgets_for_args(
24302453
context: &RewriteContext<'_>,
24312454
result: &str,

‎src/lists.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -683,7 +683,7 @@ pub fn get_comment_end(
683683

684684
// Account for extra whitespace between items. This is fiddly
685685
// because of the way we divide pre- and post- comments.
686-
fn has_extra_newline(post_snippet: &str, comment_end: usize) -> bool {
686+
pub fn has_extra_newline(post_snippet: &str, comment_end: usize) -> bool {
687687
if post_snippet.is_empty() || comment_end == 0 {
688688
return false;
689689
}

‎tests/source/issue-3198.rs

+99
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
impl TestTrait {
2+
fn foo_one_pre(/* Important comment1 */
3+
self) {
4+
}
5+
6+
fn foo_one_post(self
7+
/* Important comment1 */) {
8+
}
9+
10+
fn foo_pre(
11+
/* Important comment1 */
12+
self,
13+
/* Important comment2 */
14+
a: i32,
15+
) {
16+
}
17+
18+
fn foo_post(
19+
self
20+
/* Important comment1 */,
21+
a: i32
22+
/* Important comment2 */,
23+
) {
24+
}
25+
26+
fn bar_pre(
27+
/* Important comment1 */
28+
&mut self,
29+
/* Important comment2 */
30+
a: i32,
31+
) {
32+
}
33+
34+
fn bar_post(
35+
&mut self
36+
/* Important comment1 */,
37+
a: i32
38+
/* Important comment2 */,
39+
) {
40+
}
41+
42+
fn baz_pre(
43+
/* Important comment1 */
44+
self: X< 'a , 'b >,
45+
/* Important comment2 */
46+
a: i32,
47+
) {
48+
}
49+
50+
fn baz_post(
51+
self: X< 'a , 'b >
52+
/* Important comment1 */,
53+
a: i32
54+
/* Important comment2 */,
55+
) {
56+
}
57+
58+
fn baz_tree_pre(
59+
/* Important comment1 */
60+
self: X< 'a , 'b >,
61+
/* Important comment2 */
62+
a: i32,
63+
/* Important comment3 */
64+
b: i32,
65+
) {
66+
}
67+
68+
fn baz_tree_post(
69+
self: X< 'a , 'b >
70+
/* Important comment1 */,
71+
a: i32
72+
/* Important comment2 */,
73+
b: i32
74+
/* Important comment3 */,){
75+
}
76+
77+
fn multi_line(
78+
self: X<'a, 'b>, /* Important comment1-1 */
79+
/* Important comment1-2 */
80+
a: i32, /* Important comment2 */
81+
b: i32, /* Important comment3 */
82+
) {
83+
}
84+
85+
fn two_line_comment(
86+
self: X<'a, 'b>, /* Important comment1-1
87+
Important comment1-2 */
88+
a: i32, /* Important comment2 */
89+
b: i32, /* Important comment3 */
90+
) {
91+
}
92+
93+
fn no_first_line_comment(
94+
self: X<'a, 'b>,
95+
/* Important comment2 */a: i32,
96+
/* Important comment3 */b: i32,
97+
) {
98+
}
99+
}

‎tests/target/issue-3198.rs

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
impl TestTrait {
2+
fn foo_one_pre(/* Important comment1 */ self) {}
3+
4+
fn foo_one_post(self /* Important comment1 */) {}
5+
6+
fn foo_pre(/* Important comment1 */ self, /* Important comment2 */ a: i32) {}
7+
8+
fn foo_post(self /* Important comment1 */, a: i32 /* Important comment2 */) {}
9+
10+
fn bar_pre(/* Important comment1 */ &mut self, /* Important comment2 */ a: i32) {}
11+
12+
fn bar_post(&mut self /* Important comment1 */, a: i32 /* Important comment2 */) {}
13+
14+
fn baz_pre(
15+
/* Important comment1 */
16+
self: X<'a, 'b>,
17+
/* Important comment2 */
18+
a: i32,
19+
) {
20+
}
21+
22+
fn baz_post(
23+
self: X<'a, 'b>, /* Important comment1 */
24+
a: i32, /* Important comment2 */
25+
) {
26+
}
27+
28+
fn baz_tree_pre(
29+
/* Important comment1 */
30+
self: X<'a, 'b>,
31+
/* Important comment2 */
32+
a: i32,
33+
/* Important comment3 */
34+
b: i32,
35+
) {
36+
}
37+
38+
fn baz_tree_post(
39+
self: X<'a, 'b>, /* Important comment1 */
40+
a: i32, /* Important comment2 */
41+
b: i32, /* Important comment3 */
42+
) {
43+
}
44+
45+
fn multi_line(
46+
self: X<'a, 'b>, /* Important comment1-1 */
47+
/* Important comment1-2 */
48+
a: i32, /* Important comment2 */
49+
b: i32, /* Important comment3 */
50+
) {
51+
}
52+
53+
fn two_line_comment(
54+
self: X<'a, 'b>, /* Important comment1-1
55+
Important comment1-2 */
56+
a: i32, /* Important comment2 */
57+
b: i32, /* Important comment3 */
58+
) {
59+
}
60+
61+
fn no_first_line_comment(
62+
self: X<'a, 'b>,
63+
/* Important comment2 */ a: i32,
64+
/* Important comment3 */ b: i32,
65+
) {
66+
}
67+
}

0 commit comments

Comments
 (0)