Trailing Comma needs to learn a 'only when at line end' style

[jfelchner]

I love using trailing commas, but there's a big difference between using trailing commas on code like this:

def foo
  {
    my: 'hash',
    is: 'amazing',
  }
end

And code like this:

def foo
  my_bar_method(
    id:     some_id,
    entry:  some_entry,)
end

I'm not going to "waste" a line on every method call that breaks multiple lines for the closing parenthesis, but in my mind, having the closing paren of the Hash line up makes more sense (and happens less frequently).

I'd love to see an option to TrailingComma which specifies that trailing commas are required only if they would be the last thing on the line.

[bbatsov]

I think we should be checking for a missing trailing comma only when there is one element per line and there's no closing paren on that line. The current implementation took a shortcut (my fault, since I recall suggesting to @jonas054 to simplify the implementation, without considering some implications). I guess we'll need to revisit this.

[jfelchner]

@bbatsov there's no way to catch all the edge cases on something like this. :)

[jfelchner]

@jonas054 I completely forgot to gittip you. That's now been resolved. 😄 Once again, you rock.

[jfelchner]

@jonas054 unfortunately this is still registering an offense even when I have the config set like so:

TrailingComma:
  EnforcedStyleForMultiline: 'comma'

False positive:

scout.throttle!(status:     'realtime',
                over_limit: true)

Any ideas?

[jfelchner]

@jonas054 I may be wrong, but it seems like params to a method call may need to be handled differently to just a regular hash since:

scout.throttle! status:     'realtime',
                over_limit: true

Cannot have a trailing ,, even though there's no ) on that last line, otherwise it results in a parse error.

[jfelchner]

Here's the failing spec to help you out:

    context 'when EnforcedStyleForMultiline is comma' do
      it 'accepts a method call with Hash as last parameter split on multiple lines' do
        inspect_source(cop, ['some_method(a: "b",',
                             '            c: "d")'])
        expect(cop.offenses).to be_empty
      end
    end

[bbatsov]

@jfelchner I think it'd be best if you opened a separate ticket for this.

[jfelchner]

np