From de5e74694005ff38c1e16c5533386a5f273103dc Mon Sep 17 00:00:00 2001 From: Denys Kurets Date: Mon, 2 Sep 2019 00:59:22 +0300 Subject: [PATCH] [Fix #7299] Implement Lint/RaiseException cop --- CHANGELOG.md | 2 + config/default.yml | 6 ++ lib/rubocop.rb | 1 + lib/rubocop/cop/lint/raise_exception.rb | 39 +++++++++ manual/cops.md | 1 + manual/cops_lint.md | 23 +++++ spec/rubocop/cop/lint/raise_exception_spec.rb | 83 +++++++++++++++++++ 7 files changed, 155 insertions(+) create mode 100644 lib/rubocop/cop/lint/raise_exception.rb create mode 100644 spec/rubocop/cop/lint/raise_exception_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fafc510f0b..777b401196b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ * [#7317](https://github.com/rubocop-hq/rubocop/pull/7317): Add new formatter `pacman`. ([@crojasaragonez][]) * [#6075](https://github.com/rubocop-hq/rubocop/issues/6075): Support `IgnoredPatterns` option for `Naming/MethodName` cop. ([@koic][]) * [#7335](https://github.com/rubocop-hq/rubocop/pull/7335): Add todo as an alias to disable. `--disable-uncorrectable` will now disable cops using `rubocop:todo` instead of `rubocop:disable`. ([@desheikh][]) +* [#7299](https://github.com/rubocop-hq/rubocop/issues/7299): Add new `Lint/RaiseException` cop. ([@denys281][]) ### Bug fixes @@ -4248,3 +4249,4 @@ [@jdkaplan]: https://github.com/jdkaplan [@cstyles]: https://github.com/cstyles [@avmnu-sng]: https://github.com/avmnu-sng +[@denys281]: https://github.com/denys281 diff --git a/config/default.yml b/config/default.yml index 85dcaf50773..a52fd9f781a 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1505,6 +1505,12 @@ Lint/PercentSymbolArray: Enabled: true VersionAdded: '0.41' +Lint/RaiseException: + Description: Checks for `raise` or `fail` statements which are raising `Exception` class. + StyleGuide: '#raise-exception' + Enabled: true + VersionAdded: '0.75' + Lint/RandOne: Description: >- Checks for `rand(1)` calls. Such calls always return `0` diff --git a/lib/rubocop.rb b/lib/rubocop.rb index ff7e682cc54..5619ddb4975 100644 --- a/lib/rubocop.rb +++ b/lib/rubocop.rb @@ -321,6 +321,7 @@ require_relative 'rubocop/cop/lint/parentheses_as_grouped_expression' require_relative 'rubocop/cop/lint/percent_string_array' require_relative 'rubocop/cop/lint/percent_symbol_array' +require_relative 'rubocop/cop/lint/raise_exception' require_relative 'rubocop/cop/lint/rand_one' require_relative 'rubocop/cop/lint/redundant_cop_disable_directive' require_relative 'rubocop/cop/lint/redundant_cop_enable_directive' diff --git a/lib/rubocop/cop/lint/raise_exception.rb b/lib/rubocop/cop/lint/raise_exception.rb new file mode 100644 index 00000000000..fed736c57e8 --- /dev/null +++ b/lib/rubocop/cop/lint/raise_exception.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Lint + # This cop checks for `raise` or `fail` statements which are + # raising `Exception` class. + # + # @example + # # bad + # raise Exception, 'Error message here' + # + # # good + # raise StandardError, 'Error message here' + class RaiseException < Cop + MSG = 'Use `StandardError` over `Exception`.' + + def_node_matcher :exception?, <<~PATTERN + (send nil? ${:raise :fail} (const _ :Exception) ... ) + PATTERN + + def_node_matcher :exception_new_with_message?, <<~PATTERN + (send nil? ${:raise :fail} + (send (const _ :Exception) :new ... )) + PATTERN + + def on_send(node) + add_offense(node) if raise_exception?(node) + end + + private + + def raise_exception?(node) + exception?(node) || exception_new_with_message?(node) + end + end + end + end +end diff --git a/manual/cops.md b/manual/cops.md index 8bdb6c60579..73e08bbc963 100644 --- a/manual/cops.md +++ b/manual/cops.md @@ -220,6 +220,7 @@ In the following section you find all available cops: * [Lint/ParenthesesAsGroupedExpression](cops_lint.md#lintparenthesesasgroupedexpression) * [Lint/PercentStringArray](cops_lint.md#lintpercentstringarray) * [Lint/PercentSymbolArray](cops_lint.md#lintpercentsymbolarray) +* [Lint/RaiseException](cops_lint.md#lintraiseexception) * [Lint/RandOne](cops_lint.md#lintrandone) * [Lint/RedundantCopDisableDirective](cops_lint.md#lintredundantcopdisabledirective) * [Lint/RedundantCopEnableDirective](cops_lint.md#lintredundantcopenabledirective) diff --git a/manual/cops_lint.md b/manual/cops_lint.md index 1a2e87d6861..8b9dbe9b0a9 100644 --- a/manual/cops_lint.md +++ b/manual/cops_lint.md @@ -1570,6 +1570,29 @@ rather than meant to be part of the resulting symbols. %i(foo bar) ``` +## Lint/RaiseException + +Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged +--- | --- | --- | --- | --- +Enabled | Yes | No | 0.75 | - + +This cop checks for `raise` or `fail` statements which are +raising `Exception` class. + +### Examples + +```ruby +# bad +raise Exception, 'Error message here' + +# good +raise StandardError, 'Error message here' +``` + +### References + +* [https://rubystyle.guide#raise-exception](https://rubystyle.guide#raise-exception) + ## Lint/RandOne Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged diff --git a/spec/rubocop/cop/lint/raise_exception_spec.rb b/spec/rubocop/cop/lint/raise_exception_spec.rb new file mode 100644 index 00000000000..4433e55b9d6 --- /dev/null +++ b/spec/rubocop/cop/lint/raise_exception_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::Lint::RaiseException do + subject(:cop) { described_class.new } + + it 'registers an offense for `raise` with `::Exception`' do + expect_offense(<<~RUBY) + raise ::Exception + ^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `raise` with `::Exception.new`' do + expect_offense(<<~RUBY) + raise ::Exception.new 'Error with exception' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `raise` with `::Exception` and message' do + expect_offense(<<~RUBY) + raise ::Exception, 'Error with exception' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `raise` with `Exception`' do + expect_offense(<<~RUBY) + raise Exception + ^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `raise` with `Exception` and message' do + expect_offense(<<~RUBY) + raise Exception, 'Error with exception' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `raise` with `Exception.new` and message' do + expect_offense(<<~RUBY) + raise Exception.new 'Error with exception' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `raise` with `Exception.new(args*)` ' do + expect_offense(<<~RUBY) + raise Exception.new('arg1', 'arg2') + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `fail` with `Exception`' do + expect_offense(<<~RUBY) + fail Exception + ^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `fail` with `Exception` and message' do + expect_offense(<<~RUBY) + fail Exception, 'Error with exception' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'registers an offense for `fail` with `Exception.new` and message' do + expect_offense(<<~RUBY) + fail Exception.new 'Error with exception' + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + RUBY + end + + it 'does not register an offense for `raise` without arguments' do + expect_no_offenses('raise') + end + + it 'does not register an offense for `fail` without arguments' do + expect_no_offenses('fail') + end +end