From af3f62e63548ef8de49a11af1b9b860b37dbca65 Mon Sep 17 00:00:00 2001
From: Nathan Gouy <gouy.inc@gmail.com>
Date: Mon, 7 Dec 2020 21:32:25 -0500
Subject: [PATCH] [Fix #1087] Ignore CreateList offense on method call
 detection

---
 CHANGELOG.md                                  |  2 ++
 .../ROOT/pages/cops_rspec_factorybot.adoc     | 10 ++++--
 .../cop/rspec/factory_bot/create_list.rb      | 34 +++++++++++++++----
 .../cop/rspec/factory_bot/create_list_spec.rb | 21 ++++++++++--
 4 files changed, 57 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7f0dc7f34..657ff5efa 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 * Fix incorrect path suggested by `RSpec/FilePath` cop when second argument contains spaces. ([@tejasbubane][])
 * Fix autocorrect for EmptyLineSeparation. ([@johnny-miyake][])
 * Add new `RSpec/Capybara/SpecificMatcher` cop. ([@ydah][])
+* Fixed false offense detection in `FactoryBot/CreateList` when a n.times block is including method calls in the factory create arguments. ([@ngouy][])
 
 ## 2.11.1 (2022-05-18)
 
@@ -699,3 +700,4 @@ Compatibility release so users can upgrade RuboCop to 0.51.0. No new features.
 [@M-Yamashita01]: https://github.com/M-Yamashita01
 [@luke-hill]: https://github.com/luke-hill
 [@johnny-miyake]: https://github.com/johnny-miyake
+[@ngouy]: https://github.com/ngouy
diff --git a/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc b/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc
index a818af922..64d62d3c0 100644
--- a/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc
+++ b/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc
@@ -79,8 +79,14 @@ This cop can be configured using the `EnforcedStyle` option
 # good
 create_list :user, 3
 
-# good
-3.times { |n| create :user, created_at: n.months.ago }
+# bad
+3.times { create :user, age: 18 }
+
+# good - index is used to alter the created models attributes
+3.times { |n| create :user, age: n }
+
+# good - contains a method call, may return different values
+3.times { create :user, age: rand }
 ----
 
 ==== `EnforcedStyle: n_times`
diff --git a/lib/rubocop/cop/rspec/factory_bot/create_list.rb b/lib/rubocop/cop/rspec/factory_bot/create_list.rb
index b27a0e909..e901bd00e 100644
--- a/lib/rubocop/cop/rspec/factory_bot/create_list.rb
+++ b/lib/rubocop/cop/rspec/factory_bot/create_list.rb
@@ -15,8 +15,14 @@ module FactoryBot
         #   # good
         #   create_list :user, 3
         #
-        #   # good
-        #   3.times { |n| create :user, created_at: n.months.ago }
+        #   # bad
+        #   3.times { create :user, age: 18 }
+        #
+        #   # good - index is used to alter the created models attributes
+        #   3.times { |n| create :user, age: n }
+        #
+        #   # good - contains a method call, may return different values
+        #   3.times { create :user, age: rand }
         #
         # @example `EnforcedStyle: n_times`
         #   # bad
@@ -33,15 +39,28 @@ class CreateList < Base
           MSG_N_TIMES = 'Prefer %<number>s.times.'
           RESTRICT_ON_SEND = %i[create_list].freeze
 
-          # @!method n_times_block_without_arg?(node)
-          def_node_matcher :n_times_block_without_arg?, <<-PATTERN
+          # @!method n_times_block?(node)
+          def_node_matcher :n_times_block?, <<-PATTERN
             (block
               (send (int _) :times)
-              (args)
               ...
             )
           PATTERN
 
+          # @!method n_times_block_with_arg_and_used?(node)
+          def_node_matcher :n_times_block_with_arg_and_used?, <<-PATTERN
+            (block
+              (send (int _) :times)
+              (args (arg _value))
+                `_value
+            )
+          PATTERN
+
+          # @!method arguments_include_method_call?(node)
+          def_node_matcher :arguments_include_method_call?, <<-PATTERN
+            (send ${nil? #factory_bot?} :create (sym $_) `$(send ...))
+          PATTERN
+
           # @!method factory_call(node)
           def_node_matcher :factory_call, <<-PATTERN
             (send ${nil? #factory_bot?} :create (sym $_) $...)
@@ -54,7 +73,10 @@ class CreateList < Base
 
           def on_block(node)
             return unless style == :create_list
-            return unless n_times_block_without_arg?(node)
+
+            return unless n_times_block?(node)
+            return if n_times_block_with_arg_and_used?(node)
+            return if arguments_include_method_call?(node.body)
             return unless contains_only_factory?(node.body)
 
             add_offense(node.send_node, message: MSG_CREATE_LIST) do |corrector|
diff --git a/spec/rubocop/cop/rspec/factory_bot/create_list_spec.rb b/spec/rubocop/cop/rspec/factory_bot/create_list_spec.rb
index 23ec3c36b..c0b56a83f 100644
--- a/spec/rubocop/cop/rspec/factory_bot/create_list_spec.rb
+++ b/spec/rubocop/cop/rspec/factory_bot/create_list_spec.rb
@@ -53,9 +53,26 @@
       RUBY
     end
 
-    it 'ignores n.times with argument' do
+    it 'ignores n.times with n as argument' do
       expect_no_offenses(<<~RUBY)
-        3.times { |n| create :user, created_at: n.days.ago }
+        3.times { |n| create :user, position: n }
+      RUBY
+    end
+
+    it 'flags n.times when create call doesn\'t have method calls' do
+      expect_offense(<<~RUBY)
+        3.times { |n| create :user, :active }
+        ^^^^^^^ Prefer create_list.
+        3.times { |n| create :user, password: '123' }
+        ^^^^^^^ Prefer create_list.
+        3.times { |n| create :user, :active, password: '123' }
+        ^^^^^^^ Prefer create_list.
+      RUBY
+    end
+
+    it 'ignores n.times when create call does have method calls' do
+      expect_no_offenses(<<~RUBY)
+        3.times { |n| create :user, repositories_count: rand }
       RUBY
     end