Skip to content

Commit

Permalink
[Fix #6902] Naming/RescuedExceptionsVariableName for multiple rescues
Browse files Browse the repository at this point in the history
As issued in #6902, the `Naming/RescuedExceptionsVariableName` cop did not
handle multiple rescues as below.

```
begin
  # something
rescue FooException => foo # <= flagged
rescue BarException => bar # <= not flagged
end
```

This changes fix that and enable the cop handle multiple rescues.
  • Loading branch information
tatsuyafw authored and bbatsov committed Apr 20, 2019
1 parent fc5acde commit b0b9d39
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -17,6 +17,7 @@
* [#6926](https://github.com/rubocop-hq/rubocop/issues/6926): [Fix #6926] Allow nil values to unset config defaults. ([@dduugg][])
* [#6946](https://github.com/rubocop-hq/rubocop/pull/6946): Allow `Rails/ReflectionClassName` to use string interpolation for `class_name`. ([@r7kamura][])
* [#6778](https://github.com/rubocop-hq/rubocop/issues/6778): Fix a false positive in `Style/HashSyntax` cop when a hash key is an interpolated string and EnforcedStyle is ruby19_no_mixed_keys. ([@tatsuyafw][])
* [#6902](https://github.com/rubocop-hq/rubocop/issues/6902): Fix a bug where `Naming/RescuedExceptionsVariableName` would handle an only first rescue for multiple rescue groups. ([@tatsuyafw][])

### Changes

Expand Down
12 changes: 12 additions & 0 deletions lib/rubocop/ast/node/resbody_node.rb
Expand Up @@ -12,6 +12,18 @@ class ResbodyNode < Node
def body
node_parts[2]
end

# Returns the exception variable of the `rescue` clause.
#
# @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]
end
end
end
end
15 changes: 10 additions & 5 deletions lib/rubocop/cop/naming/rescued_exceptions_variable_name.rb
Expand Up @@ -64,17 +64,22 @@ def on_resbody(node)
return if @exception_name.const_type? ||
variable_name == preferred_name

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

def autocorrect(_node)
def autocorrect(node)
lambda do |corrector|
corrector.replace(location, preferred_name)
corrector.replace(offense_range(node), preferred_name)
end
end

private

def offense_range(resbody)
variable = resbody.exception_variable
variable.loc.expression
end

def preferred_name
@preferred_name ||= begin
name = cop_config.fetch('PreferredName', 'e')
Expand All @@ -84,11 +89,11 @@ def preferred_name
end

def variable_name
@variable_name ||= location.source
location.source
end

def location
@location ||= @exception_name.loc.expression
@exception_name.loc.expression
end

def message(_node = nil)
Expand Down
20 changes: 20 additions & 0 deletions spec/rubocop/ast/resbody_node_spec.rb
Expand Up @@ -13,6 +13,26 @@
it { expect(resbody_node.is_a?(described_class)).to be(true) }
end

describe '#exception_variable' do
context 'for an explicit rescue' do
let(:source) { 'begin; beginbody; rescue Error => ex; rescuebody; end' }

it { expect(resbody_node.exception_variable.source).to eq('ex') }
end

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

it { expect(resbody_node.exception_variable.source).to eq('ex') }
end

context 'when an exception variable is not given' do
let(:source) { 'begin; beginbody; rescue; rescuebody; end' }

it { expect(resbody_node.exception_variable).to be(nil) }
end
end

describe '#body' do
let(:source) { 'begin; beginbody; rescue Error => ex; :rescuebody; end' }

Expand Down
49 changes: 49 additions & 0 deletions spec/rubocop/cop/naming/rescued_exceptions_variable_name_spec.rb
Expand Up @@ -44,6 +44,31 @@
RUBY
end

it 'registers offenses when using `foo` and `bar` ' \
'in multiple rescues' do
expect_offense(<<-RUBY.strip_indent)
begin
something
rescue FooException => foo
^^^ Use `e` instead of `foo`.
# do something
rescue BarException => bar
^^^ Use `e` instead of `bar`.
# do something
end
RUBY

expect_correction(<<-RUBY.strip_indent)
begin
something
rescue FooException => e
# do something
rescue BarException => e
# do something
end
RUBY
end

it 'does not register an offense when using `e`' do
expect_no_offenses(<<-RUBY.strip_indent)
begin
Expand Down Expand Up @@ -198,6 +223,30 @@
RUBY
end

it 'registers offenses when using `foo` and `bar` in multiple rescues' do
expect_offense(<<-RUBY.strip_indent)
begin
something
rescue FooException => foo
^^^ Use `exception` instead of `foo`.
# do something
rescue BarException => bar
^^^ Use `exception` instead of `bar`.
# do something
end
RUBY

expect_correction(<<-RUBY.strip_indent)
begin
something
rescue FooException => exception
# do something
rescue BarException => exception
# do something
end
RUBY
end

it 'does not register an offense when using `exception`' do
expect_no_offenses(<<-RUBY.strip_indent)
begin
Expand Down

0 comments on commit b0b9d39

Please sign in to comment.