Trailing commas

[bbatsov]

I'd like us to add a cop for trailing commas. Code like this would be of interest to such a cop:

arr = [1, 2, 3,]

method(1, 2, 3,)

hash = { a: 1, b: 2, }

Obviously a trailing , in a method invocation is always a bad idea, while some people like to add trailing commas to array and hash literals to avoid having to add them in the future (or more precisely - to avoid forgetting to add them). Currently, however, the space after comma check we employ conflicts with a trailing comma check.

@jonas054 How about altering the existing cop and adding a new cop TrailingComma with two supported styles - Require and Forbid? Does this sound sensible to you?

[jonas054]

To me the difference is between multi line and single line. Whether it's a method parameter list or a literal; I can see how adding a comma after the last item makes sense. It's easier to add another item, and the diff (in git or similar) is smaller. I would still not make it the default choice, though.

Example:

  VERSION_NUMBERS = [
    VERSION_MAJOR = 3,
    VERSION_MINOR = 0,
    VERSION_BUILD = 3,
  ]

Can we always forbid trailing comma for single line, and make it configurable for multi line?

[bbatsov]

Good point about the multi-line literals. I guess if we're looking at a factory method with varargs trailing commas make sense there as well. Let's go with your suggestion - commas are always forbidden on a single line and configurable for multi-line (forbidden by default). We should also add this to the style guide itself.

[jonas054]

We've talked about the very clear cases of one line vs separate lines, but there are some in-between cases I need to discuss.

# 1. Opening bracket together with first value
VALUES = [1001,
          2020,
          3333,
         ]

# 2. Closing bracket together with last value
VALUES = [
          1001,
          2020,
          3333, ]

# 3. Values not on separate lines
VALUES = [
          1001, 2020,
          3333,
         ]

... and then various combinations of them.

The last value needs to be alone on its line for a trailing comma to make any sense, I think. Should we put any other restrictions on when a trailing comma is enforced?

[bbatsov]

I think a simpler approach would be check if the literal spans more than a line. If it spans more than a line it might make some sense to have a trailing comma (although the enforced style would be the inverse by default). In a single-line literal it's obviously pointless to have a trailing comma.

[jonas054]

OK, I'll make it simple then. Thanks.