diff options
| -rw-r--r-- | rubocop/cop/rspec/factories_in_migration_specs.rb | 40 | ||||
| -rw-r--r-- | rubocop/rubocop.rb | 1 | ||||
| -rw-r--r-- | rubocop/spec_helpers.rb | 13 | ||||
| -rw-r--r-- | spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb | 48 | ||||
| -rw-r--r-- | spec/spec_helper.rb | 1 | ||||
| -rw-r--r-- | spec/support/helpers/expect_offense.rb | 20 |
6 files changed, 122 insertions, 1 deletions
diff --git a/rubocop/cop/rspec/factories_in_migration_specs.rb b/rubocop/cop/rspec/factories_in_migration_specs.rb new file mode 100644 index 00000000000..0c5aa838a2c --- /dev/null +++ b/rubocop/cop/rspec/factories_in_migration_specs.rb @@ -0,0 +1,40 @@ +require_relative '../../spec_helpers' + +module RuboCop + module Cop + module RSpec + # This cop checks for the usage of factories in migration specs + # + # @example + # + # # bad + # let(:user) { create(:user) } + # + # # good + # let(:users) { table(:users) } + # let(:user) { users.create!(name: 'User 1', username: 'user1') } + class FactoriesInMigrationSpecs < RuboCop::Cop::Cop + include SpecHelpers + + MESSAGE = "Don't use FactoryBot.%s in migration specs, use `table` instead.".freeze + FORBIDDEN_METHODS = %i[build build_list create create_list].freeze + + def_node_search :forbidden_factory_usage?, <<~PATTERN + (send {(const nil? :FactoryBot) nil?} {#{FORBIDDEN_METHODS.map(&:inspect).join(' ')}} ...) + PATTERN + + # Following is what node.children looks like on a match: + # - Without FactoryBot namespace: [nil, :build, s(:sym, :user)] + # - With FactoryBot namespace: [s(:const, nil, :FactoryBot), :build, s(:sym, :user)] + def on_send(node) + return unless in_migration_spec?(node) + return unless forbidden_factory_usage?(node) + + method = node.children[1] + + add_offense(node, location: :expression, message: MESSAGE % method) + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 0b4c7d31442..406ec95ffc9 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -21,4 +21,5 @@ require_relative 'cop/migration/update_column_in_batches' require_relative 'cop/migration/update_large_table' require_relative 'cop/project_path_helper' require_relative 'cop/rspec/env_assignment' +require_relative 'cop/rspec/factories_in_migration_specs' require_relative 'cop/sidekiq_options_queue' diff --git a/rubocop/spec_helpers.rb b/rubocop/spec_helpers.rb index a702a083958..6c0f0193b1a 100644 --- a/rubocop/spec_helpers.rb +++ b/rubocop/spec_helpers.rb @@ -6,7 +6,18 @@ module RuboCop def in_spec?(node) path = node.location.expression.source_buffer.name - !SPEC_HELPERS.include?(File.basename(path)) && path.start_with?(File.join(Dir.pwd, 'spec')) + !SPEC_HELPERS.include?(File.basename(path)) && + path.start_with?(File.join(Dir.pwd, 'spec'), File.join(Dir.pwd, 'ee', 'spec')) + end + + # Returns true if the given node originated from a migration spec. + def in_migration_spec?(node) + path = node.location.expression.source_buffer.name + + in_spec?(node) && + path.start_with?( + File.join(Dir.pwd, 'spec', 'migrations'), + File.join(Dir.pwd, 'ee', 'spec', 'migrations')) end end end diff --git a/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb b/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb new file mode 100644 index 00000000000..2763f2bda21 --- /dev/null +++ b/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb @@ -0,0 +1,48 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/rspec/factories_in_migration_specs' + +describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do + include CopHelper + + let(:source_file) { 'spec/migrations/foo_spec.rb' } + + subject(:cop) { described_class.new } + + shared_examples 'an offensive factory call' do |namespace| + %i[build build_list create create_list].each do |forbidden_method| + namespaced_forbidden_method = "#{namespace}#{forbidden_method}(:user)" + + it "registers an offense for #{namespaced_forbidden_method}" do + expect_offense(<<-RUBY) + describe 'foo' do + let(:user) { #{namespaced_forbidden_method} } + #{'^' * namespaced_forbidden_method.size} Don't use FactoryBot.#{forbidden_method} in migration specs, use `table` instead. + end + RUBY + end + end + end + + context 'in a migration spec file' do + before do + allow(cop).to receive(:in_migration_spec?).and_return(true) + end + + it_behaves_like 'an offensive factory call', '' + it_behaves_like 'an offensive factory call', 'FactoryBot.' + end + + context 'outside of a migration spec file' do + it "does not register an offense" do + expect_no_offenses(<<-RUBY) + describe 'foo' do + let(:user) { create(:user) } + end + RUBY + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index beabba99cf5..83664bae046 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -66,6 +66,7 @@ RSpec.configure do |config| config.include MigrationsHelpers, :migration config.include StubFeatureFlags config.include StubENV + config.include ExpectOffense config.infer_spec_type_from_file_location! diff --git a/spec/support/helpers/expect_offense.rb b/spec/support/helpers/expect_offense.rb new file mode 100644 index 00000000000..35718ba90c5 --- /dev/null +++ b/spec/support/helpers/expect_offense.rb @@ -0,0 +1,20 @@ +require 'rubocop/rspec/support' + +# https://github.com/backus/rubocop-rspec/blob/master/spec/support/expect_offense.rb +# rubocop-rspec gem extension of RuboCop's ExpectOffense module. +# +# This mixin is the same as rubocop's ExpectOffense except the default +# filename ends with `_spec.rb` +module ExpectOffense + include RuboCop::RSpec::ExpectOffense + + DEFAULT_FILENAME = 'example_spec.rb'.freeze + + def expect_offense(source, filename = DEFAULT_FILENAME) + super + end + + def expect_no_offenses(source, filename = DEFAULT_FILENAME) + super + end +end |