diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a9dcde1..a5b4bbaa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Master (Unreleased) - Fix a false positive for `FactoryBot/FactoryNameStyle` when namespaced models. ([@ydah]) +- Add new `FactoryBot/ExcessiveCreateList` cop. ([@ddieulivol]) ## 2.24.0 (2023-09-18) @@ -76,6 +77,7 @@ [@bquorning]: https://github.com/bquorning [@composerinteralia]: https://github.com/composerinteralia [@darhazer]: https://github.com/Darhazer +[@ddieulivol]: https://github.com/ddieulivol [@dmitrytsepelev]: https://github.com/dmitrytsepelev [@harrylewis]: https://github.com/harrylewis [@jfragoulis]: https://github.com/jfragoulis diff --git a/config/default.yml b/config/default.yml index 96717770..61ae3322 100644 --- a/config/default.yml +++ b/config/default.yml @@ -64,6 +64,18 @@ FactoryBot/CreateList: VersionChanged: '2.24' Reference: https://www.rubydoc.info/gems/rubocop-factory_bot/RuboCop/Cop/FactoryBot/CreateList +FactoryBot/ExcessiveCreateList: + Description: Check for excessive model creation in a list. + Enabled: pending + Include: + - "**/*_spec.rb" + - "**/spec/**/*" + - "**/test/**/*" + - "**/features/support/factories/**/*.rb" + MaxAmount: 10 + VersionAdded: "<>" + Reference: https://www.rubydoc.info/gems/rubocop-factory_bot/RuboCop/Cop/FactoryBot/ExcessiveCreateList + FactoryBot/FactoryAssociationWithStrategy: Description: Use definition in factory association instead of hard coding a strategy. Enabled: pending diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index e64c63d9..b5f503f5 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -6,6 +6,7 @@ * xref:cops_factorybot.adoc#factorybotattributedefinedstatically[FactoryBot/AttributeDefinedStatically] * xref:cops_factorybot.adoc#factorybotconsistentparenthesesstyle[FactoryBot/ConsistentParenthesesStyle] * xref:cops_factorybot.adoc#factorybotcreatelist[FactoryBot/CreateList] +* xref:cops_factorybot.adoc#factorybotexcessivecreatelist[FactoryBot/ExcessiveCreateList] * xref:cops_factorybot.adoc#factorybotfactoryassociationwithstrategy[FactoryBot/FactoryAssociationWithStrategy] * xref:cops_factorybot.adoc#factorybotfactoryclassname[FactoryBot/FactoryClassName] * xref:cops_factorybot.adoc#factorybotfactorynamestyle[FactoryBot/FactoryNameStyle] diff --git a/docs/modules/ROOT/pages/cops_factorybot.adoc b/docs/modules/ROOT/pages/cops_factorybot.adoc index 3932abf4..991926ad 100644 --- a/docs/modules/ROOT/pages/cops_factorybot.adoc +++ b/docs/modules/ROOT/pages/cops_factorybot.adoc @@ -348,6 +348,66 @@ create_list :user, 3 * https://www.rubydoc.info/gems/rubocop-factory_bot/RuboCop/Cop/FactoryBot/CreateList +== FactoryBot/ExcessiveCreateList + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| No +| <> +| - +|=== + +Check for excessive model creation in a list. + +=== Examples + +==== MaxAmount: 10 (default) + +[source,ruby] +---- +# We do not allow more than 10 items to be created + +# bad +create_list(:merge_request, 1000, state: :opened) + +# good +create_list(:merge_request, 10, state: :opened) +---- + +==== MaxAmount: 20 + +[source,ruby] +---- +# We do not allow more than 20 items to be created + +# bad +create_list(:merge_request, 1000, state: :opened) + +# good +create_list(:merge_request, 15, state: :opened) +---- + +=== Configurable attributes + +|=== +| Name | Default value | Configurable values + +| Include +| `+**/*_spec.rb+`, `+**/spec/**/*+`, `+**/test/**/*+`, `+**/features/support/factories/**/*.rb+` +| Array + +| MaxAmount +| `10` +| Integer +|=== + +=== References + +* https://www.rubydoc.info/gems/rubocop-factory_bot/RuboCop/Cop/FactoryBot/ExcessiveCreateList + == FactoryBot/FactoryAssociationWithStrategy |=== diff --git a/lib/rubocop/cop/factory_bot/attribute_defined_statically.rb b/lib/rubocop/cop/factory_bot/attribute_defined_statically.rb index 8fc2d3df..33018417 100644 --- a/lib/rubocop/cop/factory_bot/attribute_defined_statically.rb +++ b/lib/rubocop/cop/factory_bot/attribute_defined_statically.rb @@ -70,7 +70,7 @@ def offensive_receiver?(receiver, node) end def receiver_matches_first_block_argument?(receiver, node) - first_block_argument = node.arguments.first + first_block_argument = node.first_argument !first_block_argument.nil? && receiver.lvar_type? && diff --git a/lib/rubocop/cop/factory_bot/excessive_create_list.rb b/lib/rubocop/cop/factory_bot/excessive_create_list.rb new file mode 100644 index 00000000..2d3f51af --- /dev/null +++ b/lib/rubocop/cop/factory_bot/excessive_create_list.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module FactoryBot + # Check for excessive model creation in a list. + # + # @example MaxAmount: 10 (default) + # # We do not allow more than 10 items to be created + # + # # bad + # create_list(:merge_request, 1000, state: :opened) + # + # # good + # create_list(:merge_request, 10, state: :opened) + # + # @example MaxAmount: 20 + # # We do not allow more than 20 items to be created + # + # # bad + # create_list(:merge_request, 1000, state: :opened) + # + # # good + # create_list(:merge_request, 15, state: :opened) + # + class ExcessiveCreateList < ::RuboCop::Cop::Base + include ConfigurableExplicitOnly + + MESSAGE = + 'Avoid using `create_list` with more than %s items.' + + # @!method create_list?(node) + def_node_matcher :create_list?, <<~PATTERN + (send #factory_call? :create_list {sym str} $(int _) ...) + PATTERN + + RESTRICT_ON_SEND = %i[create_list].freeze + + def on_send(node) + number_node = create_list?(node) + return unless number_node + + max_amount = cop_config['MaxAmount'] + return if number_node.value <= max_amount + + add_offense(number_node, message: + format(MESSAGE, max_amount: max_amount)) + end + end + end + end +end diff --git a/lib/rubocop/cop/factory_bot_cops.rb b/lib/rubocop/cop/factory_bot_cops.rb index 17ba1e2d..ee3282d4 100644 --- a/lib/rubocop/cop/factory_bot_cops.rb +++ b/lib/rubocop/cop/factory_bot_cops.rb @@ -4,6 +4,7 @@ require_relative 'factory_bot/attribute_defined_statically' require_relative 'factory_bot/consistent_parentheses_style' require_relative 'factory_bot/create_list' +require_relative 'factory_bot/excessive_create_list' require_relative 'factory_bot/factory_association_with_strategy' require_relative 'factory_bot/factory_class_name' require_relative 'factory_bot/factory_name_style' diff --git a/spec/rubocop/cop/factory_bot/excessive_create_list_spec.rb b/spec/rubocop/cop/factory_bot/excessive_create_list_spec.rb new file mode 100644 index 00000000..0369a0f2 --- /dev/null +++ b/spec/rubocop/cop/factory_bot/excessive_create_list_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +RSpec.describe RuboCop::Cop::FactoryBot::ExcessiveCreateList do + let(:cop_config) do + { 'MaxAmount' => max_amount } + end + + let(:max_amount) { 10 } + + it 'ignores code that does not contain create_list' do + expect_no_offenses(<<~RUBY) + expect(true).to be_truthy + RUBY + end + + it 'ignores create_list with non-integer value' do + expect_no_offenses(<<~RUBY) + create_list(:merge_requests, value) + RUBY + end + + it 'ignores create_list with less than 10 items' do + expect_no_offenses(<<~RUBY) + create_list(:merge_requests, 9) + RUBY + end + + it 'ignores create_list for 10 items' do + expect_no_offenses(<<~RUBY) + create_list(:merge_requests, 10) + RUBY + end + + it 'registers an offense for create_list for more than 10 items' do + expect_offense(<<~RUBY) + create_list(:merge_requests, 11) + ^^ Avoid using `create_list` with more than 10 items. + RUBY + end + + it 'registers an offense for FactoryBot.create_list' do + expect_offense(<<~RUBY) + FactoryBot.create_list(:merge_requests, 11) + ^^ Avoid using `create_list` with more than 10 items. + RUBY + end + + context 'when create_list has the factory name as a string' do + it 'registers an offense' do + expect_offense(<<~RUBY) + FactoryBot.create_list('warehouse/user', 11) + ^^ Avoid using `create_list` with more than 10 items. + RUBY + end + end +end