Skip to content
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

PreferSameLine brace_style with trailing_comma Vertical produces weird results #2321

Open
djc opened this issue Dec 30, 2017 · 3 comments · May be fixed by #5556 or #5895
Open

PreferSameLine brace_style with trailing_comma Vertical produces weird results #2321

djc opened this issue Dec 30, 2017 · 3 comments · May be fixed by #5556 or #5895
Assignees
Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors only-with-option requires a non-default option value to reproduce poor-formatting

Comments

@djc
Copy link
Contributor

djc commented Dec 30, 2017

Upgrading rustfmt-nightly from 0.2.15 to 0.3.4 created this change:

@@ -104,8 +122,7 @@ impl FetchCommandMessages {
 
 pub trait FetchBuilderMessages
 where
-    Self: Sized,
-{
+    Self: Sized, {
     fn prepare(self) -> FetchCommandMessages;
 
     fn num(self, num: u32) -> FetchCommandMessages {

This seems like a bug, since trailing_comma = "Vertical" should not trigger if the opening brace is on the same line.

@topecongiro topecongiro added only-with-option requires a non-default option value to reproduce poor-formatting labels Dec 30, 2017
@ytmimi
Copy link
Contributor

ytmimi commented Jul 19, 2022

Confirming this is reproducible with rustfmt 1.5.1-nightly (0cb294f0 2022-07-12) and configuration options trailing_comma = "Vertical" and brace_style=PreferSameLine.

Input

pub trait FetchBuilderMessages
where
    Self: Sized,
{
     fn prepare(self) -> FetchCommandMessages;
}

output

pub trait FetchBuilderMessages
where
    Self: Sized, { // <--- Note trailing comma is not removed from the bound
    fn prepare(self) -> FetchCommandMessages;
}

If someone is interested in taking this on, I'd start by checking out these lines inrewrite_bounds_on_where_clause:

rustfmt/src/items.rs

Lines 2872 to 2876 in a7bf009

let comma_tactic = if where_clause_option.suppress_comma || force_single_line {
SeparatorTactic::Never
} else {
context.config.trailing_comma()
};

@ytmimi ytmimi added the good first issue Issues up for grabs, also good candidates for new rustfmt contributors label Jul 19, 2022
@ytmimi
Copy link
Contributor

ytmimi commented Jul 20, 2022

Linking tracking issues for brace_style(#3376) and trailing_comma (#3379)

@gentoid
Copy link
Contributor

gentoid commented Aug 27, 2023

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues up for grabs, also good candidates for new rustfmt contributors only-with-option requires a non-default option value to reproduce poor-formatting
Projects
None yet
4 participants