Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Style/TrailingCommaInArguments false positive with interpretation of multi-lined arguments? #3586

Closed
brandonweiss opened this issue Oct 7, 2016 · 2 comments
Labels

Comments

@brandonweiss
Copy link
Contributor

If I set this configuration:

Style/TrailingCommaInArguments:
  Enabled: true,
  EnforcedStyleForMultiline: consistent_comma

Then it enforces this code should have a comma like this:

EmailWorker.perform_async({
  subject: "hey there",
  email: "foo@bar.com"
},)

This doesn't seem right to me? It's a nuanced difference, but I don't consider this multi-lined arguments—the argument actually starts on the same line as the parentheses. The argument itself is multi-lined for readability, but the arguments aren't multi-lined. This is what I think multi-lined arguments look like:

EmailWorker.perform_async(
  "foo",
  "bar,
)

Or in the context of a hash:

EmailWorker.perform_async(
  {
    subject: "hey there",
    email: "foo@bar.com"
  },
)

Thoughts?

@brandonweiss brandonweiss changed the title Style/TrailingCommaInArguments false positive Style/TrailingCommaInArguments false positive with interpretation of multi-lined arguments? Oct 7, 2016
@jonas054
Copy link
Collaborator

I agree. The purpose of the trailing comma cops is to make it possible to enforce a style that minimizes the diff when another element is added at the end. And the comma in your example is useless for that purpose.

In this code from lib/rubocop/cop/mixin/trailing_comma.rb, we see that the intentions are correct, but the implementation is wrong.

      # Returns true if the round/square/curly brackets of the given node are
      # on different lines, and each item within is on its own line, and the
      # closing bracket is on its own line.
      def multiline?(node)

The closing bracket, ) in your case, is not on its own line.

I noted BTW that you also need the following configuration to reproduce the problem:

Style/BracesAroundHashParameters:
  EnforcedStyle: braces

@jonas054 jonas054 added the bug label Nov 13, 2016
jonas054 added a commit to jonas054/rubocop that referenced this issue Nov 12, 2018
The method TrailingComma#multiline? should not return true
for a single argument (possibly spanning multiple lines)
followed by a closing bracket that's not on its own line.
@brandonweiss
Copy link
Contributor Author

@jonas054 ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants