7492 Add Style/TrailingCommaInBlockArgs

[ty-porter]

Fixes #7492. This PR adds new cop Style/TrailingCommaInBlockArgs for blocks with multiple arguments. Trailing commas are only required in blocks with one argument, else a trailing comma should be omitted.

---

Before submitting the PR make sure the following are checked:

• Wrote good commit messages.
• Commit message starts with [Fix #issue-number] (if the related issue exists).
• Feature branch is up-to-date with master (if not - rebase it).
• Squashed related commits together.
• Added tests.
• Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
• The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

[koic]

There are the following differences:

% ruby -wve "{foo: 1, bar: 2, baz: 3}.each {|key,| p key }"
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-darwin17]
:foo
:bar
:baz

% ruby -wve "{foo: 1, bar: 2, baz: 3}.each {|key| p key }"
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-darwin17]
[:foo, 1]
[:bar, 2]
[:baz, 3]

So, some are my thoughts.

• I think it's Style department, not Lint department
• The good and bad cases handled by this cop are incompatible and unsafe
• The trailing comma is an idiom that I sometimes see for me. I think disabled by default is preferable.

[ty-porter]

This is a lot of feedback to take in. Let me see if I can go back and make the required changes, or if not, pass along to someone who knows better than I.

[ty-porter]

@koic

• I think it's Style department, not Lint department

I didn't change this yet, but can do so if necessary.

• The good and bad cases handled by this cop are incompatible and unsafe

This shouldn't touch trailing commas that are required (for instance a trailing comma after a single argument), so it should preserve syntax in both cases.

• The trailing comma is an idiom that I sometimes see for me. I think disabled by default is preferable.

I disabled by default.

[bbatsov]

I think we need a better name for this cop, as right now it's too generic. Unfortunately I can't think of something that's not very long like RedundantCommaInBlockParameters.

[bbatsov]

This can potentially get a style guide entry as well.

[ty-porter]

I think we need a better name for this cop, as right now it's too generic. Unfortunately I can't think of something that's not very long like RedundantCommaInBlockParameters.

Just noticed as I was looking for some naming convention that a TrailingCommaInArguments cop also exists but handles redundant commas in method calls, so maybe something like TrailingCommaInBlockArgs?

[bbatsov]

Just noticed as I was looking for some naming convention that a TrailingCommaInArguments cop also exists but handles redundant commas in method calls, so maybe something like TrailingCommaInBlockArgs?

@pawptart Yeah, I think that's a good idea.

It seems also you'll have to do some rebasing looking at the list of merge conflicts. :-)

[ty-porter]

@pawptart Yeah, I think that's a good idea.

It seems also you'll have to do some rebasing looking at the list of merge conflicts. :-)

@bbatsov

Taken care of :)

[bbatsov]

Thanks!