[Fix #4843] Fix shadow rescued Exceptions of same error code (#4844)

* [Fix #4843] Fix shadow rescued Exceptions of same error code

Note: System dependent error code depends on runtime environment.

CHANGELOG.md

@@ -29,6 +29,7 @@
 * [#4830](https://github.com/bbatsov/rubocop/issues/4830): Prevent `Lint/BooleanSymbol` from truncating symbol's value in the message when offense is located in the new syntax hash. ([@akhramov][])
 * [#4747](https://github.com/bbatsov/rubocop/issues/4747): Fix `Rails/HasManyOrHasOneDependent` cop incorrectly flags `with_options` blocks. ([@koic][])
 * [#4836](https://github.com/bbatsov/rubocop/issues/4836): Make `Rails/OutputSafety` aware of safe navigation operator. ([@drenmi][])
+* [#4843](https://github.com/bbatsov/rubocop/issues/4843): Make `Lint/ShadowedException` cop aware of same system error code. ([@koic][])
 
 ### Changes
 

lib/rubocop/cop/lint/shadowed_exception.rb

@@ -19,8 +19,6 @@ module Lint
       #     handle_standard_error
       #   end
       #
-      # @example
-      #
       #   # good
       #
       #   begin
@@ -30,6 +28,21 @@ module Lint
       #   rescue Exception
       #     handle_exception
       #   end
+      #
+      #   # good, however depending on runtime environment.
+      #   #
+      #   # This is a special case for system call errors.
+      #   # System dependent error code depends on runtime environment.
+      #   # For example, whether `Errno::EAGAIN` and `Errno::EWOULDBLOCK` are
+      #   # the same error code or different error code depends on environment.
+      #   # This good case is for `Errno::EAGAIN` and `Errno::EWOULDBLOCK` with
+      #   # the same error code.
+      #   begin
+      #     something
+      #   rescue Errno::EAGAIN, Errno::EWOULDBLOCK
+      #     handle_standard_error
+      #   end
+      #
       class ShadowedException < Cop
         include RescueNode
 
@@ -77,7 +90,27 @@ def contains_multiple_levels_of_exceptions?(group)
             return !(group.size == 2 && group.include?(NilClass))
           end
 
-          group.combination(2).any? { |a, b| a && b && a <=> b }
+          group.combination(2).any? do |a, b|
+            compare_exceptions(a, b)
+          end
+        end
+
+        def compare_exceptions(exception, other_exception)
+          if system_call_err?(exception) && system_call_err?(other_exception)
+            # This condition logic is for special case.
+            # System dependent error code depends on runtime environment.
+            # For example, whether `Errno::EAGAIN` and `Errno::EWOULDBLOCK` are
+            # the same error code or different error code depends on runtime
+            # environment. This checks the error code for that.
+            exception.const_get(:Errno) != other_exception.const_get(:Errno) &&
+              exception <=> other_exception
+          else
+            exception && other_exception && exception <=> other_exception
+          end
+        end
+
+        def system_call_err?(error)
+          error && error.ancestors[1] == SystemCallError
         end
 
         def evaluate_exceptions(rescue_group)

manual/cops_lint.md

@@ -1704,8 +1704,7 @@ rescue Exception
 rescue StandardError
   handle_standard_error
 end
-```
-```ruby
+
 # good
 
 begin
@@ -1715,6 +1714,20 @@ rescue StandardError
 rescue Exception
   handle_exception
 end
+
+# good, however depending on runtime environment.
+#
+# This is a special case for system call errors.
+# System dependent error code depends on runtime environment.
+# For example, whether `Errno::EAGAIN` and `Errno::EWOULDBLOCK` are
+# the same error code or different error code depends on environment.
+# This good case is for `Errno::EAGAIN` and `Errno::EWOULDBLOCK` with
+# the same error code.
+begin
+  something
+rescue Errno::EAGAIN, Errno::EWOULDBLOCK
+  handle_standard_error
+end
 ```
 
 ## Lint/ShadowingOuterLocalVariable

spec/rubocop/cop/lint/shadowed_exception_spec.rb

@@ -60,6 +60,21 @@
       RUBY
     end
 
+    it 'accepts rescue containing multiple same error code exceptions' do
+      # System dependent error code depends on runtime environment.
+      stub_const('Errno::EAGAIN::Errno', 35)
+      stub_const('Errno::EWOULDBLOCK::Errno', 35)
+      stub_const('Errno::ECONNABORTED::Errno', 53)
+
+      expect_no_offenses(<<-RUBY.strip_indent)
+        begin
+          something
+        rescue Errno::EAGAIN, Errno::EWOULDBLOCK, Errno::ECONNABORTED
+          handle_exception
+        end
+      RUBY
+    end
+
     it 'registers an offense rescuing exceptions that are ' \
       'ancestors of each other ' do
       inspect_source(<<-RUBY.strip_indent)