Add new RSpec/RepeatedSubjectCall cop

.rubocop.yml

@@ -179,6 +179,8 @@ RSpec/RedundantAround:
   Enabled: true
 RSpec/RedundantPredicateMatcher:
   Enabled: true
+RSpec/RepeatedSubjectCall:
+  Enabled: true
 RSpec/SkipBlockInsideExample:
   Enabled: true
 RSpec/SortMetadata:

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - Add new `RSpec/RedundantPredicateMatcher` cop. ([@ydah])
 - Add support for correcting "it will" (future tense) for `RSpec/ExampleWording`. ([@jdufresne])
+- Add new `RSpec/RepeatedSubjectCall` cop. ([@drcapulet])
 
 ## 2.25.0 (2023-10-27)
 
@@ -836,6 +837,7 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
 [@deivid-rodriguez]: https://github.com/deivid-rodriguez
 [@dgollahon]: https://github.com/dgollahon
 [@dmitrytsepelev]: https://github.com/dmitrytsepelev
+[@drcapulet]: https://github.com/drcapulet
 [@drowze]: https://github.com/Drowze
 [@dswij]: https://github.com/dswij
 [@dvandersluis]: https://github.com/dvandersluis

config/default.yml

@@ -807,6 +807,12 @@ RSpec/RepeatedIncludeExample:
   VersionAdded: '1.44'
   Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/RepeatedIncludeExample
 
+RSpec/RepeatedSubjectCall:
+  Description: Checks for repeated calls to subject missing that it is memoized.
+  Enabled: pending
+  VersionAdded: "<<next>>"
+  Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/RepeatedSubjectCall
+
 RSpec/ReturnFromStub:
   Description: Checks for consistent style of stub's return setting.
   Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -86,6 +86,7 @@
 * xref:cops_rspec.adoc#rspecrepeatedexamplegroupbody[RSpec/RepeatedExampleGroupBody]
 * xref:cops_rspec.adoc#rspecrepeatedexamplegroupdescription[RSpec/RepeatedExampleGroupDescription]
 * xref:cops_rspec.adoc#rspecrepeatedincludeexample[RSpec/RepeatedIncludeExample]
+* xref:cops_rspec.adoc#rspecrepeatedsubjectcall[RSpec/RepeatedSubjectCall]
 * xref:cops_rspec.adoc#rspecreturnfromstub[RSpec/ReturnFromStub]
 * xref:cops_rspec.adoc#rspecscatteredlet[RSpec/ScatteredLet]
 * xref:cops_rspec.adoc#rspecscatteredsetup[RSpec/ScatteredSetup]

docs/modules/ROOT/pages/cops_rspec.adoc

@@ -4792,6 +4792,46 @@ end
 
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/RepeatedIncludeExample
 
+== RSpec/RepeatedSubjectCall
+
+|===
+| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed
+
+| Pending
+| Yes
+| No
+| <<next>>
+| -
+|===
+
+Checks for repeated calls to subject missing that it is memoized.
+
+=== Examples
+
+[source,ruby]
+----
+# bad
+it do
+  subject
+  expect { subject }.to not_change { A.count }
+end
+
+it do
+  expect { subject }.to change { A.count }
+  expect { subject }.to not_change { A.count }
+end
+
+# good
+it do
+  expect { my_method }.to change { A.count }
+  expect { my_method }.to not_change { A.count }
+end
+----
+
+=== References
+
+* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/RepeatedSubjectCall
+
 == RSpec/ReturnFromStub
 
 |===

lib/rubocop/cop/rspec/repeated_subject_call.rb

@@ -0,0 +1,121 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Checks for repeated calls to subject missing that it is memoized.
+      #
+      # @example
+      #   # bad
+      #   it do
+      #     subject
+      #     expect { subject }.to not_change { A.count }
+      #   end
+      #
+      #   it do
+      #     expect { subject }.to change { A.count }
+      #     expect { subject }.to not_change { A.count }
+      #   end
+      #
+      #   # good
+      #   it do
+      #     expect { my_method }.to change { A.count }
+      #     expect { my_method }.to not_change { A.count }
+      #   end
+      #
+      class RepeatedSubjectCall < Base
+        include TopLevelGroup
+
+        BLOCK_MSG = 'Calls to subject are memoized, this block is misleading'
+
+        # @!method subject?(node)
+        #   Find a named or unnamed subject definition
+        #
+        #   @example anonymous subject
+        #     subject?(parse('subject { foo }').ast) do |name|
+        #       name # => :subject
+        #     end
+        #
+        #   @example named subject
+        #     subject?(parse('subject(:thing) { foo }').ast) do |name|
+        #       name # => :thing
+        #     end
+        #
+        #   @param node [RuboCop::AST::Node]
+        #
+        #   @yield [Symbol] subject name
+        def_node_matcher :subject?, <<-PATTERN
+          (block
+            (send nil?
+              { #Subjects.all (sym $_) | $#Subjects.all }
+            ) args ...)
+        PATTERN
+
+        # @!method subject_calls(node, method_name)
+        def_node_search :subject_calls, <<~PATTERN
+          (send nil? %)
+        PATTERN
+
+        def on_top_level_group(node)
+          @subjects_by_node = detect_subjects_in_scope(node)
+
+          detect_offenses_in_block(node)
+        end
+
+        private
+
+        def detect_offense(example_node, subject_node)
+          walker = subject_node
+
+          while walker.parent? && walker.parent != example_node.body
+            walker = walker.parent
+
+            if walker.block_type? && walker.method?(:expect)
+              add_offense(walker, message: BLOCK_MSG)
+              return
+            end
+          end
+        end
+
+        def detect_offenses_in_block(node, subject_names = [])
+          subject_names = [*subject_names, *@subjects_by_node[node]]
+
+          if example?(node)
+            return detect_offenses_in_example(node, subject_names)
+          end
+
+          node.each_child_node(:send, :def, :block, :begin) do |child|
+            detect_offenses_in_block(child, subject_names)
+          end
+        end
+
+        def detect_offenses_in_example(node, subject_names)
+          return unless node.body
+
+          subjects_used = Hash.new(false)
+
+          subject_calls(node.body, Set[*subject_names, :subject]).each do |call|
+            if subjects_used[call.method_name]
+              detect_offense(node, call)
+            else
+              subjects_used[call.method_name] = true
+            end
+          end
+        end
+
+        def detect_subjects_in_scope(node)
+          node.each_descendant(:block).with_object({}) do |child, h|
+            name = subject?(child)
+            next unless name
+
+            outer_example_group = child.each_ancestor(:block).find do |a|
+              example_group?(a)
+            end
+
+            (h[outer_example_group] ||= []) << name
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rspec_cops.rb

@@ -112,6 +112,7 @@
 require_relative 'rspec/repeated_example_group_body'
 require_relative 'rspec/repeated_example_group_description'
 require_relative 'rspec/repeated_include_example'
+require_relative 'rspec/repeated_subject_call'
 require_relative 'rspec/return_from_stub'
 require_relative 'rspec/scattered_let'
 require_relative 'rspec/scattered_setup'

spec/rubocop/cop/rspec/repeated_subject_call_spec.rb

@@ -0,0 +1,91 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::RSpec::RepeatedSubjectCall do
+  it 'registers an offense for a singular block' do
+    expect_offense(<<-RUBY)
+      RSpec.describe Foo do
+        it do
+          subject
+          expect { subject }.to not_change { Foo.count }
+          ^^^^^^^^^^^^^^^^^^ Calls to subject are memoized, this block is misleading
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense for repeated blocks' do
+    expect_offense(<<-RUBY)
+      RSpec.describe Foo do
+        it do
+          expect { subject }.to change { Foo.count }
+          expect { subject }.to not_change { Foo.count }
+          ^^^^^^^^^^^^^^^^^^ Calls to subject are memoized, this block is misleading
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense for nested blocks' do
+    expect_offense(<<-RUBY)
+      RSpec.describe Foo do
+        it do
+          expect(subject.a).to eq(3)
+          nested_block do
+            expect { on_shard(:europe) { subject } }.to not_change { Foo.count }
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Calls to subject are memoized, this block is misleading
+          end
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense for custom subjects' do
+    expect_offense(<<-RUBY)
+      RSpec.describe Foo do
+        subject(:bar) { do_something }
+
+        it do
+          bar
+          expect { bar }.to not_change { Foo.count }
+          ^^^^^^^^^^^^^^ Calls to subject are memoized, this block is misleading
+        end
+      end
+    RUBY
+  end
+
+  it 'registers no offenses for no block' do
+    expect_no_offenses(<<~RUBY)
+      RSpec.describe Foo do
+        it do
+          expect(subject.a).to eq(3)
+          expect(subject.b).to eq(4)
+        end
+      end
+    RUBY
+  end
+
+  it 'registers no offenses for block first' do
+    expect_no_offenses(<<~RUBY)
+      RSpec.describe Foo do
+        it do
+          expect { subject }.to change { Foo.count }
+          expect(subject.b).to eq(4)
+        end
+      end
+    RUBY
+  end
+
+  it 'registers no offenses different subjects' do
+    expect_no_offenses(<<-RUBY)
+      RSpec.describe Foo do
+        subject { do_something_else }
+        subject(:bar) { do_something }
+
+        it do
+          expect { bar }.to not_change { Foo.count }
+          expect { subject }.to not_change { Foo.count }
+        end
+      end
+    RUBY
+  end
+end