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

MultilineMethodCallIndentation false positive? #3468

Closed
jacob-carlborg opened this issue Sep 2, 2016 · 5 comments
Closed

MultilineMethodCallIndentation false positive? #3468

jacob-carlborg opened this issue Sep 2, 2016 · 5 comments
Labels

Comments

@jacob-carlborg
Copy link

jacob-carlborg commented Sep 2, 2016

When running RuboCop on a file with the following content:

def dependencies
  @dependencies ||= begin
    p 'foo'
    DEFAULT_DEPENDENCIES
      .reject { |e| e }
      .map { |e| e }
  end
end

I get the following offenses:

Inspecting 1 file
C

Offenses:

foo.rb:5:7: C: Align .reject with begin on line 2.
      .reject { |e| e }
      ^^^^^^^
foo.rb:6:7: C: Align .map with begin on line 2.
      .map { |e| e }
      ^^^^

Is that the expected behavior? Following what RuboCop suggests, I would need to write the code like this:

def dependencies
  @dependencies ||= begin
    p 'foo'
    DEFAULT_DEPENDENCIES
                    .reject { |e| e }
                    .map { |e| e }
  end
end

Formatting the code like that looks very strange to me. Is anyone actually writing code formatted like that?

0.42.0 (using Parser 2.3.1.2, running on ruby 2.1.5 x86_64-darwin15.0)

@maxjacobson
Copy link
Contributor

I think this is fixed in #3464 -- want to check and maybe review that fix?

@maxjacobson
Copy link
Contributor

Actually I misread, it's unrelated 😬. I think your concern can be fixed by changing the configuration to match your preference.

@jacob-carlborg
Copy link
Author

jacob-carlborg commented Sep 2, 2016

I just don't see how my code is related to the examples in the documentation: http://www.rubydoc.info/github/bbatsov/RuboCop/RuboCop/Cop/Style/MultilineMethodCallIndentation

I don't see why it would look a the begin keyword. It's on a completely separate line. The multiline method call starts at the line that begins with DEFAULT_DEPENDENCIES. I update the code example to make it even more clear.

@maxjacobson
Copy link
Contributor

I see what you're saying. Agreed, that looks like a bug in the output of the message. FWIW, the following config produces no offenses and I believe is more in line with your preference:

MultilineMethodCallIndentation:
  EnforcedStyle: indented

@jacob-carlborg
Copy link
Author

I don't agree, I think changing the style is a workaround. If I remove the begin block, then there are no complains without changing the style.

@jonas054 jonas054 added the bug label Oct 15, 2016
jonas054 added a commit to jonas054/rubocop that referenced this issue Oct 16, 2016
We already have special handling for a number of keywords when
it comes to multiline indentation/alignment. Add kwbegin to the
list.
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

3 participants