Skip to content

Commit

Permalink
Merge pull request #9400 from dvandersluis/issue/7766
Browse files Browse the repository at this point in the history
[Fixes #7766] Fix `Naming/RescuedExceptionsVariableName` autocorrection issues.
  • Loading branch information
koic authored Jan 22, 2021
2 parents bfb6a06 + b74ab41 commit dd2c93f
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 6 deletions.
1 change: 0 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

### Bug fixes

* [#7766](https://github.com/rubocop-hq/rubocop/issues/7766): Rename rescue body vars when renaming exception name. ([@asterite][])
* [#9342](https://github.com/rubocop-hq/rubocop/issues/9342): Fix an error for `Lint/RedundantDirGlobSort` when using `collection.sort`. ([@koic][])
* [#9304](https://github.com/rubocop-hq/rubocop/issues/9304): Do not register an offense for `Style/ExplicitBlockArgument` when the `yield` arguments are not an exact match with the block arguments. ([@dvandersluis][])
* [#8281](https://github.com/rubocop-hq/rubocop/issues/8281): Fix Style/WhileUntilModifier handling comments and assignment when correcting to modifier form. ([@Darhazer][])
Expand Down
2 changes: 2 additions & 0 deletions changelog/fix_fix_namingrescuedexceptionsvariablename.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
* [#7766](https://github.com/rubocop-hq/rubocop/issues/7766): Fix `Naming/RescuedExceptionsVariableName` autocorrection when the rescue body returns the exception variable. ([@asterite][])
* [#7766](https://github.com/rubocop-hq/rubocop/issues/7766): Fix `Naming/RescuedExceptionsVariableName` autocorrection to not change variables if the exception variable has been reassigned. ([@dvandersluis][])
43 changes: 38 additions & 5 deletions lib/rubocop/cop/naming/rescued_exceptions_variable_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,7 @@ def on_resbody(node)
add_offense(range, message: message) do |corrector|
corrector.replace(range, preferred_name)

node.body&.each_node(:lvar) do |var|
next unless var.children.first == offending_name

corrector.replace(var, preferred_name)
end
correct_node(corrector, node.body, offending_name, preferred_name)
end
end

Expand All @@ -86,6 +82,43 @@ def offense_range(resbody)
variable.loc.expression
end

def variable_name_matches?(node, name)
if node.masgn_type?
node.each_descendant(:lvasgn).any? do |lvasgn_node|
variable_name_matches?(lvasgn_node, name)
end
else
node.children.first == name
end
end

def correct_node(corrector, node, offending_name, preferred_name)
return unless node

node.each_node(:lvar, :lvasgn, :masgn) do |child_node|
next unless variable_name_matches?(child_node, offending_name)

corrector.replace(child_node, preferred_name) if child_node.lvar_type?

if child_node.masgn_type? || child_node.lvasgn_type?
correct_reassignment(corrector, child_node, offending_name, preferred_name)
break
end
end
end

# If the exception variable is reassigned, that assignment needs to be corrected.
# Further `lvar` nodes will not be corrected though since they now refer to a
# different variable.
def correct_reassignment(corrector, node, offending_name, preferred_name)
if node.lvasgn_type?
correct_node(corrector, node.child_nodes.first, offending_name, preferred_name)
elsif node.masgn_type?
# With multiple assign, the assignments are in an array as the last child
correct_node(corrector, node.children.last, offending_name, preferred_name)
end
end

def preferred_name(variable_name)
preferred_name = cop_config.fetch('PreferredName', 'e')
if variable_name.to_s.start_with?('_')
Expand Down
71 changes: 71 additions & 0 deletions spec/rubocop/cop/naming/rescued_exceptions_variable_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,77 @@
RUBY
end
end

context 'when the variable is reassigned' do
it 'only corrects uses of the exception' do
expect_offense(<<~RUBY)
def main
raise
rescue StandardError => error
^^^^^ Use `e` instead of `error`.
error = {
error_message: error.message
}
puts error
end
RUBY

expect_correction(<<~RUBY)
def main
raise
rescue StandardError => e
error = {
error_message: e.message
}
puts error
end
RUBY
end

it 'does not correct other variables or assignments' do
expect_offense(<<~RUBY)
def main
raise
rescue StandardError => error
^^^^^ Use `e` instead of `error`.
message = error.message
puts message
end
RUBY

expect_correction(<<~RUBY)
def main
raise
rescue StandardError => e
message = e.message
puts message
end
RUBY
end
end

context 'when the variable is reassigned using multiple assignment' do
it 'only corrects uses of the exception' do
expect_offense(<<~RUBY)
def main
raise
rescue StandardError => error
^^^^^ Use `e` instead of `error`.
error, foo = 1, error
puts error
end
RUBY

expect_correction(<<~RUBY)
def main
raise
rescue StandardError => e
error, foo = 1, e
puts error
end
RUBY
end
end
end

context 'with the `PreferredName` setup' do
Expand Down

0 comments on commit dd2c93f

Please sign in to comment.