diff --git a/CHANGELOG.md b/CHANGELOG.md index 4fc272d8ef3..358357a04a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/rubocop/ast/node/resbody_node.rb b/lib/rubocop/ast/node/resbody_node.rb index f51e83e59cf..9f23f97d8d5 100644 --- a/lib/rubocop/ast/node/resbody_node.rb +++ b/lib/rubocop/ast/node/resbody_node.rb @@ -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 diff --git a/lib/rubocop/cop/naming/rescued_exceptions_variable_name.rb b/lib/rubocop/cop/naming/rescued_exceptions_variable_name.rb index 5b87fab8bed..e22c5329755 100644 --- a/lib/rubocop/cop/naming/rescued_exceptions_variable_name.rb +++ b/lib/rubocop/cop/naming/rescued_exceptions_variable_name.rb @@ -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') @@ -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) diff --git a/spec/rubocop/ast/resbody_node_spec.rb b/spec/rubocop/ast/resbody_node_spec.rb index c5aeb736a8f..fe6f0fc74fe 100644 --- a/spec/rubocop/ast/resbody_node_spec.rb +++ b/spec/rubocop/ast/resbody_node_spec.rb @@ -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' } diff --git a/spec/rubocop/cop/naming/rescued_exceptions_variable_name_spec.rb b/spec/rubocop/cop/naming/rescued_exceptions_variable_name_spec.rb index 333182727da..13efecb3062 100644 --- a/spec/rubocop/cop/naming/rescued_exceptions_variable_name_spec.rb +++ b/spec/rubocop/cop/naming/rescued_exceptions_variable_name_spec.rb @@ -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 @@ -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