Skip to content

Commit

Permalink
Fix an incorrect autocorrect for Style/RedundantException when mess…
Browse files Browse the repository at this point in the history
…age is not string
  • Loading branch information
ydah authored and bbatsov committed Oct 11, 2023
1 parent e162c99 commit 873cb9f
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#12177](https://github.com/rubocop/rubocop/pull/12177): Fix an incorrect autocorrect for `Style/RedundantException`. ([@ydah][])
44 changes: 32 additions & 12 deletions lib/rubocop/cop/style/redundant_exception.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,21 @@ module Cop
module Style
# Checks for RuntimeError as the argument of raise/fail.
#
# It checks for code like this:
#
# @example
# # Bad
# # bad
# raise RuntimeError, 'message'
#
# # Bad
# raise RuntimeError.new('message')
#
# # Good
# # good
# raise 'message'
#
# # bad - message is not a string
# raise RuntimeError, Object.new
# raise RuntimeError.new(Object.new)
#
# # good
# raise Object.new.to_s
#
class RedundantException < Base
extend AutoCorrector

Expand All @@ -30,26 +34,42 @@ def on_send(node)
fix_exploded(node) || fix_compact(node)
end

private

def fix_exploded(node)
exploded?(node) do |command, message|
add_offense(node, message: MSG_1) do |corrector|
if node.parenthesized?
corrector.replace(node, "#{command}(#{message.source})")
else
corrector.replace(node, "#{command} #{message.source}")
end
corrector.replace(node, replaced_exploded(node, command, message))
end
end
end

def replaced_exploded(node, command, message)
arg = string_message?(message) ? message.source : "#{message.source}.to_s"
arg = node.parenthesized? ? "(#{arg})" : " #{arg}"
"#{command}#{arg}"
end

def string_message?(message)
message.str_type? || message.dstr_type? || message.xstr_type?
end

def fix_compact(node)
compact?(node) do |new_call, message|
add_offense(node, message: MSG_2) do |corrector|
corrector.replace(new_call, message.source)
corrector.replace(new_call, replaced_compact(message))
end
end
end

def replaced_compact(message)
if string_message?(message)
message.source
else
"#{message.source}.to_s"
end
end

# @!method exploded?(node)
def_node_matcher :exploded?, <<~PATTERN
(send nil? ${:raise :fail} (const {nil? cbase} :RuntimeError) $_)
Expand Down
94 changes: 80 additions & 14 deletions spec/rubocop/cop/style/redundant_exception_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,59 +4,125 @@
shared_examples 'common behavior' do |keyword, runtime_error|
it "reports an offense for a #{keyword} with #{runtime_error}" do
expect_offense(<<~RUBY, keyword: keyword, runtime_error: runtime_error)
%{keyword} %{runtime_error}, msg
^{keyword}^^{runtime_error}^^^^^ Redundant `RuntimeError` argument can be removed.
%{keyword} %{runtime_error}, "message"
^{keyword}^^{runtime_error}^^^^^^^^^^^ Redundant `RuntimeError` argument can be removed.
RUBY

expect_correction(<<~RUBY)
#{keyword} msg
#{keyword} "message"
RUBY
end

it "reports an offense for a #{keyword} with #{runtime_error} and ()" do
expect_offense(<<~RUBY, keyword: keyword, runtime_error: runtime_error)
%{keyword}(%{runtime_error}, msg)
^{keyword}^^{runtime_error}^^^^^^ Redundant `RuntimeError` argument can be removed.
%{keyword}(%{runtime_error}, "message")
^{keyword}^^{runtime_error}^^^^^^^^^^^^ Redundant `RuntimeError` argument can be removed.
RUBY

expect_correction(<<~RUBY)
#{keyword}(msg)
#{keyword}("message")
RUBY
end

it "reports an offense for a #{keyword} with #{runtime_error}.new" do
expect_offense(<<~RUBY, keyword: keyword, runtime_error: runtime_error)
%{keyword} %{runtime_error}.new msg
^{keyword}^^{runtime_error}^^^^^^^^ Redundant `RuntimeError.new` call can be replaced with just the message.
%{keyword} %{runtime_error}.new "message"
^{keyword}^^{runtime_error}^^^^^^^^^^^^^^ Redundant `RuntimeError.new` call can be replaced with just the message.
RUBY

expect_correction(<<~RUBY)
#{keyword} msg
#{keyword} "message"
RUBY
end

it "reports an offense for a #{keyword} with #{runtime_error}.new" do
expect_offense(<<~RUBY, keyword: keyword, runtime_error: runtime_error)
%{keyword} %{runtime_error}.new(msg)
^{keyword}^^{runtime_error}^^^^^^^^^ Redundant `RuntimeError.new` call can be replaced with just the message.
%{keyword} %{runtime_error}.new("message")
^{keyword}^^{runtime_error}^^^^^^^^^^^^^^^ Redundant `RuntimeError.new` call can be replaced with just the message.
RUBY

expect_correction(<<~RUBY)
#{keyword} msg
#{keyword} "message"
RUBY
end

it "accepts a #{keyword} with #{runtime_error} if it does not have 2 args" do
expect_no_offenses("#{keyword} #{runtime_error}, msg, caller")
expect_no_offenses("#{keyword} #{runtime_error}, 'message', caller")
end

it 'accepts rescue w/ non redundant error' do
expect_no_offenses "#{keyword} OtherError, msg"
expect_no_offenses "#{keyword} OtherError, 'message'"
end
end

include_examples 'common behavior', 'raise', 'RuntimeError'
include_examples 'common behavior', 'raise', '::RuntimeError'
include_examples 'common behavior', 'fail', 'RuntimeError'
include_examples 'common behavior', 'fail', '::RuntimeError'

it 'registers an offense for raise with RuntimeError, "#{message}"' do
expect_offense(<<~'RUBY')
raise RuntimeError, "#{message}"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError` argument can be removed.
RUBY

expect_correction(<<~'RUBY')
raise "#{message}"
RUBY
end

it 'registers an offense for raise with RuntimeError, `command`' do
expect_offense(<<~RUBY)
raise RuntimeError, `command`
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError` argument can be removed.
RUBY

expect_correction(<<~RUBY)
raise `command`
RUBY
end

it 'registers an offense for raise with RuntimeError, Object.new' do
expect_offense(<<~RUBY)
raise RuntimeError, Object.new
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError` argument can be removed.
RUBY

expect_correction(<<~RUBY)
raise Object.new.to_s
RUBY
end

it 'registers an offense for raise with RuntimeError.new, Object.new and parans' do
expect_offense(<<~RUBY)
raise RuntimeError.new(Object.new)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError.new` call can be replaced with just the message.
RUBY

expect_correction(<<~RUBY)
raise Object.new.to_s
RUBY
end

it 'registers an offense for raise with RuntimeError.new, Object.new no parens' do
expect_offense(<<~RUBY)
raise RuntimeError.new Object.new
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError.new` call can be replaced with just the message.
RUBY

expect_correction(<<~RUBY)
raise Object.new.to_s
RUBY
end

it 'registers an offense for raise with RuntimeError, valiable' do
expect_offense(<<~RUBY)
raise RuntimeError, valiable
^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Redundant `RuntimeError` argument can be removed.
RUBY

expect_correction(<<~RUBY)
raise valiable.to_s
RUBY
end
end

0 comments on commit 873cb9f

Please sign in to comment.