`Lint/InheritException` copにテコ入れした

rubocop

Standard gem でのイシューについてジャスティンからメンションをもらったのがきっかけで、Lint/InheritException がいろいろと🤔だったのでテコ入れした。対象となる以下の PR に書いていることのサマリーとなる。

Lint/InheritException cop は、カスタム例外クラスの親クラスに Exception クラスを指定している場合、StandardError クラスか RuntimeError クラスに変えるように警告をする cop (RuboCop 1.25.1 時点では RuntimeError がデフォルト) 。とりわけ Java だとカスタムの例外クラスを作る際に java.lang.Exception を継承するので、Java から Ruby へスキル転換で不慣れな人のコードでかつて何度か見た覚えがあった。Lint として存在しているのは、そんなところが始まりかなと思っている (調べたら自分が RuboCop にパッチを送るようになる以前に導入されていた) 。

StandardError を継承しない、組み込みの例外クラスを許容するようにした

ひとつ目のテコ入れ。組み込みの Interrupt 例外クラスに対しても RuntimeError にするよう警告するのは、継承ツリーのまったく異なるものへの置換でおかしいのではというのが個人的な見解。以下、PR に書いた AA から抜粋。

        ---------------
        |   Object    |
        ---------------
               ^
               |
        ---------------
        |  Exception  |
        ---------------
               ^
               |
      |------------------|
---------------  -----------------
|StandardError|  |SignalException|
---------------  -----------------
      ^                  ^
      |                  |
---------------  -----------------
|RuntimeError |  |   Interrupt   |
---------------  -----------------

Interrupt と同様に、SystemStackError, NoMemoryError, SecurityError, NotImplementedError, LoadError, SyntaxError, ScriptError, SignalException, SystemExit にも同様のことが起きるようになっていて、通常これらを直接拡張することはないものの、ライブラリなどで意図して拡張したいのであれば良いのではということから許容するようにした (実際手元にある OSS リポジトリでも対象となるものがあった) 。Style ならともかく Lint としては厳しすぎる気がするし、The Open-Closed Principle の観点からも拡張に対して開けていて良かろうという判断。

Exception の変わりに RuntimeError ではなく StandardError をデフォルトで継承するようにした

ふたつ目のテコ入れとして、Exception の置換先のデフォルトが RuntimeError だったのを StandardError にした。これも PR に書いた AA を抜粋しておく。

---------------
|  Exception  |
---------------
      ^
      |
---------------
|StandardError| (default for `rescue`)
---------------
      ^
      |
---------------
|RuntimeError | (default for `raise`)
---------------

おさらいとして、StandardError (とそのサブクラス) は rescue に例外クラスが指定されていないときにデフォルトで補足するクラス、RuntimeError は例外クラスを指定せずに raise する際のデフォルトのクラスとなる。

カスタム例外クラスの親クラスをデフォルトで RuntimeError の継承とするとクラス指定なしの raise と、クラス指定ありの raise のいずれも is-a として違いがでなくなる。コンテキストによるので、RuntimeError の継承が間違っているわけではないが、抽象度の高い StandardError の方が良かろうと Exception のデフォルトの置換先を StandardError に変更した。

安全でない自動修正としてマークした

最後のテコ入れ、そもそもこれらのクラスについて振る舞いが変わるため、安全でない自動修正とマークした。

次のリリース (1.25.1 の次) でこのあたりの変更がざっと入る予定です。