Merge branch '378167-add-rubocop' into 'master'

Add rubocop linting for misplacement of ee only classes

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/165869



Merged-by: Allison Browne <abrowne@gitlab.com>
Approved-by: Allison Browne <abrowne@gitlab.com>
Reviewed-by: Doug Stull <dstull@gitlab.com>
Co-authored-by: Doug Stull <dstull@gitlab.com>

.rubocop.yml

@@ -531,6 +531,11 @@ Gitlab/AvoidGitlabInstanceChecks:
     - 'db/post_migrate/*.rb'
     - 'ee/db/fixtures/**/*'
 
+Gitlab/EeOnlyClass:
+  Enabled: true
+  Include:
+    - 'ee/**/ee/**/*.rb'
+
 # See https://gitlab.com/gitlab-org/gitlab/-/issues/373194
 Gitlab/RSpec/AvoidSetup:
   Enabled: true

.rubocop_todo/gitlab/ee_only_class.yml

+---
+Gitlab/EeOnlyClass:
+  Details: grace period
+  Exclude:
+    - 'ee/app/controllers/concerns/ee/routable_actions/sso_enforcement_redirect.rb'
+    - 'ee/app/graphql/ee/resolvers/pending_group_members_resolver.rb'
+    - 'ee/app/graphql/ee/resolvers/validate_codeowner_file_resolver.rb'
+    - 'ee/app/graphql/ee/types/gitlab_subscriptions/member_management/member_approval_status_enum.rb'
+    - 'ee/app/graphql/ee/types/gitlab_subscriptions/member_management/member_approval_type.rb'
+    - 'ee/app/graphql/ee/types/gitlab_subscriptions/member_management/user_promotion_status_enum.rb'
+    - 'ee/app/graphql/ee/types/gitlab_subscriptions/member_management/users_queued_for_role_promotion_type.rb'
+    - 'ee/app/graphql/ee/types/list_limit_metric_enum.rb'
+    - 'ee/app/graphql/ee/types/repository/code_owner_error_type.rb'
+    - 'ee/app/graphql/ee/types/repository/code_owner_validation_type.rb'
+    - 'ee/app/serializers/ee/commit_ai_entity.rb'
+    - 'ee/app/serializers/ee/commit_serializer.rb'
+    - 'ee/app/serializers/ee/issue_ai_entity.rb'
+    - 'ee/app/serializers/ee/merge_request_ai_entity.rb'
+    - 'ee/app/services/ee/allowed_email_domains/update_service.rb'
+    - 'ee/app/services/ee/ip_restrictions/update_service.rb'
+    - 'ee/app/services/ee/null_notification_service.rb'
+    - 'ee/app/services/ee/projects/remove_paid_features_service.rb'
+    - 'ee/app/services/ee/protected_branches/base_policy_check.rb'
+    - 'ee/app/services/ee/resource_events/synthetic_iteration_notes_builder_service.rb'
+    - 'ee/app/services/ee/resource_events/synthetic_weight_notes_builder_service.rb'
+    - 'ee/lib/ee/api/entities/analytics/code_review/merge_request.rb'
+    - 'ee/lib/ee/api/entities/analytics/deployment_frequency.rb'
+    - 'ee/lib/ee/api/entities/analytics/group_activity/issues_count.rb'
+    - 'ee/lib/ee/api/entities/analytics/group_activity/merge_requests_count.rb'
+    - 'ee/lib/ee/api/entities/analytics/group_activity/new_members_count.rb'
+    - 'ee/lib/ee/api/entities/approval_rule.rb'
+    - 'ee/lib/ee/api/entities/approval_rule_short.rb'
+    - 'ee/lib/ee/api/entities/approval_settings.rb'
+    - 'ee/lib/ee/api/entities/approval_state.rb'
+    - 'ee/lib/ee/api/entities/approver.rb'
+    - 'ee/lib/ee/api/entities/approver_group.rb'
+    - 'ee/lib/ee/api/entities/audit_event.rb'
+    - 'ee/lib/ee/api/entities/billable_member.rb'
+    - 'ee/lib/ee/api/entities/billable_membership.rb'
+    - 'ee/lib/ee/api/entities/ci/minutes/additional_pack.rb'
+    - 'ee/lib/ee/api/entities/dependencies_vulnerabilities.rb'
+    - 'ee/lib/ee/api/entities/dependency.rb'
+    - 'ee/lib/ee/api/entities/dependency_list_export.rb'
+    - 'ee/lib/ee/api/entities/epic.rb'
+    - 'ee/lib/ee/api/entities/epic_issue.rb'
+    - 'ee/lib/ee/api/entities/epic_issue_link.rb'
+    - 'ee/lib/ee/api/entities/experiment.rb'
+    - 'ee/lib/ee/api/entities/geo_node.rb'
+    - 'ee/lib/ee/api/entities/geo_node_status.rb'
+    - 'ee/lib/ee/api/entities/geo_site.rb'
+    - 'ee/lib/ee/api/entities/geo_site_status.rb'
+    - 'ee/lib/ee/api/entities/gitlab_license.rb'
+    - 'ee/lib/ee/api/entities/gitlab_license_with_active_users.rb'
+    - 'ee/lib/ee/api/entities/gitlab_subscriptions/add_on_purchase.rb'
+    - 'ee/lib/ee/api/entities/group_approval_rule.rb'
+    - 'ee/lib/ee/api/entities/group_hook.rb'
+    - 'ee/lib/ee/api/entities/group_push_rule.rb'
+    - 'ee/lib/ee/api/entities/identity_detail.rb'
+    - 'ee/lib/ee/api/entities/issuable_metric_image.rb'
+    - 'ee/lib/ee/api/entities/ldap_group.rb'
+    - 'ee/lib/ee/api/entities/ldap_group_link.rb'
+    - 'ee/lib/ee/api/entities/linked_epic.rb'
+    - 'ee/lib/ee/api/entities/managed_license.rb'
+    - 'ee/lib/ee/api/entities/member_role.rb'
+    - 'ee/lib/ee/api/entities/merge_request_approval_rule.rb'
+    - 'ee/lib/ee/api/entities/merge_request_approval_setting_rule.rb'
+    - 'ee/lib/ee/api/entities/merge_request_approval_settings.rb'
+    - 'ee/lib/ee/api/entities/merge_request_approval_state.rb'
+    - 'ee/lib/ee/api/entities/merge_request_approval_state_rule.rb'
+    - 'ee/lib/ee/api/entities/merge_request_dependency.rb'
+    - 'ee/lib/ee/api/entities/merge_trains/car.rb'
+    - 'ee/lib/ee/api/entities/project_alias.rb'
+    - 'ee/lib/ee/api/entities/project_approval_rule.rb'
+    - 'ee/lib/ee/api/entities/project_approval_setting_rule.rb'
+    - 'ee/lib/ee/api/entities/project_approval_settings.rb'
+    - 'ee/lib/ee/api/entities/project_push_rule.rb'
+    - 'ee/lib/ee/api/entities/protected_environment.rb'
+    - 'ee/lib/ee/api/entities/resource_iteration_event.rb'
+    - 'ee/lib/ee/api/entities/resource_weight_event.rb'
+    - 'ee/lib/ee/api/entities/saml_group_link.rb'
+    - 'ee/lib/ee/api/entities/scim/conflict.rb'
+    - 'ee/lib/ee/api/entities/scim/emails.rb'
+    - 'ee/lib/ee/api/entities/scim/error.rb'
+    - 'ee/lib/ee/api/entities/scim/not_found.rb'
+    - 'ee/lib/ee/api/entities/scim/user.rb'
+    - 'ee/lib/ee/api/entities/scim/user_name.rb'
+    - 'ee/lib/ee/api/entities/scim/users.rb'
+    - 'ee/lib/ee/api/entities/security_policy_configuration.rb'
+    - 'ee/lib/ee/api/entities/special_board_filter.rb'
+    - 'ee/lib/ee/api/entities/ssh_certificate.rb'
+    - 'ee/lib/ee/api/entities/vulnerability.rb'
+    - 'ee/lib/ee/api/entities/vulnerability_export.rb'
+    - 'ee/lib/ee/api/entities/vulnerability_issue_link.rb'
+    - 'ee/lib/ee/api/entities/vulnerability_related_issue.rb'
+    - 'ee/lib/ee/api/group_boards.rb'
+    - 'ee/lib/ee/gitlab/auth/ldap/access_levels.rb'
+    - 'ee/lib/ee/gitlab/auth/ldap/group.rb'
+    - 'ee/lib/ee/gitlab/auth/ldap/sync/admin_users.rb'
+    - 'ee/lib/ee/gitlab/auth/ldap/sync/external_users.rb'
+    - 'ee/lib/ee/gitlab/auth/ldap/sync/group.rb'
+    - 'ee/lib/ee/gitlab/auth/ldap/sync/groups.rb'
+    - 'ee/lib/ee/gitlab/auth/ldap/sync/proxy.rb'
+    - 'ee/lib/ee/gitlab/auth/ldap/sync/users.rb'
+    - 'ee/lib/ee/gitlab/checks/push_rule_check.rb'
+    - 'ee/lib/ee/gitlab/checks/push_rules/branch_check.rb'
+    - 'ee/lib/ee/gitlab/checks/push_rules/commit_check.rb'
+    - 'ee/lib/ee/gitlab/checks/push_rules/file_size_check.rb'
+    - 'ee/lib/ee/gitlab/checks/push_rules/tag_check.rb'
+    - 'ee/lib/ee/gitlab/ci/pipeline/quota/size.rb'
+    - 'ee/lib/ee/gitlab/ci/variables/builder/scan_execution_policies.rb'
+    - 'ee/lib/ee/gitlab/import_export/after_export_strategies/custom_template_export_import_strategy.rb'
+    - 'ee/lib/ee/gitlab/namespace_storage_size_error_message.rb'
+    - 'ee/lib/ee/gitlab/personal_access_tokens/expiry_date_calculator.rb'
+    - 'ee/lib/ee/gitlab/personal_access_tokens/service_account_token_validator.rb'
+    - 'ee/lib/ee/gitlab/scim/attribute_transform.rb'
+    - 'ee/lib/ee/gitlab/scim/base_deprovisioning_service.rb'
+    - 'ee/lib/ee/gitlab/scim/base_provisioning_service.rb'
+    - 'ee/lib/ee/gitlab/scim/deprovisioning_service.rb'
+    - 'ee/lib/ee/gitlab/scim/filter_parser.rb'
+    - 'ee/lib/ee/gitlab/scim/group/deprovisioning_service.rb'
+    - 'ee/lib/ee/gitlab/scim/group/provisioning_service.rb'
+    - 'ee/lib/ee/gitlab/scim/group/reprovisioning_service.rb'
+    - 'ee/lib/ee/gitlab/scim/params_parser.rb'
+    - 'ee/lib/ee/gitlab/scim/provisioning_response.rb'
+    - 'ee/lib/ee/gitlab/scim/provisioning_service.rb'
+    - 'ee/lib/ee/gitlab/scim/reprovisioning_service.rb'
+    - 'ee/lib/ee/gitlab/scim/value_parser.rb'
+    - 'ee/lib/ee/sidebars/explore/menus/dependencies_menu.rb'
+    - 'ee/spec/support/helpers/ee/markdown_feature.rb'

gems/config/rubocop.yml

@@ -58,6 +58,10 @@ Gitlab/BoundedContexts:
 Gitlab/DocUrl:
   Enabled: false
 
+# This cop doesn't make sense in the context of gems
+Gitlab/EeOnlyClass:
+  Enabled: false
+
 # This cop doesn't make sense in the context of gems
 Gitlab/NamespacedClass:
   Enabled: false

rubocop/cop/gitlab/ee_only_class.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Gitlab
+      class EeOnlyClass < RuboCop::Cop::Base
+        # Cop that checks for incorrect placement of classes in the ee/**/ee subdirectories.
+        #
+        # see https://docs.gitlab.com/ee/development/ee_features.html#extend-ce-features-with-ee-backend-code
+        #
+        #  # bad
+        #  # filename: ee/app/services/ee/null_notification_service.rb
+        #  module EE
+        #    class NullNotificationService
+        #    end
+        #  end
+        #
+        #  # good
+        #  # filename: ee/app/services/null_notification_service.rb
+        #  class NullNotificationService
+        #  end
+        #
+        MSG = <<~TEXT
+          This area is meant for extending CE functionality with modules.
+          It is likely this file should be removed from the sub ee directory it is currently in. Please read this
+          for the rationale behind it:
+
+          https://docs.gitlab.com/ee/development/ee_features.html#extend-ce-features-with-ee-backend-code
+        TEXT
+
+        def on_class(node)
+          # If class name matches file name - offense, if not, then no offense as it is valid
+          # to create classes inside a module.
+          return if to_class_name(File.basename(processed_source.file_path)) != node.loc.name.source
+
+          add_offense(node)
+        end
+
+        private
+
+        def to_class_name(basename)
+          # Our case here is likely purely for `.rb` files, but we'll remain flexible in general for
+          # haml_lint, .erb, etc
+          # Not using ActiveSupport since we do not want to declare that as a dependency on this generic ruby concept.
+          words = basename.sub(/\..*/, '').split('_')
+          words.map(&:capitalize).join
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/gitlab/ee_only_class_spec.rb

+# frozen_string_literal: true
+
+require 'rubocop_spec_helper'
+require_relative '../../../../rubocop/cop/gitlab/ee_only_class'
+
+RSpec.describe RuboCop::Cop::Gitlab::EeOnlyClass, feature_category: :shared do
+  describe 'bad examples' do
+    shared_examples 'reference offense' do
+      it 'registers an offense' do
+        expect_offense(<<~CODE, file_name)
+          module EE
+            class NullNotificationService
+            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This area is meant for extending CE [...]
+              def execute
+              end
+            end
+          end
+        CODE
+      end
+    end
+
+    context 'when class is defined and matches the file basename' do
+      let(:file_name) { 'ee/app/services/ee/null_notification_service.rb' }
+
+      include_examples 'reference offense'
+    end
+  end
+
+  describe 'good examples' do
+    context 'when class is defined and does not match file basename' do
+      let(:file_name) { 'ee/app/services/ee/some_extended_ce_code.rb' }
+
+      it 'does not register an offense' do
+        expect_no_offenses(<<~CODE, file_name)
+          module EE
+            module SomeExtendedCeCode
+              def execute
+              end
+
+              class NullNotificationService
+                def execute
+                end
+              end
+            end
+          end
+        CODE
+      end
+    end
+  end
+end