Skip to content

fix the comment for self are swallowed #3415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 46 additions & 23 deletions src/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ use crate::expr::{
ExprType, RhsTactics,
};
use crate::lists::{
definitive_tactic, itemize_list, write_list, ListFormatting, ListItem, Separator,
definitive_tactic, extract_post_comment, extract_pre_comment, get_comment_end,
has_extra_newline, itemize_list, write_list, ListFormatting, ListItem, Separator,
};
use crate::macros::{rewrite_macro, MacroPosition};
use crate::overflow;
Expand Down Expand Up @@ -2280,6 +2281,10 @@ fn rewrite_args(
variadic: bool,
generics_str_contains_newline: bool,
) -> Option<String> {
let terminator = ")";
let separator = ",";
let next_span_start = span.hi();

let mut arg_item_strs = args
.iter()
.map(|arg| {
Expand All @@ -2289,11 +2294,20 @@ fn rewrite_args(
.collect::<Vec<_>>();

// Account for sugary self.
// FIXME: the comment for the self argument is dropped. This is blocked
// on rust issue #27522.
let mut pre_comment_str = "";
let mut post_comment_str = "";
let min_args = explicit_self
.and_then(|explicit_self| rewrite_explicit_self(explicit_self, args, context))
.map_or(1, |self_str| {
pre_comment_str = context.snippet(mk_sp(span.lo(), args[0].pat.span.lo()));

let next_start = if args.len() > 1 {
args[1].pat.span().lo()
} else {
span.hi()
};
post_comment_str = context.snippet(mk_sp(args[0].ty.span.hi(), next_start));

arg_item_strs[0] = self_str;
2
});
Expand All @@ -2310,14 +2324,18 @@ fn rewrite_args(
// it is explicit.
if args.len() >= min_args || variadic {
let comment_span_start = if min_args == 2 {
let second_arg_start = if arg_has_pattern(&args[1]) {
args[1].pat.span.lo()
let remove_comma_byte_pos = context
.snippet_provider
.span_after(mk_sp(args[0].ty.span.hi(), args[1].pat.span.lo()), ",");
let first_post_and_second_pre_span =
mk_sp(remove_comma_byte_pos, args[1].pat.span.lo());
if count_newlines(context.snippet(first_post_and_second_pre_span)) > 0 {
context
.snippet_provider
.span_after(first_post_and_second_pre_span, "\n")
} else {
args[1].ty.span.lo()
};
let reduced_span = mk_sp(span.lo(), second_arg_start);

context.snippet_provider.span_after_last(reduced_span, ",")
remove_comma_byte_pos
}
} else {
span.lo()
};
Expand All @@ -2342,8 +2360,8 @@ fn rewrite_args(
.iter()
.map(ArgumentKind::Regular)
.chain(variadic_arg),
")",
",",
terminator,
separator,
|arg| match *arg {
ArgumentKind::Regular(arg) => span_lo_for_arg(arg),
ArgumentKind::Variadic(start) => start,
Expand All @@ -2357,18 +2375,31 @@ fn rewrite_args(
ArgumentKind::Variadic(..) => Some("...".to_owned()),
},
comment_span_start,
span.hi(),
next_span_start,
false,
);

arg_items.extend(more_items);
}

let arg_items_len = arg_items.len();
let fits_in_one_line = !generics_str_contains_newline
&& (arg_items.is_empty()
|| arg_items.len() == 1 && arg_item_strs[0].len() <= one_line_budget);
|| arg_items_len == 1 && arg_item_strs[0].len() <= one_line_budget);

for (item, arg) in arg_items.iter_mut().zip(arg_item_strs) {
for (index, (item, arg)) in arg_items.iter_mut().zip(arg_item_strs).enumerate() {
// add pre comment and post comment for first arg(self)
if index == 0 && explicit_self.is_some() {
let (pre_comment, pre_comment_style) = extract_pre_comment(pre_comment_str);
item.pre_comment = pre_comment;
item.pre_comment_style = pre_comment_style;

let comment_end =
get_comment_end(post_comment_str, separator, terminator, arg_items_len == 1);

item.new_lines = has_extra_newline(post_comment_str, comment_end);
item.post_comment = extract_post_comment(post_comment_str, comment_end, separator);
}
item.item = Some(arg);
}

Expand Down Expand Up @@ -2418,14 +2449,6 @@ fn rewrite_args(
write_list(&arg_items, &fmt)
}

fn arg_has_pattern(arg: &ast::Arg) -> bool {
if let ast::PatKind::Ident(_, ident, _) = arg.pat.node {
ident != symbol::keywords::Invalid.ident()
} else {
true
}
}

fn compute_budgets_for_args(
context: &RewriteContext<'_>,
result: &str,
Expand Down
2 changes: 1 addition & 1 deletion src/lists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ pub fn get_comment_end(

// Account for extra whitespace between items. This is fiddly
// because of the way we divide pre- and post- comments.
fn has_extra_newline(post_snippet: &str, comment_end: usize) -> bool {
pub fn has_extra_newline(post_snippet: &str, comment_end: usize) -> bool {
if post_snippet.is_empty() || comment_end == 0 {
return false;
}
Expand Down
99 changes: 99 additions & 0 deletions tests/source/issue-3198.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
impl TestTrait {
fn foo_one_pre(/* Important comment1 */
self) {
}

fn foo_one_post(self
/* Important comment1 */) {
}

fn foo_pre(
/* Important comment1 */
self,
/* Important comment2 */
a: i32,
) {
}

fn foo_post(
self
/* Important comment1 */,
a: i32
/* Important comment2 */,
) {
}

fn bar_pre(
/* Important comment1 */
&mut self,
/* Important comment2 */
a: i32,
) {
}

fn bar_post(
&mut self
/* Important comment1 */,
a: i32
/* Important comment2 */,
) {
}

fn baz_pre(
/* Important comment1 */
self: X< 'a , 'b >,
/* Important comment2 */
a: i32,
) {
}

fn baz_post(
self: X< 'a , 'b >
/* Important comment1 */,
a: i32
/* Important comment2 */,
) {
}

fn baz_tree_pre(
/* Important comment1 */
self: X< 'a , 'b >,
/* Important comment2 */
a: i32,
/* Important comment3 */
b: i32,
) {
}

fn baz_tree_post(
self: X< 'a , 'b >
/* Important comment1 */,
a: i32
/* Important comment2 */,
b: i32
/* Important comment3 */,){
}

fn multi_line(
self: X<'a, 'b>, /* Important comment1-1 */
/* Important comment1-2 */
a: i32, /* Important comment2 */
b: i32, /* Important comment3 */
) {
}

fn two_line_comment(
self: X<'a, 'b>, /* Important comment1-1
Important comment1-2 */
a: i32, /* Important comment2 */
b: i32, /* Important comment3 */
) {
}

fn no_first_line_comment(
self: X<'a, 'b>,
/* Important comment2 */a: i32,
/* Important comment3 */b: i32,
) {
}
}
67 changes: 67 additions & 0 deletions tests/target/issue-3198.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
impl TestTrait {
fn foo_one_pre(/* Important comment1 */ self) {}

fn foo_one_post(self /* Important comment1 */) {}

fn foo_pre(/* Important comment1 */ self, /* Important comment2 */ a: i32) {}

fn foo_post(self /* Important comment1 */, a: i32 /* Important comment2 */) {}

fn bar_pre(/* Important comment1 */ &mut self, /* Important comment2 */ a: i32) {}

fn bar_post(&mut self /* Important comment1 */, a: i32 /* Important comment2 */) {}

fn baz_pre(
/* Important comment1 */
self: X<'a, 'b>,
/* Important comment2 */
a: i32,
) {
}

fn baz_post(
self: X<'a, 'b>, /* Important comment1 */
a: i32, /* Important comment2 */
) {
}

fn baz_tree_pre(
/* Important comment1 */
self: X<'a, 'b>,
/* Important comment2 */
a: i32,
/* Important comment3 */
b: i32,
) {
}

fn baz_tree_post(
self: X<'a, 'b>, /* Important comment1 */
a: i32, /* Important comment2 */
b: i32, /* Important comment3 */
) {
}

fn multi_line(
self: X<'a, 'b>, /* Important comment1-1 */
/* Important comment1-2 */
a: i32, /* Important comment2 */
b: i32, /* Important comment3 */
) {
}

fn two_line_comment(
self: X<'a, 'b>, /* Important comment1-1
Important comment1-2 */
a: i32, /* Important comment2 */
b: i32, /* Important comment3 */
) {
}

fn no_first_line_comment(
self: X<'a, 'b>,
/* Important comment2 */ a: i32,
/* Important comment3 */ b: i32,
) {
}
}