diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-20 15:52:10 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-02-20 15:52:10 +0300 |
commit | dba864470fbcbb6bdd5b94eb510acdce62c962d8 (patch) | |
tree | e8ead0b84e7b814f5891d2c8cd3db2d6b635fb64 /rubocop | |
parent | b7d29500f28ff59c8898cdf889a40d3da908f162 (diff) |
Add latest changes from gitlab-org/gitlab@12-8-stable-ee
Diffstat (limited to 'rubocop')
-rw-r--r-- | rubocop/cop/gitlab/keys-first-and-values-first.rb | 55 | ||||
-rw-r--r-- | rubocop/cop/include_action_view_context.rb | 26 | ||||
-rw-r--r-- | rubocop/cop/migration/update_large_table.rb | 1 | ||||
-rw-r--r-- | rubocop/cop/scalability/bulk_perform_with_context.rb | 46 | ||||
-rw-r--r-- | rubocop/cop/scalability/cron_worker_context.rb | 39 | ||||
-rw-r--r-- | rubocop/rubocop.rb | 64 |
6 files changed, 145 insertions, 86 deletions
diff --git a/rubocop/cop/gitlab/keys-first-and-values-first.rb b/rubocop/cop/gitlab/keys-first-and-values-first.rb new file mode 100644 index 00000000000..9b68957cba2 --- /dev/null +++ b/rubocop/cop/gitlab/keys-first-and-values-first.rb @@ -0,0 +1,55 @@ +module RuboCop + module Cop + module Gitlab + class KeysFirstAndValuesFirst < RuboCop::Cop::Cop + FIRST_PATTERN = /\Afirst\z/.freeze + + def message(used_method) + <<~MSG + Don't use `.keys.first` and `.values.first`. + Instead use `.each_key.first` and `.each_value.first` (or `.first.first` and `first.second`) + + This will reduce memory usage and execution time. + MSG + end + + def on_send(node) + if find_on_keys_or_values?(node) + add_offense(node, location: :selector, message: message(node.method_name)) + end + end + + def autocorrect(node) + lambda do |corrector| + replace_with = if node.descendants.first.method_name == :values + '.each_value' + elsif node.descendants.first.method_name == :keys + '.each_key' + else + throw("Expect '.values.first' or '.keys.first', but get #{node.descendants.first.method_name}.first") + end + + upto_including_keys_or_values = node.descendants.first.source_range + before_keys_or_values = node.descendants[1].source_range + range_to_replace = node.source_range + .with(begin_pos: before_keys_or_values.end_pos, + end_pos: upto_including_keys_or_values.end_pos) + corrector.replace(range_to_replace, replace_with) + end + end + + def find_on_keys_or_values?(node) + chained_on_node = node.descendants.first + node.method_name.to_s =~ FIRST_PATTERN && + chained_on_node.is_a?(RuboCop::AST::SendNode) && + [:keys, :values].include?(chained_on_node.method_name) && + node.descendants[1] + end + + def method_name_for_node(node) + children[1].to_s + end + end + end + end +end diff --git a/rubocop/cop/include_action_view_context.rb b/rubocop/cop/include_action_view_context.rb deleted file mode 100644 index 52c84a711c9..00000000000 --- a/rubocop/cop/include_action_view_context.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -module RuboCop - module Cop - # Cop that makes sure workers include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`. - class IncludeActionViewContext < RuboCop::Cop::Cop - MSG = 'Include `::Gitlab::ActionViewOutput::Context`, not `ActionView::Context`, for Rails 5.'.freeze - - def_node_matcher :includes_action_view_context?, <<~PATTERN - (send nil? :include (const (const nil? :ActionView) :Context)) - PATTERN - - def on_send(node) - return unless includes_action_view_context?(node) - - add_offense(node.arguments.first, location: :expression) - end - - def autocorrect(node) - lambda do |corrector| - corrector.replace(node.source_range, '::Gitlab::ActionViewOutput::Context') - end - end - end - end -end diff --git a/rubocop/cop/migration/update_large_table.rb b/rubocop/cop/migration/update_large_table.rb index c15eec22d04..94bba31c249 100644 --- a/rubocop/cop/migration/update_large_table.rb +++ b/rubocop/cop/migration/update_large_table.rb @@ -34,6 +34,7 @@ module RuboCop namespaces notes projects + project_ci_cd_settings routes users ].freeze diff --git a/rubocop/cop/scalability/bulk_perform_with_context.rb b/rubocop/cop/scalability/bulk_perform_with_context.rb new file mode 100644 index 00000000000..3c5d7f39680 --- /dev/null +++ b/rubocop/cop/scalability/bulk_perform_with_context.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' +require_relative '../../code_reuse_helpers' + +module RuboCop + module Cop + module Scalability + class BulkPerformWithContext < RuboCop::Cop::Cop + include RuboCop::MigrationHelpers + include RuboCop::CodeReuseHelpers + + MSG = <<~MSG + Prefer using `Worker.bulk_perform_async_with_contexts` and + `Worker.bulk_perform_in_with_context` over the methods without a context + if your worker deals with specific projects or namespaces + The context is required to add metadata to our logs. + + If there is already a parent context that will apply to the jobs + being scheduled, please disable this cop with a comment explaing which + context will be applied. + + Read more about it https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#worker-context + MSG + + def_node_matcher :schedules_in_batch_without_context?, <<~PATTERN + (send (...) {:bulk_perform_async :bulk_perform_in} _*) + PATTERN + + def on_send(node) + return if in_migration?(node) || in_spec?(node) + return unless schedules_in_batch_without_context?(node) + return if name_of_receiver(node) == "BackgroundMigrationWorker" + + add_offense(node, location: :expression) + end + + private + + def in_spec?(node) + file_path_for_node(node).end_with?("_spec.rb") + end + end + end + end +end diff --git a/rubocop/cop/scalability/cron_worker_context.rb b/rubocop/cop/scalability/cron_worker_context.rb new file mode 100644 index 00000000000..8e754af4dcf --- /dev/null +++ b/rubocop/cop/scalability/cron_worker_context.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module RuboCop + module Cop + module Scalability + class CronWorkerContext < RuboCop::Cop::Cop + MSG = <<~MSG + Manually define an ApplicationContext for cronjob-workers. The context + is required to add metadata to our logs. + + If there is no relevant metadata, please disable the cop with a comment + explaining this. + + Read more about it https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#worker-context + MSG + + def_node_matcher :includes_cronjob_queue?, <<~PATTERN + (send nil? :include (const nil? :CronjobQueue)) + PATTERN + + def_node_search :defines_contexts?, <<~PATTERN + (send nil? :with_context _) + PATTERN + + def_node_search :schedules_with_batch_context?, <<~PATTERN + (send (...) {:bulk_perform_async_with_contexts :bulk_perform_in_with_contexts} _*) + PATTERN + + def on_send(node) + return unless includes_cronjob_queue?(node) + return if defines_contexts?(node.parent) + return if schedules_with_batch_context?(node.parent) + + add_offense(node.arguments.first, location: :expression) + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 1479dc3384a..08e09747ae2 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -1,60 +1,4 @@ -require_relative 'cop/gitlab/const_get_inherit_false' -require_relative 'cop/gitlab/module_with_instance_variables' -require_relative 'cop/gitlab/predicate_memoization' -require_relative 'cop/gitlab/httparty' -require_relative 'cop/gitlab/finder_with_find_by' -require_relative 'cop/gitlab/union' -require_relative 'cop/gitlab/rails_logger' -require_relative 'cop/include_action_view_context' -require_relative 'cop/include_sidekiq_worker' -require_relative 'cop/safe_params' -require_relative 'cop/active_record_association_reload' -require_relative 'cop/avoid_return_from_blocks' -require_relative 'cop/avoid_break_from_strong_memoize' -require_relative 'cop/avoid_route_redirect_leading_slash' -require_relative 'cop/line_break_around_conditional_block' -require_relative 'cop/prefer_class_methods_over_module' -require_relative 'cop/put_project_routes_under_scope' -require_relative 'cop/put_group_routes_under_scope' -require_relative 'cop/migration/add_column' -require_relative 'cop/migration/add_column_with_default' -require_relative 'cop/migration/add_concurrent_foreign_key' -require_relative 'cop/migration/add_concurrent_index' -require_relative 'cop/migration/add_index' -require_relative 'cop/migration/add_limit_to_string_columns' -require_relative 'cop/migration/add_reference' -require_relative 'cop/migration/add_timestamps' -require_relative 'cop/migration/datetime' -require_relative 'cop/migration/hash_index' -require_relative 'cop/migration/remove_column' -require_relative 'cop/migration/remove_concurrent_index' -require_relative 'cop/migration/remove_index' -require_relative 'cop/migration/reversible_add_column_with_default' -require_relative 'cop/migration/safer_boolean_column' -require_relative 'cop/migration/timestamps' -require_relative 'cop/migration/update_column_in_batches' -require_relative 'cop/migration/update_large_table' -require_relative 'cop/project_path_helper' -require_relative 'cop/rspec/any_instance_of' -require_relative 'cop/rspec/be_success_matcher' -require_relative 'cop/rspec/env_assignment' -require_relative 'cop/rspec/factories_in_migration_specs' -require_relative 'cop/rspec/top_level_describe_path' -require_relative 'cop/rspec/have_gitlab_http_status' -require_relative 'cop/qa/element_with_pattern' -require_relative 'cop/qa/ambiguous_page_object_name' -require_relative 'cop/sidekiq_options_queue' -require_relative 'cop/scalability/file_uploads' -require_relative 'cop/destroy_all' -require_relative 'cop/ruby_interpolation_in_translation' -require_relative 'code_reuse_helpers' -require_relative 'cop/code_reuse/finder' -require_relative 'cop/code_reuse/service_class' -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' -require_relative 'cop/graphql/authorize_types' -require_relative 'cop/graphql/descriptions' -require_relative 'cop/ignored_columns' +# frozen_string_literal: true + +# Auto-require all cops under `rubocop/cop/**/*.rb` +Dir[File.join(__dir__, 'cop', '**', '*.rb')].each(&method(:require)) |