diff options
Diffstat (limited to 'rubocop/cop')
| -rw-r--r-- | rubocop/cop/background_migration/feature_category.rb | 29 | ||||
| -rw-r--r-- | rubocop/cop/gemfile/missing_feature_category.rb | 62 | ||||
| -rw-r--r-- | rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb | 54 | ||||
| -rw-r--r-- | rubocop/cop/gitlab/feature_available_usage.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/migration/background_migrations.rb | 4 | ||||
| -rw-r--r-- | rubocop/cop/migration/prevent_index_creation.rb | 9 | ||||
| -rw-r--r-- | rubocop/cop/migration/unfinished_dependencies.rb | 51 | ||||
| -rw-r--r-- | rubocop/cop/qa/fabricate_usage.rb | 65 | ||||
| -rw-r--r-- | rubocop/cop/rspec/feature_category.rb | 85 | ||||
| -rw-r--r-- | rubocop/cop/rspec/httparty_basic_auth.rb | 2 | ||||
| -rw-r--r-- | rubocop/cop/rspec/invalid_feature_category.rb | 104 | ||||
| -rw-r--r-- | rubocop/cop/rspec/missing_feature_category.rb | 44 | ||||
| -rw-r--r-- | rubocop/cop/search/namespaced_class.rb | 1 |
13 files changed, 338 insertions, 174 deletions
diff --git a/rubocop/cop/background_migration/feature_category.rb b/rubocop/cop/background_migration/feature_category.rb index f6b68e03736..7a96271224d 100644 --- a/rubocop/cop/background_migration/feature_category.rb +++ b/rubocop/cop/background_migration/feature_category.rb @@ -2,6 +2,7 @@ require_relative '../../migration_helpers' require_relative '../../code_reuse_helpers' +require_relative '../../feature_categories' module RuboCop module Cop @@ -10,32 +11,20 @@ module RuboCop class FeatureCategory < RuboCop::Cop::Base include MigrationHelpers - FEATURE_CATEGORIES_FILE_PATH = File.expand_path("../../../config/feature_categories.yml", __dir__) - MSG = "'feature_category' should be defined to better assign the ownership for batched migration jobs. " \ "For more details refer: " \ "https://docs.gitlab.com/ee/development/feature_categorization/#batched-background-migrations" - INVALID_FEATURE_CATEGORY_MSG = "'feature_category' is invalid. " \ - "List of valid ones can be found in #{FEATURE_CATEGORIES_FILE_PATH}".freeze + INVALID_FEATURE_CATEGORY_MSG = + "'feature_category' is invalid. " \ + "List of valid ones can be found in #{FeatureCategories::CONFIG_PATH}".freeze RESTRICT_ON_SEND = [:feature_category].freeze - class << self - attr_accessor :available_feature_categories - end - def_node_search :feature_category?, <<~PATTERN (:send nil? :feature_category ...) PATTERN - def on_new_investigation - super - - # Defined only once per rubocop whole run instead of each file. - fetch_available_feature_categories unless self.class.available_feature_categories.present? - end - def on_class(node) return unless in_background_migration?(node) && node.parent_class&.short_name == :BatchedMigrationJob @@ -48,15 +37,15 @@ module RuboCop add_offense(node, message: INVALID_FEATURE_CATEGORY_MSG) unless valid_feature_category?(node) end + def external_dependency_checksum + FeatureCategories.config_checksum + end + private def valid_feature_category?(node) feature_category = node.descendants.first.value - self.class.available_feature_categories.include?(feature_category.to_s) - end - - def fetch_available_feature_categories - self.class.available_feature_categories = YAML.load_file(FEATURE_CATEGORIES_FILE_PATH).to_set + FeatureCategories.available.include?(feature_category.to_s) end end end diff --git a/rubocop/cop/gemfile/missing_feature_category.rb b/rubocop/cop/gemfile/missing_feature_category.rb new file mode 100644 index 00000000000..18e29fc30b2 --- /dev/null +++ b/rubocop/cop/gemfile/missing_feature_category.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +require_relative '../../feature_categories' + +module RuboCop + module Cop + module Gemfile + # Ensures that feature categories are specified for each gems. + # + # @example + # + # # bad + # gem 'foo' + # gem 'bar', feature_category: :invalid + # + # # good + # gem 'foo', feature_category: :wiki + # gem 'bar', feature_category: :tooling + # gem 'baz', feature_category: :shared + # + class MissingFeatureCategory < RuboCop::Cop::Base + DOCUMENT_LINK = 'https://docs.gitlab.com/ee/development/feature_categorization/#gemfile' + + def_node_matcher :send_gem, <<~PATTERN + (send nil? :gem ...) + PATTERN + + def_node_matcher :feature_category_value, <<~PATTERN + (send nil? :gem + ... + (hash <(pair (sym :feature_category) $_) ...>) + ... + ) + PATTERN + + def on_send(node) + return unless send_gem(node) + + value_node = feature_category_value(node) + + feature_categories.check( + value_node: value_node, + document_link: DOCUMENT_LINK + ) do |message| + add_offense(value_node || node, message: message) + end + end + + def external_dependency_checksum + FeatureCategories.config_checksum + end + + private + + def feature_categories + @feature_categories ||= + FeatureCategories.new(FeatureCategories.available_with_custom) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb b/rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb new file mode 100644 index 00000000000..962a58cfb4a --- /dev/null +++ b/rubocop/cop/gitlab/avoid_gitlab_instance_checks.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module Gitlab + # This cop checks for use of gitlab instance specific checks. + # + # @example + # + # # bad + # if Gitlab.com? + # Ci::Runner::FORM_EDITABLE + Ci::Runner::MINUTES_COST_FACTOR_FIELDS + # else + # Ci::Runner::FORM_EDITABLE + # end + # + # # good + # if Gitlab::Saas.feature_available?('purchases/additional_minutes') + # Ci::Runner::FORM_EDITABLE + Ci::Runner::MINUTES_COST_FACTOR_FIELDS + # else + # Ci::Runner::FORM_EDITABLE + # end + # + class AvoidGitlabInstanceChecks < RuboCop::Cop::Base + MSG = 'Avoid the use of `%{name}`. Use Gitlab::Saas.feature_available?. ' \ + 'See https://docs.gitlab.com/ee/development/ee_features.html#saas-only-feature' + RESTRICT_ON_SEND = %i[ + com? com_except_jh? com_and_canary? com_but_not_canary? org_or_com? should_check_namespace_plan? + ].freeze + + # @!method gitlab?(node) + def_node_matcher :gitlab?, <<~PATTERN + (send (const {nil? (cbase)} :Gitlab) ...) + PATTERN + + # @!method should_check_namespace_plan?(node) + def_node_matcher :should_check_namespace_plan?, <<~PATTERN + (send + (const + (const + {nil? (cbase)} :Gitlab) :CurrentSettings) :should_check_namespace_plan?) + PATTERN + + def on_send(node) + return unless gitlab?(node) || should_check_namespace_plan?(node) + + add_offense(node, message: format(MSG, name: node.method_name)) + end + end + end + end +end diff --git a/rubocop/cop/gitlab/feature_available_usage.rb b/rubocop/cop/gitlab/feature_available_usage.rb index cbfb89d3053..307ff7ea6f6 100644 --- a/rubocop/cop/gitlab/feature_available_usage.rb +++ b/rubocop/cop/gitlab/feature_available_usage.rb @@ -32,7 +32,7 @@ module RuboCop ].freeze EE_FEATURES = %i[requirements].freeze ALL_FEATURES = (FEATURES + EE_FEATURES).freeze - SPECIAL_CLASS = %w[License].freeze + SPECIAL_CLASS = %w[License Gitlab::Saas].freeze def on_send(node) return unless method_name(node) == OBSERVED_METHOD diff --git a/rubocop/cop/migration/background_migrations.rb b/rubocop/cop/migration/background_migrations.rb index 7dcc2101fb1..e131ab42318 100644 --- a/rubocop/cop/migration/background_migrations.rb +++ b/rubocop/cop/migration/background_migrations.rb @@ -14,11 +14,11 @@ module RuboCop def on_send(node) name = node.children[1] - disabled_methods = %i( + disabled_methods = %i[ queue_background_migration_jobs_by_range_at_intervals requeue_background_migration_jobs_by_range_at_intervals migrate_in - ) + ] add_offense(node.loc.selector) if disabled_methods.include? name end diff --git a/rubocop/cop/migration/prevent_index_creation.rb b/rubocop/cop/migration/prevent_index_creation.rb index 4c39032eb0f..aa0ab7b1e50 100644 --- a/rubocop/cop/migration/prevent_index_creation.rb +++ b/rubocop/cop/migration/prevent_index_creation.rb @@ -8,9 +8,11 @@ module RuboCop class PreventIndexCreation < RuboCop::Cop::Base include MigrationHelpers - FORBIDDEN_TABLES = %i[ci_builds].freeze + FORBIDDEN_TABLES = %i[ci_builds namespaces].freeze - MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden, see https://gitlab.com/gitlab-org/gitlab/-/issues/332886".freeze + MSG = "Adding new index to #{FORBIDDEN_TABLES.join(", ")} is forbidden. " \ + "For `ci_builds` see https://gitlab.com/gitlab-org/gitlab/-/issues/332886, " \ + "for `namespaces` see https://gitlab.com/groups/gitlab-org/-/epics/11543".freeze def on_new_investigation super @@ -40,6 +42,9 @@ module RuboCop def on_def(node) return unless in_migration?(node) + direction = node.children[0] + return if direction == :down + node.each_descendant(:send) do |send_node| add_offense(send_node.loc.selector) if offense?(send_node) end diff --git a/rubocop/cop/migration/unfinished_dependencies.rb b/rubocop/cop/migration/unfinished_dependencies.rb new file mode 100644 index 00000000000..1e0741c8411 --- /dev/null +++ b/rubocop/cop/migration/unfinished_dependencies.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' +require_relative '../../batched_background_migrations' + +module RuboCop + module Cop + module Migration + # Checks if there are any unfinished dependent batched bg migrations + class UnfinishedDependencies < RuboCop::Cop::Base + include MigrationHelpers + + NOT_FINALIZED_MSG = <<-MESSAGE.delete("\n").squeeze(' ').strip + Dependent migration with queued version %{version} is not yet finalized. + Consider finalizing the dependent migration and update it's finalized_by attr in the dictionary. + MESSAGE + + FINALIZED_BY_LATER_MIGRATION_MSG = <<-MESSAGE.delete("\n").squeeze(' ').strip + Dependent migration with queued version %{version} is finalized by later migration, + it has to be finalized before the current migration. + MESSAGE + + def_node_matcher :dependent_migration_versions, <<~PATTERN + (casgn nil? :DEPENDENT_BATCHED_BACKGROUND_MIGRATIONS (array $...)) + PATTERN + + def on_casgn(node) + return unless in_migration?(node) + + migration_version = version(node) + + dependent_migration_versions(node)&.each do |dependent_migration_version_node| + dependent_migration_version = dependent_migration_version_node.value + finalized_by_version = fetch_finalized_by(dependent_migration_version) + + next if finalized_by_version.present? && finalized_by_version.to_s < migration_version.to_s + + msg = finalized_by_version.present? ? FINALIZED_BY_LATER_MIGRATION_MSG : NOT_FINALIZED_MSG + add_offense(node, message: format(msg, version: dependent_migration_version)) + end + end + + private + + def fetch_finalized_by(queued_migration_version) + BatchedBackgroundMigrations.new(queued_migration_version).finalized_by + end + end + end + end +end diff --git a/rubocop/cop/qa/fabricate_usage.rb b/rubocop/cop/qa/fabricate_usage.rb new file mode 100644 index 00000000000..1787df0e7df --- /dev/null +++ b/rubocop/cop/qa/fabricate_usage.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require_relative '../../qa_helpers' + +module RuboCop + module Cop + module QA + # This cop checks for the usages of API fabrications and suggests using + # the corresponding QA factories instead. + # @example + # Good: + # create(:project) + # Bad: + # Resource::Project.fabricate_via_api! + # @example + # Good: + # create(:group) + # Bad: + # Resource::Group.fabricate_via_api! + # @example + # Good: + # create(:user, username: 'username', password: 'password') + # Bad: + # Resource::User.fabricate_via_api! do |user| + # user.username = 'username' + # user.password = 'password' + # end + class FabricateUsage < RuboCop::Cop::Base + MESSAGE = "Prefer create(:%{factory}[, ...]) here." + RESOURCES_TO_CHECK = { + 'Resource::Project' => :project, + 'Resource::Group' => :group, + 'Resource::Issue' => :issue, + 'Resource::User' => :user, + 'Resource::Pipeline' => :pipeline, + 'Resource::Job' => :job, + 'Resource::File' => :file, + 'Resource::GroupAccessToken' => :group_access_token, + 'Resource::ProjectAccessToken' => :project_access_token, + 'Resource::GroupLabel' => :group_label, + 'Resource::ProjectLabel' => :project_label, + 'Resource::GroupRunner' => :group_runner, + 'Resource::ProjectRunner' => :project_runner, + 'Resource::GroupMilestone' => :group_milestone, + 'Resource::ProjectMilestone' => :project_milestone, + 'Resource::Snippet' => :snippet, + 'Resource::ProjectSnippet' => :project_snippet + }.freeze + + RESTRICT_ON_SEND = %i[fabricate_via_api!].freeze + + def_node_matcher :const_receiver, <<~PATTERN + (send $const ...) + PATTERN + + def on_send(node) + factory = RESOURCES_TO_CHECK[const_receiver(node)&.const_name] + return unless factory + + add_offense(node, message: format(MESSAGE, factory: factory)) + end + end + end + end +end diff --git a/rubocop/cop/rspec/feature_category.rb b/rubocop/cop/rspec/feature_category.rb new file mode 100644 index 00000000000..a247bd807b0 --- /dev/null +++ b/rubocop/cop/rspec/feature_category.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'rubocop/cop/rspec/base' +require 'rubocop/cop/rspec/mixin/top_level_group' + +require_relative '../../feature_categories' + +module RuboCop + module Cop + module RSpec + # Ensures that feature categories in specs are present and valid. + # + # @example + # + # # bad + # RSpec.describe 'foo' do + # end + # + # RSpec.describe 'foo', feature_category: :invalid do + # context 'a context', feature_category: :aip do + # end + # end + # + # RSpec.describe 'foo', feature_category: :not_owned do + # end + # + # # good + # + # RSpec.describe 'foo', feature_category: :wiki do + # context 'a context', feature_category: :api do + # end + # end + # + # RSpec.describe 'foo', feature_category: :tooling do + # end + # + class FeatureCategory < RuboCop::Cop::RSpec::Base + include RuboCop::Cop::RSpec::TopLevelGroup + + DOCUMENT_LINK = 'https://docs.gitlab.com/ee/development/feature_categorization/#rspec-examples' + + # @!method feature_category?(node) + def_node_matcher :feature_category_value, <<~PATTERN + (block + (send #rspec? {#ExampleGroups.all #Examples.all} ... + (hash <(pair (sym :feature_category) $_) ...>) + ) + ... + ) + PATTERN + + def on_top_level_example_group(node) + check_feature_category(node, optional: false) + end + + def on_block(node) + check_feature_category(node, optional: true) + end + + def external_dependency_checksum + FeatureCategories.config_checksum + end + + private + + def check_feature_category(node, optional:) + value_node = feature_category_value(node) + return if optional && !value_node + + feature_categories.check( + value_node: value_node, + document_link: DOCUMENT_LINK + ) do |message| + add_offense(value_node || node, message: message) + end + end + + def feature_categories + @feature_categories ||= + FeatureCategories.new(FeatureCategories.available_with_custom) + end + end + end + end +end diff --git a/rubocop/cop/rspec/httparty_basic_auth.rb b/rubocop/cop/rspec/httparty_basic_auth.rb index d188002673f..68d48ca4f82 100644 --- a/rubocop/cop/rspec/httparty_basic_auth.rb +++ b/rubocop/cop/rspec/httparty_basic_auth.rb @@ -19,7 +19,7 @@ module RuboCop MESSAGE = "`basic_auth: { user: ... }` does not work - replace `user:` with `username:`" - RESTRICT_ON_SEND = %i(get put post delete).freeze + RESTRICT_ON_SEND = %i[get put post delete].freeze def_node_matcher :httparty_basic_auth?, <<~PATTERN (send diff --git a/rubocop/cop/rspec/invalid_feature_category.rb b/rubocop/cop/rspec/invalid_feature_category.rb deleted file mode 100644 index 9ef880d6aac..00000000000 --- a/rubocop/cop/rspec/invalid_feature_category.rb +++ /dev/null @@ -1,104 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop/cop/rspec/base' -require 'rubocop/cop/rspec/mixin/top_level_group' -require 'did_you_mean' - -module RuboCop - module Cop - module RSpec - # Ensures that feature categories in specs are valid. - # - # @example - # - # # bad - # RSpec.describe 'foo', feature_category: :invalid do - # end - # - # RSpec.describe 'foo', feature_category: :not_owned do - # end - # - # # good - # - # RSpec.describe 'foo', feature_category: :wiki do - # end - # - # RSpec.describe 'foo', feature_category: :tooling do - # end - # - class InvalidFeatureCategory < RuboCop::Cop::RSpec::Base - MSG = 'Please use a valid feature category. %{msg_suggestion}' \ - 'See https://docs.gitlab.com/ee/development/feature_categorization/#rspec-examples.' - - MSG_DID_YOU_MEAN = 'Did you mean `:%{suggestion}`? ' - - MSG_SYMBOL = 'Please use a symbol as value.' - - FEATURE_CATEGORIES_PATH = File.expand_path('../../../config/feature_categories.yml', __dir__).freeze - - # List of feature categories which are not defined in config/feature_categories.yml - CUSTOM_FEATURE_CATEGORIES = [ - # https://docs.gitlab.com/ee/development/feature_categorization/#tooling-feature-category - :tooling, - # https://docs.gitlab.com/ee/development/feature_categorization/#shared-feature-category - :shared - ].to_set.freeze - - # @!method feature_category?(node) - def_node_matcher :feature_category_value, <<~PATTERN - (block - (send #rspec? {#ExampleGroups.all #Examples.all} ... - (hash <(pair (sym :feature_category) $_) ...>) - ) - ... - ) - PATTERN - - def on_block(node) - value_node = feature_category_value(node) - return unless value_node - - unless value_node.sym_type? - add_offense(value_node, message: MSG_SYMBOL) - return - end - - return if valid_feature_category?(value_node) - - message = format(MSG, msg_suggestion: suggestion_message(value_node)) - add_offense(value_node, message: message) - end - - # Used by RuboCop to invalidate its cache if the contents of - # config/feature_categories.yml changes. - def external_dependency_checksum - @external_dependency_checksum ||= - Digest::SHA256.file(FEATURE_CATEGORIES_PATH).hexdigest - end - - private - - def suggestion_message(value_node) - spell = DidYouMean::SpellChecker.new(dictionary: self.class.feature_categories) - - suggestions = spell.correct(value_node.value) - return if suggestions.none? - - format(MSG_DID_YOU_MEAN, suggestion: suggestions.first) - end - - def valid_feature_category?(node) - self.class.feature_categories.include?(node.value) - end - - def self.feature_categories - @feature_categories ||= YAML - .load_file(FEATURE_CATEGORIES_PATH) - .map(&:to_sym) - .to_set - .union(CUSTOM_FEATURE_CATEGORIES) - end - end - end - end -end diff --git a/rubocop/cop/rspec/missing_feature_category.rb b/rubocop/cop/rspec/missing_feature_category.rb deleted file mode 100644 index 0f517473982..00000000000 --- a/rubocop/cop/rspec/missing_feature_category.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop/cop/rspec/base' -require 'rubocop/cop/rspec/mixin/top_level_group' - -module RuboCop - module Cop - module RSpec - # Ensures that top level example group contains a feature category. - # - # @example - # - # # bad - # RSpec.describe 'foo' do - # end - # - # # good - # RSpec.describe 'foo', feature_category: :wiki do - # end - class MissingFeatureCategory < RuboCop::Cop::RSpec::Base - include RuboCop::Cop::RSpec::TopLevelGroup - - MSG = 'Please add missing feature category. ' \ - 'See https://docs.gitlab.com/ee/development/feature_categorization/#rspec-examples.' - - # @!method feature_category?(node) - def_node_matcher :feature_category?, <<~PATTERN - (block - (send ... - (hash <(pair (sym :feature_category) _) ...>) - ) - ... - ) - PATTERN - - def on_top_level_example_group(node) - return if feature_category?(node) - - add_offense(node.children.first) - end - end - end - end -end diff --git a/rubocop/cop/search/namespaced_class.rb b/rubocop/cop/search/namespaced_class.rb index 7d0901cb6f6..009bcfd67b1 100644 --- a/rubocop/cop/search/namespaced_class.rb +++ b/rubocop/cop/search/namespaced_class.rb @@ -23,6 +23,7 @@ module RuboCop PERMITTED_NAMESPACES = %w[ Search EE::Search API::Search EE::API::Search API::Admin::Search RuboCop::Cop::Search API::Entities::Search::Zoekt + API::Internal::Search::Zoekt ].map { |x| x.split('::') }.freeze SEARCH_REGEXES = [ |