Fix false negatives/positives

Author: ydakuka

config/default.yml

@@ -315,13 +315,7 @@ RSpec/DiscardedMatcher:
   Description: Checks for matchers that are used in void context.
   Enabled: pending
   VersionAdded: "<<next>>"
-  MatcherMethods:
-    - change
-    - have_received
-    - output
-    - receive
-    - receive_messages
-    - receive_message_chain
+  CustomMatcherMethods: []
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/DiscardedMatcher
 
 RSpec/DuplicatedMetadata:

lib/rubocop/cop/rspec/discarded_matcher.rb

@@ -9,7 +9,8 @@ module RSpec
       # standalone expressions have their result silently discarded.
       # This usually means a missing `.and` to chain compound matchers.
       #
-      # The list of matcher methods can be configured with `MatcherMethods`.
+      # The list of matcher methods can be configured
+      # with `CustomMatcherMethods`.
       #
       # @example
       #   # bad
@@ -37,10 +38,10 @@ class DiscardedMatcher < Base
         MSG = 'The result of `%<method>s` is not used. ' \
               'Did you mean to chain it with `.and`?'
 
-        # @!method includes_expectation?(node)
-        def_node_search :includes_expectation?, <<~PATTERN
-          (send nil? #Expectations.all ...)
-        PATTERN
+        MATCHER_METHODS = %i[
+          change have_received output
+          receive receive_messages receive_message_chain
+        ].to_set.freeze
 
         def on_send(node)
           check_discarded_matcher(node, node)
@@ -55,31 +56,50 @@ def on_block(node) # rubocop:disable InternalAffairs/NumblockHandler
         def check_discarded_matcher(send_node, node)
           return unless matcher_call?(send_node)
           return unless inside_example?(node)
+          return unless example_with_matcher_expectation?(node)
 
           target = find_outermost_chain(node)
           return unless void_value?(target)
 
           add_offense(target, message: format(MSG, method: node.method_name))
         end
 
+        def example_with_matcher_expectation?(node)
+          example_node =
+            node.each_ancestor(:block).find { |ancestor| example?(ancestor) }
+
+          example_node.each_descendant(:send).any? do |send_node|
+            expectation_with_matcher?(send_node)
+          end
+        end
+
+        def expectation_with_matcher?(node)
+          %i[to to_not not_to].include?(node.method_name) &&
+            node.arguments.any? do |arg|
+              arg.each_node(:send).any? { |s| matcher_call?(s) }
+            end
+        end
+
         def void_value?(node)
           case node.parent.type
-          when :begin
-            true
           when :block
             example?(node.parent)
-          when :when, :case
+          when :begin, :case, :when
             void_value?(node.parent)
           end
         end
 
         def matcher_call?(node)
-          node.receiver.nil? && matcher_methods.include?(node.method_name)
+          node.receiver.nil? && all_matcher_methods.include?(node.method_name)
+        end
+
+        def all_matcher_methods
+          @all_matcher_methods ||=
+            (MATCHER_METHODS + custom_matcher_methods).freeze
         end
 
-        def matcher_methods
-          @matcher_methods ||=
-            cop_config.fetch('MatcherMethods', []).to_set(&:to_sym).freeze
+        def custom_matcher_methods
+          cop_config.fetch('CustomMatcherMethods', []).map(&:to_sym)
         end
 
         def find_outermost_chain(node)

spec/rubocop/cop/rspec/discarded_matcher_spec.rb

@@ -60,6 +60,16 @@
     RUBY
   end
 
+  it 'does not register for `output` unrelated to matcher' do
+    expect_no_offenses(<<~RUBY)
+      specify do
+        output.rewind
+        parsed = JSON.parse(output.read.chomp)
+        expect(parsed["message"]).to eq("error message")
+      end
+    RUBY
+  end
+
   it 'registers an offense for standalone `have_received`' do
     expect_offense(<<~RUBY)
       specify do
@@ -104,6 +114,12 @@
     RUBY
   end
 
+  it 'does not register an offense for `change` in parenthesized arg' do
+    expect_no_offenses(<<~RUBY)
+      expect { result }.to (change { obj.bar })
+    RUBY
+  end
+
   it 'does not register an offense when `receive` is used with `allow`' do
     expect_no_offenses(<<~RUBY)
       specify do
@@ -183,7 +199,8 @@
     RUBY
   end
 
-  it 'does not register an offense for `change` in non-void `if` and `else` branches' do
+  it 'does not register an offense ' \
+     'for `change` in non-void `if` and `else` branches' do
     expect_no_offenses(<<~RUBY)
       specify do
         result = if condition
@@ -285,25 +302,18 @@ def expect_action
     RUBY
   end
 
-  context 'with custom MatcherMethods' do
+  context 'with custom CustomMatcherMethods' do
     let(:cop_config) do
-      { 'MatcherMethods' => %w[custom_matcher] }
+      { 'CustomMatcherMethods' => %w[custom_matcher] }
     end
 
     it 'registers an offense for configured custom matcher' do
       expect_offense(<<~RUBY)
         specify do
-          expect(foo).to receive(:bar)
-          custom_matcher(:foo)
-          ^^^^^^^^^^^^^^^^^^^^ The result of `custom_matcher` is not used. Did you mean to chain it with `.and`?
-        end
-      RUBY
-    end
-
-    it 'does not register an offense for default matchers not in config' do
-      expect_no_offenses(<<~RUBY)
-        specify do
-          change { obj.bar }
+          expect(foo).to \\
+            receive(:bar)
+            custom_matcher(:foo)
+            ^^^^^^^^^^^^^^^^^^^^ The result of `custom_matcher` is not used. Did you mean to chain it with `.and`?
         end
       RUBY
     end