diff --git a/CHANGELOG.md b/CHANGELOG.md index 46eadc32ddb5..920d61c26495 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/rubocop/cop/lint/shadowed_exception.rb b/lib/rubocop/cop/lint/shadowed_exception.rb index e7496bd8d993..0f7b9cde6497 100644 --- a/lib/rubocop/cop/lint/shadowed_exception.rb +++ b/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) diff --git a/manual/cops_lint.md b/manual/cops_lint.md index 50d5aaa1a0d7..edf604d98a5b 100644 --- a/manual/cops_lint.md +++ b/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 diff --git a/spec/rubocop/cop/lint/shadowed_exception_spec.rb b/spec/rubocop/cop/lint/shadowed_exception_spec.rb index 8ae7e6a95e70..9117587332c4 100644 --- a/spec/rubocop/cop/lint/shadowed_exception_spec.rb +++ b/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)