False positive for Style/TrailingComma cop

[sferik]

In RuboCop version 0.32.0, given the following configuration:

Style/TrailingComma:
  EnforcedStyleForMultiline: 'comma'

And the following code:

Foo.new({})

RuboCop generates the following offense:

Put a comma after the last parameter of a multiline method call.
Foo.new({})
        ^^

With autocorrect enabled, the code is changed to:

Foo.new({},)

Something is clearly wrong. This was not a problem in earlier versions of RuboCop.

[mattjmcnaughton]

I made a pull request with a possible fix.

Here's what I think is going on (although I might be wrong). Basically the cop interpreted the expression, correctly, as a method call. As can be seen in the code, because the last argument, the {} has no comma, and comma is the setting, an offense was reported.

I simply added a check to not report an offense if the first argument of the function is an empty hash, as can be seen here.

My only concern, as mentioned on the pull request is that my solution would create false negatives. For example, it would not report an offense for Foo.new({}, ... \n /// 5), when perhaps it should.

Thoughts? Let me know if there are any changes I can make to make it better!

[sferik]

The error message says:

Put a comma after the last parameter of a multiline method call.

IMHO, it should only apply to multiline method calls. For example:

Foo.new(
  "first arg",
  "second arg",
)

[mattjmcnaughton]

Hmm so maybe the problem is that Foo.new({}) should never have been called multiline in the first place. I'm guessing that has something to do with the new definition of multiline (seen here), although I may be totally wrong about that.

[sferik]

@philoserf This is not the default. I am overwriting the default, as I say in the first line of the description with the configuration:

Style/TrailingComma:
  EnforcedStyleForMultiline: 'comma'

This is just a bug. 🐛

Not a bloody nose. 🚫 💉 👃

[philoserf]

My miss. Comment retracted.

[mattjmcnaughton]

I updated my pull request off of @sferik's observation that this method should not have been labelled multiline? in the first place. Let me know if I can make any changes to improve it!

[jfelchner]

My setup is exactly like @sferik and I'm also getting this problem.

[denisdefreyne]

I’m running into the same exact issue.

Here’s another case where the cop gives incorrect output:

Foo.new(
  'blah blah',
  { owner: 'me', author: 'myself' },
  'more blah',
)

RuboCop reports:

stuff.rb:6:14: C: Avoid comma after the last parameter of a method call, unless each item is on its own line.
  'more blah',
             ^

This is the RuboCop configuration:

TrailingComma:
  EnforcedStyleForMultiline: comma