Merge pull request #450 from rubocop-rspec/create_list_cop

Add FactoryGirl/CreateList cop. Fixes #411

CHANGELOG.md

@@ -9,6 +9,7 @@
 * Fix `RSpec/NestedGroups` cop support --auto-gen-config. ([@walf443][])
 * Fix false positives in `Capybara/FeatureMethods` when feature methods are used as property names in a factory. ([@Darhazer][])
 * Allow configuring enabled methods in `Capybara/FeatureMethods`. ([@Darhazer][])
+* Add `FactoryBot/CreateList` cop. ([@Darhazer][])
 
 ## 1.24.0 (2018-03-06)
 

config/default.yml

@@ -380,6 +380,15 @@ Capybara/FeatureMethods:
   EnabledMethods: []
   StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/Capybara/FeatureMethods
 
+FactoryBot/CreateList:
+  Description: Checks for create_list usage.
+  Enabled: true
+  EnforcedStyle: create_list
+  SupportedStyles:
+  - create_list
+  - n_times
+  StyleGuide: http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/CreateList
+
 FactoryBot/DynamicAttributeDefinedStatically:
   Description: Prefer declaring dynamic attribute values in a block.
   Enabled: true

lib/rubocop/cop/rspec/factory_bot/create_list.rb

@@ -0,0 +1,148 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      module FactoryBot
+        # Checks for create_list usage.
+        #
+        # This cop can be configured using the `EnforcedStyle` option
+        #
+        # @example `EnforcedStyle: create_list`
+        #   # bad
+        #   3.times { create :user }
+        #
+        #   # good
+        #   create_list :user, 3
+        #
+        #   # good
+        #   3.times { |n| create :user, created_at: n.months.ago }
+        #
+        # @example `EnforcedStyle: n_times`
+        #   # bad
+        #   create_list :user, 3
+        #
+        #   # good
+        #   3.times { create :user }
+        class CreateList < Cop
+          include ConfigurableEnforcedStyle
+
+          MSG_CREATE_LIST = 'Prefer create_list.'.freeze
+          MSG_N_TIMES = 'Prefer %<number>s.times.'.freeze
+
+          def_node_matcher :n_times_block?, <<-PATTERN
+            (block
+              (send (int _) :times)
+              ...
+            )
+          PATTERN
+
+          def_node_matcher :factory_call, <<-PATTERN
+            (send ${(const nil? {:FactoryGirl :FactoryBot}) nil?} :create (sym $_) $...)
+          PATTERN
+
+          def_node_matcher :factory_list_call, <<-PATTERN
+            (send ${(const nil? {:FactoryGirl :FactoryBot}) nil?} :create_list (sym $_) (int $_) $...)
+          PATTERN
+
+          def on_block(node)
+            return unless style == :create_list
+            return unless n_times_block?(node)
+            return unless contains_only_factory?(node.body)
+
+            add_offense(node.send_node,
+                        location: :expression, message: MSG_CREATE_LIST)
+          end
+
+          def on_send(node)
+            return unless style == :n_times
+
+            factory_list_call(node) do |_receiver, _factory, count, _|
+              add_offense(
+                node,
+                location: :selector,
+                message: format(MSG_N_TIMES, number: count)
+              )
+            end
+          end
+
+          def autocorrect(node)
+            if style == :create_list
+              autocorrect_n_times_to_create_list(node)
+            else
+              autocorrect_create_list_to_n_times(node)
+            end
+          end
+
+          private
+
+          def contains_only_factory?(node)
+            if node.block_type?
+              factory_call(node.send_node)
+            else
+              factory_call(node)
+            end
+          end
+
+          def autocorrect_n_times_to_create_list(node)
+            block = node.parent
+            count = block.receiver.source
+            replacement = factory_call_replacement(block.body, count)
+
+            lambda do |corrector|
+              corrector.replace(block.loc.expression, replacement)
+            end
+          end
+
+          def autocorrect_create_list_to_n_times(node)
+            replacement = generate_n_times_block(node)
+            lambda do |corrector|
+              corrector.replace(node.loc.expression, replacement)
+            end
+          end
+
+          def generate_n_times_block(node)
+            receiver, factory, count, options = *factory_list_call(node)
+
+            arguments = ":#{factory}"
+            options = build_options_string(options)
+            arguments += ", #{options}" unless options.empty?
+
+            replacement = format_receiver(receiver)
+            replacement += format_method_call(node, 'create', arguments)
+            "#{count}.times { #{replacement} }"
+          end
+
+          def factory_call_replacement(body, count)
+            receiver, factory, options = *factory_call(body)
+
+            arguments = ":#{factory}, #{count}"
+            options = build_options_string(options)
+            arguments += ", #{options}" unless options.empty?
+
+            replacement = format_receiver(receiver)
+            replacement += format_method_call(body, 'create_list', arguments)
+            replacement
+          end
+
+          def build_options_string(options)
+            options.map(&:source).join(', ')
+          end
+
+          def format_method_call(node, method, arguments)
+            if node.parenthesized?
+              "#{method}(#{arguments})"
+            else
+              "#{method} #{arguments}"
+            end
+          end
+
+          def format_receiver(receiver)
+            return '' unless receiver
+            "#{receiver.source}."
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rspec_cops.rb

@@ -1,6 +1,7 @@
 require_relative 'rspec/capybara/current_path_expectation'
 require_relative 'rspec/capybara/feature_methods'
 
+require_relative 'rspec/factory_bot/create_list'
 require_relative 'rspec/factory_bot/dynamic_attribute_defined_statically'
 require_relative 'rspec/factory_bot/static_attribute_defined_dynamically'
 

manual/cops.md

@@ -6,6 +6,7 @@
 
 #### Department [FactoryBot](cops_factorybot.md)
 
+* [FactoryBot/CreateList](cops_factorybot.md#factorybotcreatelist)
 * [FactoryBot/DynamicAttributeDefinedStatically](cops_factorybot.md#factorybotdynamicattributedefinedstatically)
 * [FactoryBot/StaticAttributeDefinedDynamically](cops_factorybot.md#factorybotstaticattributedefineddynamically)
 

manual/cops_factorybot.md

@@ -1,5 +1,49 @@
 # FactoryBot
 
+## FactoryBot/CreateList
+
+Enabled by default | Supports autocorrection
+--- | ---
+Enabled | Yes
+
+Checks for create_list usage.
+
+This cop can be configured using the `EnforcedStyle` option
+
+### Examples
+
+#### `EnforcedStyle: create_list`
+
+```ruby
+# bad
+3.times { create :user }
+
+# good
+create_list :user, 3
+
+# good
+3.times { |n| create :user, created_at: n.months.ago }
+```
+#### `EnforcedStyle: n_times`
+
+```ruby
+# bad
+create_list :user, 3
+
+# good
+3.times { create :user }
+```
+
+### Configurable attributes
+
+Name | Default value | Configurable values
+--- | --- | ---
+EnforcedStyle | `create_list` | `create_list`, `n_times`
+
+### References
+
+* [http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/CreateList](http://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/CreateList)
+
 ## FactoryBot/DynamicAttributeDefinedStatically
 
 Enabled by default | Supports autocorrection

spec/rubocop/cop/rspec/factory_bot/create_list_spec.rb

@@ -0,0 +1,140 @@
+RSpec.describe RuboCop::Cop::RSpec::FactoryBot::CreateList, :config do
+  subject(:cop) { described_class.new(config) }
+
+  let(:cop_config) do
+    { 'EnforcedStyle' => enforced_style }
+  end
+
+  context 'when EnforcedStyle is :create_list' do
+    let(:enforced_style) { :create_list }
+
+    it 'flags usage of n.times with no arguments' do
+      expect_offense(<<-RUBY)
+        3.times { create :user }
+        ^^^^^^^ Prefer create_list.
+      RUBY
+    end
+
+    it 'flags usage of n.times when FactoryGirl.create is used' do
+      expect_offense(<<-RUBY)
+        3.times { FactoryGirl.create :user }
+        ^^^^^^^ Prefer create_list.
+      RUBY
+    end
+
+    it 'flags usage of n.times when FactoryBot.create is used' do
+      expect_offense(<<-RUBY)
+        3.times { FactoryBot.create :user }
+        ^^^^^^^ Prefer create_list.
+      RUBY
+    end
+
+    it 'ignores create method of other object' do
+      expect_no_offenses(<<-RUBY)
+        3.times { SomeFactory.create :user }
+      RUBY
+    end
+
+    it 'ignores create in other block' do
+      expect_no_offenses(<<-RUBY)
+        allow(User).to receive(:create) { create :user }
+      RUBY
+    end
+
+    it 'flags n.times with argument' do
+      expect_offense(<<-RUBY)
+        3.times { |n| create :user, created_at: n.days.ago }
+        ^^^^^^^ Prefer create_list.
+      RUBY
+    end
+
+    it 'ignores n.times when there is no create call inside' do
+      expect_no_offenses(<<-RUBY)
+        3.times { do_something }
+      RUBY
+    end
+
+    it 'ignores n.times when there is other calls but create' do
+      expect_no_offenses(<<-RUBY)
+        used_passwords = []
+        3.times do
+          u = create :user
+          expect(used_passwords).not_to include(u.password)
+          used_passwords << u.password
+        end
+      RUBY
+    end
+
+    it 'flags FactoryGirl.create calls with a block' do
+      expect_offense(<<-RUBY)
+        3.times do
+        ^^^^^^^ Prefer create_list.
+          create(:user) { |user| create :account, user: user }
+        end
+      RUBY
+    end
+
+    include_examples 'autocorrect',
+                     '5.times { create :user }',
+                     'create_list :user, 5'
+
+    include_examples 'autocorrect',
+                     '5.times { create(:user, :trait) }',
+                     'create_list(:user, 5, :trait)'
+
+    include_examples 'autocorrect',
+                     '5.times { create :user, :trait, key: val }',
+                     'create_list :user, 5, :trait, key: val'
+
+    include_examples 'autocorrect',
+                     '5.times { FactoryGirl.create :user }',
+                     'FactoryGirl.create_list :user, 5'
+  end
+
+  context 'when EnforcedStyle is :n_times' do
+    let(:enforced_style) { :n_times }
+
+    it 'flags usage of create_list' do
+      expect_offense(<<-RUBY)
+        create_list :user, 3
+        ^^^^^^^^^^^ Prefer 3.times.
+      RUBY
+    end
+
+    it 'flags usage of FactoryGirl.create_list' do
+      expect_offense(<<-RUBY)
+       FactoryGirl.create_list :user, 3
+                   ^^^^^^^^^^^ Prefer 3.times.
+      RUBY
+    end
+
+    it 'flags usage of FactoryGirl.create_list with a block' do
+      expect_offense(<<-RUBY)
+       FactoryGirl.create_list(:user, 3) { |user| user.points = rand(1000) }
+                   ^^^^^^^^^^^ Prefer 3.times.
+      RUBY
+    end
+
+    it 'ignores create method of other object' do
+      expect_no_offenses(<<-RUBY)
+        SomeFactory.create_list :user, 3
+      RUBY
+    end
+
+    include_examples 'autocorrect',
+                     'create_list :user, 5',
+                     '5.times { create :user }'
+
+    include_examples 'autocorrect',
+                     'create_list(:user, 5, :trait)',
+                     '5.times { create(:user, :trait) }'
+
+    include_examples 'autocorrect',
+                     'create_list :user, 5, :trait, key: val',
+                     '5.times { create :user, :trait, key: val }'
+
+    include_examples 'autocorrect',
+                     'FactoryGirl.create_list :user, 5',
+                     '5.times { FactoryGirl.create :user }'
+  end
+end