rubocop: Bug encountered with `Layout/RescueEnsureAlignment` Cop and a memoized method

Expected behavior

Expected RuboCop to not flag the method.

Actual behavior

$ rubocop test.rb
Inspecting 1 file
C

Offenses:

test.rb:8:5: C: Layout/RescueEnsureAlignment: rescue at 8, 4 is not aligned with begin at 6, 13.
    rescue StandardError
    ^^^^^^

Steps to reproduce the problem

Create test.rb:

# frozen_string_literal: true

# Dummy Class
class Foo
  def bar
    @bar ||= begin
      expensive_method
    rescue StandardError
      fall_back
    end
  end
end

Run rubocop test.rb

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.59.0

Inference

Rubocop recommending to place rescue at position 13 in this example is a distasteful 🐛 /cc @rrosenblum

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 8
Comments: 19 (5 by maintainers)

Commits related to this issue

[Fix #6254] Fix RescueEnsureAlignment for non-local assignments — committed to marcotc/rubocop by marcotc 5 years ago
Merge pull request #6671 from marcotc/fix-raise [Fix #6254] Fix RescueEnsureAlignment for non-local assignments — committed to rubocop/rubocop by Drenmi 5 years ago

Most upvoted comments

Barring all personal opinion on what is readable and not, I labelled this as a feature request to add a configuration option to make indentation respect the assignment LHS like so:

@bar ||= begin
  expensive_method
rescue StandardError
  fall_back
end

when used as RHS of an assignment.

I added a separate bug report for the auto-correct problem in #6258.

Drenmi on Sep 10, 2018

I apologize for introducing this new bug. It was introduced while fixing another bug, #6246. I switched the check for alignment to be based on the keyword rather than end. Unfortunately, the cop was missing tests for several important scenarios.

I agree with @Drenmi and @bquorning on handling this with new configuration.

rrosenblum on Sep 11, 2018

@oehlschl I agree with you. My test case is failing with rubocop-0.64.0.

What’s worse is that the autocorrect result is not correct as well:

Original script

# frozen_string_literal: true

# Dummy Class
class Foo
  def bar
    @bar ||= begin
      expensive_method
    rescue StandardError
      fall_back
    end
  end
end

Running RuboCop

rubocop -V
# => 0.64.0 (using Parser 2.6.0.0, running on ruby 2.4.4 x64-mingw32)

rubocop test.rb --auto-correct
Inspecting 1 file
C

Offenses:

test.rb:8:5: C: [Corrected] Layout/RescueEnsureAlignment: rescue at 8, 4 is not aligned with begin at 6, 13.
    rescue StandardError
    ^^^^^^
test.rb:9:7: C: [Corrected] Layout/IndentationWidth: Use 2 (not -7) spaces for indentation.
      fall_back
      ^^^^^^^

1 file inspected, 2 offenses detected, 2 offenses corrected

Result

# frozen_string_literal: true

# Dummy Class
class Foo
  def bar
    @bar ||= begin
      expensive_method
             rescue StandardError
               fall_back
    end
  end
end

ashmaroli on Feb 16, 2019

I can’t get it to show what I want.

@bbatsov I understand that you’re recommending the following

# frozen_string_literal: true

# Dummy Class
class Foo
  def bar
    @bar ||= begin
               expensive_method
             rescue StandardError
               fall_back
             end
  end
end

but IMO, that just reduces the readability, IMO…

allow you to configure fixed indent with respect to the first line as well

I could not find any docs on how to configure this cop…

Why do you think this is a bug?

Because to me, the existing code is more readable / visually pleasing:

class Foo
  def bar
    @bar ||= begin
      expensive_method
    rescue StandardError
      fall_back
    end
  end
end

I would’ve even expected Rubocop to recommend aligning with the def column… but that’s incorrect as well…

ashmaroli on Sep 9, 2018