Add new RSpec/MemoizedMatcher cop

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Master (Unreleased)
 
+* Add new `RSpec/MemoizedMatcher` cop. ([@pirj][])
+
 ## 2.2.0 (2021-02-02)
 
 * Fix `HooksBeforeExamples`, `LeadingSubject`, `LetBeforeExamples` and `ScatteredLet` autocorrection to take into account inline comments and comments immediately before the moved node. ([@Darhazer][])

config/default.yml

@@ -433,6 +433,12 @@ RSpec/LetSetup:
   VersionAdded: '1.7'
   StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/LetSetup
 
+RSpec/MemoizedMatcher:
+  Description: Matchers used in expectations should not be defined in memoized helpers.
+  Enabled: pending
+  VersionAdded: '2.3'
+  StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MemoizedMatcher
+
 RSpec/MessageChain:
   Description: Check that chains of messages are not being stubbed.
   Enabled: true

docs/modules/ROOT/pages/cops.adoc

@@ -46,6 +46,7 @@
 * xref:cops_rspec.adoc#rspecleakyconstantdeclaration[RSpec/LeakyConstantDeclaration]
 * xref:cops_rspec.adoc#rspecletbeforeexamples[RSpec/LetBeforeExamples]
 * xref:cops_rspec.adoc#rspecletsetup[RSpec/LetSetup]
+* xref:cops_rspec.adoc#rspecmemoizedmatcher[RSpec/MemoizedMatcher]
 * xref:cops_rspec.adoc#rspecmessagechain[RSpec/MessageChain]
 * xref:cops_rspec.adoc#rspecmessageexpectation[RSpec/MessageExpectation]
 * xref:cops_rspec.adoc#rspecmessagespies[RSpec/MessageSpies]

docs/modules/ROOT/pages/cops_rspec.adoc

@@ -2287,6 +2287,57 @@ end
 
 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/LetSetup
 
+== RSpec/MemoizedMatcher
+
+|===
+| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+
+| Pending
+| Yes
+| No
+| 2.3
+| -
+|===
+
+Matchers used in expectations should not be defined in memoized helpers.
+
+=== Examples
+
+[source,ruby]
+----
+# bad - plain data
+let(:expected_value) { {a: 1, b: 2, c: 3} }
+
+it 'returns a proper hash' do
+  expect(parser.parse).to eq expected_value
+end
+
+# bad - compound matcher defined in a memoized helper
+let(:expected) { be_positive.and be_rational }
+
+it 'sums up to a positive rational' do
+  expect(calculator.sum).to expected
+end
+
+# good - plain data used inline
+it 'returns a proper hash' do
+  expect(parser.parse).to eq({a: 1, b: 2, c: 3})
+end
+
+# good - compound matcher is extracted to a method
+def be_positive_and_rational
+  be_positive.and be_rational
+end
+
+it 'sums up to a positive rational' do
+  expect(calculator.sum).to be_positive_and_rational
+end
+----
+
+=== References
+
+* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MemoizedMatcher
+
 == RSpec/MessageChain
 
 |===

lib/rubocop/cop/rspec/memoized_matcher.rb

@@ -0,0 +1,124 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Matchers used in expectations should not be defined in memoized helpers.
+      #
+      # @example
+      #   # bad - plain data
+      #   let(:expected_value) { {a: 1, b: 2, c: 3} }
+      #
+      #   it 'returns a proper hash' do
+      #     expect(parser.parse).to eq expected_value
+      #   end
+      #
+      #   # bad - compound matcher defined in a memoized helper
+      #   let(:expected) { be_positive.and be_rational }
+      #
+      #   it 'sums up to a positive rational' do
+      #     expect(calculator.sum).to expected
+      #   end
+      #
+      #   # good - plain data used inline
+      #   it 'returns a proper hash' do
+      #     expect(parser.parse).to eq({a: 1, b: 2, c: 3})
+      #   end
+      #
+      #   # good - compound matcher is extracted to a method
+      #   def be_positive_and_rational
+      #     be_positive.and be_rational
+      #   end
+      #
+      #   it 'sums up to a positive rational' do
+      #     expect(calculator.sum).to be_positive_and_rational
+      #   end
+      class MemoizedMatcher < Base
+        include Variable
+
+        MSG = 'Do not memoize matchers'
+
+        # @!method expectation_matchers(example_group)
+        #   Match expectation matchers used in the example group
+        #
+        #   @example source that matches
+        #     describe Foo do
+        #       it 'contains a list with a single one' do
+        #         is_expected.to contain_exactly(eq_one)
+        #       end
+        #     end
+        #
+        #   @param example_group [RuboCop::AST::Node]
+        #   @yieldparam [RuboCop::AST::Node] matchers
+        def_node_search :expectation_matchers, <<~PATTERN
+          (send
+            {
+              (send nil? #Expectations.all _ ?)          # expect(...), expect_any_instance_of or is_expected
+              (block (send nil? #Expectations.all) ...)  # expect { ... }
+            }
+            #Runners.all  # .to or .not_to
+            $send         # matcher
+            _ ?           # an optional expectation failure message
+          )
+        PATTERN
+
+        # @!method memoized_helper?(statement)
+        #   Match memoized helpers
+        #
+        #   @example source that matches
+        #     let(:be_heavy) { be > 5.ounces }
+        #
+        #   @param statement [RuboCop::AST::Node]
+        #   @yield [helper_name, block_body] Gives helper name and definition
+        def_node_matcher :memoized_helper?, <<~PATTERN
+          (block
+            (send nil?
+              { #Helpers.all #Subjects.all }
+              ({sym str} $_name)
+            )
+            _block_args $_block_body
+          )
+        PATTERN
+
+        # @!method individual_matchers(matcher)
+        #   Match individual matchers in a compound matcher
+        #
+        #   @example source that matches
+        #     contains_exactly(be_one, be_two)
+        #
+        #   @param matcher [RuboCop::AST::Node]
+        #   @yieldparam [RuboCop::AST::Node] individual matchers
+        def_node_search :individual_matchers, <<~PATTERN
+          (send nil? $%)
+        PATTERN
+
+        def on_block(example_group)
+          return unless example_group?(example_group)
+
+          helpers = helpers(example_group)
+          helpers_names = Set.new(helpers.keys)
+
+          expectation_matchers(example_group) do |matcher|
+            individual_matchers(matcher, helpers_names) do |helper_name|
+              add_offense(helpers[helper_name])
+            end
+          end
+        end
+
+        private
+
+        def helpers(example_group)
+          helpers = {}
+          return helpers unless example_group.body
+
+          example_group.body.each_child_node do |statement|
+            memoized_helper?(statement) do |name, body|
+              helpers[name.to_sym] = body
+            end
+          end
+          helpers
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rspec_cops.rb

@@ -58,6 +58,7 @@
 require_relative 'rspec/leaky_constant_declaration'
 require_relative 'rspec/let_before_examples'
 require_relative 'rspec/let_setup'
+require_relative 'rspec/memoized_matcher'
 require_relative 'rspec/message_chain'
 require_relative 'rspec/message_expectation'
 require_relative 'rspec/message_spies'

spec/rubocop/cop/rspec/memoized_matcher_spec.rb

@@ -0,0 +1,152 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::RSpec::MemoizedMatcher do
+  it 'flags memoized matchers' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:eq_one) { eq(1) }
+                       ^^^^^ Do not memoize matchers
+        it { is_expected.to eq_one }
+      end
+    RUBY
+  end
+
+  it 'ignores memoized helpers when not used in an expectation' do
+    expect_no_offenses(<<~RUBY)
+      describe Foo do
+        let(:eq_one) { eq(1) }
+        it { is_expected.to be_ok }
+      end
+    RUBY
+  end
+
+  it 'ignores memoized helpers when used as an actual' do
+    expect_no_offenses(<<~RUBY)
+      describe Foo do
+        let(:one) { eq(1) }
+        it { expect(one).to be_match(1) }
+      end
+    RUBY
+  end
+
+  it 'flags memoized matchers on all levels' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:eq_one) { eq(1) }
+                       ^^^^^ Do not memoize matchers
+        context do
+          it { is_expected.to eq_one }
+        end
+      end
+    RUBY
+  end
+
+  it 'flags memoized matchers when `expect` is used' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:eq_one) { eq(1) }
+                       ^^^^^ Do not memoize matchers
+        it { expect(1).to eq_one }
+      end
+    RUBY
+  end
+
+  it 'flags memoized matchers with a failure message in expectation' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:eq_one) { eq(1) }
+                       ^^^^^ Do not memoize matchers
+        it { is_expected.to eq_one, 'not one' }
+      end
+    RUBY
+  end
+
+  it 'flags memoized matchers with block expectations' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:change_something) { change { something } }
+                                 ^^^^^^^^^^^^^^^^^^^^ Do not memoize matchers
+        it do
+          expect { nothing }.to change_something
+        end
+      end
+    RUBY
+  end
+
+  it 'flags memoized matchers used in an compound expectation' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:eq_one) { eq(1) }
+                       ^^^^^ Do not memoize matchers
+        it { is_expected.to be_positive.and eq_one }
+      end
+    RUBY
+  end
+
+  it 'flags memoized argument matchers' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:eq_one) { eq(1) }
+                       ^^^^^ Do not memoize matchers
+        it { is_expected.to contain_exactly(eq_one) }
+      end
+    RUBY
+  end
+
+  it 'flags memoized matchers defined using a string' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let('eq_one') { eq(1) }
+                        ^^^^^ Do not memoize matchers
+        it { is_expected.to eq_one }
+      end
+    RUBY
+  end
+
+  it 'flags several memoized matchers' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:something) { 1 }
+        let(:eq_one) { eq(1) }
+                       ^^^^^ Do not memoize matchers
+        let(:eq_two) { eq(2) }
+                       ^^^^^ Do not memoize matchers
+        it { is_expected.to eq_one }
+        it { is_expected.to eq_two }
+        it { is_expected.to be_true }
+      end
+    RUBY
+  end
+
+  it 'flags a named `subject` used as a matcher' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        subject(:eq_one) { eq(1) }
+                           ^^^^^ Do not memoize matchers
+        it { is_expected.to eq_one }
+      end
+    RUBY
+  end
+
+  it 'flags memoized matchers with `expect_any_instance_of`' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:become_closed) { receive(:close!).with(:immediately) }
+                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Do not memoize matchers
+        it { expect_any_instance_of(Office).to become_closed }
+      end
+    RUBY
+  end
+
+  it 'flags memoized matchers used in a nested example group' do
+    expect_offense(<<~RUBY)
+      describe Foo do
+        let(:eq_one) { eq(1) }
+                       ^^^^^ Do not memoize matchers
+        context 'at the bar' do
+          it { is_expected.to eq_one }
+        end
+      end
+    RUBY
+  end
+end