[Fix #6526] Fix a wrong line highlight in Lint/ShadowedException cop

When a rescue group contained a higher level exception on the same line as a lower level exception, and a lower level exception was on one of the preceding lines in the same group, the wrong line was highlighted by Lint/ShadowedException cop. ``` begin something rescue ZeroDivisionError ^^^^^^^^^^^^^^^^^^^^^^^^ handle_exception rescue NoMethodError, StandardError handle_standard_error end ``` This change fixes the above incorrect behavior.
rubocop · Jan 5, 2019 · 89c7e9b · 89c7e9b
1 parent bf4264d
commit 89c7e9b
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@
 
 * [#6623](https://github.com/rubocop-hq/rubocop/pull/6623): Fix heredoc detection in trailing comma. ([@palkan][])
 * [#6100](https://github.com/rubocop-hq/rubocop/issues/6100): Fix a false positive in `Naming/ConstantName` cop when rhs is a conditional expression. ([@tatsuyafw][])
+* [#6526](https://github.com/rubocop-hq/rubocop/issues/6526): Fix a wrong line highlight in `Lint/ShadowedException` cop. ([@tatsuyafw][])
 
 ## 0.62.0 (2019-01-01)
 

diff --git a/lib/rubocop/cop/lint/shadowed_exception.rb b/lib/rubocop/cop/lint/shadowed_exception.rb
@@ -53,11 +53,7 @@ def on_rescue(node)
           return if rescue_modifier?(node)
 
           _body, *rescues, _else = *node
-          rescued_groups = rescues.each_with_object([]) do |group, exceptions|
-            rescue_group, = *group
-
-            exceptions << evaluate_exceptions(rescue_group)
-          end
+          rescued_groups = rescued_groups_for(rescues)
 
           rescue_group_rescues_multiple_levels = rescued_groups.any? do |group|
             contains_multiple_levels_of_exceptions?(group)
@@ -72,17 +68,16 @@ def on_rescue(node)
         private
 
         def offense_range(rescues)
-          first_rescue = rescues.first
-          last_rescue = rescues.last
-          last_exceptions, = *last_rescue
-          # last_rescue clause may not specify exception class
-          end_pos = if last_exceptions
-                      last_exceptions.loc.expression.end_pos
-                    else
-                      last_rescue.loc.keyword.end_pos
-                    end
-
-          range_between(first_rescue.loc.expression.begin_pos, end_pos)
+          shadowing_rescue = find_shadowing_rescue(rescues)
+          expression = shadowing_rescue.loc.expression
+          range_between(expression.begin_pos, expression.end_pos)
+        end
+
+        def rescued_groups_for(rescues)
+          rescues.map do |group|
+            rescue_group, = *group
+            evaluate_exceptions(rescue_group)
+          end
         end
 
         def contains_multiple_levels_of_exceptions?(group)
@@ -153,6 +148,17 @@ def rescued_exceptions(rescue_group)
             klass.source
           end.compact
         end
+
+        def find_shadowing_rescue(rescues)
+          rescued_groups = rescued_groups_for(rescues)
+          rescued_groups.zip(rescues).each do |group, res|
+            return res if contains_multiple_levels_of_exceptions?(group)
+          end
+
+          rescued_groups.each_cons(2).with_index do |group_pair, i|
+            return rescues[i] unless sorted?(group_pair)
+          end
+        end
       end
     end
   end

diff --git a/spec/rubocop/cop/lint/shadowed_exception_spec.rb b/spec/rubocop/cop/lint/shadowed_exception_spec.rb
@@ -198,6 +198,8 @@
       expect_offense(<<-RUBY.strip_indent)
         begin
           something
+        rescue NoMethodError
+          handle_no_method_error
         rescue Exception
         ^^^^^^^^^^^^^^^^ Do not shadow rescued Exceptions.
           handle_exception
@@ -222,6 +224,20 @@
       RUBY
     end
 
+    it 'registers an offense for two exceptions when there are ' \
+       'multiple levels of exceptions in the same rescue' do
+      expect_offense(<<-RUBY.strip_indent)
+        begin
+          something
+        rescue ZeroDivisionError
+          handle_exception
+        rescue NoMethodError, StandardError
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not shadow rescued Exceptions.
+          handle_standard_error
+        end
+      RUBY
+    end
+
     it 'registers an offense rescuing out of order exceptions when there ' \
        'is an ensure' do
       expect_offense(<<-RUBY.strip_indent)