Skip to content

Delimited expressions should be allowed as last argument #149

Open
rust-lang/rust
#114764

Description

(this was originally a comment in #61)

I'm trying to dig up why the rustfmt overflow_delimited_expr shouldn't be enabled by default. And I found it derives from the following statement in the guide

Only where the multi-line sub-expression is a closure with an explicit block, this combining behaviour may be used where there are other arguments, as long as all the arguments and the first line of the closure fit on the first line, the closure is the last argument, and there is only one closure argument

History

I see in this comment @joshtriplett says

This should never apply if the outer construct contains more than just the single expression

// don't do this
let thing = Struct(first, OtherStruct(
    second,
    third,
));

// don't do this either
foo(thing, bar(
    param1,
    param2,
    param3,
));

In the next comment they suggest a possible exception for format macros

As a possible exception to the requirement that the outer construct must have only one argument, you can also apply this to an invocation of print!, println!, or format!, where the first argument consists of a format string and the second argument consists of a single expression:

println!("Thing: {}", generate_thing(
    long_argument_one,
    long_argument_two,
    long_argument_three,
));

AFAIK this did not make it into the guide

Then @nrc suggests an exception for closures as the last argument in this comment and @joshtriplett agrees

Re closures, I would like to restrict this to block closures, and in this case we allow any number of parameters as long as everything up to the { of the closure fits on one line (including inside the 'short' heuristic for function args), e.g.,:

foo(1 + 2, x, some_variable.bar(), |x| {
    closure_body();
    ret_value
})

This did make it into the guide

Then there's a lot of discussion about what control flow expressions should be allowed.

Then there's discussion about arrays where @kennytm brings up arrays and @joshtriplett agrees that it should be treated similarly.

However, there was never any discussion about allowing arrays, match, and other delimited expressions as the last argument.

Getting to the point

I don't see any reason why other multi-line delimited expressions shouldn't be allowed by default in the same context as closures. For instance, right now, rustfmt will format a match inside a closure and a match without a closure differently:

    // with a closure
    let f = fn_call(123, |cond| match cond {
        A => 1,
        B => 2,
    });
    // without a closure, not consistent
    let f = fn_call(
        123,
        match cond {
            A => 1,
            B => 2,
        },
    );
    // works fine with normal blocks though
    let f = fn_call(123, {
        let a = 1;
        let b = 2;
        a + b
    });
    // or without any other arguments
    let f = fn_call(match cond {
        A => 1,
        B => 2,
    });

That doesn't make any sense to me, and looks super ugly. Also, consider an append macro, used to push a list of elements to a Vec without copying or cloning:

     append!(v, [
        352732685652347985,
        348974392769832796759287,
        4738973482759382759832,
        178957349857932759834729857,
    ]);

Awesome for cleaning up dozens of lines of v.push(...) calls into an easily readable format. But rustfmt just kills me here (without enabling the overflow_delimited_expr option):

     append!(
        v,
        [
            352732685652347985,
            348974392769832796759287,
            4738973482759382759832,
            178957349857932759834729857,
        ]
    );

So my question is, why does this "last argument exception" not apply to match, arrays, vec![], etc? Seems inconsistent, yet alone ugly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions