diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb
index 6fc018bb04c5697a049c281a4d4a03961a90ac3d..2a7c3fb58fb90e2450e8b8ddab018fd719e49624 100644
--- a/app/models/ci/pipeline.rb
+++ b/app/models/ci/pipeline.rb
@@ -20,7 +20,7 @@ class Pipeline < ActiveRecord::Base
has_many :sourced_pipelines, class_name: Ci::Sources::Pipeline, foreign_key: :source_pipeline_id
has_one :triggered_by_pipeline, through: :source_pipeline, source: :source_pipeline
- has_many :triggered_pipelines, through: :sourced_pipelines, source: :pipeline
+ has_many :triggered_pipelines, -> { auto_include(false) }, through: :sourced_pipelines, source: :pipeline
has_many :auto_canceled_pipelines, class_name: 'Ci::Pipeline', foreign_key: 'auto_canceled_by_id'
has_many :auto_canceled_jobs, class_name: 'CommitStatus', foreign_key: 'auto_canceled_by_id'
diff --git a/app/models/ci/runner.rb b/app/models/ci/runner.rb
index 05aeb6b20e91026f75a4655bb9e8b62cdc6e50ba..a1c04d7bdf86b78bcb4c625b8afed00536644ef8 100644
--- a/app/models/ci/runner.rb
+++ b/app/models/ci/runner.rb
@@ -14,7 +14,7 @@ class Runner < ActiveRecord::Base
has_many :builds
has_many :runner_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
- has_many :projects, through: :runner_projects
+ has_many :projects, -> { auto_include(false) }, through: :runner_projects
has_one :last_build, ->() { order('id DESC') }, class_name: 'Ci::Build'
diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb
index 89a74b7dcb1d94a8fb612121454d1ce1698b4219..858b7ef533e7ad137295269e42a26b3498847af2 100644
--- a/app/models/deploy_key.rb
+++ b/app/models/deploy_key.rb
@@ -2,7 +2,7 @@ class DeployKey < Key
include IgnorableColumn
has_many :deploy_keys_projects, inverse_of: :deploy_key, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
- has_many :projects, through: :deploy_keys_projects
+ has_many :projects, -> { auto_include(false) }, through: :deploy_keys_projects
scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
scope :are_public, -> { where(public: true) }
diff --git a/app/models/deploy_token.rb b/app/models/deploy_token.rb
index fe726b156d43a5bbba27ebb30c79e340b875ed47..b47b2ff4c3fb1d08b2ef2b7b4e4aaf14a08f8b56 100644
--- a/app/models/deploy_token.rb
+++ b/app/models/deploy_token.rb
@@ -8,7 +8,7 @@ class DeployToken < ActiveRecord::Base
default_value_for(:expires_at) { Forever.date }
has_many :project_deploy_tokens, inverse_of: :deploy_token
- has_many :projects, through: :project_deploy_tokens
+ has_many :projects, -> { auto_include(false) }, through: :project_deploy_tokens
validate :ensure_at_least_one_scope
before_save :ensure_token
diff --git a/app/models/fork_network.rb b/app/models/fork_network.rb
index 7f1728e8c772ba76555cfa5ca0d6aae1c6ad56de..aad3509b895620f4fea95ac929b8d9958b9be337 100644
--- a/app/models/fork_network.rb
+++ b/app/models/fork_network.rb
@@ -1,7 +1,7 @@
class ForkNetwork < ActiveRecord::Base
belongs_to :root_project, class_name: 'Project'
has_many :fork_network_members
- has_many :projects, through: :fork_network_members
+ has_many :projects, -> { auto_include(false) }, through: :fork_network_members
after_create :add_root_as_member, if: :root_project
diff --git a/app/models/group.rb b/app/models/group.rb
index 90d28b17dd5473b5825e622ff47a26b8cfe611a8..b50a72c874551ce01d303873ee5befcfb33f577b 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -15,9 +15,9 @@ class Group < Namespace
has_many :group_members, -> { where(requested_at: nil) }, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
alias_method :members, :group_members
- has_many :users, through: :group_members
+ has_many :users, -> { auto_include(false) }, through: :group_members
has_many :owners,
- -> { where(members: { access_level: Gitlab::Access::OWNER }) },
+ -> { where(members: { access_level: Gitlab::Access::OWNER }).auto_include(false) },
through: :group_members,
source: :user
@@ -26,7 +26,7 @@ class Group < Namespace
has_many :milestones
has_many :project_group_links, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
- has_many :shared_projects, through: :project_group_links, source: :project
+ has_many :shared_projects, -> { auto_include(false) }, through: :project_group_links, source: :project
has_many :notification_settings, dependent: :destroy, as: :source # rubocop:disable Cop/ActiveRecordDependent
has_many :labels, class_name: 'GroupLabel'
has_many :variables, class_name: 'Ci::GroupVariable'
diff --git a/app/models/lfs_object.rb b/app/models/lfs_object.rb
index 6e7e200f0fc1ab1c132544f65547a5ce4f8962b3..7ab2be36b9ad9e8d66abb023790f1689d2cdd91e 100644
--- a/app/models/lfs_object.rb
+++ b/app/models/lfs_object.rb
@@ -4,7 +4,7 @@ class LfsObject < ActiveRecord::Base
include ObjectStorage::BackgroundMove
has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
- has_many :projects, through: :lfs_objects_projects
+ has_many :projects, -> { auto_include(false) }, through: :lfs_objects_projects
scope :with_files_stored_locally, -> { where(file_store: [nil, LfsObjectUploader::Store::LOCAL]) }
scope :with_files_stored_remotely, -> { where(file_store: LfsObjectUploader::Store::REMOTE) }
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 194e01815f573000cd3b14b6dadbb49c25eba79d..b8544f06c5df2d5416afb4b33120b9b43a3b1c45 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -26,7 +26,7 @@ class Milestone < ActiveRecord::Base
has_many :boards
has_many :issues
- has_many :labels, -> { distinct.reorder('labels.title') }, through: :issues
+ has_many :labels, -> { distinct.reorder('labels.title').auto_include(false) }, through: :issues
has_many :merge_requests
has_many :events, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
diff --git a/app/models/project.rb b/app/models/project.rb
index 9e369d94047b07a2ab3600f2744c690707ae1720..16bc8dc14bca51baa9000793f7e133154cee29d3 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -143,11 +143,11 @@ class Project < ActiveRecord::Base
has_one :packagist_service
# TODO: replace these relations with the fork network versions
- has_one :forked_project_link, foreign_key: "forked_to_project_id"
- has_one :forked_from_project, through: :forked_project_link
+ has_one :forked_project_link, foreign_key: "forked_to_project_id"
+ has_one :forked_from_project, -> { auto_include(false) }, through: :forked_project_link
has_many :forked_project_links, foreign_key: "forked_from_project_id"
- has_many :forks, through: :forked_project_links, source: :forked_to_project
+ has_many :forks, -> { auto_include(false) }, through: :forked_project_links, source: :forked_to_project
# TODO: replace these relations with the fork network versions
has_one :root_of_fork_network,
@@ -155,7 +155,7 @@ class Project < ActiveRecord::Base
inverse_of: :root_project,
class_name: 'ForkNetwork'
has_one :fork_network_member
- has_one :fork_network, through: :fork_network_member
+ has_one :fork_network, -> { auto_include(false) }, through: :fork_network_member
# Merge Requests for target project should be removed with it
has_many :merge_requests, foreign_key: 'target_project_id'
@@ -172,12 +172,12 @@ class Project < ActiveRecord::Base
has_many :protected_tags
has_many :project_authorizations
- has_many :authorized_users, through: :project_authorizations, source: :user, class_name: 'User'
+ has_many :authorized_users, -> { auto_include(false) }, through: :project_authorizations, source: :user, class_name: 'User'
has_many :project_members, -> { where(requested_at: nil) },
as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
alias_method :members, :project_members
- has_many :users, through: :project_members
+ has_many :users, -> { auto_include(false) }, through: :project_members
has_many :requesters, -> { where.not(requested_at: nil) },
as: :source, class_name: 'ProjectMember', dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
@@ -186,13 +186,13 @@ class Project < ActiveRecord::Base
has_many :deploy_keys_projects
has_many :deploy_keys, -> { auto_include(false) }, through: :deploy_keys_projects
has_many :users_star_projects
- has_many :starrers, through: :users_star_projects, source: :user
+ has_many :starrers, -> { auto_include(false) }, through: :users_star_projects, source: :user
has_many :releases
has_many :lfs_objects_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
- has_many :lfs_objects, through: :lfs_objects_projects
+ has_many :lfs_objects, -> { auto_include(false) }, through: :lfs_objects_projects
has_many :lfs_file_locks
has_many :project_group_links
- has_many :invited_groups, through: :project_group_links, source: :group
+ has_many :invited_groups, -> { auto_include(false) }, through: :project_group_links, source: :group
has_many :pages_domains
has_many :todos
has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
@@ -223,14 +223,14 @@ class Project < ActiveRecord::Base
has_many :builds, class_name: 'Ci::Build', inverse_of: :project, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :build_trace_section_names, class_name: 'Ci::BuildTraceSectionName'
has_many :runner_projects, class_name: 'Ci::RunnerProject'
- has_many :runners, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
+ has_many :runners, -> { auto_include(false) }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
has_many :variables, class_name: 'Ci::Variable'
has_many :triggers, class_name: 'Ci::Trigger'
has_many :environments
has_many :deployments
has_many :pipeline_schedules, class_name: 'Ci::PipelineSchedule'
has_many :project_deploy_tokens
- has_many :deploy_tokens, through: :project_deploy_tokens
+ has_many :deploy_tokens, -> { auto_include(false) }, through: :project_deploy_tokens
has_many :active_runners, -> { active.auto_include(false) }, through: :runner_projects, source: :runner, class_name: 'Ci::Runner'
diff --git a/app/models/user.rb b/app/models/user.rb
index cde119c4feeb07617a40a0f0542016e2b41c4c61..4241b0ed36bdf0006085e3a995aa1b46af854131 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -103,18 +103,18 @@ def update_tracked_fields!(request)
has_many :masters_groups, -> { where(members: { access_level: Gitlab::Access::MASTER }).auto_include(false) }, through: :group_members, source: :group
# Projects
- has_many :groups_projects, through: :groups, source: :projects
- has_many :personal_projects, through: :namespace, source: :projects
+ has_many :groups_projects, -> { auto_include(false) }, through: :groups, source: :projects
+ has_many :personal_projects, -> { auto_include(false) }, through: :namespace, source: :projects
has_many :project_members, -> { where(requested_at: nil) }
- has_many :projects, through: :project_members
- has_many :created_projects, foreign_key: :creator_id, class_name: 'Project'
+ has_many :projects, -> { auto_include(false) }, through: :project_members
+ has_many :created_projects, foreign_key: :creator_id, class_name: 'Project'
has_many :users_star_projects, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
- has_many :starred_projects, through: :users_star_projects, source: :project
+ has_many :starred_projects, -> { auto_include(false) }, through: :users_star_projects, source: :project
has_many :project_authorizations
- has_many :authorized_projects, through: :project_authorizations, source: :project
+ has_many :authorized_projects, -> { auto_include(false) }, through: :project_authorizations, source: :project
has_many :user_interacted_projects
- has_many :project_interactions, through: :user_interacted_projects, source: :project, class_name: 'Project'
+ has_many :project_interactions, -> { auto_include(false) }, through: :user_interacted_projects, source: :project, class_name: 'Project'
has_many :snippets, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
has_many :notes, dependent: :destroy, foreign_key: :author_id # rubocop:disable Cop/ActiveRecordDependent
@@ -134,7 +134,7 @@ def update_tracked_fields!(request)
has_many :triggers, dependent: :destroy, class_name: 'Ci::Trigger', foreign_key: :owner_id # rubocop:disable Cop/ActiveRecordDependent
has_many :issue_assignees
- has_many :assigned_issues, class_name: "Issue", through: :issue_assignees, source: :issue
+ has_many :assigned_issues, -> { auto_include(false) }, class_name: "Issue", through: :issue_assignees, source: :issue
has_many :assigned_merge_requests, dependent: :nullify, foreign_key: :assignee_id, class_name: "MergeRequest" # rubocop:disable Cop/ActiveRecordDependent
has_many :custom_attributes, class_name: 'UserCustomAttribute'
diff --git a/rubocop/cop/gitlab/has_many_through_scope.rb b/rubocop/cop/gitlab/has_many_through_scope.rb
new file mode 100644
index 0000000000000000000000000000000000000000..770a2a0529f28a59afbd675d2b73376deb8fc357
--- /dev/null
+++ b/rubocop/cop/gitlab/has_many_through_scope.rb
@@ -0,0 +1,45 @@
+require 'gitlab/styles/rubocop/model_helpers'
+
+module RuboCop
+ module Cop
+ module Gitlab
+ class HasManyThroughScope < RuboCop::Cop::Cop
+ include ::Gitlab::Styles::Rubocop::ModelHelpers
+
+ MSG = 'Always provide an explicit scope calling auto_include(false) when using has_many :through'.freeze
+
+ def_node_search :through?, <<~PATTERN
+ (pair (sym :through) _)
+ PATTERN
+
+ def_node_matcher :has_many_through?, <<~PATTERN
+ (send nil? :has_many ... #through?)
+ PATTERN
+
+ def_node_search :disables_auto_include?, <<~PATTERN
+ (send _ :auto_include false)
+ PATTERN
+
+ def_node_matcher :scope_disables_auto_include?, <<~PATTERN
+ (block (send nil? :lambda) _ #disables_auto_include?)
+ PATTERN
+
+ def on_send(node)
+ return unless in_model?(node)
+ return unless has_many_through?(node)
+
+ target = node
+ scope_argument = node.children[3]
+
+ if scope_argument.children[0].children.last == :lambda
+ return if scope_disables_auto_include?(scope_argument)
+
+ target = scope_argument
+ end
+
+ add_offense(target, location: :expression)
+ end
+ end
+ end
+ end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index 406ec95ffc93f1f0e3ee637f7212ed5d5be2f6a6..c2254332e7dd5290acef51ee4a7c15a5ab77463e 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -1,7 +1,8 @@
# rubocop:disable Naming/FileName
+require_relative 'cop/gitlab/has_many_through_scope'
+require_relative 'cop/gitlab/httparty'
require_relative 'cop/gitlab/module_with_instance_variables'
require_relative 'cop/gitlab/predicate_memoization'
-require_relative 'cop/gitlab/httparty'
require_relative 'cop/include_sidekiq_worker'
require_relative 'cop/line_break_around_conditional_block'
require_relative 'cop/migration/add_column'
diff --git a/spec/rubocop/cop/gitlab/has_many_through_scope_spec.rb b/spec/rubocop/cop/gitlab/has_many_through_scope_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..6d769c8e6fd00edda48b06652a8d2ef56a6c0536
--- /dev/null
+++ b/spec/rubocop/cop/gitlab/has_many_through_scope_spec.rb
@@ -0,0 +1,74 @@
+require 'spec_helper'
+
+require 'rubocop'
+require 'rubocop/rspec/support'
+
+require_relative '../../../../rubocop/cop/gitlab/has_many_through_scope'
+
+describe RuboCop::Cop::Gitlab::HasManyThroughScope do # rubocop:disable RSpec/FilePath
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ context 'in a model file' do
+ before do
+ allow(cop).to receive(:in_model?).and_return(true)
+ end
+
+ context 'when the model does not use has_many :through' do
+ it 'does not register an offense' do
+ expect_no_offenses(<<-RUBY)
+ class User < ActiveRecord::Base
+ has_many :tags, source: 'UserTag'
+ end
+ RUBY
+ end
+ end
+
+ context 'when the model uses has_many :through' do
+ context 'when the association has no scope defined' do
+ it 'registers an offense on the association' do
+ expect_offense(<<-RUBY)
+ class User < ActiveRecord::Base
+ has_many :tags, through: :user_tags
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+ end
+ RUBY
+ end
+ end
+
+ context 'when the association has a scope defined' do
+ context 'when the scope does not disable auto-loading' do
+ it 'registers an offense on the scope' do
+ expect_offense(<<-RUBY)
+ class User < ActiveRecord::Base
+ has_many :tags, -> { where(active: true) }, through: :user_tags
+ ^^^^^^^^^^^^^^^^^^^^^^^^^^ #{described_class::MSG}
+ end
+ RUBY
+ end
+ end
+
+ context 'when the scope has auto_include(false)' do
+ it 'does not register an offense' do
+ expect_no_offenses(<<-RUBY)
+ class User < ActiveRecord::Base
+ has_many :tags, -> { where(active: true).auto_include(false).reorder(nil) }, through: :user_tags
+ end
+ RUBY
+ end
+ end
+ end
+ end
+ end
+
+ context 'outside of a migration spec file' do
+ it 'does not register an offense' do
+ expect_no_offenses(<<-RUBY)
+ class User < ActiveRecord::Base
+ has_many :tags, through: :user_tags
+ end
+ RUBY
+ end
+ end
+end