Enums and variants

[nrc]

Including struct variants. I'd like to discuss generics separately (#29).

[nrc]

This is pretty much covered in the guide: https://github.com/rust-lang-nursery/fmt-rfcs/blob/master/guide/guide.md#enums

I'd like to cover multi-line tuple variants, empty struct variants (are these even legal?), and clarify our position on short struct variants.

[nrc]

Proposal:

multi-line tuple variants

If a tuple variant will not fit on one line, each type should go on its own line, using block indent. In this case, you should use a trailing comma; e.g.,

enum Foo {
    Bar(
        Type1,
        Type2,
        Type3,
    ),
}

We recommend using struct variants, or a single stuct in a tuple variant, when variants get large like this.

empty struct and tuple variants

We recommend avoiding these. If you must use them, there should be no space between parentheses/braces or before the opening parenthesis, and a space before the opening brace. E.g.,

enum Foo {
    Bar(),
    Baz {},
}

struct variants

If a struct variant is 'short', it can be formatted on one line, e.g.,

Foo {
    Bar { x: i32 },
    Baz { x: u32, y: u32 },
    //    ^            ^ - indicates 'length of fields'.
}

We define short as having 3 or fewer fields and having a total length of fields less than 25 characters.

[joshtriplett]

@nrc If we want to recommend against empty struct and tuple variants, we should 1) give a reason, and 2) say what to do instead.

[nrc]

If we want to recommend against empty struct and tuple variants, we should 1) give a reason, and 2) say what to do instead.

1 - I don't see any reason why you should use them, they seem to only exist as a consequence of uniformity, rather than having any actual use case. Perhaps that is reason enough?
2 - Use a unit variant.

[solson]

They are most likely to be used by generated code, e.g. by macros. I also don't see why you would use them directly.

[joshtriplett]

@nrc I agree on both fronts; I'm just saying that the document should explain that, and mention using Variant instead of Variant() or Variant {}.

[strega-nil]

@joshtriplett I dunno if that's the style guide's job, seems more like @steveklabnik's :)

[joshtriplett]

@nrc The definition of "short" seems too prescriptive to me, and introducing a magic number like 25 seems strange; I'd rather define it as "put it on one line if it fits, or on multiple lines if it doesn't". As well as "put it on multiple lines if it has comments on each field".

[nrc]

We discussed this at the style meeting today, in particular we covered @joshtriplett's comment above in some detail. I proposed the following counter-example for why I believe we need the length limit:

Foo {
    Baz { field_foo: ALongishType, field_2_bar: ANormalSizeType, field_2_bar: ANormalSizeType },
    Baz { field_foo: ALongishType, field_2_bar: ANormalSizeType, field_2_bar: ANormalSizeType },
}

I find this hard to read.

We tried, mostly in vain, to find some underlying rules for why this feels bad, but the same field/types in an function declaration do not. I vaguely proposed that the analogy was wrong and we should compare to function bodies, instead of argument lists. Some possible, concrete reasons for why there is a difference: fields are the primary component of a struct/struct variant, but arguments are secondary to a function body. Or, there is more whitespace/indentation between/in function sigs c.f., struct variants.

We also tried to find examples beyond x/y/z or r/g/b, but could not. I maintain that these are good enough as examples (c.f., as function args) since it is reasonable to use them in struct variants.

[joshtriplett]

@nrc Thanks for your summary.

I personally don't find x/y/z and r/g/b very compelling, because in most cases I'd want those as tuple variants or top-level tuple structs, rather than struct variants. And in every other case I can think of, splitting each item to its own line seems fine.

I'd propose the following alternative for consideration:

• Allow a single field on the same line. For any multi-field struct variant, put each field on its own indented line. Never put two fields on the same line.

I also wouldn't object strongly to "allow multiple fields on the same line subject only to the line length limit", but I do find the readability argument against it reasonable.

[nrc]

@joshtriplett I think that we can disagree on how we would represent these simple data structures, but the fact that we have disagreement here means we should cope with the case well, even if we conclude that there are better designs.

I do believe that having a length heuristic is useful in other places too - struct literals seem to frequently be formatted better, and function calls also benefit. Given that we will probably want the length heuristic elsewhere (and thus, this case will not be an anomaly), I don't see a reason to avoid it here.

[joshtriplett]

@nrc While I'd still like to avoid introducing more magic numbers other than 99/100 and 4, if we do end up doing so, can I suggest that 1) we still do it in terms of overall line length limit, and 2) we make the length of a "short" line common for any other parts of the spec that want a number for "short" line length?

(That said, for function calls I'm much more inclined towards "only use the normal line length limit".)

[nrc]

@joshtriplett 2 is a very good point, and I agree. I have thus filed #47 to discuss the definition of 'short' and propose we move discussion there (and for this issue we just specify formatting for 'short' and not 'short', but not what 'short' actually means).

I'm not sure about making it relative to line length - I imagine that even if a person likes longer lines, that doesn't mean they will automatically prefer longer one-line struct variants.

[nrc]

nominating for discussion again. I'd like to FCP, but I'm not sure what to propose wrt 'short' lines

[solson]

I don't think there's anything necessary to propose for 'short' in this issue. I would use the term without defining it like you suggested above. Rustfmt can use some arbitrary good-enough measure until #47 is decided.

[joshtriplett]

I'd agree with @solson; however, I would suggest using the (not yet defined) term "simple" instead of "short". That leaves open the possibility of defining "simple" in terms of characters or in terms of some other complexity measure (e.g. "contains only field_name: SingleTokenTypeName pairs; must not contain any multi-token type name, any attribute, or make the line too long").

Personally, I would favor a heuristic like that over anything based on a character count. I'd write any number of r: u32, g: u32, b: u32, a: u32 on one line, but something with multi-token types (since the field names can't have any additional complexity) I'd write on multiple lines long before it hit the line length.

That rule would produce the following (or we could limit it to enum struct variants only and not standalone structs if you prefer):

struct Point { x: f64, y: f64, z: f64 }

struct Vec4 { x: f64, y: f64, z: f64, w: f64 }

enum Color {
    RGB { r: u32, g: u32, b: u32 },
    RGBA { r: u32, g: u32, b: u32, a: u32 },
}

struct NonTrivial<T> {
    thing: Foo<T>,
    len: usize,
}

Which I think roughly matches what we tend to do naturally.

[nrc]

I don't think using "short" vs "simple" matters too much, in particular I don't feel like calling it "short" constrains us to any particular definition. We can also come back and change the word we use later.

So, let us call this FCP and we'll define short/simple later.

[joshtriplett]

@nrc What do you think of the alternative measure I suggested for "simple" (or "short" if you prefer)? I think as a heuristic, it seems practical, and I think it produces good results for a variety of code. Can you think of any examples that you'd rather not see written using that heuristic?

Note that I'd have no objection to limiting the use of "simple"/"short" to enum struct variants and not standalone structs; I'd just like to know if that heuristic produces results you like or not.

Still really not a fan of the idea of using a character-count-based metric here. I could perhaps live with a token-count-based metric, though I think the complexity-based metric I posted in #31 (comment) will produce better results in practice.

[nrc]

What do you think of the alternative measure I suggested for "simple" (or "short" if you prefer)?

I think it is best discussed on #47, rather than here.

I think for this issue we should stick with just using "short" and say it is defined elsewhere.

[nrc]

We've changed our process: rather than requiring a full RFC, a PR to close this issue only requires making the changes discussed here to the style guide. You can see more details about the process in the readme for this repo.

If you'd like to help with this work, let me know, I'd be very happy to help you help us.

[nrc]

cc rust-lang/rustfmt#1809

Here the issue is that having a mix of single line and multi-line struct variants looks messy and I tend to agree. The alternative is always making structs multi-line (as we do with regular structs). The downside of that is that single line tuple variants and mutli-line struct variants might look messy together (but at least they look like different things). Also an enum with only single-line variants would take up more space (which suggests that we could apply the single-line struct thing when all struct variants meet the criteria, rather than consider each variant separately).

[porglezomp]

I just ran into this when running a format: a serious downside of making the breaking all-or-nothing is that you could have an enum with a lot of variants, and then adding a single new variant that's slightly too long causes every other variant to suddenly break, hitting them all with a useless diff.

I also agree that the length restriction is an annoying criteria for line breaking, since I have two variants that are exactly the same except one is msg: String and the other is character: char and the character variant gets broken across multiple lines all over the place. I'm considering renaming the field to char even though it conflicts with the type name just because the formatting with character is so annoying.

[nrc]

We discussed the struct variant/multi-line issue at the style team meeting today. We did not reach a conclusion. To summarise the discussion:

• We could not decide whether it was actually a problem or not - while there was some sympathy for the inconsistency, there was also some feeling that it was an acceptable formatting
• There was not much enthusiasm for an 'all variants' heuristic
• We could not agree whether we preferred all multi-line or 'all' single-line
• In favour of single line
  • when reading such enums, one often wants to compare variants with each other and this is easier with the single-line format
  • consistency with tuple and unit variants
• In favour of multi-lines:
  • enum decls are rare (c.f., expressions) and so conserving vertical space is less important
  • consistency with structs
  • better readability (declarations are read like docs)
  • encourages documentation

[ghost]

I vote for multi-lines. My reasoning can be summarized with:

// Good.
struct RGB(u32, u32, u32);
struct RGBA(u32, u32, u32, u32);

// Bad.
struct RGB { r: u32, g: u32, b: u32 }
struct RGBA { r: u32, g: u32, b: u32, a: u32 }

// Good.
enum Color {
    RGB(u32, u32, u32),
    RGBA(u32, u32, u32, u32),
}

// Bad.
enum Color {
    RGB { r: u32, g: u32, b: u32 },
    RGBA { r: u32, g: u32, b: u32, a: u32 },
}

// Good.
enum Color {
    RGB {
        r: u32,
        g: u32,
        b: u32,
    },
    RGBA {
        r: u32,
        g: u32,
        b: u32,
        a: u32,
    },
}

// Very bad.
enum Color {
    RGB { r: u32, g: u32, b: u32 },
    RGBA {
        r: u32,
        g: u32,
        b: u32,
        a: u32,
    },
}

Do we have any more real-world examples?

I feel like a lot of styles can be given a pass in these toy examples with single-letter names (r/g/b/a), single-word types (u32), and short names (RGB/RGBA). Things sort of get aligned and don't look too bad.

But with longer names, more variants, and more complex types, code can easily get misaligned and start looking messy.

[ghost]

Here's another real-world example.

Current rustfmt formatting:

enum Garbage {
    Destroy {
        object: *mut u8,
        size: usize,
        destroy: unsafe fn(*mut u8, usize),
    },
    Free { object: *mut u8, size: usize },
    Fn { f: Option<SendBoxFnOnce<(), ()>> }
}

Multi-line formatting:

enum Garbage {
    Destroy {
        object: *mut u8,
        size: usize,
        destroy: unsafe fn(*mut u8, usize),
    },
    Free {
        object: *mut u8,
        size: usize,
    },
    Fn {
        f: Option<SendBoxFnOnce<(), ()>>,
    },
}

[solson]

After seeing some examples and mulling over the pros listed in @nrc's comment, I'm leaning towards multi-line as @stjepang has illustrated above.

Particularly, I think documentation-like readability outweighs vertical compactness for type declarations, and moreover the multi-line style makes it natural to insert /// comments for fields.

[joshtriplett]

As long as this doesn't push us towards unconditional multi-line for struct/enum literal expressions, that seems fine to me.

[strega-nil]

I might be alone in writing @stjepang's example like the first one, with the mixture of multi-lines and single-lines, but I'd like to say there is at least one person following that style.