Skip to content

Commit

Permalink
[Fix #1087] Ignore CreateList offense on method call detection
Browse files Browse the repository at this point in the history
  • Loading branch information
ngouy authored and pirj committed Jun 15, 2022
1 parent 426513d commit af3f62e
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
10 changes: 8 additions & 2 deletions docs/modules/ROOT/pages/cops_rspec_factorybot.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
34 changes: 28 additions & 6 deletions lib/rubocop/cop/rspec/factory_bot/create_list.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 $_) $...)
Expand All @@ -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|
Expand Down
21 changes: 19 additions & 2 deletions spec/rubocop/cop/rspec/factory_bot/create_list_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit af3f62e

Please sign in to comment.