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

FirstMethodArgumentLineBreak plays poorly with arrays and hashes #6493

Open
Epigene opened this issue Nov 17, 2018 · 11 comments
Open

FirstMethodArgumentLineBreak plays poorly with arrays and hashes #6493

Epigene opened this issue Nov 17, 2018 · 11 comments

Comments

@Epigene
Copy link

Epigene commented Nov 17, 2018

Maybe this is intended behavior, but it seems that Layout/FirstMethodArgumentLineBreak does nothing if the first argument is a Hash or Array literal.

Additionally, I'd like the closing parameter parenthesis to be aligned consistently, but Layout/ClosingParenthesisIndentation is no help in this (perhaps I need to use a different cop?).


Expected behavior

Given rubocop config:

AllCops:
  DisabledByDefault: true 
  UseCache: false
  TargetRubyVersion: 2.3.4

Layout/AlignArray:
  Enabled: true

Layout/ClosingParenthesisIndentation:
  Enabled: true

Layout/FirstMethodArgumentLineBreak:
  Enabled: true

Layout/IndentationWidth:
  Enabled: true
  Width: 2
 
Layout/MultilineMethodCallBraceLayout:
  Enabled: true
  EnforcedStyle: new_line

And offending (contrived) code:

begin
  begin
    sheet.add_row([
      a[0],
      a[1],
    ])
  end
end

begin
  begin
    sheet.add_row({
      k1: :v1,
      k2: :v2
    })
  end
end

I'd expect the autofixed code to be:

begin
  begin
    sheet.add_row(
      [
        a[0],
        a[1],
      ]
    )
  end
end

begin
  begin
    sheet.add_row(
      {
        k1: :v1,
        k2: :v2
      }
    )
  end
end

Actual behavior

begin
  begin
    sheet.add_row([
      a[0],
      a[1],
    ]
                 )
  end
end

begin
  begin
    sheet.add_row({
      k1: :v1,
      k2: :v2
    }
                 )
  end
end

RuboCop version

$ rubocop -V
0.60.0 (using Parser 2.5.3.0, running on ruby 2.3.4 x86_64-darwin17)
marcotc added a commit to marcotc/rubocop that referenced this issue Feb 22, 2019
marcotc added a commit to marcotc/rubocop that referenced this issue Mar 21, 2019
@athornton2012
Copy link

@marcotc we came across this issue today as well! It is the exact same as @Epigene where hashes and arrays are not getting new lines in method calls. Do you think that a fix will be released soon?
Thanks!

@stale
Copy link

stale bot commented May 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label May 8, 2019
@AlexWayfer
Copy link
Contributor

Additional example:

sheet.add_row(k1:
  v1)

@stale stale bot removed the stale Issues that haven't been active in a while label May 20, 2019
@lamont-granquist
Copy link

Dunno if this would be considered a separate issue but heredocs don't seem to be considered multiline either:

foo(<<~EOF
  here
EOF
   )

@allentsai
Copy link

I was writing a feature request, but found this issue already filed. I agree with above.

After adding

Layout/FirstMethodArgumentLineBreak:
  Enabled: true

Layout/FirstHashElementLineBreak:
  Enabled: true

Layout/ClosingParenthesisIndentation:
  Enabled: true

Layout/MultilineHashBraceLayout:
  Enabled: true
  EnforcedStyle: new_line

We end up with an autocorrection as follows.
Before:

  def public_api_as_json(options = {})
    as_json({
      only: [:id, :nature, :deleted, :token],
      methods: [:definition, :description, :label, :size],
    }.merge(options))
  end

After:

  def public_api_as_json(options = {})
    as_json({
      only: [:id, :nature, :deleted, :token],
      methods: [:definition, :description, :label, :size],
    }.merge(options)
           )
  end

I would have liked to end up with

  def public_api_as_json(options = {})
    as_json({
      only: [:id, :nature, :deleted, :token],
      methods: [:definition, :description, :label, :size],
    }.merge(options))
  end

or

  def public_api_as_json(options = {})
    as_json(
      {
        only: [:id, :nature, :deleted, :token],
        methods: [:definition, :description, :label, :size],
      }.merge(options)
    )
  end

@allentsai
Copy link

The cop might be somewhat broken, rubocop does not complain about:

        with(:get_object,
             custom_audience.fb_id,
             fields: request_fields
            ).

I would expect it to return:

        with(
          :get_object,
          custom_audience.fb_id,
          fields: request_fields
        ).

@stale
Copy link

stale bot commented Feb 12, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label Feb 12, 2020
@lamont-granquist
Copy link

still there AFAIK

@stale stale bot removed the stale Issues that haven't been active in a while label Feb 12, 2020
@stale
Copy link

stale bot commented Aug 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label Aug 11, 2020
@stale
Copy link

stale bot commented Nov 9, 2020

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

@stale stale bot closed this as completed Nov 9, 2020
@lamont-granquist
Copy link

confirmed bug still exists in rubocop 1.2.0

@koic koic reopened this Nov 9, 2020
@stale stale bot removed the stale Issues that haven't been active in a while label Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants