diff --git a/.rubocop.yml b/.rubocop.yml index 9ceb846627056d25c69779f03846b7bb02b1b125..c6d2d48db9bb3f39f2bf1458b121fff9dcbc3528 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1012,3 +1012,8 @@ Graphql/ResourceNotAvailableError: Exclude: # Definition of `raise_resource_not_available_error!` - 'lib/gitlab/graphql/authorize/authorize_resource.rb' + +RSpec/FactoryBot/LocalStaticAssignment: + Include: + - spec/factories/**/*.rb + - ee/spec/factories/**/*.rb diff --git a/.rubocop_todo/rspec/factory_bot/local_static_assignment.yml b/.rubocop_todo/rspec/factory_bot/local_static_assignment.yml new file mode 100644 index 0000000000000000000000000000000000000000..7a4201ca0270a8ef8fcfe91e202422c0bfd70b84 --- /dev/null +++ b/.rubocop_todo/rspec/factory_bot/local_static_assignment.yml @@ -0,0 +1,3 @@ +--- +RSpec/FactoryBot/LocalStaticAssignment: + Details: grace period diff --git a/rubocop/cop/rspec/factory_bot/local_static_assignment.rb b/rubocop/cop/rspec/factory_bot/local_static_assignment.rb new file mode 100644 index 0000000000000000000000000000000000000000..1a26bc31c78e30f0da5c0a759ce900d6c4ecdbc6 --- /dev/null +++ b/rubocop/cop/rspec/factory_bot/local_static_assignment.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module RSpec + module FactoryBot + # Flags local assignments during factory "load time". This leads to + # static data definitions. + # + # Move these definitions into attribute block or + # `transient` block to ensure that the data is evaluated during + # "runtime" and remains dynamic. + # + # @example + # # bad + # factory :foo do + # random = rand(23) + # baz { "baz-#{random}" } + # + # trait :a_trait do + # random = rand(23) + # baz { "baz-#{random}" } + # end + # + # transient do + # random = rand(23) + # baz { "baz-#{random}" } + # end + # end + # + # # good + # factory :foo do + # baz { "baz-#{random}" } + # + # trait :a_trait do + # baz { "baz-#{random}" } + # end + # + # transient do + # random { rand(23) } + # end + # end + class LocalStaticAssignment < RuboCop::Cop::Base + MSG = 'Avoid local static assignemnts in factories which lead to static data definitions.' + + RESTRICT_ON_SEND = %i[factory transient trait].freeze + + def_node_search :local_assignment, <<~PATTERN + (begin $(lvasgn ...)) + PATTERN + + def on_send(node) + return unless node.parent&.block_type? + + node.parent.each_child_node(:begin) do |begin_node| + begin_node.each_child_node(:lvasgn) do |lvasgn_node| + add_offense(lvasgn_node) + end + end + end + end + end + end + end +end diff --git a/spec/factories/design_management/designs.rb b/spec/factories/design_management/designs.rb index d16fd0c297b58f305da22dea360a85e037e444e7..b284c7f57374e7186ee31a271a9c31bb92570fed 100644 --- a/spec/factories/design_management/designs.rb +++ b/spec/factories/design_management/designs.rb @@ -26,7 +26,7 @@ sequence(:relative_position) { |n| n * 1000 } end - create_versions = ->(design, evaluator, commit_version) do + create_versions = ->(design, evaluator, commit_version) do # rubocop:disable RSpec/FactoryBot/LocalStaticAssignment unless evaluator.versions_count == 0 project = design.project issue = design.issue diff --git a/spec/rubocop/cop/rspec/factory_bot/local_static_assignment_spec.rb b/spec/rubocop/cop/rspec/factory_bot/local_static_assignment_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..de86435616cdec0b401262692304b6d12f424093 --- /dev/null +++ b/spec/rubocop/cop/rspec/factory_bot/local_static_assignment_spec.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' + +require_relative '../../../../../rubocop/cop/rspec/factory_bot/local_static_assignment' + +RSpec.describe RuboCop::Cop::RSpec::FactoryBot::LocalStaticAssignment, feature_category: :tooling do + shared_examples 'local static assignment' do |block| + it "flags static local assignment in `#{block}`" do + expect_offense(<<~RUBY, block: block) + %{block} do + age + name + + random_number = rand(23) + ^^^^^^^^^^^^^^^^^^^^^^^^ Avoid local static assignemnts in factories which lead to static data definitions. + + random_string = SecureRandom.uuid + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid local static assignemnts in factories which lead to static data definitions. + + project + end + RUBY + end + + it 'does not flag correct use' do + expect_no_offenses(<<~RUBY) + #{block} do + age do + random_number = rand(23) + random_number + 1 + end + end + RUBY + end + end + + it_behaves_like 'local static assignment', 'factory :project' + it_behaves_like 'local static assignment', 'transient' + it_behaves_like 'local static assignment', 'trait :closed' + + it 'does not flag local assignments in unrelated blocks' do + expect_no_offenses(<<~RUBY) + factory :project do + sequence(:number) do |n| + random_number = rand(23) + random_number * n + end + + name do + random_string = SecureRandom.uuid + random_string + "-name" + end + + initialize_with do + random_string = SecureRandom.uuid + new(name: random_string) + end + end + RUBY + end +end