Skip to content

Commit

Permalink
[Fix #7842] Add AcceptImplicitNamespaces to Lint/RaiseException
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
koic authored and bbatsov committed Apr 13, 2020
1 parent d4a9bf5 commit 42ac781
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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

Expand Down
2 changes: 2 additions & 0 deletions config/default.yml
Expand Up @@ -1580,6 +1580,8 @@ Lint/RaiseException:
StyleGuide: '#raise-exception'
Enabled: pending
VersionAdded: '0.81'
AllowedImplicitNamespaces:
- 'Gem'

Lint/RandOne:
Description: >-
Expand Down
48 changes: 42 additions & 6 deletions lib/rubocop/cop/lint/raise_exception.rb
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions manual/cops_lint.md
Expand Up @@ -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
Expand All @@ -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

Expand Down
29 changes: 27 additions & 2 deletions 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)
Expand Down Expand Up @@ -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

0 comments on commit 42ac781

Please sign in to comment.