Skip to content

Commit

Permalink
Fix a false positive for Lint/InheritException
Browse files Browse the repository at this point in the history
Fixes standardrb/standard#388 and
reverts #3427.

This PR prevents the following false positive and auto-correction.

```diff
 class TestClass
-  FatalError = Class.new(Interrupt)
+  FatalError = Class.new(RuntimeError)
 end
```

Below is an inheritance tree for the exception classes.

```
        ---------------
        | BasicObject |
        ---------------
               ^
               |
        ---------------
        |   Kernel    |
        ---------------
               ^
               |
        ---------------
        |   Object    |
        ---------------
               ^
               |
        ---------------
        |  Exception  |
        ---------------
               ^
               |
      |------------------|
---------------  -----------------
|StandardError|  |SignalException|
---------------  -----------------
      ^                  ^
      |                  |
---------------  -----------------
|RuntimeError |  |   Interrupt   |
---------------  -----------------
```

As the inheritance tree shows, `Interrupt` is not `is_a?` in either
`StandardError` or `RuntimeError`.

```ruby
RuntimeError.new.is_a?(StandardError) #=> true
Interrupt.new.is_a?(StandardError)    #=> false
Interrupt.new.is_a?(RuntimeError)     #=> false
```

This goes against The Liskov Substitution Principle.
`Interup` should not be replaced with `StandardError` or its subclasses.
Also, according to The Open/Closed Principle, `Interrup` class should be
open to inheritance.

The same is true for `SystemStackError`, `NoMemoryError`, `SecurityError`,
`NotImplementedError`, `LoadError`, `SyntaxError`, `ScriptError`,
`SignalException`, and `SystemExit` classes.

This PR allows for exception handling according to Object-oriented principles
and Ruby exception handling mechanism.
  • Loading branch information
koic authored and bbatsov committed Feb 13, 2022
1 parent c0136af commit 02fba0d
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 22 deletions.
@@ -0,0 +1 @@
* [#10406](https://github.com/rubocop/rubocop/pull/10406): Fix a false positive for `Lint/InheritException` when inheriting a standard lib exception class that is not a subclass of `StandardError`. ([@koic][])
30 changes: 8 additions & 22 deletions lib/rubocop/cop/lint/inherit_exception.rb
Expand Up @@ -3,10 +3,9 @@
module RuboCop
module Cop
module Lint
# This cop looks for error classes inheriting from `Exception`
# and its standard library subclasses, excluding subclasses of
# `StandardError`. It is configurable to suggest using either
# `StandardError` (default) or `RuntimeError` instead.
# This cop looks for error classes inheriting from `Exception`.
# It is configurable to suggest using either `StandardError` (default) or
# `RuntimeError` instead.
#
# @safety
# This cop's autocorrection is unsafe because `rescue` that omit
Expand Down Expand Up @@ -42,24 +41,11 @@ class InheritException < Base
include ConfigurableEnforcedStyle
extend AutoCorrector

MSG = 'Inherit from `%<prefer>s` instead of `%<current>s`.'
MSG = 'Inherit from `%<prefer>s` instead of `Exception`.'
PREFERRED_BASE_CLASS = {
runtime_error: 'RuntimeError',
standard_error: 'StandardError'
}.freeze
ILLEGAL_CLASSES = %w[
Exception
SystemStackError
NoMemoryError
SecurityError
NotImplementedError
LoadError
SyntaxError
ScriptError
Interrupt
SignalException
SystemExit
].freeze

RESTRICT_ON_SEND = %i[new].freeze

Expand All @@ -71,7 +57,7 @@ class InheritException < Base
PATTERN

def on_class(node)
return unless node.parent_class && illegal_class_name?(node.parent_class)
return unless node.parent_class && exception_class?(node.parent_class)

message = message(node.parent_class)

Expand All @@ -82,7 +68,7 @@ def on_class(node)

def on_send(node)
constant = class_new_call?(node)
return unless constant && illegal_class_name?(constant)
return unless constant && exception_class?(constant)

message = message(constant)

Expand All @@ -97,8 +83,8 @@ def message(node)
format(MSG, prefer: preferred_base_class, current: node.const_name)
end

def illegal_class_name?(class_node)
ILLEGAL_CLASSES.include?(class_node.const_name)
def exception_class?(class_node)
class_node.const_name == 'Exception'
end

def preferred_base_class
Expand Down
16 changes: 16 additions & 0 deletions spec/rubocop/cop/lint/inherit_exception_spec.rb
Expand Up @@ -28,6 +28,14 @@ class C < RuntimeError; end
RUBY
end
end

context 'when inheriting a standard lib exception class that is not a subclass of `StandardError`' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class C < Interrupt; end
RUBY
end
end
end

context 'with enforced style set to `standard_error`' do
Expand Down Expand Up @@ -56,6 +64,14 @@ class C < StandardError; end
RUBY
end
end

context 'when inheriting a standard lib exception class that is not a subclass of `StandardError`' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class C < Interrupt; end
RUBY
end
end
end
end
end

0 comments on commit 02fba0d

Please sign in to comment.