Skip to content

Fix/Inconsistent Struct Body Opening Brace Placement After Where Clause #5508

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 12 commits into
base: master
Choose a base branch
from
Open

Fix/Inconsistent Struct Body Opening Brace Placement After Where Clause #5508

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0ee1308
Improvements: Adding the tests cases for the issue
jmj0502 Aug 18, 2022
2e46b4c
Improvements: The formatting for generic structs containing where cla…
jmj0502 Aug 22, 2022
58e2a54
Bugfix: Removing unnecesary value.
jmj0502 Aug 22, 2022
b3e3651
Improvements: A refactor was performed in order to improve readabilit…
jmj0502 Aug 23, 2022
b5323ca
Improvements: Removing unnecessary comments.
jmj0502 Aug 23, 2022
0c0fb58
Improvements: Removing additional line break.
jmj0502 Aug 23, 2022
254c829
Improvements: Now empty impl blocks and impl blocks containing single…
jmj0502 Sep 17, 2022
088369e
Improvements: replacing &snippet[comment_start..] for comment_snippet…
jmj0502 Sep 17, 2022
f526382
Improvements: Removing unnecesary match clause (fields.is_empty()) [i…
jmj0502 Oct 7, 2022
c5098f8
Improvements: Making changes to the signature of the set_struct_brace…
jmj0502 Oct 13, 2022
5cd84e1
Improvements: Now the inner_span variable is hoisted on the outter mo…
jmj0502 Oct 13, 2022
64a2321
Merge branch 'master' into fix/issue-5507
jmj0502 Aug 30, 2023
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
33 changes: 28 additions & 5 deletions src/items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1262,6 +1262,29 @@ fn format_unit_struct(
Some(format!("{}{};", header_str, generics_str))
}

fn set_brace_pos(has_fields: bool, has_content: bool, version: Version) -> BracePos {
return match version {
Version::One => {
if has_fields {
return BracePos::Auto;
}
BracePos::ForceSameLine
}
Version::Two => {
match (has_fields, has_content) {
// Doesn't have fields, there is nothing between {}, has generics.
(true, true) => BracePos::ForceSameLine,

// Doesn't have fields, has something between { }, has generics.
(true, false) => BracePos::Auto,

// Has fields, has generics.
(false, _) => BracePos::Auto,
}
}
};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Couple of notes:

  1. the return in return match version is redundant, since the match is the only statement in this function. remove the trailing semicolon from the match statement to convert it to an expression and have it implicitly return its value.
  2. I'm all for extracting this logic into a stand alone function, but I think we can simplify the logic see my next comment for details.


pub(crate) fn format_struct_struct(
context: &RewriteContext<'_>,
struct_parts: &StructParts<'_>,
Expand Down Expand Up @@ -1289,11 +1312,11 @@ pub(crate) fn format_struct_struct(
context,
g,
context.config.brace_style(),
if fields.is_empty() {
BracePos::ForceSameLine
} else {
BracePos::Auto
},
set_brace_pos(
!fields.is_empty(),
span.hi() <= body_lo + BytePos(5),
Copy link
Contributor

Choose a reason for hiding this comment

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

To be honest, this check span.hi() <= body_lo + BytePos(5) has got me scratching my head and it's not obvious from reading the code what it's supposed to be doing. I'm all for moving this logic into a standalone function, but I think this check could be simplified to something like:

            if fields.is_empty() {
                let snippet = context.snippet(mk_sp(body_lo, span.hi()));
                let body_contains_commets = contains_comment(snippet);
                if context.config.version() == Version::Two && body_contains_commets {
                    BracePos::Auto
                } else {
                    BracePos::ForceSameLine
                }
            } else {
                BracePos::Auto
            }

context.config.version(),
),
offset,
// make a span that starts right after `struct Foo`
mk_sp(header_hi, body_lo),
Expand Down
20 changes: 20 additions & 0 deletions tests/source/issue-5507/issue-5507-indent-style-block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// rustfmt-version: Two

struct EmptyBody<T>
where T: Eq {
}

struct LineComment<T>
where T: Eq {
// body
}

struct BlockComment<T>
where T: Eq {
/* block comment */
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also add a MultiLineBlockComment case

struct MultiLineBlockComment<T>
    where T: Eq {
    /* block comment 
     * block comment
     */
}


struct HasBody<T>
where T: Eq {
x: T
}
21 changes: 21 additions & 0 deletions tests/source/issue-5507/issue-5507-indent-style-visual.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// rustfmt-indent_style: Visual
// rustfmt-version: Two

struct EmptyBody<T>
where T: Eq {
}

struct LineComment<T>
where T: Eq {
// body
}

struct BlockComment<T>
where T: Eq {
/* block comment */
}

struct HasBody<T>
where T: Eq {
x: T
}
25 changes: 25 additions & 0 deletions tests/target/issue-5507/issue-5507-indent-style-block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// rustfmt-version: Two

struct EmptyBody<T>
where
T: Eq,
{}

struct LineComment<T>
where
T: Eq,
{
// body
}

struct BlockComment<T>
where
T: Eq,
{/* block comment */}

struct HasBody<T>
where
T: Eq,
{
x: T,
}
22 changes: 22 additions & 0 deletions tests/target/issue-5507/issue-5507-indent-style-visual.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// rustfmt-indent_style: Visual
// rustfmt-version: Two

struct EmptyBody<T>
where T: Eq
{}

struct LineComment<T>
where T: Eq
{
// body
}

struct BlockComment<T>
where T: Eq
{/* block comment */}

struct HasBody<T>
where T: Eq
{
x: T,
}