If-chain wrapping when `use_small_heuristics = "Max"`

[digama0]

This is a follow up to https://github.com/rust-lang/style-team/issues/169#issuecomment-1723205191. Currently, rustfmt is formatting if-chains on nightly, but it uses a line-break-heavy style that is the current front-runner on https://github.com/rust-lang/style-team/issues/169 and other popular styles also exist. In particular, use_small_heuristics = "Max" is IMO a strong indication of intent from the author to not have spurious line breaks, and rustfmt should instead format let chains the same as regular binary operators, filling the line if space permits. (An option specific to if-chains is also reasonable, but use_small_heuristics = "Max" should imply it.)

before:

if let 1 = 1
  && true
{
}

after:

if let 1 = 1 && true {}

This relates to a discussion on Zulip.

[ytmimi]

Thanks for opening the issue. I Just wanted to double check if this was also a case you'd expect to be formatted on a single line even though there's more than one let in the chain.

if let 1 = 1 && let 2 = 2 && true {
}

Or in this case would this be preferred:

if let 1 = 1
    && let 2 = 2
    && true
{
}

[eminence]

For me personally, I would expect that use_small_heuristics = "Max" would cause the multi-let version to also be formatted on one line

[calebcartwright]

I think that it'd be best to take a step back and think about desired formatting outcomes vs. a specific means to that end.

use_small_heuristics, though perhaps not well understood, isn't vague and overarching, nor is it a matter of representing the developers overarching intent for an entire codebase. It's a single option which functions as a single big club to simplify controlling multiple granular width related options that respectively govern the width limits for various, enumerated, specific AST constructs: https://rust-lang.github.io/rustfmt/?version=v1.6.0&search=#use_small_heuristics

let-chains are, at least from an AST perspective, just a binary expression with a let expr and some other things contained within. Those are formatted according to their own respective rules and configurations, and use_small_heuristics (and I think more granularly, single_line_if_else_max_width) ,only comes into play so long as the other conditions are true.

i.e. single_line_if_else_max_width only comes into the picture if the if's expression itself could independently be formatted on a single line based on all other rules and configs.

what should come first, imo, is a config option that controls the behavior of the "inner" let chain, which could then be wired up into use_small_heuristics

[calebcartwright]

perhaps more succinctly and with an example:

if fooooooooooooooooooooooooooooooooo(really_really_really_really_really_long_ident, some_other_call(), "long                                            string                                                              ", "another llllllllllllllllllllllllllllooooooooooooooooooooooooooooooonnnnnnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggg string") { .. } else { ..}

that can't go on one line just because use_small_heuristics is set to max. there's other rules that require that fooooooooooooooooooooooooooooooooo call to be multilined, and so the entire if/else expression has to follow the multiline rules regardless.

the only difference here is that instead of a function call we've got the let chain/binary expression, and atm, the default formatting requires that to be multilined, and we've got no configuration option that allows users to do something different

[digama0]

what should come first, imo, is a config option that controls the behavior of the "inner" let chain, which could then be wired up into use_small_heuristics

Agreed on this. I have no particular suggestion for the name of the option, but it makes sense to me that there would be an option specifically for choosing between different flavors of if-chain formatting, and use_small_heuristics = "Max" is a meta-option that sets this option among many others.

[workingjubilee]

use_small_heuristics, though perhaps not well understood, isn't vague and overarching, nor is it a matter of representing the developers overarching intent for an entire codebase. It's a single option which functions as a single big club to

thogitha want big smash

[eminence]

Also agreed with @calebcartwright and in the interest of being more clear and explicit about my expectations:

I expect that there should be an option (maybe a yet-to-be-invented option) that would allow if let 1 = 1 && let 2 = 2 && true {} to be formatted on one line. (But I also stand by my earlier comment, as it accurately reflects what my expectations for use_small_heuristics are, absent any other info)

[calebcartwright]

@ytmimi what I think we should do from the rustfmt team side is either implement (or guide someone in the implementation of) the foundation/option to control this (and fwiw i'd be fine with something as vague as "let_chain_style"), naturally with the default set to where the style guide appears to be heading.

that will allow others to suggest/implement other variants, with hopefully relatively minimal input from our end

the only concern/question i have from a design perspective is whether that'll suffice. in the style team meetings we basically went through all the different kinds of expressions and weighed whether each could be included in the "allowed to stay single line" list

i.e. what if someone is fine with single line formatting for calls, but only calls that had 0 args. but then what if someone else wants to control the number of args, or the types of args, and then... I think you start to get the picture :smile:

i think our lens should primarily be: how can we provide some level of control over the wrapping behavior, without grotesquely complicating the config option surface and internal codebase

[digama0]

perhaps more succinctly and with an example:

if fooooooooooooooooooooooooooooooooo(really_really_really_really_really_long_ident, some_other_call(), "long                                            string                                                              ", "another llllllllllllllllllllllllllllooooooooooooooooooooooooooooooonnnnnnnnnnnnnnnnnnnnnnnnnnngggggggggggggggggggggggggggggg string") { .. } else { ..}

that can't go on one line just because use_small_heuristics is set to max. there's other rules that require that fooooooooooooooooooooooooooooooooo call to be multilined, and so the entire if/else expression has to follow the multiline rules regardless.

Well this one has to be multilined anyway because it literally doesn't fit. It's use_small_heuristics = "Max" (meaning line length), not line_length = infinity. Maybe a better example along the same lines would be an expression which is less than the line width but nevertheless has forced line breaks even with use_small_heuristics = "Max":

if ({ let x = true; x }) {
  ...
}

[calebcartwright]

re: https://github.com/rust-lang/rustfmt/issues/5948#issuecomment-1778224434 yes, thanks for the correction @digama0 . I was rushing and provided a bad example :sweat_smile:

the point i was trying to make is that use_small_heuristics doesn't trump everything else. it's more of a width-based last line line of defense. the element/construct width limits controlled by use_small_heuristics are applied IFF everything contained within an expression can be formatted in a single line, and then if so, are only done so if the single line version of that element/construct would not exceed the width limits set via use_small_heuristics

[ytmimi]

@ytmimi what I think we should do from the rustfmt team side is either implement (or guide someone in the implementation of) the foundation/option to control this (and fwiw i'd be fine with something as vague as "let_chain_style"), naturally with the default set to where the style guide appears to be heading.

@calebcartwright I just opened https://github.com/rust-lang/rustfmt/pull/5983 to add let_chain_style.