Skip to content

Fix Comment Preservation in PatKind::Paren Patterns #6111

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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix Comment Preservation in PatKind::Paren Patterns #6111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b7f2154
add:`rewrite_with_parens`in `PatKind::Paren`
yuvraj-wale Mar 11, 2024
88cfed1
add: manual comment recovery
yuvraj-wale Mar 15, 2024
930b412
add: test cases
yuvraj-wale Mar 16, 2024
be1585c
fix: test case errors
yuvraj-wale Mar 16, 2024
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
10 changes: 7 additions & 3 deletions src/patterns.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,9 +271,13 @@ impl Rewrite for Pat {
PatKind::MacCall(ref mac) => {
rewrite_macro(mac, None, context, shape, MacroPosition::Pat)
}
PatKind::Paren(ref pat) => pat
.rewrite(context, shape.offset_left(1)?.sub_width(1)?)
.map(|inner_pat| format!("({})", inner_pat)),
PatKind::Paren(ref pat) => {
let inner_pat = pat.rewrite(context, shape)?;
let pre_snippet = context.snippet(mk_sp(self.span.lo(), pat.span.lo()));
let post_snippet = context.snippet(mk_sp(pat.span.hi(), self.span.hi()));
Comment on lines +276 to +277
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I was suggesting we return context.snippet(span) I was thinking more along the lines of the following approach where we don't apply any formatting, not even to the inner pattern.

let pre_snippet = context.snippet(self.span.lo().between(pat.span.lo()));
let post_snippet = context.snippet(pat.span.lo().between(self.span.hi()));

if contains_comment(pre_snippet) || contains_comment(post_snippet) {
    return Some(context.snippet(self.span).to_owned())
}

Your approach may also work though! As I mentioned I'd like you to add some test cases to better prove this out.

Copy link
Author

@yuvraj-wale yuvraj-wale Mar 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey I've added some test cases, let me know if there's anything specific you'd like to see.

let result = format!("{}{}{}", pre_snippet, inner_pat, post_snippet);
Some(result)
}
}
}
}
Expand Down
54 changes: 54 additions & 0 deletions tests/source/comments_in_paren/comments_in_paren.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// rustfmt-wrap_comments: true

// Ensure comment preservation in parenthesis
fn main() {
let (/* */ () | () /* */) = ();
let (/**/ () | () /**/) = ();
let (/*comment*/ () | () /*comment*/) = ();
let (/*multi-line
comment*/ () | () /*multi-line
comment */) = ();
let (/*comment with new line
*/
() | ()
/*comment with new line
*/
) = ();
}

// Ensure proper handling of line comments
fn line_comments() {
let (// Before
()
|
()
// After
) = ();
}

// Ensure proper handling of block comments with new lines
fn block_comments_with_new_lines() {
let (/* Before
* with new line */
()
|
()
/* After
* with new line */
) = ();
}

// Ensure inner pattern is getting formatted properly
// whilst preserving comments in outer parenthesis
fn inner_pat_formatting() {
let (/*comment*/
(

)


| (

)
/*comment*/) = ();
}
43 changes: 43 additions & 0 deletions tests/target/comments_in_paren/comments_in_paren.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// rustfmt-wrap_comments: true

// Ensure comment preservation in parenthesis
fn main() {
let (/* */ () | () /* */) = ();
let (/**/ () | () /**/) = ();
let (/*comment*/ () | () /*comment*/) = ();
let (/*multi-line
comment*/ () | () /*multi-line
comment */) = ();
let (/*comment with new line
*/
() | ()
/*comment with new line
*/
) = ();
}

// Ensure proper handling of line comments
fn line_comments() {
let (// Before
() | ()
// After
) = ();
}

// Ensure proper handling of block comments with new lines
fn block_comments_with_new_lines() {
let (/* Before
* with new line */
() | ()
/* After
* with new line */
) = ();
}

// Ensure inner pattern is getting formatted properly
// whilst preserving comments in outer parenthesis
fn inner_pat_formatting() {
let (/*comment*/
() | ()
/*comment*/) = ();
}