Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lint/RaiseException not aware of namespaced "Exception" classes #7842

Closed
sos4nt opened this issue Apr 2, 2020 · 0 comments · Fixed by #7846
Closed

Lint/RaiseException not aware of namespaced "Exception" classes #7842

sos4nt opened this issue Apr 2, 2020 · 0 comments · Fixed by #7846
Labels

Comments

@sos4nt
Copy link

sos4nt commented Apr 2, 2020

Rubocop 0.81 introduced Lint/RaiseException which checks if we attempt to raise the top-level Exception class.

Unfortunately, it also complains about subclasses with the name Exception, e.g.

raise Gem::Exception, 'some gem error'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
# W: Lint/RaiseException: Use StandardError over Exception.

But the above is perfectly fine given that Gem::Exception is a subclass of RuntimeError:

Gem::Exception.ancestors
#=> [Gem::Exception, RuntimeError, StandardError, Exception, ...]

I don't think there's an easy fix because even a plain raise Exception could be valid from within the correct namespace, e.g.:

module Gem
  def self.foo
    raise Exception
  end
end

Gem.foo #=> Gem::Exception (Gem::Exception)

The current behavior merely ensures that we don't use exceptions with the name Exception which seem to be of little use to me.

@koic koic added the bug label Apr 3, 2020
koic added a commit to koic/rubocop that referenced this issue Apr 3, 2020
Resolve part of rubocop#7842.

This PR fixes a false positive for `Lint/RaiseException`
when raising Exception with explicit namespace.

`Exception` belonging to a namespace is expected to
inherit `StandardError`.

This PR makes `Lint/RaiseException` aware of the following differences:

```ruby
Gem::Exception.new.is_a?(StandardError) # => true
Exception.new.is_a?(StandardError)      # => false
```

On the other hand, the following case have not been resolved by this PR.

```ruby
module Gem
  def self.foo
    raise Exception
  end
end

Gem.foo #=> Gem::Exception
```

The above case will be resolved separately from this PR.
koic added a commit that referenced this issue Apr 3, 2020
…exception

[#7842] Fix a false positive for `Lint/RaiseException`
koic added a commit to koic/rubocop that referenced this issue Apr 4, 2020
…tion`

Fixes rubocop#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.
bbatsov pushed a commit that referenced this issue Apr 13, 2020
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants