Merge branch 'ee-rubocop-prepend' into 'master'

EE: Added Cop for injecting EE modules

See merge request gitlab-org/gitlab-ee!8843

.rubocop.yml

@@ -175,3 +175,9 @@ GitlabSecurity/PublicSend:
     - 'ee/db/**/*'
     - 'ee/lib/**/*.rake'
     - 'ee/spec/**/*'
+
+Cop/InjectEnterpriseEditionModule:
+  Enabled: true
+  Exclude:
+    - 'spec/**/*'
+    - 'ee/spec/**/*'

app/controllers/confirmations_controller.rb

 # frozen_string_literal: true
 
 class ConfirmationsController < Devise::ConfirmationsController
-  prepend ::EE::ConfirmationsController
+  prepend ::EE::ConfirmationsController # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   include AcceptsPendingInvitations
 

app/controllers/ldap/omniauth_callbacks_controller.rb

@@ -32,5 +32,5 @@ class Ldap::OmniauthCallbacksController < OmniauthCallbacksController
   end
 end
 
-Ldap::OmniauthCallbacksController.prepend(EE::OmniauthCallbacksController)
+Ldap::OmniauthCallbacksController.prepend(EE::OmniauthCallbacksController) # rubocop: disable Cop/InjectEnterpriseEditionModule
 Ldap::OmniauthCallbacksController.prepend(EE::Ldap::OmniauthCallbacksController)

app/finders/license_template_finder.rb

@@ -13,7 +13,7 @@
 class LicenseTemplateFinder
   include Gitlab::Utils::StrongMemoize
 
-  prepend ::EE::LicenseTemplateFinder
+  prepend ::EE::LicenseTemplateFinder # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   attr_reader :project, :params
 

app/finders/projects_finder.rb

@@ -24,7 +24,7 @@
 class ProjectsFinder < UnionFinder
   include CustomAttributesFilter
 
-  prepend ::EE::ProjectsFinder
+  prepend ::EE::ProjectsFinder # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   attr_accessor :params
   attr_reader :current_user, :project_ids_relation

app/finders/template_finder.rb

@@ -3,7 +3,7 @@
 class TemplateFinder
   include Gitlab::Utils::StrongMemoize
 
-  prepend ::EE::TemplateFinder
+  prepend ::EE::TemplateFinder # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   VENDORED_TEMPLATES = HashWithIndifferentAccess.new(
     dockerfiles: ::Gitlab::Template::DockerfileTemplate,

app/helpers/application_settings_helper.rb

@@ -227,7 +227,7 @@ module ApplicationSettingsHelper
   end
 end
 
-ApplicationSettingsHelper.prepend(EE::ApplicationSettingsHelper)
+ApplicationSettingsHelper.prepend(EE::ApplicationSettingsHelper) # rubocop: disable Cop/InjectEnterpriseEditionModule
 
 # The methods in `EE::ApplicationSettingsHelper` should be available as both
 # instance and class methods.

app/helpers/auth_helper.rb

@@ -100,7 +100,7 @@ module AuthHelper
   extend self
 end
 
-AuthHelper.prepend(EE::AuthHelper)
+AuthHelper.prepend(EE::AuthHelper) # rubocop: disable Cop/InjectEnterpriseEditionModule
 
 # The methods added in EE should be available as both class and instance
 # methods, just like the methods provided by `AuthHelper` itself.

app/helpers/environments_helper.rb

 # frozen_string_literal: true
 
 module EnvironmentsHelper
-  prepend ::EE::EnvironmentsHelper
+  prepend ::EE::EnvironmentsHelper # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   def environments_list_data
     {

app/helpers/form_helper.rb

 # frozen_string_literal: true
 
 module FormHelper
-  prepend ::EE::FormHelper
+  prepend ::EE::FormHelper # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   def form_errors(model, type: 'form')
     return unless model.errors.any?

app/helpers/projects_helper.rb

 # frozen_string_literal: true
 
 module ProjectsHelper
-  prepend ::EE::ProjectsHelper
+  prepend ::EE::ProjectsHelper # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   def link_to_project(project)
     link_to namespace_project_path(namespace_id: project.namespace, id: project), title: h(project.name) do

app/helpers/services_helper.rb

@@ -45,7 +45,7 @@ module ServicesHelper
   extend self
 end
 
-ServicesHelper.prepend(EE::ServicesHelper)
+ServicesHelper.prepend(EE::ServicesHelper) # rubocop: disable Cop/InjectEnterpriseEditionModule
 
 # The methods in `EE::ServicesHelper` should be available as both instance and
 # class methods.

app/helpers/sorting_helper.rb

 # frozen_string_literal: true
 
 module SortingHelper
-  prepend ::EE::SortingHelper
+  prepend ::EE::SortingHelper # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   def sort_options_hash
     {

app/helpers/system_note_helper.rb

@@ -39,7 +39,7 @@ module SystemNoteHelper
   extend self
 end
 
-SystemNoteHelper.prepend(EE::SystemNoteHelper)
+SystemNoteHelper.prepend(EE::SystemNoteHelper) # rubocop: disable Cop/InjectEnterpriseEditionModule
 
 # The methods in `EE::SystemNoteHelper` should be available as both instance and
 # class methods.

app/mailers/emails/merge_requests.rb

@@ -2,7 +2,7 @@
 
 module Emails
   module MergeRequests
-    prepend Emails::EE::MergeRequests
+    prepend Emails::EE::MergeRequests # rubocop: disable Cop/InjectEnterpriseEditionModule
 
     def new_merge_request_email(recipient_id, merge_request_id, reason = nil)
       setup_merge_request_mail(merge_request_id, recipient_id, present: true)

app/mailers/emails/notes.rb

@@ -2,7 +2,7 @@
 
 module Emails
   module Notes
-    prepend Emails::EE::Notes
+    prepend Emails::EE::Notes # rubocop: disable Cop/InjectEnterpriseEditionModule
 
     def note_commit_email(recipient_id, note_id)
       setup_note_mail(note_id, recipient_id)

app/mailers/emails/projects.rb

@@ -2,7 +2,7 @@
 
 module Emails
   module Projects
-    prepend Emails::EE::Projects
+    prepend Emails::EE::Projects # rubocop: disable Cop/InjectEnterpriseEditionModule
 
     def project_was_moved_email(project_id, user_id, old_path_with_namespace)
       @current_user = @user = User.find user_id

app/mailers/notify.rb

 # frozen_string_literal: true
 
 class Notify < BaseMailer
-  prepend ::EE::Notify
+  prepend ::EE::Notify # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   include ActionDispatch::Routing::PolymorphicRoutes
   include GitlabRoutingHelper

app/models/commit_status.rb

@@ -7,7 +7,7 @@ class CommitStatus < ActiveRecord::Base
   include Presentable
   include EnumWithNil
 
-  prepend ::EE::CommitStatus
+  prepend ::EE::CommitStatus # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   self.table_name = 'ci_builds'
 

app/models/concerns/prometheus_adapter.rb

@@ -5,7 +5,8 @@ module PrometheusAdapter
 
   included do
     include ReactiveCaching
-    prepend EE::PrometheusAdapter # We can't prepend outside of this model due to the use of `included`, so this must stay here.
+    # We can't prepend outside of this model due to the use of `included`, so this must stay here.
+    prepend EE::PrometheusAdapter # rubocop: disable Cop/InjectEnterpriseEditionModule
 
     self.reactive_cache_key = ->(adapter) { [adapter.class.model_name.singular, adapter.id] }
     self.reactive_cache_lease_timeout = 30.seconds

app/models/concerns/protected_ref_access.rb

@@ -52,7 +52,7 @@ module ProtectedRefAccess
 end
 
 ProtectedRefAccess.include(EE::ProtectedRefAccess::Scopes)
-ProtectedRefAccess.prepend(EE::ProtectedRefAccess)
+ProtectedRefAccess.prepend(EE::ProtectedRefAccess) # rubocop: disable Cop/InjectEnterpriseEditionModule
 
 # When using `prepend` (or `include` for that matter), the `ClassMethods`
 # constants are not merged. This means that `class_methods` in

app/models/discussion_note.rb

@@ -4,7 +4,8 @@
 #
 # A note of this type can be resolvable.
 class DiscussionNote < Note
-  prepend EE::DiscussionNote # This prepend must stay here because the `validates` below depends on it.
+  # This prepend must stay here because the `validates` below depends on it.
+  prepend EE::DiscussionNote # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   # Names of all implementers of `Noteable` that support discussions.
   def self.noteable_types

app/models/global_milestone.rb

@@ -3,7 +3,7 @@
 class GlobalMilestone
   include Milestoneish
 
-  include ::EE::GlobalMilestone
+  include ::EE::GlobalMilestone # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   EPOCH = DateTime.parse('1970-01-01')
   STATE_COUNT_HASH = { opened: 0, closed: 0, all: 0 }.freeze

app/models/group_milestone.rb

 # frozen_string_literal: true
 # Group Milestones are milestones that can be shared among many projects within the same group
 class GroupMilestone < GlobalMilestone
-  include ::EE::GroupMilestone
+  include ::EE::GroupMilestone # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   attr_accessor :group
 

app/models/list.rb

 # frozen_string_literal: true
 
 class List < ActiveRecord::Base
-  prepend ::EE::List
+  prepend ::EE::List # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   belongs_to :board
   belongs_to :label

app/models/members_preloader.rb

 # frozen_string_literal: true
 
 class MembersPreloader
-  prepend EE::MembersPreloader
+  prepend EE::MembersPreloader # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   attr_reader :members
 

app/models/merge_request.rb

@@ -25,7 +25,7 @@ class MergeRequest < ActiveRecord::Base
                 :ref_fetched,
                 :deleted_at
 
-  prepend ::EE::MergeRequest
+  prepend ::EE::MergeRequest # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   belongs_to :target_project, class_name: "Project"
   belongs_to :source_project, class_name: "Project"

app/models/milestone.rb

@@ -18,7 +18,7 @@ class Milestone < ActiveRecord::Base
   include Milestoneish
   include Gitlab::SQL::Pattern
 
-  prepend ::EE::Milestone
+  prepend ::EE::Milestone # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   cache_markdown_field :title, pipeline: :single_line
   cache_markdown_field :description

app/models/project_authorization.rb

@@ -2,7 +2,7 @@
 
 class ProjectAuthorization < ActiveRecord::Base
   include FromUnion
-  prepend ::EE::ProjectAuthorization
+  prepend ::EE::ProjectAuthorization # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   belongs_to :user
   belongs_to :project

app/models/project_import_data.rb

@@ -3,7 +3,7 @@
 require 'carrierwave/orm/activerecord'
 
 class ProjectImportData < ActiveRecord::Base
-  prepend ::EE::ProjectImportData
+  prepend ::EE::ProjectImportData # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   belongs_to :project, inverse_of: :import_data
   attr_encrypted :credentials,

app/presenters/commit_status_presenter.rb

@@ -17,7 +17,7 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated
 
   presents :build
 
-  prepend ::EE::CommitStatusPresenter
+  prepend ::EE::CommitStatusPresenter # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   def self.callout_failure_messages
     CALLOUT_FAILURE_MESSAGES

app/serializers/build_details_entity.rb

 # frozen_string_literal: true
 
 class BuildDetailsEntity < JobEntity
-  prepend ::EE::BuildDetailEntity
+  prepend ::EE::BuildDetailEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :coverage, :erased_at, :duration
   expose :tag_list, as: :tags

app/serializers/environment_entity.rb

@@ -2,7 +2,7 @@
 
 class EnvironmentEntity < Grape::Entity
   include RequestAwareEntity
-  prepend ::EE::EnvironmentEntity
+  prepend ::EE::EnvironmentEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :id
   expose :name

app/serializers/issuable_sidebar_entity.rb

@@ -3,7 +3,7 @@
 class IssuableSidebarEntity < Grape::Entity
   include TimeTrackableEntity
   include RequestAwareEntity
-  prepend ::EE::IssuableSidebarEntity
+  prepend ::EE::IssuableSidebarEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :participants, using: ::API::Entities::UserBasic do |issuable|
     issuable.participants(request.current_user)

app/serializers/issue_board_entity.rb

@@ -2,7 +2,7 @@
 
 class IssueBoardEntity < Grape::Entity
   include RequestAwareEntity
-  prepend ::EE::IssueBoardEntity
+  prepend ::EE::IssueBoardEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :id
   expose :iid

app/serializers/issue_entity.rb

@@ -2,7 +2,7 @@
 
 class IssueEntity < IssuableEntity
   include TimeTrackableEntity
-  prepend ::EE::IssueEntity
+  prepend ::EE::IssueEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :state
   expose :milestone_id

app/serializers/issue_sidebar_entity.rb

 # frozen_string_literal: true
 
 class IssueSidebarEntity < IssuableSidebarEntity
-  prepend ::EE::IssueSidebarEntity
+  prepend ::EE::IssueSidebarEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :assignees, using: API::Entities::UserBasic
 end

app/serializers/merge_request_widget_entity.rb

 # frozen_string_literal: true
 
 class MergeRequestWidgetEntity < IssuableEntity
-  prepend ::EE::MergeRequestWidgetEntity
+  prepend ::EE::MergeRequestWidgetEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :state
   expose :in_progress_merge_commit_sha

app/serializers/project_mirror_entity.rb

 # frozen_string_literal: true
 
 class ProjectMirrorEntity < Grape::Entity
-  prepend ::EE::ProjectMirrorEntity
+  prepend ::EE::ProjectMirrorEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :id
 

app/serializers/variable_entity.rb

 # frozen_string_literal: true
 
 class VariableEntity < Grape::Entity
-  prepend ::EE::VariableEntity
+  prepend ::EE::VariableEntity # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   expose :id
   expose :key

app/services/base_count_service.rb

@@ -3,7 +3,7 @@
 # Base class for services that count a single resource such as the number of
 # issues for a project.
 class BaseCountService
-  prepend ::EE::BaseCountService
+  prepend ::EE::BaseCountService # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   def relation_for_count
     raise(

app/services/issuable_base_service.rb

 # frozen_string_literal: true
 
 class IssuableBaseService < BaseService
-  prepend ::EE::IssuableBaseService
+  prepend ::EE::IssuableBaseService # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   private
 

ee/app/models/ee/issue.rb

@@ -11,7 +11,7 @@ module EE
       WEIGHT_ANY = 'Any'.freeze
       WEIGHT_NONE = 'None'.freeze
 
-      prepend EE::RelativePositioning
+      prepend EE::RelativePositioning # rubocop: disable Cop/InjectEnterpriseEditionModule
       include Elastic::IssuesSearch
 
       scope :order_weight_desc, -> { reorder ::Gitlab::Database.nulls_last_order('weight', 'DESC') }

ee/app/models/ee/project.rb

@@ -10,13 +10,13 @@ module EE
     extend ::Gitlab::Utils::Override
     extend ::Gitlab::Cache::RequestCache
     include ::Gitlab::Utils::StrongMemoize
-    include ::EE::GitlabRoutingHelper
+    include ::EE::GitlabRoutingHelper # rubocop: disable Cop/InjectEnterpriseEditionModule
 
     GIT_LFS_DOWNLOAD_OPERATION = 'download'.freeze
 
     prepended do
       include Elastic::ProjectsSearch
-      include EE::DeploymentPlatform
+      include EE::DeploymentPlatform # rubocop: disable Cop/InjectEnterpriseEditionModule
       include EachBatch
 
       ignore_column :mirror_last_update_at,

ee/app/services/ee/system_note_service.rb

@@ -12,7 +12,7 @@ module EE
       # ::SystemNoteService wants the methods to be available as both class and
       # instance methods. This removes the need for having to both `include` and
       # `extend` this module everywhere it is used.
-      extend(EE::SystemNoteService)
+      extend(EE::SystemNoteService) # rubocop: disable Cop/InjectEnterpriseEditionModule
     end
 
     #

ee/lib/ee/gitlab/geo_git_access.rb

@@ -2,7 +2,7 @@ module EE
   module Gitlab
     module GeoGitAccess
       include ::Gitlab::ConfigHelper
-      include ::EE::GitlabRoutingHelper
+      include ::EE::GitlabRoutingHelper # rubocop: disable Cop/InjectEnterpriseEditionModule
       include GrapePathHelpers::NamedRouteMatcher
       extend ::Gitlab::Utils::Override
 

lib/feature.rb

@@ -4,7 +4,7 @@ require 'flipper/adapters/active_record'
 require 'flipper/adapters/active_support_cache_store'
 
 class Feature
-  prepend EE::Feature
+  prepend EE::Feature # rubocop: disable Cop/InjectEnterpriseEditionModule
 
   # Classes to override flipper table names
   class FlipperFeature < Flipper::Adapters::ActiveRecord::Feature

rubocop/cop/inject_enterprise_edition_module.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Cop that blacklists the injecting of EE specific modules anywhere but on
+    # the last line of a file. Injecting a module in the middle of a file will
+    # cause merge conflicts, while placing it on the last line will not.
+    class InjectEnterpriseEditionModule < RuboCop::Cop::Cop
+      MSG = 'Injecting EE modules must be done on the last line of this file' \
+        ', outside of any class or module definitions'
+
+      METHODS = Set.new(%i[include extend prepend]).freeze
+
+      def_node_matcher :ee_const?, <<~PATTERN
+        (const (const _ :EE) _)
+      PATTERN
+
+      def on_send(node)
+        return unless METHODS.include?(node.children[1])
+        return unless ee_const?(node.children[2])
+
+        line = node.location.line
+        buffer = node.location.expression.source_buffer
+        last_line = buffer.last_line
+
+        # Parser treats the final newline (if present) as a separate line,
+        # meaning that a simple `line < last_line` would yield true even though
+        # the expression is the last line _of code_.
+        last_line -= 1 if buffer.source.end_with?("\n")
+
+        add_offense(node) if line < last_line
+      end
+
+      # Automatically correcting these offenses is not always possible, as
+      # sometimes code needs to be refactored to make this work. As such, we
+      # only allow developers to easily blacklist existing offenses.
+      def autocorrect(node)
+        lambda do |corrector|
+          corrector.insert_after(
+            node.source_range,
+            " # rubocop: disable #{cop_name}"
+          )
+        end
+      end
+    end
+  end
+end

rubocop/rubocop.rb

@@ -41,3 +41,4 @@ require_relative 'cop/code_reuse/presenter'
 require_relative 'cop/code_reuse/serializer'
 require_relative 'cop/code_reuse/active_record'
 require_relative 'cop/group_public_or_visible_to_user'
+require_relative 'cop/inject_enterprise_edition_module'

spec/rubocop/cop/inject_enterprise_edition_module_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/inject_enterprise_edition_module'
+
+describe RuboCop::Cop::InjectEnterpriseEditionModule do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  it 'flags the use of `prepend EE` in the middle of a file' do
+    expect_offense(<<~SOURCE)
+    class Foo
+      prepend EE::Foo
+      ^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+    end
+    SOURCE
+  end
+
+  it 'flags the use of `prepend ::EE` in the middle of a file' do
+    expect_offense(<<~SOURCE)
+    class Foo
+      prepend ::EE::Foo
+      ^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+    end
+    SOURCE
+  end
+
+  it 'flags the use of `include EE` in the middle of a file' do
+    expect_offense(<<~SOURCE)
+    class Foo
+      include EE::Foo
+      ^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+    end
+    SOURCE
+  end
+
+  it 'flags the use of `include ::EE` in the middle of a file' do
+    expect_offense(<<~SOURCE)
+    class Foo
+      include ::EE::Foo
+      ^^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+    end
+    SOURCE
+  end
+
+  it 'flags the use of `extend EE` in the middle of a file' do
+    expect_offense(<<~SOURCE)
+    class Foo
+      extend EE::Foo
+      ^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+    end
+    SOURCE
+  end
+
+  it 'flags the use of `extend ::EE` in the middle of a file' do
+    expect_offense(<<~SOURCE)
+    class Foo
+      extend ::EE::Foo
+      ^^^^^^^^^^^^^^^^ Injecting EE modules must be done on the last line of this file, outside of any class or module definitions
+    end
+    SOURCE
+  end
+
+  it 'does not flag prepending of regular modules' do
+    expect_no_offenses(<<~SOURCE)
+    class Foo
+      prepend Foo
+    end
+    SOURCE
+  end
+
+  it 'does not flag including of regular modules' do
+    expect_no_offenses(<<~SOURCE)
+    class Foo
+      include Foo
+    end
+    SOURCE
+  end
+
+  it 'does not flag extending using regular modules' do
+    expect_no_offenses(<<~SOURCE)
+    class Foo
+      extend Foo
+    end
+    SOURCE
+  end
+
+  it 'does not flag the use of `prepend EE` on the last line' do
+    expect_no_offenses(<<~SOURCE)
+    class Foo
+    end
+
+    Foo.prepend(EE::Foo)
+    SOURCE
+  end
+
+  it 'does not flag the use of `include EE` on the last line' do
+    expect_no_offenses(<<~SOURCE)
+    class Foo
+    end
+
+    Foo.include(EE::Foo)
+    SOURCE
+  end
+
+  it 'does not flag the use of `extend EE` on the last line' do
+    expect_no_offenses(<<~SOURCE)
+    class Foo
+    end
+
+    Foo.extend(EE::Foo)
+    SOURCE
+  end
+
+  it 'autocorrects offenses by just disabling the Cop' do
+    source = <<~SOURCE
+    class Foo
+      prepend EE::Foo
+      include Bar
+    end
+    SOURCE
+
+    expect(autocorrect_source(source)).to eq(<<~SOURCE)
+    class Foo
+      prepend EE::Foo # rubocop: disable Cop/InjectEnterpriseEditionModule
+      include Bar
+    end
+    SOURCE
+  end
+end