From 3eedcf3b3b0bd48ebafeae149806ad67be2e7439 Mon Sep 17 00:00:00 2001 From: ddieulivol Date: Thu, 9 Nov 2023 15:11:53 +0100 Subject: [PATCH] Add an excessive_create_list cop Cop used to signal when we create a lot of factory objects with FactoryBot (configurable with the MaxAmount option) --- .rubocop.yml | 3 ++ CHANGELOG.md | 2 + config/default.yml | 7 +++ docs/modules/ROOT/pages/cops.adoc | 1 + .../ROOT/pages/cops_rspec_factorybot.adoc | 47 ++++++++++++++++ .../factory_bot/excessive_create_list.rb | 53 +++++++++++++++++++ lib/rubocop/cop/rspec_cops.rb | 1 + lib/rubocop/rspec/config_formatter.rb | 1 + .../factory_bot/excessive_create_list_spec.rb | 44 +++++++++++++++ 9 files changed, 159 insertions(+) create mode 100644 lib/rubocop/cop/rspec/factory_bot/excessive_create_list.rb create mode 100644 spec/rubocop/cop/rspec/factory_bot/excessive_create_list_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index 1412e6432..16fabde4c 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -98,6 +98,9 @@ RSpec/ExampleLength: - heredoc Max: 11 +RSpec/FactoryBot/ExcessiveCreateList: + Enabled: true + RSpec/FilePath: Enabled: false diff --git a/CHANGELOG.md b/CHANGELOG.md index e7974d61b..441fd74f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Master (Unreleased) +- Add new `RSpec/FactoryBot/ExcessiveCreateList` cop. ([@ddieulivol]) + ## 2.25.0 (2023-10-27) - Add support single quoted string and percent string and heredoc for `RSpec/Rails/HttpStatus`. ([@ydah]) diff --git a/config/default.yml b/config/default.yml index f5e731437..8d3c26501 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1067,6 +1067,13 @@ RSpec/FactoryBot/CreateList: VersionChanged: '2.23' Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/CreateList +RSpec/FactoryBot/ExcessiveCreateList: + Description: Check for create_list FactoryBot declarations higher than configured + MaxAmount. + Enabled: pending + VersionAdded: '2.26' + Reference: https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/ExcessiveCreateList + RSpec/FactoryBot/FactoryClassName: Description: Use string value when setting the class attribute explicitly. Enabled: true diff --git a/docs/modules/ROOT/pages/cops.adoc b/docs/modules/ROOT/pages/cops.adoc index 9f0962c12..49dfddf01 100644 --- a/docs/modules/ROOT/pages/cops.adoc +++ b/docs/modules/ROOT/pages/cops.adoc @@ -122,6 +122,7 @@ * xref:cops_rspec_factorybot.adoc#rspecfactorybot/attributedefinedstatically[RSpec/FactoryBot/AttributeDefinedStatically] * xref:cops_rspec_factorybot.adoc#rspecfactorybot/consistentparenthesesstyle[RSpec/FactoryBot/ConsistentParenthesesStyle] * xref:cops_rspec_factorybot.adoc#rspecfactorybot/createlist[RSpec/FactoryBot/CreateList] +* xref:cops_rspec_factorybot.adoc#rspecfactorybot/excessivecreatelist[RSpec/FactoryBot/ExcessiveCreateList] * xref:cops_rspec_factorybot.adoc#rspecfactorybot/factoryclassname[RSpec/FactoryBot/FactoryClassName] * xref:cops_rspec_factorybot.adoc#rspecfactorybot/factorynamestyle[RSpec/FactoryBot/FactoryNameStyle] * xref:cops_rspec_factorybot.adoc#rspecfactorybot/syntaxmethods[RSpec/FactoryBot/SyntaxMethods] diff --git a/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc b/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc index 979216201..4cf947acd 100644 --- a/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc +++ b/docs/modules/ROOT/pages/cops_rspec_factorybot.adoc @@ -189,6 +189,53 @@ create_list :user, 3 * https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/CreateList +== RSpec/FactoryBot/ExcessiveCreateList + +|=== +| Enabled by default | Safe | Supports autocorrection | Version Added | Version Changed + +| Pending +| Yes +| No +| 2.26 +| - +|=== + +Check for create_list FactoryBot declarations +higher than configured MaxAmount. + +=== Examples + +==== 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) +---- + +==== 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) +---- + +=== References + +* https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/FactoryBot/ExcessiveCreateList + == RSpec/FactoryBot/FactoryClassName |=== diff --git a/lib/rubocop/cop/rspec/factory_bot/excessive_create_list.rb b/lib/rubocop/cop/rspec/factory_bot/excessive_create_list.rb new file mode 100644 index 000000000..a2f73ba66 --- /dev/null +++ b/lib/rubocop/cop/rspec/factory_bot/excessive_create_list.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Rubocop + module Cop + module RSpec + module FactoryBot + # Check for create_list FactoryBot declarations + # higher than configured MaxAmount. + # + # @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) + # + # @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) + # + class ExcessiveCreateList < RuboCop::Cop::RSpec::Base + MESSAGE = + 'Avoid using `create_list` with more than %s items.' + + # @!method create_list?(node) + def_node_matcher :create_list?, <<~PATTERN + (send nil? :create_list (sym ...) $(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 +end diff --git a/lib/rubocop/cop/rspec_cops.rb b/lib/rubocop/cop/rspec_cops.rb index b6d8d83d7..2928b84c3 100644 --- a/lib/rubocop/cop/rspec_cops.rb +++ b/lib/rubocop/cop/rspec_cops.rb @@ -12,6 +12,7 @@ require_relative 'rspec/factory_bot/attribute_defined_statically' require_relative 'rspec/factory_bot/consistent_parentheses_style' require_relative 'rspec/factory_bot/create_list' +require_relative 'rspec/factory_bot/excessive_create_list' require_relative 'rspec/factory_bot/factory_class_name' require_relative 'rspec/factory_bot/factory_name_style' require_relative 'rspec/factory_bot/syntax_methods' diff --git a/lib/rubocop/rspec/config_formatter.rb b/lib/rubocop/rspec/config_formatter.rb index f41985e42..406b5278d 100644 --- a/lib/rubocop/rspec/config_formatter.rb +++ b/lib/rubocop/rspec/config_formatter.rb @@ -19,6 +19,7 @@ class ConfigFormatter RSpec/FactoryBot/AttributeDefinedStatically RSpec/FactoryBot/ConsistentParenthesesStyle RSpec/FactoryBot/CreateList + RSpec/FactoryBot/ExcessiveCreateList RSpec/FactoryBot/FactoryClassName RSpec/FactoryBot/FactoryNameStyle RSpec/FactoryBot/SyntaxMethods diff --git a/spec/rubocop/cop/rspec/factory_bot/excessive_create_list_spec.rb b/spec/rubocop/cop/rspec/factory_bot/excessive_create_list_spec.rb new file mode 100644 index 000000000..cfbf7a2ee --- /dev/null +++ b/spec/rubocop/cop/rspec/factory_bot/excessive_create_list_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Rubocop::Cop::RSpec::FactoryBot::ExcessiveCreateList do + let(:cop_config) do + { 'MaxAmount' => max_amount } + end + + context 'when MaxAmount is set to 50' do + let(:max_amount) { 50 } + + 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 50 items' do + expect_no_offenses(<<~RUBY) + create_list(:merge_requests, 30) + RUBY + end + + it 'ignores create_list for 50 items' do + expect_no_offenses(<<~RUBY) + create_list(:merge_requests, 50) + RUBY + end + + it 'registers an offense for create_list for more than 50 items' do + expect_offense(<<~RUBY) + create_list(:merge_requests, 51) + ^^ Avoid using `create_list` with more than 50 items. + RUBY + end + end +end