Chains of fields and method calls section is maybe underspecified

[ytmimi]

Asking this in response to rust-lang/rustfmt#6332.

Not sure if this is a style guide bug, a rustfmt bug, or if the style guide is underspecified (or maybe I'm looking in the wrong section), but the Chains of fields and method calls doesn't fully describe scenarios where the last line of the chain's root isn't indented. For example Foo/foo in the code below:

fn main() {
    Foo {
        field_a: value_a,
        field_b: value_b,
    }
    .call_some_method()
    .do_something(lots, of, arguments)
    .do_another_thing(also, many, arguments);

    foo(
        Some(value_a),
        Some(value_b),
        Some(value_c),
        Some(value_d),
        Some(value_e),
        Some(value_f),
        Some(value_g),
    )
    .call_some_method()
    .do_something(lots, of, arguments)
    .do_another_thing(also, many, arguments);
}

The guide states the following (emphasis mine):

If formatting on multiple lines, put each field access or method call in the chain on its own line, with the line-break before the . and after any ?. Block-indent each subsequent line

Again emphasis mine, and in the Multi-line elements subsection the guide states:

If any element in a chain is formatted across multiple lines, put that element and any later elements on their own lines.

a.b.c()?
    .foo(
        an_expr,
        another_expr,
    )
    .bar
    .baz

Note there is block indent due to the chain and the function call in the above example.

I think the current formatting produced by rustfmt is correct, but the guide uses language that suggests that the chained calls be block indented, so should the Foo/foo example above actually be formatted like this?

fn main() {
    Foo {
        field_a: value_a,
        field_b: value_b,
    }
        .call_some_method()
        .do_something(lots, of, arguments)
        .do_another_thing(also, many, arguments);

    foo(
        Some(value_a),
        Some(value_b),
        Some(value_c),
        Some(value_d),
        Some(value_e),
        Some(value_f),
        Some(value_g),
    )
        .call_some_method()
        .do_something(lots, of, arguments)
        .do_another_thing(also, many, arguments);
}

[calebcartwright]

This was discussed back in the lengthy #66, but I'd agree that there were a number of key decisions made there, and implemented (correctly) in rustfmt, that didn't make their way into explicit text in the guide and which would be helpful to do so.

the best tl;dr i could give would be that it's another case avoiding the appearance of floating/unparented elements

[upsuper]

Thanks for the context. The discussion in #66 doesn't seem to be conclusive, and this exact case was raised close to the end of the thread with no one seemingly against it. I also feel the current formatting is quite unfortunate.

Another case worth mentioning in the rustfmt issue is its consequence in function call formatting. Rustfmt currently produces output like

fn main() {
    foo(Foo {
        field_a: value_a,
        field_b: value_b,
    }
    .call_some_method()
    .do_something(lots, of, arguments)
    .do_another_thing(also, many, arguments))
}

However, the style guide says:

If function call is not small, it would otherwise over-run the max width, or any argument or the callee is multi-line, then format the call across multiple lines. In this case, put each argument on its own block-indented line, break after the opening parenthesis and before the closing parenthesis, and use a trailing comma:

a_function_call(
    arg1,
    a_nested_call(a, b),
)

which indicate it should probably be at least formatted as

fn main() {
    foo(
        Foo {
            field_a: value_a,
            field_b: value_b,
        }
        .call_some_method()
        .do_something(lots, of, arguments)
        .do_another_thing(also, many, arguments),
    )
}

[upsuper]

Wait, isn't this very issue mentioned explicitly in the guide here:

If the length of the last line of the first element plus its indentation is less than or equal to the indentation of the second line, then combine the first and second lines if they fit. Apply this rule recursively.

// ...

foo(
    expr1,
    expr2,
).baz?
   .qux();

which indicate the case in question is not unspecified but should be

fn main() {
    Foo {
        field_a: value_a,
        field_b: value_b,
    }.call_some_method()
        .do_something(lots, of, arguments)
        .do_another_thing(also, many, arguments);

    foo(
        Some(value_a),
        Some(value_b),
        Some(value_c),
        Some(value_d),
        Some(value_e),
        Some(value_f),
        Some(value_g),
    ).call_some_method()
        .do_something(lots, of, arguments)
        .do_another_thing(also, many, arguments);
}

?

[calebcartwright]

which indicate it should probably be at least formatted as

The Style Guide sections can't be reviewed and considered in isolation of the rest of the guide, and formatting has to follow prescriptions from all other relevant sections, e.g. in this case like Combinable Expressions

which indicate the case in question is not unspecified but should be

I believe there was a previous acknowledgement that there was an issue with this particular snippet 👇 (at least I vaguely recall a PR that was removing it but which ultimately got closed because it had other things)

foo(
    expr1,
    expr2,
).baz?
   .qux();

Because that snippet doesn't match the rules for multiline elements
https://doc.rust-lang.org/nightly/style-guide/expressions.html?highlight=chain#multi-line-elements

If any element in a chain is formatted across multiple lines, put that element and any later elements on their own lines.

---

A general pattern with the example snippets is that they don't necessarily always fully comply with the full text of the style guide, they are just meant as a quick visual aid for human readers.

For example, the snippet in question would have to be written as a single line chain according to the rules for function calls and chains anyway (i.e. foo would never wrap). We try to clean up these occasional cases when we find them, but I wouldn't overindex on them

This rule 👇

If the length of the last line of the first element plus its indentation is less than or equal to the indentation of the second line, then combine the first and second lines if they fit. Apply this rule recursively.

is perhaps more easily visualized with a snippet like this

fn main() {
    a.b.c()?.d.e(foo, bar, baz, "something much much much longer").f.d();
    
    ab.c.d.e(foo, bar, baz, "something much much much longer").f.d()
}

the rule is using column width to specify where to make the break to avoid floating scenarios like this:

    a.b
        .c()?
...

and that's why the application of these rules on the input snippet breaks at d in the top but e in the second

fn main() {
    a.b.c()?
        .d
        .e(foo, bar, baz, "something much much much longer")
        .f
        .d();

    ab.c.d
        .e(foo, bar, baz, "something much much much longer")
        .f
        .d()
}

It's not trying to tack on subsequent chain elements onto the closing token of a preceding element that had to be wrapped across multiple lines, nor is it intended to contradict the "after a multiline element put all subsequent elements on their own line". The early RFCs and pre-1.0 release of rustfmt worked hand in hand to test things out, and sometimes that surfaced undesirable results, and the accompanying findings for #66 (at least some of them) on the beta rustfmt side were rust-lang/rustfmt#2932

[calebcartwright]

Perhaps a simpler way to think about the existing rules (speaking for myself and not on behalf of the style team) is that the goal is to avoid "floating" code, and that once a chain has to be broken, to use indentation of the subsequent elements to visually line up the remaining child elements as close as possible vertically to the the last token of the prior element where the break occurred (and while still adhering to all other rules)

The Style team will most likely be looking at ways to improve rules for the 2027 edition, but this is the way I've perceived the intent/objective of the current rule and while I think there's always opportunity for wording things more clearly (or fixing any specious examples), I don't see any cases of rustfmt violating existing rules

[upsuper]

@calebcartwright Thanks for all the context!