Function argument/parameter indentation

[doy]

From the style guide:

• For multiline function signatures, each new line should align with the first parameter. Multiple parameters per line are permitted:

fn frobnicate(a: Bar, b: Bar,
              c: Bar, d: Bar)
              -> Bar {
    ...
}

fn foo<T: This,
       U: That>(
       a: Bar,
       b: Bar)
       -> Baz {
    ...
}

I really don’t think this is a good idea. It’s quite hard to skim the code when everything is indented differently, and it gets especially unreadable as function names get longer. It also makes diffs unnecessarily large, since you need to reindent pretty much every time you rename the function. As a general rule, I don’t think that variable leading indentation is a good idea, but especially so in this case. The guide has a question about whether this should also be allowed:

[OPEN] Do we also want to allow the following?

frobnicate(
    arg1,
    arg2,
    arg3)

This style could ease the conflict between line length and functions with many parameters (or long method chains).

I’m of the opinion that this should actually be the preferred formatting. What’s the reasoning behind the current guideline?

[tbu]

I actually like transferring the style of the {} braces to the standard parantheses (), like

foobar(
    arg1,
    arg2,
    arg3,
)

IIRC the Python PEP 8 explicitely allows this.

[cmr]

The second question is about function call, not declaration. Personally, I am very much in favor of the rule as it stands. All of the function arguments line up nicely, and they are at a different indentation level from the function body (I often have trouble in some of the older code in rustc where the arguments/return value run at the same indentation level as the function body, making it hard to scan where it starts.)

What would you prefer the frobnicate/foo examples look like?

[tbu]

The reason why I don’t particulary like the style that the next line starts next to the opening ( is that it sometimes conflicts with maximum line lengths. With functions definitions this might not become a problem, as there should be a maximum of 3-4 indentations (so up to 16 spaces).

However I think that this is a problem for function calls, so as to remain consistent with my suggestion (I think that function call and function definition should have similar style), I’d propose to write the functions as follows:

fn frobnicate(
    a: Bar, b: Bar,
    c: Bar, d: Bar,
) -> Bar {
    // ...
}

and

fn foo<T: This, U: That>(
    a: Bar,
    b: Bar,
) -> Baz {
    // ...
}

and for longer type parameters:

fn foo<
    T: Clone + Hash + Add + Eq,
    U: Copy + Send,
>(thing: &T) -> U {
    // ...
}

[cmr]

I find this incredibly ugly. My only real criticism is “why should function definition look like function call”? Later I’ll pull up some examples from rustc/servo and see how they would look under this.

[Almale]

Niko in [his “where clauses” RFC][1] quoted a real life code example from Rust source:

fn set_var_to_merged_bounds<T:Clone + InferStr + LatticeValue,                                 V:Clone+Eq+ToStr+Vid+UnifyVid<Bounds>>(                                 &self,                                 v_id: V,                                 a: &Bounds,                                 b: &Bounds,                                 rank: uint)                                 -> ures;

He admitted this is unreadable, but he blamed the unreadability on the absense of where clauses. However, the truth is this is unreadable because of the extreme effort in the code format style to save lines. By adding 3 new lines:

fn set_var_to_merged_bounds <                     T : Clone + InferStr + LatticeValue,                     V : Clone + Eq + ToStr + Vid + UnifyVid<Bounds>         > (             &self,             v_id : V,             a : &Bounds,             b : &Bounds,             rank : uint         )         -> ures;

By adding 2 more lines:

fn set_var_to_merged_bounds             <                 T : Clone + InferStr + LatticeValue,                 V : Clone + Eq + ToStr + Vid + UnifyVid<Bounds>             >             (                 &self,                 v_id : V,                 a : &Bounds,                 b : &Bounds,                 rank : uint             )             -> ures;

[1]: https://github.com/rust-lang/rfcs/pull/135

[doy]

For calling functions with a long argument list, I typically use @tbu’s style. For declarations, I’d probably go with something similar, although it’s less clear what the right answer is - something like this perhaps?

fn frobnicate(
    a: Bar, b: Bar, c: Bar, d: Bar
) -> Bar {
    ...
}

fn foo<
    T: This, U: That
>(
    a: Bar, b: Bar
) -> Bar {
    ...
}

or perhaps

fn frobnicate(
        a: Bar, b: Bar,
        c: Bar, d: Bar) -> Bar {
    ...
}

fn foo<
        T: This, U: That>(
        a: Bar, b: Bar) -> Bar {
    ...
}

I just know I have worked on codebases in the past that used the current style and it really falls apart in the edge cases, especially in a language like Rust where types can be quite long, with generics and closure types. How does the current style hold up when you have a function that already has a long name, and it needs to take parameters with long types, such that they don’t fit in the space available? For instance, from libserialize:

        fn read_enum_struct_variant_field<T>(&mut self,
                                             name: &str,
                                             idx: uint,
                                             f: |&mut Decoder<'doc>| -> DecodeResult<T>)
                                             -> DecodeResult<T> {

That already gets pretty close to running off the end of the page, and that was just in the first page of things I searched for.

[P1start]

I agree that the style that is currently in the style guide is not very good. It makes it very difficult to distinguish between type parameters and function arguments.

I much prefer this style, which is just a subtle change over the original:

fn foo<T: This,
       U: That>
      (a: Bar,
       b: Bar)
    -> Baz {
    ...
}

This makes it easier to see the difference between type parameters and normal parameters by placing the parenthesis in the ‘margin’ to act as a separator. It still retains the problem of having to re-indent everything each time the function name is updated, but function names are very rarely updated in any case.

I dislike the style of indenting the arguments by 4 spaces regardless of the length of the function. That places the function arguments on the same level as the function body, which can make it confusing to distinguish between the two (especially when there are no return values, or the return value appears on the same line). Example:

fn frobnicate(
    a: Bar, b: Bar,
    c: Bar, d: Bar) {
    a.bizbaz(c.to_foobar());
    d.frob(d).qux();
}

Placing the ) on its own (dedented) line does help, but it still can’t change the fact that the parameters and body are on the same line.

---

Niko’s example with my proposed style (and some other clean up):

fn set_var_to_merged_bounds<T: Clone + InferStr + LatticeValue,
                            V: Clone + Eq + ToStr + Vid
                              + UnifyVid<Bounds<T>>>
                           (&self,
                            v_id: V,
                            a: &Bounds<T>,
                            b: &Bounds<T>,
                            rank: uint)
                         -> ures;

Looking at that, all one has to do to see where the type parameters, arguments, and return value start is look at the whitespace ‘margin’ on the right.

[arcto]

Having the same indentation for parameters and function body is extremely confusing. They should at least be different.

[tbu]

Is this so even when there is an unintented line in between?

[Almale]

Parameters and body really should have different indentiation. I think noone argued differently. The OP suggested only that the opening braces should be followed by a line break.

[doy]

Yeah, I really don’t care how far the parameters are indented by, as long as it’s consistent (my second example used 8-space indents for each parameter line, for instance). Having an unindented line in between does help differentiate as well. I do agree that figuring out how to differentiate type parameters from normal parameters is still an issue, but it’s equally an issue with the current standard.

[o11c]

My rule is to always indent in multiples of 4 spaces. Indent once for every matched delimter you’re inside, plus once if you are forced to wrap in the middle of a line (does "not after a , or ;" cover all the cases?).

If you try to align, you get noise when refactoring.

[arcto]

+1 for line break after opening parenthesis, but only if the indentation is different to that of the body.

[arcto]

As for invocations, a line break after the opening parenthesis could be useful if the function name is long and/or the arguments are long. This keeps lines shorter and helps staving off line wraps as well as noise when refactoring. Good point, @o11c!

---

To emphasise the last point:

bar = get_bar(arg1,
              arg2,
              arg3,
              arg4);

The invocation above is in line with the current guidelines:

Multiline function invocations generally follow the same rule as for signatures.

However, changing the left hand side (bar) generates refactoring noise from the realignment the rest of the arguments. Renaming the function generates the same realignment noise for all invocations in the code base.

[erickt]

I use @tbu’s style for my personal code. I really enjoy the consistency. I tried to get this to be the style years ago, but I didn’t write bother to write a rustfmt so it was hard to impose my wishes on everyone else

Also, as the writer of that statement in libserialize. I hate it too. Fortunately my next version doesn’t suffer from huge lines like that.

[superlukas]

Just to recap what I said in #20, the style I use is:

frobnicate(arg1, arg2,
        arg3, arg4);

let really_really_really_long_name =
        "really really really long value too";

This obviously only concerns arguments and other line continuations, not parameters. I’m not sure where the style comes from, I think it made it’s way from C to C++.

LLVM uses this style and I’m pretty sure Google does so too.

---

Edit: And here is my take on function declarations:

fn some_func<T: This, U: That,
             V: SomeMore, W: EvenMore>
            (arg1: Type1, arg2: Type2, arg3: Type3,
             arg4: Type4, arg5: Type5)
            -> ReturnType {
    // ...
}

[phaylon]

I’ve personally settled on tbu’s approach above, having overlong contents of <...> and (...) in function definitions broken up and indented once (the same level of the function).

I do agree that it is very ugly. Though I also find it the most easily readable and (for me as newcomer possibly more important) scannable. It is easy to see what is what, even without syntax highlighting (for example as in diffs and other terminal output). I also appreciate the fact that it minimizes the lines that change in commits.

I have experimented with having the separator code >( for type-parameters to function-parameters half-indented (2 instead of 4 spaces). But that didn’t really make anything stand out more, and looked a bit out of place.

Is there a RFC or demonstration of the current proposed where clause syntax somewhere? I could imagine that cleaning things a bit up, since you can indent the where and align next to that. If the where part can’t change, there’s no risk of commits affecting lines they otherwise wouldn’t need to.

[rpjohnst]

+1 to @tbu’s style. As another data point, I don’t see it as ugly at all. I prefer it because it doesn’t require a lot of extra manipulation (whether manually or through a tool) to get things formatted “right” when you move code around.

On the other hand, I find styles that indent to non-multiples of 4 to be irritating, especially when refactoring or renaming things. They are one of the biggest reasons people use spaces- with indentation always at a multiple of 4 (or something else), tabs don’t cause nearly as many problems (mostly just online code).

[retep998]

+1 to @tbu as well I’ve been finding myself using this style a lot lately, and I love it.

    fn get_host_addresses(
        &mut self,
        host: Option<&str>,
        servname: Option<&str>,
        hint: Option<AddrinfoHint>
    ) -> IoResult<Vec<AddrinfoInfo>> {
        iocpabort!("nope")
    }

It makes it so much easier to work with the code in a text editor. If something is mass renamed I don’t have to hunt down each case and manually fix the indenting (and thus causing overly large diffs). And it plays nice with editors who can’t cope with the other tabbing scheme, like Atom.

On a somewhat related note, trying out this style revealed a bug with Rust’s parser: https://github.com/rust-lang/rust/issues/15887