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

Layout/RescueEnsureAlignment incorrect behaviour when used as assignment #6918

Closed
rhys-vdw opened this issue Apr 11, 2019 · 12 comments
Closed

Comments

@rhys-vdw
Copy link

rhys-vdw commented Apr 11, 2019

Layout/RescueEnsureAlignment does not support assignment


Expected behavior

Should consider indentation difference from first non-whitespace character in line containing begin.

values = begin
  get_values_array
rescue
  []
end

Actual behavior

(for actual use case, example is simplified)

app/models/concerns/demographics.rb:21:5: C: Layout/RescueEnsureAlignment: rescue at 21, 4 is not aligned with begin at 19, 13.
    rescue ActiveRecord::StatementInvalid => error
    ^^^^^^

Steps to reproduce the problem

Follow example.

RuboCop version

Include the output of rubocop -V or bundle exec rubocop -V if using Bundler. Here's an example:

$ bundle exec rubocop -V
0.64.0 (using Parser 2.6.0.0, running on ruby 2.4.5 x86_64-darwin18)
@rhys-vdw
Copy link
Author

rhys-vdw commented Apr 11, 2019

Should probably behave in the same way as Layout/EndAlignment by offering the EnforcedStyleAlignWith: variable option.

@stale
Copy link

stale bot commented May 11, 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 11, 2019
@rhys-vdw
Copy link
Author

This bot makes no sense. Why are you closing legitimate issues? To create to appearance that the project is maintained? If you have lots of bugs and nobody's fixing them, then you should have a high issue count.

@stale stale bot removed the stale Issues that haven't been active in a while label May 15, 2019
@kepstin
Copy link

kepstin commented May 30, 2019

The alignment that the cop accepts here is hilariously bad :/

values = begin
  get_values_array
         rescue
           []
end

This seems related to #6774 and #6254 and appears to have been introduced in #6246 (the old behaviour of having rescue/ensure match end gave good results here)

@jakeonrails
Copy link

I have a similar example of bad output but not from assignment:

  coerce_input(lambda do |value, _ctx|
    URI(value)
  rescue URI::InvalidURIError
    nil
  end)

# Was autocorrected to:

  coerce_input(lambda do |value, _ctx|
    URI(value)
               rescue URI::InvalidURIError
                 nil
  end)

yelvert added a commit to ComplyMD/dotfiles that referenced this issue Jun 12, 2019
@Draiken
Copy link

Draiken commented Aug 1, 2019

I'm still seeing this. I agree with @rhys-vdw that it should have an option similar to the Layout/EndAlignment

@dvandersluis
Copy link
Member

Agree with the previous commentators, this is burning us too, even in 0.75.1.

@stale
Copy link

stale bot commented Apr 18, 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 Apr 18, 2020
@ekalinichev
Copy link

FYI issue is still there

@stale stale bot removed the stale Issues that haven't been active in a while label May 17, 2020
dylanahsmith added a commit to dylanahsmith/rubocop that referenced this issue Jul 2, 2020
@mvz
Copy link
Contributor

mvz commented Sep 26, 2020

There's now Layout/BeginEndAlignment which, when enabled, should make the examples given here be flagged and/or corrected correctly.

@ianfixes
Copy link
Contributor

ianfixes commented Nov 4, 2020

via #7531 (comment)

I'm curious if this fix extends to cases with memoize, where in rubocop 0.77.0 it prefers this:

  private memoize def do_something
    "Something"
                  rescue NameError
                    nil
  end

but without memoize, Rubocop acts sensibly and allows this:

  private def do_something
    "Something"
  rescue NameError
    nil
  end

@jonas054
Copy link
Collaborator

The originally reported expected behavior is now the actual behavior with default configuration. Closing this issue.

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

Successfully merging a pull request may close this issue.

9 participants