Skip to content

Inconsistent Struct Body Opening Brace Placement After Where Clause #5507

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

Open
ytmimi opened this issue Aug 18, 2022 · 5 comments · May be fixed by #5508
Open

Inconsistent Struct Body Opening Brace Placement After Where Clause #5507

ytmimi opened this issue Aug 18, 2022 · 5 comments · May be fixed by #5508
Assignees

Comments

@ytmimi
Copy link
Contributor

ytmimi commented Aug 18, 2022

When the body of a struct is empty or it only contains comments, the opening brace of the struct body is placed on the same line as the where clause. If the struct body is not empty, the opening brace is placed on a newline.

No configuration values are needed, and I was using rustfmt 1.5.1-nightly (841b4542 2022-08-17) when I came across this.

Input

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
}

Output

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,
}
@jmj0502
Copy link
Contributor

jmj0502 commented Aug 18, 2022

@ytmimi I would like to work on this issue as well. I may need some clues on where to start but that's ok tho' c:

@jmj0502
Copy link
Contributor

jmj0502 commented Aug 18, 2022

@rustbot claim

@ytmimi
Copy link
Contributor Author

ytmimi commented Aug 18, 2022

I may need some clues on where to start but that's ok tho' c:

You'll likely want to start at format_struct, and follow the code down through format_struct_struct

rustfmt/src/items.rs

Lines 982 to 997 in 949da52

fn format_struct(
context: &RewriteContext<'_>,
struct_parts: &StructParts<'_>,
offset: Indent,
one_line_width: Option<usize>,
) -> Option<String> {
match *struct_parts.def {
ast::VariantData::Unit(..) => format_unit_struct(context, struct_parts, offset),
ast::VariantData::Tuple(ref fields, _) => {
format_tuple_struct(context, struct_parts, fields, offset)
}
ast::VariantData::Struct(ref fields, _) => {
format_struct_struct(context, struct_parts, fields, offset, one_line_width)
}
}
}

rustfmt/src/items.rs

Lines 1265 to 1301 in 949da52

pub(crate) fn format_struct_struct(
context: &RewriteContext<'_>,
struct_parts: &StructParts<'_>,
fields: &[ast::FieldDef],
offset: Indent,
one_line_width: Option<usize>,
) -> Option<String> {
let mut result = String::with_capacity(1024);
let span = struct_parts.span;
let header_str = struct_parts.format_header(context, offset);
result.push_str(&header_str);
let header_hi = struct_parts.ident.span.hi();
let body_lo = if let Some(generics) = struct_parts.generics {
// Adjust the span to start at the end of the generic arguments before searching for the '{'
let span = span.with_lo(generics.span.hi());
context.snippet_provider.span_after(span, "{")
} else {
context.snippet_provider.span_after(span, "{")
};
let generics_str = match struct_parts.generics {
Some(g) => format_generics(
context,
g,
context.config.brace_style(),
if fields.is_empty() {
BracePos::ForceSameLine
} else {
BracePos::Auto
},
offset,
// make a span that starts right after `struct Foo`
mk_sp(header_hi, body_lo),
last_line_width(&result),
)?,

If I had to guess what the issue is it's that we set the brace_pos argument of format_generics on line 1292 to BracePos::ForceSameLine when the struct fields are empty.

@jmj0502
Copy link
Contributor

jmj0502 commented Aug 20, 2022

@ytmimi Hey! hope you're doing great!
While testing I noticed that there are some idempotence tests that were written with the wrong format. Should I also correct those? (I'm still working on some edge cases but it'll be good to know c:)
For example:

struct Foo<T: Eq+Clone, U>
where
U: Eq+Clone, {
// body
}

@ytmimi
Copy link
Contributor Author

ytmimi commented Aug 22, 2022

It's likely that any change we make would need to be version gated to prevent breaking formatting changes. so we'd only be able to change the behavior when version=Two.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants