Added new option "chain_count" to limit number of chained functions per line.

[PokeyOne]

See Issue #2263

This PR adds a new option to limit the number of chained method calls based on the number of method calls in addition to the number of characters. This is my first PR to the rustfmt repository, so I apologise if anything about this PR is out of order or if any changes are needed. I want to get this done the right way.

This PR adds the new option chain_count, which is a usize type with a default value of 0. This is the maximum number of chained method calls to put on a single line. Setting to 0 effectively disables the option, while setting it to 1 has the effect of forcing method chaining to be on new lines.

Setting the option to 2 would have the effect that the following would be fine:

a.b().c()

However this:

a.b().c().d()

would become this:

a.b()
    .c()
    .d()

I originally considered making an option force_multiline_chains, however did not persue this because setting chain_count to 1 has the same effect, and this way there is more freedom for those who would prefer have a couple before it wraps.

Examples

0 (default):

fn main() {
    a.foo().bar().baz();
}

1:

fn main() {
    a.foo()
        .bar()
        .baz();
}

2:

fn main() {
    a.foo()
        .bar()
        .baz();
}

3:

fn main() {
    a.foo().bar().baz();
}

Interactions with Other Config Options

chain_width - Does not override. The chain will be made multiline if either of these rules are triggered.

force_one_line_chain - Overrides this option. (This was chosen arbitrarily and I am open to changing this 👍 )

[PokeyOne]

Sorry, looks like I missed that the tests with config files have to work on both stable and nightly. I will update this PR, and ask for help in the Discord if needed.

Is there a best practice for if I should add a new commit that fixes it, or amend my previous commit for the sake of a cleaner git history?

[ytmimi]

Appreciate you opening up a PR for a long standing issue. I know that @calebcartwright is currently working to improve how we handle chains in rustfmt, so he's the best reviewer for this PR. By the way, did you read through the entire discussion in #2263? From my understanding of the thread having a chain_count option might also lead to issues where it applies nicely to some chains, but not to all the chains in a project. Was wondering if you considered that?

Sorry, looks like I missed that the tests with config files have to work on both stable and nightly

You actually don't need to add a config file. You can just leave a comment at the top of the test file in the form //rustfmt-<config-name>:<config value> and the config will be applied. Check out the create-test-cases section of the Contributing Guide

Typically, default tests for config options are located in tests/source/configs/<config-name> and tests/target/configs/<config-name>

Also we'll likely need to add a lot more tests to fully prove out this idea. We'll want tests with a combination of long and short chains, and simple and complex chains. we'll want to pair those initial inputs with different values for the chain_count to get a better sense of how this proposed config option is going to affect chain formatting.

Is there a best practice for if I should add a new commit that fixes it

As you continue to work on this you can choose what makes sense for you. If you feel that adding new commits is the best approach then do that. If there are multiple commits we'll likely squash them when merging.

[calebcartwright]

@PokeyOne - thank you for the PR but I'm going to close this because even if we were to support such behavior, we'd need a different implementation that's predicated on some other work being finished first.

[PokeyOne]

Ok, thanks for letting me know, @calebcartwright . Is there something I should look for if I would like to help get this done after said other work being finished?

[calebcartwright]

I'd recommend subscribing to #2263. As noted in that issue, I'm skeptical this is something we'll actually want to support, but once the new option and associated internal framework have been in place for a while we can reevaluate. At some point down the road I'll either drop a note in that issue that we'd be willing to take on a new variant to support by-call logic, or I'll close it noting that we don't intend to support it. Obviously if it ends up being the former then you'd be more than welcome to try to pick it up at that time