Disable trailing comma by default

perlun commented 7 years ago

Introduction and background

When I ran rustfmt yesterday on some code I was writing, I was once again reminded by the fact that our current defaults are the following:

             where_trailing_comma <boolean> Default: false
                                  Put a trailing comma on where clauses
            struct_trailing_comma [Always|Never|Vertical] Default: Vertical
                                  If there is a trailing comma on structs
        struct_lit_trailing_comma [Always|Never|Vertical] Default: Vertical
                                  If there is a trailing comma on literal structs
              enum_trailing_comma <boolean> Default: true
                                  Put a trailing comma on enum declarations
       match_block_trailing_comma <boolean> Default: false
                                  Put a trailing comma after a block based match arm (non-block arms are not affected)
    match_wildcard_trailing_comma <boolean> Default: true
                                  Put a trailing comma after a wildcard arm

I know that some of this is ongoing discussion in https://github.com/rust-lang-nursery/fmt-rfcs/issues/34, since it specifically mentions the trailing comma in some situations:

The last match arm should have a comma if the body is not a block:

I want to take the chance before #34 is merged to broaden that discussion a bit more, and try to reach consensus on trailing commas in general. (I know that they are sometimes needed; I consider that a bug and this discussion is more related to the areas where it is syntactically optional, but a common de-facto standard in the Rust community and/or enforced by rustfmt.)

I don't claim to be unbiased; I have a specific opinion that I am voicing in this RFC. However, what I do claim is to represent the different standpoints in a factual way. If you feel that I am missing or misrepresenting some argument for either position, please write a comment with the missing part and I'll try to adjust the proposal continously.

Basic presumption

I hope that (almost) everyone can agree that our current position - to sometimes advocate a trailing comma and sometimes not, as shown above - is not optimal. It feels counterintuitive and also like something you'd have to teach people about ("the style guide recommends it here, but not there, for whatever reason" - not saying that there aren't reasons for the inconsistency, just saying that an inconsistent set of rules in an area is much harder to defend and also harder for people to remember.)

Arguments used to support the addition of trailing commas

It's commonly used within the rust community

This is a fact. Servo uses it, rustc uses it, probably a bunch of other projects as well. The precedent is there, there's not so much to say about that. There are probably a number of people who like this style, otherwise it wouldn't have been started to be used anyway. Moving away from this style would risk discouraging those people from contributing to the community.

Counter-argument

It's not that common outside the Rust community.
The current Rust code in existance will hopefully eventually be a very small minority of the cumulative amount of rust code in the years to come. In other words, the vast majority of the rust code eventually written does not yet exist.
Once rustfmt is stable enough, we should be able to reformat larger codebases (like servo and rustc) with it anyway.
Regarding the "people might like it" part, it's a fact regarding any part of the style guide. The style guide cannot emphasise individual people so much, but should rather focus on the greater good for the community as a whole.

It makes changing the code easier

This argument is also factually correct; when adding a new entry to an array, you never have to remember adding a trailing comma. Perhaps more importantly so, when rearranging items in an array you don't have to visually scan the array to find which line now has the missing comma (so you can add it and make the code compile again).

Counter-argument

While the argument is factually correct, it's pretty much a matter of what you are used to. Good tooling (i.e. instant feedback on the validity of the code, perhaps powered by rls or simiilar) can make this be more of a moot point. If the editor you are using instantly shows you (or hints at) where the missing comma is, this is less of a point as before. (but of course, not all people can be expected to use a single editor/a RLS-powered editor etc.)

It produces shorter, less verbose diffs

This argument is based on the simple fact that if the trailing comma is already present on the last line of an {array|enum|match|etc}, that line will not have to be edited if you add a new element to the {array|enum|match}. In other words, the diff produced when changing the code will only include the relevant line (the new functionality being added), you won't have to touch an adjacent line to make it compile.

This is a very relevant and factual argument, since it avoids the cognitive load put on the brain when parsing the diff. You can focus on the actual change, and have to think less about the noise.

Counter-argument

While this is a a factually correct assertion, @bbatsov (the author of Rubocop, a linter for Ruby), wrote that:

The defaults will not be changed [to enable trailing commas by default] - using VCS diff limitations to justify a coding convention doesn't make sense

Of course, here is where facts end and opinions start: I personally agree with Boris, that this is too weak an argument to justify the current position. Others might, and will, say the opposite (that this is such a strong argument that the current position should be retained).

It breaks git blame

This is actually a pretty good argument. It will appear as if the person who added the comma was the author of that line even though his/her contribution was only the single character. Can't really say much about this, this is a reasonable, factual argument. (However, it's again letting the limitations of a particular tool mandate the style choices)

Arguments used to reject the addition of trailing commas

Opinionated: It looks like a typo

One of my early encounters with the Rust community was actually because of this; I read some code in the Rust compiler source code which struck me as "looking weird", because of the trailing comma. I have been doing programming for about 20 years, of which 14 years of it has been "for a living". Never during these years have I encountered a community that was advocating trailing commas.

I'm quite certain I'm not alone with this experience (which the SO links further down in this RFC indicates; people tend to get confused when they read code written like this). In my eyes, the current position definitely is not "mainstream" enough for a language with high ambitions to become a broad competitor to languages like C, C++ and perhaps Java, C# and others as well. If that's really what we're aiming for, we should try to aim for a style that "feels natural" to most people reading code without having seen what the style guide advocates.

Our language is so great because of the other features it provides (immmutability, the whole borrowing concept, proper concurrency etc) that I think that is what should be sticking out when people read our code: our superiority in these areas. Not that we write code that looks like an array element is missing from the end; that someone has forgotten to finish his sentence. Again, extremely opinionated, but it makes people question the code: is this really what the author intended to write? Is there a bug here, did I just discover an unpleasant surprise?

It is contrary to the recommended style guide/linter defaults for other programming languages

A non-comprehensive on some of the findings I could gather:

rubocop, a linting tool for Ruby, produces warnings for this syntax with its default config. The issue is discussed more in https://github.com/bbatsov/rubocop/issues/713, https://github.com/bbatsov/rubocop/pull/735 and at length in https://github.com/bbatsov/ruby-style-guide/issues/273. Other style guides like the Thoughtbot one advocates trailing comma, but I'd say the former one is the more popular one (and definitely a more comprehensive guide with much more details about the recommendations).
eslint, a linting tool for Javascript, has it disabled by default but with options for enforcing it for various scenarios (multi-line only, always, etc).
tslint, a linter for Typescript, seems to take the approach to let you configure it either way, but don't seem to advocate a particular default. (tslint is not a reformatter in that sense, so it can take a more relaxed stance on the matter)
The PHP manual says that "[h]aving a trailing comma after the last defined array entry, while unusual, is a valid syntax."
It has been said to be common in Python, but I couldn't find a style guide to back this claim.

Other languages/style guides don't typically disallow it, but it's a common source for people to scratch their heads and wonder "why is this valid C#/Python/etc code?". Take a look at this and this link (Python), this link (C#), this link (Java) etc. It is definitely not a universally accepted style, recommended by a large majority of the style guides for programming languages in general or anything like that.

To put it bluntly: if we want to keep making people confused, we should keep the current recommendation as it is.

It is invalid code in some languages/environments:

JSON: Valid code

{
"foo": "bar",
"baz": "zot"
}

Invalid code, parsers will reject this:

{
"foo": "bar",
"baz": "zot",
}

Opinionated: It's not really intended for humans writing code

The fact that virtually all programming languages (except from JSON, which isn't really a "programming language") allows it seems to be generally motivated by the fact that it's intended for code generation, where it's obviously a whole lot easier for the code generating code to not have to always check "is this the last element of the array being printed to stdout" and so forth. C# 2005 Programmer's Reference (p. 208) seems to imply this, and this SO thread gives the same impression. It was initially allowed in C back from the days of K&R, and many other languages have included support for it since.

One could argue that if this "feature" is designed for code generators, let the code generators use it and let humans produce code that is more aesthetically pleasing and straightforward.

Conclusion and proposed adjustment

I find the communities for various programming languages a bit scattered on the subject. Some people prefer this style very strongly, others (like me) dislike it equally strongly and would never ever want to run a tool to reformat our code with the default settings as they are right now. Others are just confused by reading code formatted like this and start to ask questions about it. That, to me, indicates that our current defaults are not so intuitive and hence I suggest that the following settings should be the new default:

             where_trailing_comma <boolean> Default: false
                                  Put a trailing comma on where clauses
            struct_trailing_comma [Always|Never|Vertical] Default: Never
                                  If there is a trailing comma on structs
        struct_lit_trailing_comma [Always|Never|Vertical] Default: Never
                                  If there is a trailing comma on literal structs
              enum_trailing_comma <boolean> Default: false
                                  Put a trailing comma on enum declarations
       match_block_trailing_comma <boolean> Default: false
                                  Put a trailing comma after a block based match arm (non-block arms are not affected)
    match_wildcard_trailing_comma <boolean> Default: false
                                  Put a trailing comma after a wildcard arm

IMHO, this would be very consistent and easy to understand for anyone: experts and new Rust enthusiasts alike.

Now, you may send the expected flames... 😉

strega-nil commented 7 years ago

It's more consistent to have the trailing comma, and, imho, it's prettier. I hate that some languages cough C++ cough don't support it.

foo(
  x,
  y,
  z,
);

is much prettier than the alternatives

foo(
  x,
  y,
  z
)

or

foo(
  x,
  y,
  z)

In short, I completely disagree.

solson commented 7 years ago

There's a lot of text here, so I'll try to respond point-by-point.

our current position - to sometimes advocate a trailing comma and sometimes not, as shown above

There isn't a current position. Most of rustfmt's defaults haven't been decided yet, so it doesn't represent the community's preference. In my experience the current majority preference is for consistently using trailing commas everywhere.

It's not that common outside the Rust community.

I disagree, I've seen a lot of non-Rust codebases that use trailing commas when their languages allow it.

Good tooling (i.e. instant feedback on the validity of the code, perhaps powered by rls or simiilar) can make this be more of a moot point. If the editor you are using instantly shows you (or hints at) where the missing comma is, this is less of a point as before. (but of course, not all people can be expected to use a single editor/a RLS-powered editor etc.)

The "sufficiently smart tooling" argument is not a good one. Even when I have something like an RLS-powered editor, I get in situations where they're not available, like editing code on GitHub, in kdiff3, or basically anywhere outside my main editor on my personal computer.

VCS diff limitations

This is blaming tools for not being smart enough again. They never end up as smart as we'd like, so it's easier to keep things simple when we can manage it, so the tools don't need smarts.

However, it's again letting the limitations of a particular tool mandate the style choices

As we should, when it's an easy win.

It looks like a typo

Eh, it's easy to get used to. Maybe it helps to think of it like this: when lists are formatted in one line, comma acts as a separator, but when lists are formatted across lines, commas acts as a terminator. At this point it looks like a typo to me when you omit the trailing comma.

It is invalid code in some languages/environments

Because of this, JSON has always been a big pain for me to edit by hand. Adding a new item constantly requires backtracking to add commas to previous lines.

It's not really intended for humans writing code

Strongly, strongly disagree. If I was designing a "better JSON" intended for human editing, then comments and trailing commas would be step 1. And it's not just me, since both YAML and TOML support trailing commas.

petrochenkov commented 7 years ago

Regarding aesthetics of structs and similar cases, I don't see much difference between Rust's , and other field separators like ; used by C-like languages.

// Rust
struct S {
    field1: u8,
    field2: u8, // <- trailing separator
}

// C++
struct S {
    uint8_t field1;
    uint8_t field2; // <- mandatory trailing separator
};

using VCS diff limitations to justify a coding convention doesn't make sense

Blaming other tools doesn't make the problems go away.

nrc commented 7 years ago

My preference is for a trailing comma before a newline:

Foo {
    f: a,
    g: b,
}

foo(
    a: T,
    b: T,
)

// but

foo(a, b);
Foo { f: a, g: b };

I think this is just subjective preference, but the diff thing is an argument in favour

perlun commented 7 years ago

I disagree, I've seen a lot of non-Rust codebases that use trailing commas when their languages allow it.

It would be interesting to see some examples of this. Also, if there are other style guides out there (apart from the Thoughtbot one which I already linked) that advocates it, please let us know.

solson commented 7 years ago

@perlun I could try taking a look around later. Ultimately, that point is not very important, though. There is reason enough to use trailing commas in Rust without reference to external precedent.

steveklabnik commented 7 years ago

It would be interesting to see some examples of this.

First few random JS/Ruby ones:

bruno-medeiros commented 7 years ago

While the argument is factually correct, it's pretty much a matter of what you are used to. Good tooling (i.e. instant feedback on the validity of the code, perhaps powered by rls or simiilar) can make this be more of a moot point. If the editor you are using instantly shows you (or hints at) where the missing comma is, this is less of a point as before. (but of course, not all people can be expected to use a single editor/a RLS-powered editor etc.)

Counter-Counter-Argument: If the editor just shows where the error is, you still have to go there and fix it, so the editing operation is not as easy for the user as the trailing comma case. It would only be as easy if the editor automatically corrected your code - which would likely be extremely complicated, and IMO, not worth it.

bruno-medeiros commented 7 years ago

@perlun Ultimately it's a subjective thing: However, you can't generally claim that users familiar with other languages that don't support or recommend trailing commas would prefer things to remain that way. I've been programming in Java for nearly all my programming career, and every once in a while I get annoyed it doesn't support trailing commas, especially since I use block indentation a lot. (These annoyances came into realization after first learning the D language many years ago, which for me was the first language I learned that supported trailing commas)

My preference is for a trailing comma before a newline:

Agreed. It's worth noting that distinction regarding the newline.

joshtriplett commented 7 years ago

@bruno-medeiros Agreed. As an example, I attempted to get Haskell to support trailing commas a while back (with little success at that time).

est31 commented 7 years ago

When I came to Rust from C++, I instantly fell in love with trailing commas being possible, and being used. I always wanted to do this on C++ but never were able to. For me its one of the many things where Rust improves over C++ (not saying that Rust only improves over C++, but here clearly it does).

Trailing commas don't break git blame and git diff, and countering your counter-argument, I think VCS is an important part of the developer's workflow these days (do you know of any big software not inside a VCS?), so a language should be VCS friendly.

That being said, if trailing commas are to be removed, there is a good style that still is compatible with git diff:

let a = Struct { a: "a"
               , b: "b"
               , c: "c"
               , d: "d"
               };

Its used for example by the elm language. But note that I clearly prefer to keep trailing commas.

solson commented 7 years ago

@est31 The leading-comma style has the same problems as no-trailing-comma. It merely shifts the problem to the initial element. On top of that it introduces visual alignment which is also VCS-unfriendly, and per https://github.com/rust-lang-nursery/fmt-rfcs/issues/8 we generally prefer block indent.

est31 commented 7 years ago

Ah you are right. Indeed, the problem is still there. Its less bad though, as I think most times you add stuff to the end of a list, not the beginning.

perlun commented 7 years ago

Thanks to all (@solson, @est31 and others) for your feedback thus far. I am currently taking care of our newborn child so time for engaging further in the discussion is a bit sparse... ☺️

Just some quick comments for now; I intend to provide more feedback as soon as I can find some time. We are not speaking about "removing" trailing comma support here, but more discussing what should be the proposed "strong set of defaults" for the Rust programming language. Obviously, some people feel very strongly about the matter and I have great respect for that.

Personally, I also feel very strongly about it (which is kind of obvious since I spent hours writing a 3-page text about it..), but from the other POV. I really dislike it, the VCS argument has never ever been a problem for me (and like everyone else, I definitely use git and live my life using git diff all the time). Clearly, this is a matter of personal preference to a large degree, and what you are used to. I'm much more bothered by the unpleasant way it looks: it confused me, before I knew why it was done like this on purpose, and I'm quite sure it will confuse others new to this particular style as well. That's one of my key arguments against it.

It's a bit like the if (100 == bar) style which has sometimes been common in certain communities; looks weird at first glance, but once you realize why someone wrote it like that, it makes a lot more sense. But I still have never opted for that style either (there are other, better ways to deal with the error of assignment vs comparison), and I likely will never ever opt for the trailing comma style either. IMHO, the case for it is simply much weaker than the case against it. Now, not trying to shove this opinion down anyone's throat or anything; just stating very clearly that I don't like it and would likely configure any projects where I can to not use this style, regardless of what the final outcome of this discussion will be (which is one of the reasons why I find #33 to be important).

solson commented 7 years ago

I have not seen people confused by trailing commas aside from a trivial "oh, you can do that?" realization. There are a lot of things that are hard about learning Rust, but this doesn't seem like one of them.

Besides, every long-term Rust user spends a small time as a beginner and a long time as an experienced user, so I give a lot more weight to making editing pleasant than to avoiding minor surprises for beginners. In terms of the "language strangeness budget", the trailing comma is an extremely cheap thing to add, with real benefits.

Given the already common use of trailing commas in the community, I think they should be the default style.

Keats commented 7 years ago

Go forces you to put a trailing command for maps before newlines:

    commits := map[string]int{
        "rsc": 3711,
        "r":   2138,
        "gri": 1908,
        "adg": 912 // <- syntax error: unexpected semicolon or newline, expecting comma or } 
    }

Overall I agree with @nrc preferences and it matches the codestyle I've used in various codebases in Python.

scottmcm commented 7 years ago

FWIW, I find the Vertical setting to be more consistent.

For example, if you have this you have terminators on all the lines:

let mut x = MyStruct::default();
x.foo = 4;
x.bar = 13;

So I like having all the terminators in

let s = MyStruct {
    foo: 4,
    bar: 13,
};

Though MyStruct { foo: 4, bar: 13, } does look weird to me.

The VCS argument is compelling to me as well. I don't see getting away from line-based tooling any time soon, and especially for bug-fixes I prefer as small a diff as possible, even when it's something simple like commas.

perlun commented 7 years ago

@solson:

Besides, every long-term Rust user spends a small time as a beginner and a long time as an experienced user, so I give a lot more weight to making editing pleasant than to avoiding minor surprises for beginners.

Fair enough. It's just that not all of us find it pleasant to edit code where redundant, arbitrary characters have been added. 😜 In fact, some of us even find it unpleasant to look at and edit. This is really a matter of personal preference.

If we really really feel the comma should be there, I think using the approach mentioned by @keats as used by Go, enforcing it in the compiler would be the way to go. I mean: make it a compile-time error if it is missing. I would find it annoying for sure, but at least it would be very consistent and stringent. This would no longer be a "style argument" but rather a "grammar argument" for the basic syntax of the language.

@solson also wrote, much earlier:

Maybe it helps to think of it like this: when lists are formatted in one line, comma acts as a separator, but when lists are formatted across lines, commas acts as a terminator.

My main problem with this argument is that commas are really not terminators. Not in Rust, nor in C, C++, C# or any other language I can think of. Not in English either. You wouldn't write: "apples, pears, bananas,", you would write either "apples, pears and bananas" or "apples, pears, and bananas" (with or without "Oxford comma") 😉 But the point is: comma is not commonly perceived as a terminator. (and yes, I know that the significant difference here is the "one field per line" use case)

Maybe that's the key problem here, that we (the Rust language) sometimes use a comma when a semicolon would be more appropriate? I wouldn't fight this, it looks reasonably sane:

let s = MyStruct {
    foo: 4; // Semicolons as terminator for each individual field value.
    bar: 13;
};

I would also accept this:

let s = MyStruct {
    foo: 4 // No trailing separator.
    bar: 13
};

...but this would have the downside in that it would make whitespace be the separator, since you must be able to specify more complex field values as well as these simple cases. And we also have the other cases, like enums and arrays (where semicolon would work but be different from pretty much any other language on the face of the earth) and match arms (where semicolons would probably not be very trivial to support).

So, anyway, what I will simply have to do is to invent my own language on top of Rust, which transpiles semicolons into the "trailing comma on each line" syntax and everybody could be happy. :trollface: 😆

joshtriplett commented 7 years ago

@perlun Worth noting that English doesn't treat semicolons as terminators either; that convention comes from programming languages as well. (Some programming languages did use semicolons as separators rather than terminators; it worked very badly.) it's just a matter of what you're used to from programming languages.

solson commented 7 years ago

Fair enough. It's just that not all of us find it pleasant to edit code where redundant, arbitrary characters have been added. :stuck_out_tongue_winking_eye: In fact, some of us even find it unpleasant to look at and edit. This is really a matter of personal preference.

I can't quite agree with this. The trailing comma isn't redundant or arbitrary. It's a very intentional method for making editing operations easier and diffs cleaner. Whether you like to look at it or not is personal preference, but its utility is more objective than that, given that we have so many dumb line-based tools that aren't going away any time soon.

Ixrec commented 7 years ago

For some of us the trailing comma is also more pleasant to edit. In most programming languages I often find myself editing the end of a list of items which does not have a trailing comma, and inevitably I'll forget to move the comma at some point and get a syntax error. I've also experienced at least a dozen git rebases where the only conflict was two commits that both appended a new item to the same list, and which would not have conflicted at all were trailing commas in use.

Since this is a hard syntax error that takes a few seconds to fix rather than a subtle runtime error like use-after-free, I don't have any strong opinion on whether it should be the default or official style, but in my subjective anecdotal experience, trailing commas results in less of my time being wasted, so I would object to removing the feature.

perlun commented 7 years ago

@joshtriplett

Worth noting that English doesn't treat semicolons as terminators either; that convention comes from programming languages as well.

That, of course, is true.

@solson

I can't quite agree with this. The trailing comma isn't redundant or arbitrary. It's a very intentional method for making editing operations easier and diffs cleaner. Whether you like to look at it or not is personal preference, but its utility is more objective than that, given that we have so many dumb line-based tools that aren't going away any time soon.

Well, it is in fact redundant, since the compiler doesn't need it to produce a valid binary. It's also not needed for people who are used to working with code formatted this way; we simply find it to be noise.

Arbitrary: OK, I can agree that it's not. That I can give you, it's a syntacically useful thing added with a clear purpose, that a number of people feel is very valuable.

@Ixrec

I've also experienced at least a dozen git rebases where the only conflict was two commits that both appended a new item to the same list, and which would not have conflicted at all were trailing commas in use.

This is a good argument to support the trailing comma. Thanks for that.

jplatte commented 7 years ago

It's also not needed for people who are used to working with code formatted this way; we simply find it to be noise.

I am very much used to no trailing commas, as I can't have them in C++ AFAIK. But I was very happy to see Rust supporting the trailing comma style so the last line in multiline lists is consistent with the preceding ones. Also, it might be worth noting that basically everyone already adds redundant separators in the form of semicolons in CSS.

And one final note, about the look of trailing separators: I personally actually dislike the look of the C / C++ / Json style multiline lists, because the last line is always inconsistent with the other ones. I always go for YAML when it comes to loading human-written data, because having to think about whether or not to put a comma after a list entry, even though it's such a minor thing, annoys me every time.

perlun commented 7 years ago

I am very much used to no trailing commas, as I can't have them in C++ AFAIK.

I have heard others say the same, and I think the answer is "it depends". This is fully valid code:

enum some_enum {
    Foo,
    Bar,
    Baz,
};

int some_array[] = {
    1,
    2,
    3,
};

...whereas this is not (as @ubsan pointed out early in this thread):

int main() {
    printf("%d %d %d", 1, 2, 3,); // Gives the error "foo.cpp:14:32: error: expected expression" with my Apple LLVM version 8.0.0 (clang-800.0.42.1)
}

And one final note, about the look of trailing separators: I personally actually dislike the look of the C / C++ / Json style multiline lists, because the last line is always inconsistent with the other ones. I always go for YAML when it comes to loading human-written data, because having to think about whether or not to put a comma after a list entry, even though it's such a minor thing, annoys me every time.

That's fine, and I don't claim that everyone using "non-trailing comma" is happy about that; sometimes they are limited by the languages they are using, but would prefer to not be. Sorry if I made it sound like that.

However, my point that there are certainly those of us who like to not add the trailing comma, who feels that the "editing argument" and the "VCS argument" is not compelling enough. We like the "traditional" way, with no extra comma at the end, and feel that the perceived "extra work" it causes isn't really a problem for us.

I think it all boils down to personal preference at the end, just like Linus Torvalds once said it:

Coding style is very personal, and I won't force my views on anybody

(quoting from linux/CodingStyle)

est31 commented 7 years ago

@perlun this is only a discussion about what the default should be, rustfmt will (most likely) be customizable. So you can set it to some value you like. I too don't like some of rustfmt's choices, but I know that I can work towards adding an option in rustfmt for them.

killercup commented 7 years ago

I have not read all comments made here, but I think one pro-trailing-comma point is missing:

You can more easily copy-paste/comment out lines of code.

Maybe I write code in a weird way, but copying/commenting out struct fields, where clauses, match arms, etc. is something I often do when fixing/refactoring stuff. (I also use trailing commas in other languages and so far it has never made my code worse to work with.)

nrc commented 7 years ago

If we really really feel the comma should be there, I think using the approach mentioned by @keats as used by Go, enforcing it in the compiler would be the way to go. I mean: make it a compile-time error if it is missing. I would find it annoying for sure, but at least it would be very consistent and stringent. This would no longer be a "style argument" but rather a "grammar argument" for the basic syntax of the language.

This approach has been somewhat rejected in Rust - generally, we try to be fairly flexible in the compiler (except where it matters, of course) and allow the user to opt-in to stricter checks using external tools such as lints or rustfmt.

perlun commented 7 years ago

@nrc

This approach has been somewhat rejected in Rust - generally, we try to be fairly flexible in the compiler (except where it matters, of course) and allow the user to opt-in to stricter checks using external tools such as lints or rustfmt.

That's nice, I like that approach. Being dogmatic with things that are really subjective (like this specific thing) can have adverse effects, like them choosing some other tool, language or platform instead. I think one reason for the success of C and C++ has been that they are quite relaxed in terms of style. (Then of course, the downside of that is that there is a plethora of various indentation styles etc over there...)

Java and C# has the advantage of a reasonable de-facto standard for style choices, where many (most) people follow the suggested style.

Back to this specific issue: should the Rust Style Guide and/or rustfmt recommend/enforce a particular choice (like "enforce trailing comma in vertical cases" which seems to be a fairly common choice among people commenting on this issue)? Or should it be one of the cases where the guide & tool is agnostic? (Please motivate either position.)

scottmcm commented 7 years ago

@perlun It should, so that people don't have to care what they type, and refactoring/codegen tools can output whatever, trusting that they'll get normalized.

nrc commented 7 years ago

There's been no new discussion here for a while, so I'm putting this into FCP. I propose that we adopt the guiding principle of using a trailing comma if it is followed by a newline (see, e.g, https://github.com/rust-lang-nursery/fmt-rfcs/issues/42#issuecomment-263742813 and https://github.com/rust-lang-nursery/fmt-rfcs/issues/42#issuecomment-263738648). I feel like this should be formally handled on a case by case basis, so there is no need for a PR here. But we could add this to the guiding principles in README.md, if someone wanted to do that.

perlun commented 7 years ago

I feel like this should be formally handled on a case by case basis, so there is no need for a PR here. But we could add this to the guiding principles in README.md, if someone wanted to do that.

Would this be clear enough? I mean: don't we need a recommendation to guide the functionality of rustfmt (and other "Rust formatting guidelines" compliant formatters, should other similar tools eventually arise.

I am currently shifting from my initial position of "remove all trailing comma" to the more pragmatic approach of "allow trailing comma, but don't enforce it". I know that many people in the Rust community are against removing it, and I respect that: the discussion in this issue has clearly shown that removing it would make a load of people annoyed, since they find it valuable. I understand their points and that's fine.

Alas, I would still dislike the default Rust formatting "forcing" me to use this particular format for anything that I would contribute to a "standard" Rust project, per #33:

All official Rust projects which use Rustfmt must use the default style and must not include a customisation file.

In other words: to get a potentially useful project that me or someone else would write to become an "official" Rust project, we would need to adopt the default Rust formatting in full. No compromise. I find that a good thing, but it makes it even more important what choices we make in areas like this.

The "trailing comma or not" is not an important enough issue to take a strong standpoint on, IMHO. It's better to leave it open for per-project customization; it's really nowhere near as important as other things like #1 (Line and indent width), so "strong force" is unjustified in this case. We should pick our battles. Various projects with different line length and indent width would be very annoying to have to switch between. Projects with and without trailing commas would be much less annoying; it's really a quite minor thing by comparison.

joshtriplett commented 7 years ago

@perlun Consistency is important, especially for official Rust projects. Making it "allowed but not enforced" would mean that code in an official Rust project doesn't consistently use it; some code would use it, and some code wouldn't.

I don't actually think rustfmt should have an option for "allow but don't enforce"; it should have an option for "use trailing commas", either enabled or disabled. For your own projects, you can disable it, and rustfmt will consistently not use trailing commas.

scottmcm commented 7 years ago

@perlun To me, rustfmt by default normalizing all the optional syntax choices is more important than any individual syntax choice.

(I have no strong opinions about what options exist. I've used Resharper tooling that reformats, like forcing this., and have run it with various things off. That may be less important if the defaults become broadly accepted.)

nrc commented 7 years ago

Would this be clear enough? I mean: don't we need a recommendation to guide the functionality of rustfmt (and other "Rust formatting guidelines" compliant formatters, should other similar tools eventually arise.

Individual style RFCs would have a specific recommendation that would specify how formatters should work.

danielpclark commented 7 years ago

I found the trailing comma odd at first. But after reading some of the initial points here I really like it for one reason: code writing code.

It was never something that bothered me. It grew on me.

Mange commented 7 years ago

For me, it's very common to add a line to a list, then select the entire list and sort the lines of it. If I wouldn't use trailing commas I would then have to scan the entire list to find the missing comma, add it, and then go to the last line to remove it from there. Granted, this is only a problem if it's the first time I sort this list, or if the item I added ended up last in the list, but then that's almost worse since I still need to make sure if this was the case or not every time.

I understand that this is just input for the defaults, but I think it's a very sane default that is more common in scripting languages as it's more commonly supported there. Maybe this is why it looks strange for someone coming from an ~older~ more experienced language. I take it that using underscore in numbers (100_250_000) would be equally confusing the first time for these developers?

JS style guides have a tendency to reject trailing commas, but only because it's invalid in IE8 and earlier. JS projects that work using transpilers and minifiers are more prone to having them in my experience.

I'm also under the impression that the rubocop rules are commonly rejected and that the BDFL of that project uses uncommon defaults that he prefers or were more common a long time ago. From what I've been able to tell, Thoughtbot's style is more common as its default for their Hound CI service that a lot of Ruby projects use.

nrc commented 7 years ago

We discussed this in the meeting today, specifically, how to preserve the decision here for posterity. We decided that we should take a PR which mentions this as part of a 'guidelines'/rationale section in the style guide.

tomprince commented 7 years ago

As a note, the python style guide (PEP8) specifically suggests the same position adopted by rust: That there be a trailing comma, when it is followed by a new line.

craftytrickster commented 7 years ago

Is this issue still open for debate? I really believe that requiring a trailing comma by default is the wrong decision. I do not mind opening up an issue in a different location, etc if desired/required.

A comma in a list implies that an item will be followed by said comma. For example, if one views [1,2, .... after seeing the comma, it is natural for them to assume that content will come afterwards.

It seems incorrect to require an erroneous (or in the very least, completely unnecessary) trailing comma in these situations to be part of the formatting rules as default. I understand that some people do have a preference of adding the unnecessary comma and I respect their preference, but I strongly believe that it is a mistake to force this upon everyone. Essentially, a trailing comma to an indicator that maybe perhaps some additional content could theoretically appear here in the future, but it is not guaranteed. Given how loose that guarantee is, why would it be considered the default? It just seems to add syntactic noise for no benefit.

joshtriplett commented 7 years ago

@craftytrickster In a certain sense, any issue is always theoretically open for new arguments, but this issue isn't open to further debate on the existing arguments that have already been considered. If you have a new point that hasn't previously been raised, please by all means raise it.

This is something where we understand that some people's mental parsers will (hopefully only temporarily) stumble on the trailing comma, but we believe that's something that people will get used to, in exchange for the various benefits already discussed elsewhere. It's common in a variety of languages, and we intend that Rust be one of them.

That said, I also believe it would be worthwhile for us to explicitly explain this, and the reasons for it, in Rust documentation somewhere, rather than assuming that people will simply pick it up by osmosis.

craftytrickster commented 7 years ago

@joshtriplett thank you for the explanation. I think that is a fair process, regardless of my opinion of the outcome.

perlun commented 7 years ago

@joshtriplett Would you or anyone else have a chance to add a list of languages/linters where the trailing comma is recommended by default? It would be valuable to me and this discussion. I think some of it has been mentioned already, but it would be super if someone could make a bullet list of it.

joshtriplett commented 7 years ago

@perlun C, in many contexts. Python, in PEP 8.

chriskrycho commented 7 years ago

Also a fair number of modern JS styles are defaulting that way. It's one of the common configuration options in ESLint, TSLint, and Prettier.

Mange commented 7 years ago

Go requires a trailing comma in some cases. Yes, the compiler regards it as a syntax error otherwise, that's how hard they enforce this style.

perlun commented 7 years ago

So the bullet list of other languages/environments advocating a trailing comma is for now:

C, in many context (which ones? Sorry to be annoying, but as many "hard facts" as possible here is best)
Python, in PEP 8
"A fair number of modern JS styles" (again, hard facts would be useful here - ESLint actually goes the other way, having it disabled by default. Please provide links to JS styles advocating its usage, for completeness. ~TSLint has a setting for it, but it's not enforced by the default as is suggested in this thread.~ It's enabled in the tslint:recommended ruleset: https://github.com/palantir/tslint/blob/master/src/configs/recommended.ts#L156-L161)
Go enforces it for certain scenarios.

Yes, the compiler regards it as a syntax error otherwise, that's how hard they enforce this style.

I don't think that's as crazy at it seems. If we really think that advocating this should be the default (which I'm already stated that I'm against, for reasons already discussed and which closely resembles those shared by @craftytrickster), making the compiler yell about it is at least extremely consistent. You cannot even compile lint-erroneous code in that case, which is one perfectly valid standpoint - it will at least make everyone abide by the linting rules. 😉

(I am not saying that I advocate making this a compiler error, I am just saying it is at least very consistent. If we would start incorporating rustfmt rules into the compiler, I think such things should be a --strict flag or something that can be turned on and off, ideally per linting rule. But it also adds significant complexity to the compiler I guess, so it might not be the best idea to implement/suggest at the moment.)

simonwep commented 4 years ago

I'm a bit confused, if , should act as separator then I would expect after each , another values as this is the meaning of a separator. I I'd write (a, b, ) then technically a value after b, is missing since the comma after it doesn't separate anything.

Compared with ; which is commonly used to terminate statements:

let a = 100; 
let b = 200;

whereby , is used to separate stuff, e.g. it should be put between expressions:

let tupel = (a, b, c, d)

The comma is, in linguistics also used as separator and regarding programming languages it's described as separator:

The use of the comma token as an operator is distinct from its use in function calls and definitions, variable declarations, enum declarations, and similar constructs, where it acts as a separator.

IMO The semicolon acts most of the time as statement-terminator, but could be a separator (as seen in loops) but the semicolon had always the meaning of separating values where trailing commas wouldn't be allowed and wont make any sense since they won't separate anymore.

joshtriplett commented 4 years ago

@Simonwep This isn't really the right place for further help on formatting/style questions; if you'd like to ask questions about Rust style, users.rust-lang.org is probably a better place.

I would suggest not thinking of , as strictly a separator, or alternatively, thinking of it as separating a previous expression from a possible subsequent expression.

Please see the history of this thread and the resulting documentation for the various benefits of using trailing commas, and the tradeoffs we considered.

sweihub commented 1 year ago

Hi, I am looking for a solution to remove the trailing comma of function arguments? It bothers me much, can anyone help? I don't find any related settings.

fn on_rsp_user_login(
    &mut self,
    pRspUserLogin: *mut CThostFtdcRspUserLoginField,
    pRspInfo: *mut CThostFtdcRspInfoField,
    nRequestID: ::std::os::raw::c_int,
    bIsLast: bool,     <==== HERE, I DON'T WHAT THIS COMMA!
) {
    debug!("user login");
    self.tx.send(Event::UserLogin).unwrap();
}

perlun commented 1 year ago

@sweihub If I interpret https://github.com/datafuselabs/databend/pull/613 correctly, this was the behavior of some previous rustfmt version. The current version does not use a trailing comma in such cases. (caveat: not currently using Rust myself, others can verify or falsify this accordingly)

rust-lang / style-team