RSpec/SubjectStub. Refactor and decrease complexity

lib/rubocop/cop/rspec/subject_stub.rb

@@ -1,5 +1,7 @@
 # frozen_string_literal: true
 
+require 'set'
+
 module RuboCop
   module Cop
     module RSpec
@@ -20,6 +22,8 @@ module RSpec
       #   end
       #
       class SubjectStub < Cop
+        include RuboCop::RSpec::TopLevelDescribe
+
         MSG = 'Do not stub methods of the object under test.'
 
         # @!method subject(node)
@@ -73,72 +77,38 @@ class SubjectStub < Cop
             } ...)
         PATTERN
 
-        def on_block(node)
-          return unless example_group?(node)
+        def on_top_level_describe(node, _args)
+          @explicit_subjects = find_all_explicit_subjects(node.parent)
 
-          find_subject_stub(node) do |stub|
+          find_subject_expectations(node.parent) do |stub|
             add_offense(stub)
           end
         end
 
         private
 
-        # Find subjects within tree and then find (send) nodes for that subject
-        #
-        # @param node [RuboCop::Node] example group
-        #
-        # @yield [RuboCop::Node] message expectations for subject
-        def find_subject_stub(node, &block)
-          find_subject(node) do |subject_name, context|
-            find_subject_expectation(context, subject_name, &block)
+        def find_all_explicit_subjects(node)
+          node.each_descendant(:block).each_with_object({}) do |child, h|
+            name = subject(child)
+            if name
+              h[child.parent.parent] ||= []
+              h[child.parent.parent] << name
+            end
           end
         end
 
-        # Find a subject message expectation
-        #
-        # @param node [RuboCop::Node]
-        # @param subject_name [Symbol] name of subject
-        #
-        # @yield [RuboCop::Node] message expectation
-        def find_subject_expectation(node, subject_name, &block)
-          # Do not search node if it is an example group with its own subject.
-          return if example_group?(node) && redefines_subject?(node)
-
-          # Yield the current node if it is a message expectation.
-          yield(node) if message_expectation?(node, subject_name)
-
-          # Recurse through node's children looking for a message expectation.
-          node.each_child_node do |child|
-            find_subject_expectation(child, subject_name, &block)
+        def find_subject_expectations(node, subject_names = [], &block)
+          if example_group?(node) && @explicit_subjects[node]
+            subject_names = @explicit_subjects[node]
           end
-        end
 
-        # Check if node's children contain a subject definition
-        #
-        # @param node [RuboCop::Node]
-        #
-        # @return [Boolean]
-        def redefines_subject?(node)
-          node.each_child_node.any? do |child|
-            subject(child) || redefines_subject?(child)
+          expectation_detected = (subject_names + [:subject]).any? do |name|
+            message_expectation?(node, name)
           end
-        end
-
-        # Find a subject definition
-        #
-        # @param node [RuboCop::Node]
-        # @param parent [RuboCop::Node,nil]
-        #
-        # @yieldparam subject_name [Symbol] name of subject being defined
-        # @yieldparam parent [RuboCop::Node] parent of subject definition
-        def find_subject(node, parent: nil, &block)
-          # An implicit subject is defined by RSpec when no subject is declared
-          subject_name = subject(node) || :subject
-
-          yield(subject_name, parent) if parent
+          return yield(node) if expectation_detected
 
           node.each_child_node do |child|
-            find_subject(child, parent: node, &block)
+            find_subject_expectations(child, subject_names, &block)
           end
         end
       end

spec/rubocop/cop/rspec/subject_stub_spec.rb

@@ -20,6 +20,26 @@
     RUBY
   end
 
+  it 'flags when subject is stubbed and there are several named subjects ' \
+     'in the same example group', :wip do
+    expect_offense(<<-RUBY)
+      describe Foo do
+        subject(:foo) { described_class.new }
+        subject(:bar) { described_class.new }
+        subject(:baz) { described_class.new }
+
+        before do
+          allow(bar).to receive(:bar).and_return(baz)
+          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not stub methods of the object under test.
+        end
+
+        it 'uses expect twice' do
+          expect(foo.bar).to eq(baz)
+        end
+      end
+    RUBY
+  end
+
   it 'flags when subject is mocked' do
     expect_offense(<<-RUBY)
       describe Foo do