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
[Fix #7842] Add AcceptImplicitNamespaces
to Lint/RaiseException
#7846
[Fix #7842] Add AcceptImplicitNamespaces
to Lint/RaiseException
#7846
Conversation
b7ba87b
to
26c1149
Compare
config/default.yml
Outdated
@@ -1567,6 +1567,8 @@ Lint/RaiseException: | |||
StyleGuide: '#raise-exception' | |||
Enabled: pending | |||
VersionAdded: '0.81' | |||
AllowImplicitNamespaces: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be named "AllowedImplicitNamespaces".
@@ -5,33 +5,64 @@ module Cop | |||
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably the explanation should be extended to mention the cop normally ignores namespaced "Exception", but might cause false positives if some module has its own Exception
class. This would make it easier for people to understand the need for the extra config. That being said - I think it's a very bad idea to name base classes "Exception" and I'd rather name them "(BaseError)" or something along those lines. Hiding built-in constants is a recipe for trouble IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for reviewing. I extended the explanation.
26c1149
to
5bf74df
Compare
…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.
5bf74df
to
59e0823
Compare
Looks good! |
Fixes #7842.
This PR adds
AcceptImplicitNamespaces
option toLint/RaiseException
.Users can specify and accept an implicit namespace as follows:
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.
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.