diff --git a/CHANGELOG.md b/CHANGELOG.md index 5626e6631d2a..22f07a006b8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ * [#3088](https://github.com/bbatsov/rubocop/pull/3088): Ignore offenses that involve conflicting HEREDOCs in the `Style/Multiline*BraceLayout` cops. ([@panthomakos][]) * [#3083](https://github.com/bbatsov/rubocop/issues/3083): Do not register an offense for splat block args in `Style/SymbolProc`. ([@rrosenblum][]) * [#3063](https://github.com/bbatsov/rubocop/issues/3063): Don't auto-correct `a + \` into `a + \\` in `Style/LineEndConcatenation`. ([@jonas054][]) +* [#3034](https://github.com/bbatsov/rubocop/issues/3034): Report offenses for `RuntimeError.new(msg)` in `Style/RedundantException`. ([@jonas054][]) ### Changes diff --git a/lib/rubocop/cop/style/redundant_exception.rb b/lib/rubocop/cop/style/redundant_exception.rb index 35c4aa358e19..c52574cb419b 100644 --- a/lib/rubocop/cop/style/redundant_exception.rb +++ b/lib/rubocop/cop/style/redundant_exception.rb @@ -6,36 +6,50 @@ module Cop module Style # This cop checks for RuntimeError as the argument of raise/fail. # - # Currently it checks for code like this: + # It checks for code like this: # # @example - # + # # Bad # raise RuntimeError, 'message' + # + # # Bad + # raise RuntimeError.new('message') + # + # # Good + # raise 'message' class RedundantException < Cop - MSG = 'Redundant `RuntimeError` argument can be removed.'.freeze - - TARGET_NODE = s(:const, nil, :RuntimeError) + MSG_1 = 'Redundant `RuntimeError` argument can be removed.'.freeze + MSG_2 = 'Redundant `RuntimeError.new` call can be replaced with ' \ + 'just the message.'.freeze def on_send(node) - return unless node.command?(:raise) || node.command?(:fail) - - _receiver, _selector, *args = *node - - return unless args.size == 2 - - first_arg, = *args - - add_offense(first_arg, :expression) if first_arg == TARGET_NODE + exploded?(node) { return add_offense(node, :expression, MSG_1) } + compact?(node) { add_offense(node, :expression, MSG_2) } end - # switch `raise RuntimeError, 'message'` to `raise 'message'` + # Switch `raise RuntimeError, 'message'` to `raise 'message'`, and + # `raise RuntimeError.new('message')` to `raise 'message'`. def autocorrect(node) - start_range = node.source_range.begin - no_comma = range_with_surrounding_comma(node.source_range.end, :right) - comma_range = start_range.join(no_comma) - final_range = range_with_surrounding_space(comma_range, :right) - ->(corrector) { corrector.replace(final_range, '') } + exploded?(node) do |command, message| + return lambda do |corrector| + corrector.replace(node.source_range, + "#{command} #{message.source}") + end + end + compact?(node) do |new_call, message| + lambda do |corrector| + corrector.replace(new_call.source_range, message.source) + end + end end + + def_node_matcher :exploded?, <<-PATTERN + (send nil ${:raise :fail} (const nil :RuntimeError) $_) + PATTERN + + def_node_matcher :compact?, <<-PATTERN + (send nil {:raise :fail} $(send (const nil :RuntimeError) :new $_)) + PATTERN end end end diff --git a/spec/rubocop/cop/style/redundant_exception_spec.rb b/spec/rubocop/cop/style/redundant_exception_spec.rb index a39d4e88cf6f..738fba982ecb 100644 --- a/spec/rubocop/cop/style/redundant_exception_spec.rb +++ b/spec/rubocop/cop/style/redundant_exception_spec.rb @@ -6,55 +6,66 @@ describe RuboCop::Cop::Style::RedundantException do subject(:cop) { described_class.new } - it 'reports an offense for a raise with RuntimeError' do - inspect_source(cop, 'raise RuntimeError, msg') - expect(cop.offenses.size).to eq(1) - end + shared_examples 'common behavior' do |keyword| + it "reports an offense for a #{keyword} with RuntimeError" do + src = "#{keyword} RuntimeError, msg" + inspect_source(cop, src) + expect(cop.highlights).to eq([src]) + expect(cop.messages) + .to eq(['Redundant `RuntimeError` argument can be removed.']) + end - it 'reports an offense for a fail with RuntimeError' do - inspect_source(cop, 'fail RuntimeError, msg') - expect(cop.offenses.size).to eq(1) - end + it "reports an offense for a #{keyword} with RuntimeError.new" do + src = "#{keyword} RuntimeError.new(msg)" + inspect_source(cop, src) + expect(cop.highlights).to eq([src]) + expect(cop.messages) + .to eq(['Redundant `RuntimeError.new` call can be replaced with ' \ + 'just the message.']) + end - it 'accepts a raise with RuntimeError if it does not have 2 args' do - inspect_source(cop, 'raise RuntimeError, msg, caller') - expect(cop.offenses).to be_empty - end + it "accepts a #{keyword} with RuntimeError if it does not have 2 args" do + inspect_source(cop, "#{keyword} RuntimeError, msg, caller") + expect(cop.offenses).to be_empty + end - it 'accepts a fail with RuntimeError if it does not have 2 args' do - inspect_source(cop, 'fail RuntimeError, msg, caller') - expect(cop.offenses).to be_empty - end + it "auto-corrects a #{keyword} RuntimeError by removing RuntimeError" do + src = "#{keyword} RuntimeError, msg" + result_src = "#{keyword} msg" + new_src = autocorrect_source(cop, src) + expect(new_src).to eq(result_src) + end - it 'auto-corrects a raise by removing RuntimeError' do - src = 'raise RuntimeError, msg' - result_src = 'raise msg' - new_src = autocorrect_source(cop, src) - expect(new_src).to eq(result_src) - end + it "auto-corrects a #{keyword} RuntimeError.new with parentheses by " \ + 'removing RuntimeError.new' do + src = "#{keyword} RuntimeError.new(msg)" + result_src = "#{keyword} msg" + new_src = autocorrect_source(cop, src) + expect(new_src).to eq(result_src) + end - it 'auto-corrects a fil by removing RuntimeError' do - src = 'fail RuntimeError, msg' - result_src = 'fail msg' - new_src = autocorrect_source(cop, src) - expect(new_src).to eq(result_src) - end + it "auto-corrects a #{keyword} RuntimeError.new without parentheses by " \ + 'removing RuntimeError.new' do + src = "#{keyword} RuntimeError.new msg" + result_src = "#{keyword} msg" + new_src = autocorrect_source(cop, src) + expect(new_src).to eq(result_src) + end - it 'does not modify raise w/ RuntimeError if it does not have 2 args' do - src = 'raise runtimeError, msg, caller' - new_src = autocorrect_source(cop, src) - expect(new_src).to eq(src) - end + it "does not modify #{keyword} w/ RuntimeError if it does not have 2 " \ + 'args' do + src = "#{keyword} runtimeError, msg, caller" + new_src = autocorrect_source(cop, src) + expect(new_src).to eq(src) + end - it 'does not modify fail w/ RuntimeError if it does not have 2 args' do - src = 'fail RuntimeError, msg, caller' - new_src = autocorrect_source(cop, src) - expect(new_src).to eq(src) + it 'does not modify rescue w/ non redundant error' do + src = "#{keyword} OtherError, msg" + new_src = autocorrect_source(cop, src) + expect(new_src).to eq(src) + end end - it 'does not modify rescue w/ non redundant error' do - src = 'fail OtherError, msg' - new_src = autocorrect_source(cop, src) - expect(new_src).to eq(src) - end + include_examples 'common behavior', 'raise' + include_examples 'common behavior', 'fail' end