diff --git a/changelog/fix_an_incorrect_autocorrect_for_style_redundant_exception.md b/changelog/fix_an_incorrect_autocorrect_for_style_redundant_exception.md new file mode 100644 index 000000000000..f1611a55d191 --- /dev/null +++ b/changelog/fix_an_incorrect_autocorrect_for_style_redundant_exception.md @@ -0,0 +1 @@ +* [#12177](https://github.com/rubocop/rubocop/pull/12177): Fix an incorrect autocorrect for `Style/RedundantException`. ([@ydah][]) diff --git a/lib/rubocop/cop/style/redundant_exception.rb b/lib/rubocop/cop/style/redundant_exception.rb index 03e09f6eaa08..ee557e732f13 100644 --- a/lib/rubocop/cop/style/redundant_exception.rb +++ b/lib/rubocop/cop/style/redundant_exception.rb @@ -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 @@ -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) $_) diff --git a/spec/rubocop/cop/style/redundant_exception_spec.rb b/spec/rubocop/cop/style/redundant_exception_spec.rb index a02ad1249ccb..1e36452690ad 100644 --- a/spec/rubocop/cop/style/redundant_exception_spec.rb +++ b/spec/rubocop/cop/style/redundant_exception_spec.rb @@ -4,54 +4,54 @@ 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 @@ -59,4 +59,70 @@ 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