[Fix rubocop#3034] Report offenses also for compact style in Redundan…

…tException

Until now we have only had support for reporting
`raise RuntimeError, msg`. Add support for reporting
`raise RuntimeError.new(msg)`.

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
 

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

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