Skip to content

Commit

Permalink
Retain trailing separator when extracting the last inline post comment
Browse files Browse the repository at this point in the history
Fixes 5042

Previously, trailing commas were removed from the last inline comment.
This lead to rustfmt refusing to format code snippets because
the original comment did not match the rewritten comment.

Now, when rustfmt extracts the last inline comment it leaves trailing
separators alone. Rustfmt does not need to remove these separators
because they are commented out.
  • Loading branch information
ytmimi authored and calebcartwright committed Feb 4, 2022
1 parent 368a9b7 commit 606894e
Show file tree
Hide file tree
Showing 9 changed files with 149 additions and 7 deletions.
27 changes: 20 additions & 7 deletions src/lists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,15 +611,30 @@ pub(crate) fn extract_post_comment(
post_snippet: &str,
comment_end: usize,
separator: &str,
is_last: bool,
) -> Option<String> {
let white_space: &[_] = &[' ', '\t'];

// Cleanup post-comment: strip separators and whitespace.
let post_snippet = post_snippet[..comment_end].trim();

let last_inline_comment_ends_with_separator = if is_last {
if let Some(line) = post_snippet.lines().last() {
line.ends_with(separator) && line.trim().starts_with("//")
} else {
false
}
} else {
false
};

let post_snippet_trimmed = if post_snippet.starts_with(|c| c == ',' || c == ':') {
post_snippet[1..].trim_matches(white_space)
} else if let Some(stripped) = post_snippet.strip_prefix(separator) {
stripped.trim_matches(white_space)
} else if last_inline_comment_ends_with_separator {
// since we're on the last item it's fine to keep any trailing separators in comments
post_snippet.trim_matches(white_space)
}
// not comment or over two lines
else if post_snippet.ends_with(',')
Expand Down Expand Up @@ -748,14 +763,12 @@ where
.snippet_provider
.span_to_snippet(mk_sp((self.get_hi)(&item), next_start))
.unwrap_or("");
let comment_end = get_comment_end(
post_snippet,
self.separator,
self.terminator,
self.inner.peek().is_none(),
);
let is_last = self.inner.peek().is_none();
let comment_end =
get_comment_end(post_snippet, self.separator, self.terminator, is_last);
let new_lines = has_extra_newline(post_snippet, comment_end);
let post_comment = extract_post_comment(post_snippet, comment_end, self.separator);
let post_comment =
extract_post_comment(post_snippet, comment_end, self.separator, is_last);

self.prev_span_end = (self.get_hi)(&item) + BytePos(comment_end as u32);

Expand Down
24 changes: 24 additions & 0 deletions tests/source/issue-5042/multi-line_comment_with_trailing_comma.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
fn main() {
// 5042 deals with trailing commas, not the indentation issue of these comments
// When a future PR fixes the inentation issues these test can be updated
let _ = std::ops::Add::add(10, 20
// ...
// ...,
);

let _ = std::ops::Add::add(10, 20
/* ... */
// ...,
);

let _ = std::ops::Add::add(10, 20
// ...,
// ...,
);

let _ = std::ops::Add::add(10, 20
// ...,
/* ...
*/,
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
fn main() {
// 5042 deals with trailing commas, not the indentation issue of these comments
// When a future PR fixes the inentation issues these test can be updated
let _ = std::ops::Add::add(10, 20
// ...
// ...
);

let _ = std::ops::Add::add(10, 20
/* ... */
// ...
);

let _ = std::ops::Add::add(10, 20
// ...
// ...
);

let _ = std::ops::Add::add(10, 20
// ...
/* ...
*/
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fn main() {
let _ = std::ops::Add::add(10, 20
// ...,
);

let _ = std::ops::Add::add(10, 20
/* ... */,
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
fn main() {
let _ = std::ops::Add::add(10, 20
// ...
);

let _ = std::ops::Add::add(10, 20
/* ... */
);
}

24 changes: 24 additions & 0 deletions tests/target/issue-5042/multi-line_comment_with_trailing_comma.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
fn main() {
// 5042 deals with trailing commas, not the indentation issue of these comments
// When a future PR fixes the inentation issues these test can be updated
let _ = std::ops::Add::add(
10, 20, // ...
// ...,
);

let _ = std::ops::Add::add(
10, 20, /* ... */
// ...,
);

let _ = std::ops::Add::add(
10, 20, // ...,
// ...,
);

let _ = std::ops::Add::add(
10, 20, // ...,
/* ...
*/
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
fn main() {
// 5042 deals with trailing commas, not the indentation issue of these comments
// When a future PR fixes the inentation issues these test can be updated
let _ = std::ops::Add::add(
10, 20, // ...
// ...
);

let _ = std::ops::Add::add(
10, 20, /* ... */
// ...
);

let _ = std::ops::Add::add(
10, 20, // ...
// ...
);

let _ = std::ops::Add::add(
10, 20, // ...
/* ...
*/
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {
let _ = std::ops::Add::add(
10, 20, // ...,
);

let _ = std::ops::Add::add(10, 20 /* ... */);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {
let _ = std::ops::Add::add(
10, 20, // ...
);

let _ = std::ops::Add::add(10, 20 /* ... */);
}

0 comments on commit 606894e

Please sign in to comment.