Merge pull request #863 from mockdeep/rf-no_let

Add MultipleMemoizedHelpers cop

Author: pirj

CHANGELOG.md

@@ -11,6 +11,7 @@
 * Improve `RSpec/NestedGroups`, `RSpec/FilePath`, `RSpec/DescribeMethod`, `RSpec/MultipleDescribes`, `RSpec/DescribeClass`'s top-level example group detection. ([@pirj][])
 * Add detection of `let!` with a block-pass or a string literal to `RSpec/LetSetup`. ([@pirj][])
 * Add `IgnoredPatterns` configuration option to `RSpec/VariableName`. ([@jtannas][])
+* Add `RSpec/MultipleMemoizedHelpers` cop. ([@mockdeep][])
 
 ## 1.42.0 (2020-07-09)
 
@@ -542,3 +543,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
 [@elliterate]: https://github.com/elliterate
 [@mlarraz]: https://github.com/mlarraz
 [@jtannas]: https://github.com/jtannas
+[@mockdeep]: https://github.com/mockdeep

config/default.yml

@@ -394,6 +394,13 @@ RSpec/MultipleExpectations:
   VersionChanged: '1.21'
   StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleExpectations
 
+RSpec/MultipleMemoizedHelpers:
+  Description: Checks if example groups contain too many `let` and `subject` calls.
+  Enabled: true
+  AllowSubject: true
+  Max: 5
+  StyleGuide: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleMemoizedHelpers
+
 RSpec/MultipleSubjects:
   Description: Checks if an example group defines `subject` multiple times.
   Enabled: true

lib/rubocop/cop/rspec/multiple_memoized_helpers.rb

@@ -0,0 +1,148 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # Checks if example groups contain too many `let` and `subject` calls.
+      #
+      # This cop is configurable using the `Max` option and the `AllowSubject`
+      # which will configure the cop to only register offenses on calls to
+      # `let` and not calls to `subject`.
+      #
+      # @example
+      #   # bad
+      #   describe MyClass do
+      #     let(:foo) { [] }
+      #     let(:bar) { [] }
+      #     let!(:baz) { [] }
+      #     let(:qux) { [] }
+      #     let(:quux) { [] }
+      #     let(:quuz) { {} }
+      #   end
+      #
+      #   describe MyClass do
+      #     let(:foo) { [] }
+      #     let(:bar) { [] }
+      #     let!(:baz) { [] }
+      #
+      #     context 'when stuff' do
+      #       let(:qux) { [] }
+      #       let(:quux) { [] }
+      #       let(:quuz) { {} }
+      #     end
+      #   end
+      #
+      #   # good
+      #   describe MyClass do
+      #     let(:bar) { [] }
+      #     let!(:baz) { [] }
+      #     let(:qux) { [] }
+      #     let(:quux) { [] }
+      #     let(:quuz) { {} }
+      #   end
+      #
+      #   describe MyClass do
+      #     context 'when stuff' do
+      #       let(:foo) { [] }
+      #       let(:bar) { [] }
+      #       let!(:booger) { [] }
+      #     end
+      #
+      #     context 'when other stuff' do
+      #       let(:qux) { [] }
+      #       let(:quux) { [] }
+      #       let(:quuz) { {} }
+      #     end
+      #   end
+      #
+      # @example when disabling AllowSubject configuration
+      #
+      #   # rubocop.yml
+      #   # RSpec/MultipleMemoizedHelpers:
+      #   #   AllowSubject: false
+      #
+      #   # bad - `subject` counts towards memoized helpers
+      #   describe MyClass do
+      #     subject { {} }
+      #     let(:foo) { [] }
+      #     let(:bar) { [] }
+      #     let!(:baz) { [] }
+      #     let(:qux) { [] }
+      #     let(:quux) { [] }
+      #   end
+      #
+      # @example with Max configuration
+      #
+      #   # rubocop.yml
+      #   # RSpec/MultipleMemoizedHelpers:
+      #   #   Max: 1
+      #
+      #   # bad
+      #   describe MyClass do
+      #     let(:foo) { [] }
+      #     let(:bar) { [] }
+      #   end
+      #
+      class MultipleMemoizedHelpers < Base
+        include ConfigurableMax
+        include RuboCop::RSpec::Variable
+
+        MSG = 'Example group has too many memoized helpers [%<count>d/%<max>d]'
+
+        def on_block(node)
+          return unless spec_group?(node)
+
+          count = all_helpers(node).uniq.count
+
+          return if count <= max
+
+          self.max = count
+          add_offense(node, message: format(MSG, count: count, max: max))
+        end
+
+        def on_new_investigation
+          @example_group_memoized_helpers = {}
+        end
+
+        private
+
+        attr_reader :example_group_memoized_helpers
+
+        def all_helpers(node)
+          [
+            *helpers(node),
+            *node.each_ancestor(:block).flat_map(&method(:helpers))
+          ]
+        end
+
+        def helpers(node)
+          @example_group_memoized_helpers[node] ||=
+            variable_nodes(node).map do |variable_node|
+              if variable_node.block_type?
+                variable_definition?(variable_node.send_node)
+              else # block-pass (`let(:foo, &bar)`)
+                variable_definition?(variable_node)
+              end
+            end
+        end
+
+        def variable_nodes(node)
+          example_group = RuboCop::RSpec::ExampleGroup.new(node)
+          if allow_subject?
+            example_group.lets
+          else
+            example_group.lets + example_group.subjects
+          end
+        end
+
+        def max
+          cop_config['Max']
+        end
+
+        def allow_subject?
+          cop_config['AllowSubject']
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rspec_cops.rb

@@ -65,6 +65,7 @@
 require_relative 'rspec/missing_example_group_argument'
 require_relative 'rspec/multiple_describes'
 require_relative 'rspec/multiple_expectations'
+require_relative 'rspec/multiple_memoized_helpers'
 require_relative 'rspec/multiple_subjects'
 require_relative 'rspec/named_subject'
 require_relative 'rspec/nested_groups'

lib/rubocop/rspec/variable.rb

@@ -8,7 +8,7 @@ module Variable
       extend RuboCop::NodePattern::Macros
 
       def_node_matcher :variable_definition?, <<~PATTERN
-        (send #rspec? #{(Helpers::ALL + Subject::ALL).node_pattern_union}
+        (send nil? #{(Helpers::ALL + Subject::ALL).node_pattern_union}
           $({sym str dsym dstr} ...) ...)
       PATTERN
     end

manual/cops.md

@@ -64,6 +64,7 @@
 * [RSpec/MissingExampleGroupArgument](cops_rspec.md#rspecmissingexamplegroupargument)
 * [RSpec/MultipleDescribes](cops_rspec.md#rspecmultipledescribes)
 * [RSpec/MultipleExpectations](cops_rspec.md#rspecmultipleexpectations)
+* [RSpec/MultipleMemoizedHelpers](cops_rspec.md#rspecmultiplememoizedhelpers)
 * [RSpec/MultipleSubjects](cops_rspec.md#rspecmultiplesubjects)
 * [RSpec/NamedSubject](cops_rspec.md#rspecnamedsubject)
 * [RSpec/NestedGroups](cops_rspec.md#rspecnestedgroups)

manual/cops_rspec.md

@@ -2119,6 +2119,108 @@ Max | `1` | Integer
 
 * [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleExpectations](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleExpectations)
 
+## RSpec/MultipleMemoizedHelpers
+
+Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
+--- | --- | --- | --- | ---
+Enabled | Yes | No | - | -
+
+Checks if example groups contain too many `let` and `subject` calls.
+
+This cop is configurable using the `Max` option and the `AllowSubject`
+which will configure the cop to only register offenses on calls to
+`let` and not calls to `subject`.
+
+### Examples
+
+```ruby
+# bad
+describe MyClass do
+  let(:foo) { [] }
+  let(:bar) { [] }
+  let!(:baz) { [] }
+  let(:qux) { [] }
+  let(:quux) { [] }
+  let(:quuz) { {} }
+end
+
+describe MyClass do
+  let(:foo) { [] }
+  let(:bar) { [] }
+  let!(:baz) { [] }
+
+  context 'when stuff' do
+    let(:qux) { [] }
+    let(:quux) { [] }
+    let(:quuz) { {} }
+  end
+end
+
+# good
+describe MyClass do
+  let(:bar) { [] }
+  let!(:baz) { [] }
+  let(:qux) { [] }
+  let(:quux) { [] }
+  let(:quuz) { {} }
+end
+
+describe MyClass do
+  context 'when stuff' do
+    let(:foo) { [] }
+    let(:bar) { [] }
+    let!(:booger) { [] }
+  end
+
+  context 'when other stuff' do
+    let(:qux) { [] }
+    let(:quux) { [] }
+    let(:quuz) { {} }
+  end
+end
+```
+#### when disabling AllowSubject configuration
+
+```ruby
+# rubocop.yml
+# RSpec/MultipleMemoizedHelpers:
+#   AllowSubject: false
+
+# bad - `subject` counts towards memoized helpers
+describe MyClass do
+  subject { {} }
+  let(:foo) { [] }
+  let(:bar) { [] }
+  let!(:baz) { [] }
+  let(:qux) { [] }
+  let(:quux) { [] }
+end
+```
+#### with Max configuration
+
+```ruby
+# rubocop.yml
+# RSpec/MultipleMemoizedHelpers:
+#   Max: 1
+
+# bad
+describe MyClass do
+  let(:foo) { [] }
+  let(:bar) { [] }
+end
+```
+
+### Configurable attributes
+
+Name | Default value | Configurable values
+--- | --- | ---
+AllowSubject | `true` | Boolean
+Max | `5` | Integer
+
+### References
+
+* [https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleMemoizedHelpers](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/MultipleMemoizedHelpers)
+
 ## RSpec/MultipleSubjects
 
 Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged