-
-
Notifications
You must be signed in to change notification settings - Fork 284
[Fix #49] Introduce RescueFromExceptionsVariableName
cop
#139
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
base: master
Are you sure you want to change the base?
[Fix #49] Introduce RescueFromExceptionsVariableName
cop
#139
Conversation
Follows rubocop/rubocop#7122 Closes rubocop#49 Inspired from @pocke [refactoring][1] in the `Naming/RescuedExceptionsVariableName` cop, this PR handle the `rescue_from` Rails version. Obstructing points: - As discussed [in this comment][2], the `PreferredName` config should not be duplicated but read from the `Naming/RescuedExceptionsVariableName` cop. How can I read the config from another cop ? Is there already some example of cop working this way ? - I haven't been able to make the parser to not take the `||` of the block arguments which make the offending message being `|my_var|` instead of `my_var`. What is the way to avoid this ? [1]: rubocop/rubocop#7122 [2]: rubocop#49 (comment)
96821f8
to
9ee85ae
Compare
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.
Do you mind rebasing to trigger the build?
# The `PreferredName` config option takes a `String`. It represents | ||
# the required name of the variable. Its default is `e` that is read | ||
# from `Naming/RescuedExceptionsVariableName` cop in the main Rubocop | ||
# repository. |
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.
WDYT of:
The
PreferredName
config option sets the preferred name for the exception variable, defaults toe
.
def autocorrect(node) | ||
lambda do |corrector| | ||
offending_name = variable_name(node) | ||
preferred_name = preferred_name(offending_name) | ||
corrector.replace(offense_range(node), "|#{preferred_name}|") | ||
end | ||
end |
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.
There's a new interface for auto-correction, see https://docs.rubocop.org/rubocop/1.22/development.html#auto-correct
|
||
private | ||
|
||
def offense_range(resbody) |
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.
It feels like this would better be expressed with node pattern. Do you need any guidance with that?
Docs: https://docs.rubocop.org/rubocop-ast/node_pattern.html
@anthony-robin Ping |
Follows rubocop/rubocop#7122
Closes #49
Inspired from @pocke refactoring in the
Naming/RescuedExceptionsVariableName
cop, this PRhandle the
rescue_from
Rails version.Obstructing points:
As discussed in this comment, the
PreferredName
config should not be duplicated but read from theNaming/RescuedExceptionsVariableName
cop.How can I read the config from another cop ? Is there already some example of cop working this way ?
I haven't been able to make the parser to not take the
||
of the block arguments which make the offending message being|my_var|
instead ofmy_var
.What is the way to avoid this ?
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).and description in grammatically correct, complete sentences.
bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.