From 835f9a6260cf6ac21700f6b6474ec760d3d40db1 Mon Sep 17 00:00:00 2001
From: Brian Williams <bwilliams@gitlab.com>
Date: Wed, 28 Aug 2024 17:00:23 +0000
Subject: [PATCH] Fix FactoriesInMigrationSpecs flagging sends with no
arguments
`...` matches on zero arguments which causes it to flag things like
`build.id`. `_` matches on at least one argument, so it should only flag
things like `build(:model)`.
---
.../cop/rspec/factories_in_migration_specs.rb | 2 +-
.../factories_in_migration_specs_spec.rb | 29 +++++++++++++++----
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/rubocop/cop/rspec/factories_in_migration_specs.rb b/rubocop/cop/rspec/factories_in_migration_specs.rb
index 7dce1264b0e1..cf90b768a22d 100644
--- a/rubocop/cop/rspec/factories_in_migration_specs.rb
+++ b/rubocop/cop/rspec/factories_in_migration_specs.rb
@@ -20,7 +20,7 @@ class FactoriesInMigrationSpecs < RuboCop::Cop::Base
FORBIDDEN_METHODS = %i[build build_list create create_list attributes_for].freeze
def_node_search :forbidden_factory_usage?, <<~PATTERN
- (send {(const nil? :FactoryBot) nil?} {#{FORBIDDEN_METHODS.map(&:inspect).join(' ')}} ...)
+ (send {(const nil? :FactoryBot) nil?} {#{FORBIDDEN_METHODS.map(&:inspect).join(' ')}} _ ...)
PATTERN
# Following is what node.children looks like on a match:
diff --git a/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb b/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb
index e41dd338387d..29964d68cf6c 100644
--- a/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb
+++ b/spec/rubocop/cop/rspec/factories_in_migration_specs_spec.rb
@@ -7,15 +7,34 @@
RSpec.describe RuboCop::Cop::RSpec::FactoriesInMigrationSpecs do
shared_examples 'an offensive factory call' do |namespace|
%i[build build_list create create_list attributes_for].each do |forbidden_method|
- namespaced_forbidden_method = "#{namespace}#{forbidden_method}(:user)"
+ namespaced_forbidden_method = "#{namespace}#{forbidden_method}"
- it "registers an offense for #{namespaced_forbidden_method}" do
- expect_offense(<<-RUBY)
+ it "registers an offense for #{namespaced_forbidden_method} with one arg" do
+ code = <<-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.
+ let(:user) { %{namespaced_method}(:user) }
+ ^{namespaced_method}^^^^^^^ Don't use FactoryBot.%{method} in migration specs, use `table` instead.
end
RUBY
+
+ expect_offense(code, method: forbidden_method, namespaced_method: namespaced_forbidden_method)
+ end
+
+ it "registers an offense for #{namespaced_forbidden_method} with multiple args" do
+ code = <<-RUBY
+ describe 'foo' do
+ let(:user) { %{namespaced_method}(:user, name: name) }
+ ^{namespaced_method}^^^^^^^^^^^^^^^^^^^ Don't use FactoryBot.%{method} in migration specs, use `table` instead.
+ end
+ RUBY
+
+ expect_offense(code, method: forbidden_method, namespaced_method: namespaced_forbidden_method)
+ end
+
+ it "does not create an offense for #{namespaced_forbidden_method} with no args" do
+ expect_no_offenses(<<~RUBY)
+ let(:id) { #{namespaced_forbidden_method}.id }
+ RUBY
end
end
end
--
GitLab