Skip to content

Commit

Permalink
[Fix #6893] Handle implicit rescue correctly in Naming/RescuedExcepti…
Browse files Browse the repository at this point in the history
…onsVariableName (#7122)

Co-authored-by: Anthony Robin <contact@anthony-robin.fr>
  • Loading branch information
2 people authored and bbatsov committed Jun 25, 2019
1 parent 239e2b6 commit efe9ab4
Show file tree
Hide file tree
Showing 5 changed files with 126 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
* [#7118](https://github.com/rubocop-hq/rubocop/pull/7118): Fix `Style/WordArray` with `encoding: binary` magic comment and non-ASCII string. ([@pocke][])
* [#7159](https://github.com/rubocop-hq/rubocop/issues/7159): Fix an error for `Lint/DuplicatedKey` when using endless range. ([@koic][])
* [#7151](https://github.com/rubocop-hq/rubocop/issues/7151): Fix `Style/WordArray` to also consider words containing hyphens. ([@fwitzke][])
* [#6893](https://github.com/rubocop-hq/rubocop/issues/6893): Handle implicit rescue correctly in `Naming/RescuedExceptionsVariableName`. ([@pocke][], [@anthony-robin][])
* [#7165](https://github.com/rubocop-hq/rubocop/issues/7165): Fix an auto-correct error for `Style/ConditionalAssignment` when without `else` branch'. ([@koic][])

### Changes
Expand Down
7 changes: 1 addition & 6 deletions lib/rubocop/ast/node/resbody_node.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,7 @@ def body
#
# @return [Node, nil] The exception variable of the `resbody`.
def exception_variable
variable = node_parts[1]
return variable if variable

# When resbody is an implicit rescue (i.e. `rescue e` style),
# the exception variable is descendants[1].
descendants[1]
node_parts[1]
end
end
end
Expand Down
41 changes: 21 additions & 20 deletions lib/rubocop/cop/naming/rescued_exceptions_variable_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,20 @@ class RescuedExceptionsVariableName < Cop
MSG = 'Use `%<preferred>s` instead of `%<bad>s`.'

def on_resbody(node)
exception_type, @exception_name = *node
return unless exception_type || @exception_name

@exception_name ||= exception_type.children.first
return if @exception_name.const_type? ||
variable_name == preferred_name
name = variable_name(node)
return unless name
return if preferred_name(name).to_sym == name

add_offense(node, location: offense_range(node))
end

def autocorrect(node)
lambda do |corrector|
offending_name = node.exception_variable.children.first
offending_name = variable_name(node)
preferred_name = preferred_name(offending_name)
corrector.replace(offense_range(node), preferred_name)

return unless node.body

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

corrector.replace(var.loc.expression, preferred_name)
Expand All @@ -89,21 +85,26 @@ def offense_range(resbody)
variable.loc.expression
end

def preferred_name
name = cop_config.fetch('PreferredName', 'e')
variable_name.to_s.start_with?('_') ? "_#{name}" : name
def preferred_name(variable_name)
preferred_name = cop_config.fetch('PreferredName', 'e')
if variable_name.to_s.start_with?('_')
"_#{preferred_name}"
else
preferred_name
end
end

def variable_name
location.source
end
def variable_name(node)
asgn_node = node.exception_variable
return unless asgn_node

def location
@exception_name.loc.expression
asgn_node.children.last
end

def message(_node = nil)
format(MSG, preferred: preferred_name, bad: variable_name)
def message(node)
offending_name = variable_name(node)
preferred_name = preferred_name(offending_name)
format(MSG, preferred: preferred_name, bad: offending_name)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/rubocop/ast/resbody_node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
end

context 'for an implicit rescue' do
let(:source) { 'begin; beginbody; rescue ex; rescuebody; end' }
let(:source) { 'begin; beginbody; rescue => ex; rescuebody; end' }

it { expect(resbody_node.exception_variable.source).to eq('ex') }
end
Expand Down
110 changes: 102 additions & 8 deletions spec/rubocop/cop/naming/rescued_exceptions_variable_name_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,100 @@
RUBY
end
end

context 'with lower letters class name' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
begin
something
rescue my_exception
# do something
end
RUBY
end
end

context 'with method as `Exception`' do
it 'does not register an offense without variable name' do
expect_no_offenses(<<~RUBY)
begin
something
rescue ActiveSupport::JSON.my_method
# do something
end
RUBY
end

it 'does not register an offense with expected variable name' do
expect_no_offenses(<<~RUBY)
begin
something
rescue ActiveSupport::JSON.my_method => e
# do something
end
RUBY
end

it 'registers an offense with unexpected variable name' do
expect_offense(<<~RUBY)
begin
something
rescue ActiveSupport::JSON.my_method => exc
^^^ Use `e` instead of `exc`.
# do something
end
RUBY

expect_correction(<<~RUBY)
begin
something
rescue ActiveSupport::JSON.my_method => e
# do something
end
RUBY
end
end

context 'with splat operator as `Exception` list' do
it 'does not register an offense without variable name' do
expect_no_offenses(<<~RUBY)
begin
something
rescue *handled
# do something
end
RUBY
end

it 'does not register an offense with expected variable name' do
expect_no_offenses(<<~RUBY)
begin
something
rescue *handled => e
# do something
end
RUBY
end

it 'registers an offense with unexpected variable name' do
expect_offense(<<~RUBY)
begin
something
rescue *handled => exc
^^^ Use `e` instead of `exc`.
# do something
end
RUBY

expect_correction(<<~RUBY)
begin
something
rescue *handled => e
# do something
end
RUBY
end
end
end

context 'with implicit rescue' do
Expand All @@ -121,16 +215,16 @@
expect_offense(<<~RUBY)
begin
something
rescue exc
^^^ Use `e` instead of `exc`.
rescue => exc
^^^ Use `e` instead of `exc`.
# do something
end
RUBY

expect_correction(<<~RUBY)
begin
something
rescue e
rescue => e
# do something
end
RUBY
Expand All @@ -140,16 +234,16 @@
expect_offense(<<~RUBY)
begin
something
rescue _exc
^^^^ Use `_e` instead of `_exc`.
rescue => _exc
^^^^ Use `_e` instead of `_exc`.
# do something
end
RUBY

expect_correction(<<~RUBY)
begin
something
rescue _e
rescue => _e
# do something
end
RUBY
Expand All @@ -159,7 +253,7 @@
expect_no_offenses(<<~RUBY)
begin
something
rescue e
rescue => e
# do something
end
RUBY
Expand All @@ -169,7 +263,7 @@
expect_no_offenses(<<~RUBY)
begin
something
rescue _e
rescue => _e
# do something
end
RUBY
Expand Down

0 comments on commit efe9ab4

Please sign in to comment.