From 42ac78116340a2a99c963682f4a3aa6b64535893 Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Sat, 4 Apr 2020 00:51:38 +0900 Subject: [PATCH] [Fix #7842] Add `AcceptImplicitNamespaces` to `Lint/RaiseException` Fixes #7842. This PR adds `AcceptImplicitNamespaces` option to `Lint/RaiseException`. Users can specify and accept an implicit namespace as follows: ```yaml Lint/RaiseException: AllowImplicitNamespaces: - 'Gem' ``` ```ruby # good module Gem def self.foo raise Exception end end Gem.foo #=> Gem::Exception ``` `Gem` is specified by default. https://ruby-doc.org/stdlib-2.7.0/libdoc/rubygems/rdoc/Gem/Exception.html This way does not prevent all false positives, but it expect some cases can be solved. --- CHANGELOG.md | 1 + config/default.yml | 2 + lib/rubocop/cop/lint/raise_exception.rb | 48 ++++++++++++++++--- manual/cops_lint.md | 23 +++++++++ spec/rubocop/cop/lint/raise_exception_spec.rb | 29 ++++++++++- 5 files changed, 95 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 457e1e063b4..b84b5a753eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ * [#7842](https://github.com/rubocop-hq/rubocop/issues/7842): Fix a false positive for `Lint/RaiseException` when raising Exception with explicit namespace. ([@koic][]) * [#7834](https://github.com/rubocop-hq/rubocop/issues/7834): Fix `Lint/UriRegexp` to register offense with array arguments. ([@tejasbubane][]) * [#7841](https://github.com/rubocop-hq/rubocop/issues/7841): Fix an error for `Style/TrailingCommaInBlockArgs` when lambda literal (`->`) has multiple arguments. ([@koic][]) +* [#7842](https://github.com/rubocop-hq/rubocop/issues/7842): Fix a false positive for `Lint/RaiseException` when Exception without cbase specified under the namespace `Gem` by adding `AllowedImplicitNamespaces` option. ([@koic][]) ### Changes diff --git a/config/default.yml b/config/default.yml index 549a38a8203..9328eb6df7e 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1580,6 +1580,8 @@ Lint/RaiseException: StyleGuide: '#raise-exception' Enabled: pending VersionAdded: '0.81' + AllowedImplicitNamespaces: + - 'Gem' Lint/RandOne: Description: >- diff --git a/lib/rubocop/cop/lint/raise_exception.rb b/lib/rubocop/cop/lint/raise_exception.rb index e49175198d4..d3d99b56098 100644 --- a/lib/rubocop/cop/lint/raise_exception.rb +++ b/lib/rubocop/cop/lint/raise_exception.rb @@ -6,32 +6,68 @@ module Lint # This cop checks for `raise` or `fail` statements which are # raising `Exception` class. # + # You can specify a module name that will be an implicit namespace + # using `AllowedImplicitNamespaces` option. The cop cause a false positive + # for namespaced `Exception` when a namespace is omitted. This option can + # prevent the false positive by specifying a namespace to be omitted for + # `Exception`. Alternatively, make `Exception` a fully qualified class + # name with an explicit namespace. + # # @example # # bad # raise Exception, 'Error message here' # # # good # raise StandardError, 'Error message here' + # + # @example AllowedImplicitNamespaces: ['Gem'] + # # good + # module Gem + # def self.foo + # raise Exception # This exception means `Gem::Exception`. + # end + # end class RaiseException < Cop MSG = 'Use `StandardError` over `Exception`.' def_node_matcher :exception?, <<~PATTERN - (send nil? ${:raise :fail} (const {cbase nil?} :Exception) ... ) + (send nil? {:raise :fail} (const ${cbase nil?} :Exception) ... ) PATTERN def_node_matcher :exception_new_with_message?, <<~PATTERN - (send nil? ${:raise :fail} - (send (const {cbase nil?} :Exception) :new ... )) + (send nil? {:raise :fail} + (send (const ${cbase nil?} :Exception) :new ... )) PATTERN def on_send(node) - add_offense(node) if raise_exception?(node) + exception?(node, &check(node)) || + exception_new_with_message?(node, &check(node)) end private - def raise_exception?(node) - exception?(node) || exception_new_with_message?(node) + def check(node) + lambda do |cbase| + return if cbase.nil? && implicit_namespace?(node) + + add_offense(node) + end + end + + def implicit_namespace?(node) + return false unless (parent = node.parent) + + if parent.module_type? + namespace = parent.identifier.source + + return allow_implicit_namespaces.include?(namespace) + end + + implicit_namespace?(parent) + end + + def allow_implicit_namespaces + cop_config['AllowedImplicitNamespaces'] || [] end end end diff --git a/manual/cops_lint.md b/manual/cops_lint.md index a1e3b3f4791..5f7d948b3ec 100644 --- a/manual/cops_lint.md +++ b/manual/cops_lint.md @@ -1496,6 +1496,13 @@ Pending | Yes | No | 0.81 | - This cop checks for `raise` or `fail` statements which are raising `Exception` class. +You can specify a module name that will be an implicit namespace +using `AllowedImplicitNamespaces` option. The cop cause a false positive +for namespaced `Exception` when a namespace is omitted. This option can +prevent the false positive by specifying a namespace to be omitted for +`Exception`. Alternatively, make `Exception` a fully qualified class +name with an explicit namespace. + ### Examples ```ruby @@ -1505,6 +1512,22 @@ raise Exception, 'Error message here' # good raise StandardError, 'Error message here' ``` +#### AllowedImplicitNamespaces: ['Gem'] + +```ruby +# good +module Gem + def self.foo + raise Exception # This exception means `Gem::Exception`. + end +end +``` + +### Configurable attributes + +Name | Default value | Configurable values +--- | --- | --- +AllowedImplicitNamespaces | `Gem` | Array ### References diff --git a/spec/rubocop/cop/lint/raise_exception_spec.rb b/spec/rubocop/cop/lint/raise_exception_spec.rb index 5e3e9a310b4..cdc208cfe01 100644 --- a/spec/rubocop/cop/lint/raise_exception_spec.rb +++ b/spec/rubocop/cop/lint/raise_exception_spec.rb @@ -1,7 +1,9 @@ # frozen_string_literal: true -RSpec.describe RuboCop::Cop::Lint::RaiseException do - subject(:cop) { described_class.new } +RSpec.describe RuboCop::Cop::Lint::RaiseException, :config do + subject(:cop) { described_class.new(config) } + + let(:cop_config) { { 'AllowedImplicitNamespaces' => ['Gem'] } } it 'registers an offense for `raise` with `::Exception`' do expect_offense(<<~RUBY) @@ -87,4 +89,27 @@ raise Foo::Exception RUBY end + + context 'when under namespace' do + it 'does not register an offense when Exception without cbase specified' do + expect_no_offenses(<<~RUBY) + module Gem + def self.foo + raise Exception + end + end + RUBY + end + + it 'does not register an offense when Exception with cbase specified' do + expect_offense(<<~RUBY) + module Gem + def self.foo + raise ::Exception + ^^^^^^^^^^^^^^^^^ Use `StandardError` over `Exception`. + end + end + RUBY + end + end end