Add support for variable alignment to Layout/RescueEnsureAlignment

[dylanahsmith]

Fixes #6918

The Layout/RescueEnsureAlignment cop was always enforcing alignment of the rescue keyword with the begin keyword, even when the begin block was being assigned to a variable. For example, it would expect code like

result = begin
           something
         rescue
           puts 'error'
         end

but reject code aligning rescue with a variable like

result = begin
  something
rescue
  puts 'error'
end

even though a variable style is supported for similar cops, such as Layout/EndAlignment.

---

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.

[pariser]

🙏 thank you kindly

[pariser]

❤️

[pariser]

Nice to make this configurable

[bbatsov]

Why not simply EnforcedStyle?

[dylanahsmith]

It looked like the alignment cops were using EnforcedStyleAlignWith, so I was just trying to be consistent with them. Why not simply EnforcedStyle for Layout/BlockAlignment, Layout/DefEndAlignment, Layout/EndAlignment and Layout/RescueEnsureAlignment? I'm fine either way. Did you want me to change it to EnforcedStyle?

[ianfixes]

It looked like the alignment cops were using EnforcedStyleAlignWith

That is my experience/expectation as well, FWIW

[bbatsov]

I no longer remember why we did it like this in the past, but I definitely don't like it today. Probably it was to avoid styles named align_with_*, but I think that's fine.

[bbatsov]

Apart from my small remark the suggested changes seem reasonable to me.

[ianfixes]

I came here from #6918 ... Can I request a test case be added?

The code in question is:

  private memoize def do_something
    "Something"
  rescue NameError
    nil
  end

Rubocop 0.77.0 wants this:

  private memoize def do_something
    "Something"
                  rescue NameError
                    nil
  end

If I remove memoize, Rubocop acts sensibly and allows this:

  private def do_something
    "Something"
  rescue NameError
    nil
  end

Should I open a separate issue for supporting EnforcedStyleAlignWith: start_of_line configuration option for this cop, or is that a trivial addition to this PR?

[dylanahsmith]

It would make sense to add support for start_of_line as well, but perhaps that should be done in another PR to keep this one focused.

[Richard-Degenne]

Any input on the status of this PR?

[tdeo]

I'd love to see that merged, @bbatsov any final decision on EnforcedStyleAlignWith (used by Layout/BlockAlignment, Layout/DefEndAlignment, Layout/EndAlignment) vs EnforcedStyle ?

[bbatsov]

I'm closing this PR due to no recent activity. Feel free to re-open it if you ever come back to it.

[ianfixes]

I was under the impression that this PR was waiting for a response by @bbatsov to the following questions:

Did you want me to change EnforcedStyleAlignWith to EnforcedStyle? (#7531 (comment))

@bbatsov any final decision on EnforcedStyleAlignWith (used by Layout/BlockAlignment, Layout/DefEndAlignment, Layout/EndAlignment) vs EnforcedStyle ? (#7531 (comment))

[bbatsov]

Well, if that's the case - I prefer the use of EnforcedStyle align_with_x. @jonas054 If you recall why we didn't go with EnforcedStyle there, please do remind me.

[dylanahsmith]

As mentioned in the referenced issue (#6918 (comment)), I think this problem is addressed by the Layout/BeginEndAlignment cop with the EnforcedStyleAlignWith: start_of_line configuration to achieved the desired style mentioned in the PR description.

[bbatsov]

A problem that solves itself - that's my favourite kind. 😄 Thanks!

[jonas054]

Well, if that's the case - I prefer the use of EnforcedStyle align_with_x. @jonas054 If you recall why we didn't go with EnforcedStyle there, please do remind me.

No, I don't remember. I've found that we changed the name from AlignWith to EnforcedStyleAlignWith in #3765, but why we didn't go all the way to EnforcedStyle, I cannot say.