Merge branch 'pl-cop-factory-association' into 'master'

Introduce cop FactoryBot/InlineAssociation

See merge request gitlab-org/gitlab!44840

.rubocop.yml

@@ -571,3 +571,9 @@ Gitlab/RailsLogger:
   Exclude:
     - 'spec/**/*.rb'
     - 'ee/spec/**/*.rb'
+
+# WIP See https://gitlab.com/gitlab-org/gitlab/-/issues/267606
+FactoryBot/InlineAssociation:
+  Include:
+    - 'spec/factories/**/*.rb'
+    - 'ee/spec/factories/**/*.rb'

.rubocop_todo.yml

@@ -1278,3 +1278,44 @@ Graphql/IDType:
     - 'app/graphql/resolvers/snippets_resolver.rb'
     - 'app/graphql/resolvers/user_merge_requests_resolver.rb'
     - 'app/graphql/resolvers/user_resolver.rb'
+
+# Offense count: 86
+# Cop supports --auto-correct.
+FactoryBot/InlineAssociation:
+  Exclude:
+    - 'ee/spec/factories/analytics/cycle_analytics/group_stages.rb'
+    - 'ee/spec/factories/ci/reports/security/findings.rb'
+    - 'ee/spec/factories/ci/reports/security/reports.rb'
+    - 'ee/spec/factories/geo/event_log.rb'
+    - 'ee/spec/factories/groups.rb'
+    - 'ee/spec/factories/merge_request_blocks.rb'
+    - 'ee/spec/factories/resource_iteration_event.rb'
+    - 'ee/spec/factories/resource_weight_events.rb'
+    - 'ee/spec/factories/vulnerabilities/feedback.rb'
+    - 'spec/factories/atlassian_identities.rb'
+    - 'spec/factories/audit_events.rb'
+    - 'spec/factories/design_management/design_at_version.rb'
+    - 'spec/factories/design_management/designs.rb'
+    - 'spec/factories/design_management/versions.rb'
+    - 'spec/factories/events.rb'
+    - 'spec/factories/git_wiki_commit_details.rb'
+    - 'spec/factories/gitaly/commit.rb'
+    - 'spec/factories/go_module_commits.rb'
+    - 'spec/factories/go_module_versions.rb'
+    - 'spec/factories/go_modules.rb'
+    - 'spec/factories/group_group_links.rb'
+    - 'spec/factories/import_export_uploads.rb'
+    - 'spec/factories/merge_requests.rb'
+    - 'spec/factories/notes.rb'
+    - 'spec/factories/packages.rb'
+    - 'spec/factories/packages/package_file.rb'
+    - 'spec/factories/prometheus_alert.rb'
+    - 'spec/factories/resource_label_events.rb'
+    - 'spec/factories/resource_milestone_event.rb'
+    - 'spec/factories/resource_state_event.rb'
+    - 'spec/factories/sent_notifications.rb'
+    - 'spec/factories/serverless/domain.rb'
+    - 'spec/factories/serverless/domain_cluster.rb'
+    - 'spec/factories/terraform/state.rb'
+    - 'spec/factories/uploads.rb'
+    - 'spec/factories/wiki_pages.rb'

rubocop/cop/rspec/factory_bot/inline_association.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      module FactoryBot
+        # This cop encourages the use of inline associations in FactoryBot.
+        # The explicit use of `create` and `build` is discouraged.
+        #
+        # See https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#inline-definition
+        #
+        # @example
+        #
+        # Context:
+        #
+        #   Factory.define do
+        #     factory :project, class: 'Project'
+        #       # EXAMPLE below
+        #     end
+        #   end
+        #
+        # # bad
+        # creator { create(:user) }
+        # creator { create(:user, :admin) }
+        # creator { build(:user) }
+        # creator { FactoryBot.build(:user) }
+        # creator { ::FactoryBot.build(:user) }
+        # add_attribute(:creator) { build(:user) }
+        #
+        # # good
+        # creator { association(:user) }
+        # creator { association(:user, :admin) }
+        # add_attribute(:creator) { association(:user) }
+        #
+        # # Accepted
+        # after(:build) do |instance|
+        #   instance.creator = create(:user)
+        # end
+        #
+        # initialize_with do
+        #   create(:project)
+        # end
+        #
+        # creator_id { create(:user).id }
+        #
+        class InlineAssociation < RuboCop::Cop::Cop
+          MSG = 'Prefer inline `association` over `%{type}`. ' \
+            'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories'
+
+          REPLACEMENT = 'association'
+
+          def_node_matcher :create_or_build, <<~PATTERN
+            (
+              send
+              ${ nil? (const { nil? (cbase) } :FactoryBot) }
+              ${ :create :build }
+              (sym _)
+              ...
+            )
+          PATTERN
+
+          def_node_matcher :association_definition, <<~PATTERN
+            (block
+              {
+                (send nil? $_)
+                (send nil? :add_attribute (sym $_))
+              }
+              ...
+            )
+          PATTERN
+
+          def_node_matcher :chained_call?, <<~PATTERN
+            (send _ _)
+          PATTERN
+
+          SKIP_NAMES = %i[initialize_with].to_set.freeze
+
+          def on_send(node)
+            _receiver, type = create_or_build(node)
+            return unless type
+            return if chained_call?(node.parent)
+            return unless inside_assocation_definition?(node)
+
+            add_offense(node, message: format(MSG, type: type))
+          end
+
+          def autocorrect(node)
+            lambda do |corrector|
+              receiver, type = create_or_build(node)
+              receiver = "#{receiver.source}." if receiver
+              expression = "#{receiver}#{type}"
+              replacement = node.source.sub(expression, REPLACEMENT)
+              corrector.replace(node.source_range, replacement)
+            end
+          end
+
+          private
+
+          def inside_assocation_definition?(node)
+            node.each_ancestor(:block).any? do |parent|
+              name = association_definition(parent)
+              name && !SKIP_NAMES.include?(name)
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/rspec/factory_bot/inline_association_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rspec-parameterized'
+require 'rubocop'
+
+require_relative '../../../../../rubocop/cop/rspec/factory_bot/inline_association'
+
+RSpec.describe RuboCop::Cop::RSpec::FactoryBot::InlineAssociation, type: :rubocop do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  shared_examples 'offense' do |code_snippet, autocorrected|
+    # We allow `create` or `FactoryBot.create` or `::FactoryBot.create`
+    let(:type) { code_snippet[/^(?:::)?(?:FactoryBot\.)?(\w+)/, 1] }
+    let(:offense_marker) { '^' * code_snippet.size }
+    let(:offense_msg) { msg(type) }
+    let(:offense) { "#{offense_marker} #{offense_msg}" }
+    let(:pristine_source) { source.sub(offense, '') }
+    let(:source) do
+      <<~RUBY
+          FactoryBot.define do
+            factory :project do
+              attribute { #{code_snippet} }
+                          #{offense}
+            end
+          end
+      RUBY
+    end
+
+    it 'registers an offense' do
+      expect_offense(source)
+    end
+
+    it 'autocorrects the source' do
+      corrected = autocorrect_source(pristine_source)
+
+      expect(corrected).not_to include(code_snippet)
+      expect(corrected).to include(autocorrected)
+    end
+  end
+
+  shared_examples 'no offense' do |code_snippet|
+    first_line = code_snippet.lines.first.chomp
+
+    context "for `#{first_line}`" do
+      it 'does not register any offenses' do
+        expect_no_offenses <<~RUBY
+          FactoryBot.define do
+            factory :project do
+              #{code_snippet}
+            end
+          end
+        RUBY
+      end
+    end
+  end
+
+  context 'offenses' do
+    using RSpec::Parameterized::TableSyntax
+
+    where(:code_snippet, :autocorrected) do
+      # create
+      'create(:user)'              | 'association(:user)'
+      'FactoryBot.create(:user)'   | 'association(:user)'
+      '::FactoryBot.create(:user)' | 'association(:user)'
+      'create(:user, :admin)'      | 'association(:user, :admin)'
+      'create(:user, name: "any")' | 'association(:user, name: "any")'
+      # build
+      'build(:user)'               | 'association(:user)'
+      'FactoryBot.build(:user)'    | 'association(:user)'
+      '::FactoryBot.build(:user)'  | 'association(:user)'
+      'build(:user, :admin)'       | 'association(:user, :admin)'
+      'build(:user, name: "any")'  | 'association(:user, name: "any")'
+    end
+
+    with_them do
+      include_examples 'offense', params[:code_snippet], params[:autocorrected]
+    end
+
+    it 'recognizes `add_attribute`' do
+      expect_offense <<~RUBY
+        FactoryBot.define do
+          factory :project, class: 'Project' do
+            add_attribute(:method) { create(:user) }
+                                     ^^^^^^^^^^^^^ #{msg(:create)}
+          end
+        end
+      RUBY
+    end
+
+    it 'recognizes `transient` attributes' do
+      expect_offense <<~RUBY
+        FactoryBot.define do
+          factory :project, class: 'Project' do
+            transient do
+              creator { create(:user) }
+                        ^^^^^^^^^^^^^ #{msg(:create)}
+            end
+          end
+        end
+      RUBY
+    end
+  end
+
+  context 'no offenses' do
+    include_examples 'no offense', 'association(:user)'
+    include_examples 'no offense', 'association(:user, :admin)'
+    include_examples 'no offense', 'association(:user, name: "any")'
+
+    include_examples 'no offense', <<~RUBY
+      after(:build) do |object|
+        object.user = create(:user)
+      end
+    RUBY
+
+    include_examples 'no offense', <<~RUBY
+      initialize_with do
+        create(:user)
+      end
+    RUBY
+
+    include_examples 'no offense', <<~RUBY
+      user_id { create(:user).id }
+    RUBY
+  end
+
+  def msg(type)
+    format(described_class::MSG, type: type)
+  end
+end