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

Changes from 1 commit
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
65 changes: 20 additions & 45 deletions src/items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use rustc_span::{symbol, BytePos, Span, DUMMY_SP};

use crate::attr::filter_inline_attrs;
use crate::comment::{
combine_strs_with_missing_comments, comment_style, contains_comment, find_comment_end,
is_last_comment_block, recover_comment_removed, recover_missing_comment_in_span,
rewrite_missing_comment, FindUncommented,
combine_strs_with_missing_comments, comment_style, contains_comment, is_last_comment_block,
recover_comment_removed, recover_missing_comment_in_span, rewrite_missing_comment,
FindUncommented,
};
use crate::config::lists::*;
use crate::config::{BraceStyle, Config, IndentStyle, Version};
Expand Down Expand Up @@ -1262,53 +1262,28 @@ fn format_unit_struct(
Some(format!("{}{};", header_str, generics_str))
}

fn set_brace_pos(
fn set_struct_brace_pos(
context: &RewriteContext<'_>,
struct_parts: &StructParts<'_>,
fields: &[ast::FieldDef],
body_lo: BytePos,
span: Span,
) -> BracePos {
if fields.is_empty() {
let span = struct_parts.span;
let generics_lo = if let Some(generics) = struct_parts.generics {
generics.span.lo()
} else {
body_lo
};
let snippet = context.snippet(mk_sp(generics_lo, span.hi()));
let body_contains_comments = contains_comment(snippet);
let has_single_line_block_comment = if body_contains_comments {
let comment_start = match snippet.find('/') {
Some(i) => i,
None => 0,
};
let comment_snippet = &snippet[comment_start..];
let comment = comment_style(comment_snippet, false);
let is_block_comment = comment.is_block_comment();
let is_single_line_comment = if is_block_comment {
let comment_end = match find_comment_end(comment_snippet) {
Some(i) => i,
None => comment_start,
};
is_single_line(&comment_snippet[..=comment_end - 1])
} else {
false
};
is_single_line_comment
} else {
false
};
if !fields.is_empty() {
return BracePos::Auto;
}
let snippet = context.snippet(span).trim();

if context.config.version() == Version::Two {
return match body_contains_comments {
true if has_single_line_block_comment => BracePos::ForceSameLine,
true => BracePos::Auto,
false => BracePos::ForceSameLine,
};
}
if snippet.is_empty() || context.config.version() == Version::One {
return BracePos::ForceSameLine;
}
BracePos::Auto

let is_single_line = is_single_line(snippet);
let is_block_comment = comment_style(snippet, false).is_block_comment();

if is_single_line && is_block_comment {
BracePos::ForceSameLine
} else {
BracePos::Auto
}
}

pub(crate) fn format_struct_struct(
Expand Down Expand Up @@ -1338,7 +1313,7 @@ pub(crate) fn format_struct_struct(
context,
g,
context.config.brace_style(),
set_brace_pos(&context, &struct_parts, &fields, body_lo),
set_struct_brace_pos(&context, &fields, mk_sp(body_lo, span.hi() - BytePos(1))),
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating the inner_span inline with mk_sp(body_lo, span.hi() - BytePos(1)) can we just hoist it up from where it's originally defined.

+ let inner_span = mk_sp(body_lo, span.hi() - BytePos(1));
  let generics_str = match struct_parts.generics {
         Some(g) => format_generics(
             context,
             g,
             context.config.brace_style(),
-            set_brace_pos(&context, &struct_parts, &fields, body_lo),
+            set_struct_brace_pos(&context, &fields, inner_span)),
             ..
         )
         ..
 };
 ...
     if fields.is_empty() {
-        let inner_span = mk_sp(body_lo, span.hi() - BytePos(1));
         format_empty_struct_or_tuple(context, inner_span, offset, &mut result, "", "}");
         return Some(result);
     }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ytmimi Hey! Hope you're doing awesome!

I was about to ask you a question about this point hehe (hositing the inner_span on the outermost scope of the function). I'll add the changes in a few minutes 👍.

offset,
// make a span that starts right after `struct Foo`
mk_sp(header_hi, body_lo),
Expand Down