[Fix #6202] Modify RescueEnsureAlignment to be based on the beginning

position rather than the end position
rubocop · Sep 5, 2018 · 766213d · 766213d
1 parent e712c8c
commit 766213d
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 24 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -31,6 +31,7 @@
 * [#6196](https://github.com/rubocop-hq/rubocop/issues/6196): Fix incorrect autocorrect for `Style/EmptyCaseCondition` when using `return` in `when` clause and assigning the return value of `case`. ([@koic][])
 * [#6142](https://github.com/rubocop-hq/rubocop/issues/6142): Ignore keyword arguments in `Rails/Delegate`. ([@sunny][])
 * [#6240](https://github.com/rubocop-hq/rubocop/issues/6240): Fix an auto-correct error for `Style/WordArray` when setting `EnforcedStyle: brackets` and using string interpolation in `%W` literal. ([@koic][])
+* [#6202](https://github.com/rubocop-hq/rubocop/issues/6202): Fix infinite loop when auto-correcting `Lint/RescueEnsureAlignment` when `end` is misaligned. The alignment and message are now based on the beginning position rather than the `end` position. ([@rrosenblum][])
 
 ### Changes
 

diff --git a/lib/rubocop/cop/layout/rescue_ensure_alignment.rb b/lib/rubocop/cop/layout/rescue_ensure_alignment.rb
@@ -25,8 +25,10 @@ class RescueEnsureAlignment < Cop
         include RangeHelp
 
         MSG = '`%<kw_loc>s` at %<kw_loc_line>d, %<kw_loc_column>d is not ' \
-              'aligned with `end` at %<end_loc_line>d, %<end_loc_column>d.'
-              .freeze
+              'aligned with `%<beginning>s` at ' \
+              '%<begin_loc_line>d, %<begin_loc_column>d.'.freeze
+        ANCESTOR_TYPES = %i[kwbegin def defs class module].freeze
+        RUBY_2_5_ANCESTOR_TYPES = (ANCESTOR_TYPES + [:block]).freeze
 
         def on_resbody(node)
           check(node) unless modifier?(node)
@@ -56,24 +58,39 @@ def autocorrect(node)
         private
 
         def check(node)
-          end_loc = ancestor_node(node).loc.end
+          ancestor_node = ancestor_node(node)
+          alignment_loc = ancestor_node.loc.expression
           kw_loc = node.loc.keyword
 
-          return if end_loc.column == kw_loc.column
-          return if end_loc.line == kw_loc.line
+          return if alignment_loc.column == kw_loc.column
+          return if alignment_loc.line == kw_loc.line
 
           add_offense(node,
                       location: kw_loc,
-                      message: format_message(kw_loc, end_loc))
+                      message: format_message(kw_loc,
+                                              alignment_loc,
+                                              ancestor_node))
         end
 
-        def format_message(kw_loc, end_loc)
+        def format_message(kw_loc, alignment_loc, ancestor_node)
           format(MSG,
                  kw_loc: kw_loc.source,
                  kw_loc_line: kw_loc.line,
                  kw_loc_column: kw_loc.column,
-                 end_loc_line: end_loc.line,
-                 end_loc_column: end_loc.column)
+                 beginning: alignment_source(ancestor_node),
+                 begin_loc_line: alignment_loc.line,
+                 begin_loc_column: alignment_loc.column)
+        end
+
+        def alignment_source(node)
+          ending = case node.type
+                   when :def, :defs, :class, :module
+                     node.loc.name
+                   when :block, :kwbegin
+                     node.loc.begin
+                   end
+
+          range_between(node.loc.expression.begin_pos, ending.end_pos).source
         end
 
         def modifier?(node)
@@ -90,8 +107,12 @@ def whitespace_range(node)
         end
 
         def ancestor_node(node)
-          types = %i[kwbegin def defs class module]
-          types << :block if target_ruby_version >= 2.5
+          types = if target_ruby_version >= 2.5
+                    RUBY_2_5_ANCESTOR_TYPES
+                  else
+                    ANCESTOR_TYPES
+                  end
+
           node.each_ancestor(*types).first
         end
       end

diff --git a/spec/rubocop/cli/cli_autocorrect_spec.rb b/spec/rubocop/cli/cli_autocorrect_spec.rb
@@ -645,8 +645,8 @@ def method
       def func
         foo
         bar
-        rescue StandardError
-          baz
+      rescue StandardError
+        baz
         end
 
       def func

diff --git a/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb b/spec/rubocop/cop/layout/rescue_ensure_alignment_spec.rb
@@ -13,7 +13,7 @@
         begin
           something
             rescue
-            ^^^^^^ `rescue` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `rescue` at 3, 4 is not aligned with `begin` at 1, 0.
             error
         end
       RUBY
@@ -44,7 +44,7 @@
         def test
           something
             rescue
-            ^^^^^^ `rescue` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `rescue` at 3, 4 is not aligned with `def test` at 1, 0.
             error
         end
       RUBY
@@ -75,7 +75,7 @@ def test
         def Test.test
           something
             rescue
-            ^^^^^^ `rescue` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `rescue` at 3, 4 is not aligned with `def Test.test` at 1, 0.
             error
         end
       RUBY
@@ -106,7 +106,7 @@ def Test.test
         class C
           something
             rescue
-            ^^^^^^ `rescue` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `rescue` at 3, 4 is not aligned with `class C` at 1, 0.
             error
         end
       RUBY
@@ -137,7 +137,7 @@ class C
         module M
           something
             rescue
-            ^^^^^^ `rescue` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `rescue` at 3, 4 is not aligned with `module M` at 1, 0.
             error
         end
       RUBY
@@ -168,7 +168,7 @@ module M
         begin
           something
             ensure
-            ^^^^^^ `ensure` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `ensure` at 3, 4 is not aligned with `begin` at 1, 0.
             error
         end
       RUBY
@@ -199,7 +199,7 @@ module M
         def test
           something
             ensure
-            ^^^^^^ `ensure` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `ensure` at 3, 4 is not aligned with `def test` at 1, 0.
             error
         end
       RUBY
@@ -230,7 +230,7 @@ def test
         def Test.test
           something
             ensure
-            ^^^^^^ `ensure` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `ensure` at 3, 4 is not aligned with `def Test.test` at 1, 0.
             error
         end
       RUBY
@@ -261,7 +261,7 @@ def Test.test
         class C
           something
             ensure
-            ^^^^^^ `ensure` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `ensure` at 3, 4 is not aligned with `class C` at 1, 0.
             error
         end
       RUBY
@@ -292,7 +292,7 @@ class C
         module M
           something
             ensure
-            ^^^^^^ `ensure` at 3, 4 is not aligned with `end` at 5, 0.
+            ^^^^^^ `ensure` at 3, 4 is not aligned with `module M` at 1, 0.
             error
         end
       RUBY
@@ -317,6 +317,19 @@ module M
     end
   end
 
+  it 'accepts end being misaligned' do
+    expect_no_offenses(<<-RUBY.strip_indent)
+      def method1
+        'foo'
+      end
+
+      def method2
+        'bar'
+      rescue
+        'baz' end
+    RUBY
+  end
+
   it 'accepts rescue and ensure on the same line' do
     expect_no_offenses('begin; puts 1; rescue; ensure; puts 2; end')
   end
@@ -371,7 +384,7 @@ def foo
             [1, 2, 3].each do |el|
               el.to_s
           rescue StandardError => _exception
-          ^^^^^^ `rescue` at 4, 0 is not aligned with `end` at 6, 2.
+          ^^^^^^ `rescue` at 4, 0 is not aligned with `[1, 2, 3].each do` at 2, 2.
               next
             end
           end