diff --git a/.rubocop.yml b/.rubocop.yml index f2dbc1e5db143bcd67c566cac542b608b5ce512f..badaa0fa929db93c5895c40a43f5186654e5c47f 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/.rubocop_todo/gitlab/ee_only_class.yml b/.rubocop_todo/gitlab/ee_only_class.yml new file mode 100644 index 0000000000000000000000000000000000000000..cdbf46ddbb8c32b52533e3d1661b8016b9c8305e --- /dev/null +++ b/.rubocop_todo/gitlab/ee_only_class.yml @@ -0,0 +1,129 @@ +--- +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' diff --git a/gems/config/rubocop.yml b/gems/config/rubocop.yml index 390c9e6b6b0270587b6e31fb52cd16ff327811e5..635e5515535f0760ecfe2afab3a7ce5717e86c65 100644 --- a/gems/config/rubocop.yml +++ b/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 diff --git a/rubocop/cop/gitlab/ee_only_class.rb b/rubocop/cop/gitlab/ee_only_class.rb new file mode 100644 index 0000000000000000000000000000000000000000..1b047e1e93a8b7a38a89be748b8f1d2d1a906f9c --- /dev/null +++ b/rubocop/cop/gitlab/ee_only_class.rb @@ -0,0 +1,51 @@ +# 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 diff --git a/spec/rubocop/cop/gitlab/ee_only_class_spec.rb b/spec/rubocop/cop/gitlab/ee_only_class_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..5c47ce77c49df1c198dbb9ccb5a76073b53b4006 --- /dev/null +++ b/spec/rubocop/cop/gitlab/ee_only_class_spec.rb @@ -0,0 +1,50 @@ +# 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