Implement let-else formatting

[ytmimi]

ref #4914

r? @calebcartwright

Implements let-else formatting based on the rules outlined in rust-lang/rust#107312. The examples listed in the style guide are included as simple test cases to validate let-else formatting.

Note This PR does not implement a "short" config for let-else.

[ytmimi]

I think the PR is good to go, but I plan to add some additional test cases with comments between the initializer expression and the else block as well as test out how the formatting is changed when users set control_brace_style.

[calebcartwright]

test out how the formatting is changed when users set control_brace_style.

This should not have an impact given the current applicable domains, as control_brace_style is only intended to apply to control flow expressions.

I recognize that there will likely be some non-zero portion of the user base that would like to force the brace onto the next line, but I think that needs to be considered against the more holistic lines of the posture outlined in #3376 (comment); I don't want to try to shoehorn yet another AST context into the existing config options

[ytmimi]

@calebcartwright, given the way I implemented let-else it's possible to use control_brace_style to configure where the else will be placed. For example, you can run rustfmt on the following input:

fn main() {
    let Some(x) = opt else {
        return;
    };
}

with control_brace_style=ClosingNextLine:

fn main() {
    let Some(x) = opt
    else {
        return;
    };
}

or with control_brace_style=AlwaysNextLine:

fn main() {
    let Some(x) = opt
    else
    {
        return;
    };
}

I implemented the else block rewriting for let-else by reusing existing functionality from if-else. If you'd prefer let-else formatting to not be influenced by control_brace_style I can certainly make an update to the PR. I thought it would be worth writing some tests for that functionality given that it's currently built in.

[calebcartwright]

If you'd prefer let-else formatting to not be influenced by control_brace_style I can certainly make an update to the PR. I thought it would be worth writing some tests for that functionality given that it's currently built in.

To clarify, I was speaking about expected formatting behavior, and not current/actual behavior of the proposed implementation. I recognize that there's a... control flow aspect to this given the divergent nature of the else block (certainly more so than other types of statement, item, etc. contexts), but I'm still iffy on whether or not we should tie control_brace_style to this

[fgsch]

Hi. Is there anything we outsiders can help with to get this moving forward?

[ytmimi]

Hi. Is there anything we outsiders can help with to get this moving forward?

@fgsch thanks for asking! I always appreciate feedback, so if anyone has thoughts on simplifying the implementation or adding more clarity to the code please let me know. Additionally, if anyone has ideas for test cases that haven't already been covered that would be a great help.

[calebcartwright]

Echo the appreciation. My assumption was that it was more of a bump to gauge status, so also want to note that this is one we'd discussed offline in chat/team meetings. I owned the action item to recap those discussions and remaining elements back here on the PR and obviously failed there, so apologies for the lack of transparency.

I do think we're just about sorted implementation wise, but also want to note that there will be at least one more implementation PR and then some release/communication work to follow before this actually gets into user's hands.

[camsteffen]

I think there's a missing case of

let Some(x) = opt else {
    return
}

Also I don't see any examples with long patterns after let. Probably good to add variation to the types of patterns in general.

[camsteffen]

In particular, struct patterns, tuples and arrays can raise some unique formatting concerns.

[ytmimi]

I think there's a missing case of

let Some(x) = opt else {
    return
}

I can add a test case for that!

Also I don't see any examples with long patterns after let

Now that you bring it up I should probably add one or two to make sure that we wrap the else block onto its own line if the pattern is multi-lined.

[B-Reif]

Another quick bump here :) Please let us outsiders know if we can be of help.

[calebcartwright]

Another quick bump here :) Please let us outsiders know if we can be of help.

Appreciate the offer, but there's nothing needed from the community nor anything the community can do to help (unless you happen to review the changes and think you've spotted an issue).

Similarly, there's no need for any continued bumps on the thread. This is not forgotten, this is currently our top priority. However, as I alluded to in #5690 (comment), there's a lot of moving parts required to get to a point where let-else support will be available in released versions of rustfmt, the majority of which are unrelated to this particular PR and which are not going to be reflected here.

It's intentional that we've not merged this PR yet, and this PR being merged vs. unmerged does not accurately portray status.

[calebcartwright]

@ytmimi we should be all set now on being able to merge, so if you could rebase whenever time permits that'd be most appreciated

[Oliboy50]

as described here, this should have been

    let Some(x) = opt else {
        // nope
        return
    };

without ; after return to make sure that a single expression is not formatted on a single line if it contains a comment