summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/workers/concerns/application_worker.rb4
-rw-r--r--rubocop/cop/include_sidekiq_worker.rb29
-rw-r--r--rubocop/cop/sidekiq_options_queue.rb27
-rw-r--r--rubocop/rubocop.rb2
-rw-r--r--spec/rubocop/cop/include_sidekiq_worker_spec.rb31
-rw-r--r--spec/rubocop/cop/sidekiq_options_queue_spec.rb26
-rw-r--r--spec/workers/every_sidekiq_worker_spec.rb4
7 files changed, 117 insertions, 6 deletions
diff --git a/app/workers/concerns/application_worker.rb b/app/workers/concerns/application_worker.rb
index 285b5bada7d..37586e161c9 100644
--- a/app/workers/concerns/application_worker.rb
+++ b/app/workers/concerns/application_worker.rb
@@ -3,7 +3,7 @@ Sidekiq::Worker.extend ActiveSupport::Concern
module ApplicationWorker
extend ActiveSupport::Concern
- include Sidekiq::Worker
+ include Sidekiq::Worker # rubocop:disable Cop/IncludeSidekiqWorker
included do
set_queue
@@ -17,7 +17,7 @@ module ApplicationWorker
def set_queue
queue_name = [queue_namespace, base_queue_name].compact.join(':')
- sidekiq_options queue: queue_name
+ sidekiq_options queue: queue_name # rubocop:disable Cop/SidekiqOptionsQueue
end
def base_queue_name
diff --git a/rubocop/cop/include_sidekiq_worker.rb b/rubocop/cop/include_sidekiq_worker.rb
new file mode 100644
index 00000000000..4a6332286a2
--- /dev/null
+++ b/rubocop/cop/include_sidekiq_worker.rb
@@ -0,0 +1,29 @@
+require_relative '../spec_helpers'
+
+module RuboCop
+ module Cop
+ # Cop that makes sure workers include `ApplicationWorker`, not `Sidekiq::Worker`.
+ class IncludeSidekiqWorker < RuboCop::Cop::Cop
+ include SpecHelpers
+
+ MSG = 'Include `ApplicationWorker`, not `Sidekiq::Worker`.'.freeze
+
+ def_node_matcher :includes_sidekiq_worker?, <<~PATTERN
+ (send nil :include (const (const nil :Sidekiq) :Worker))
+ PATTERN
+
+ def on_send(node)
+ return if in_spec?(node)
+ return unless includes_sidekiq_worker?(node)
+
+ add_offense(node.arguments.first, :expression)
+ end
+
+ def autocorrect(node)
+ lambda do |corrector|
+ corrector.replace(node.source_range, 'ApplicationWorker')
+ end
+ end
+ end
+ end
+end
diff --git a/rubocop/cop/sidekiq_options_queue.rb b/rubocop/cop/sidekiq_options_queue.rb
new file mode 100644
index 00000000000..43b35ba0214
--- /dev/null
+++ b/rubocop/cop/sidekiq_options_queue.rb
@@ -0,0 +1,27 @@
+require_relative '../spec_helpers'
+
+module RuboCop
+ module Cop
+ # Cop that prevents manually setting a queue in Sidekiq workers.
+ class SidekiqOptionsQueue < RuboCop::Cop::Cop
+ include SpecHelpers
+
+ MSG = 'Do not manually set a queue; `ApplicationWorker` sets one automatically.'.freeze
+
+ def_node_matcher :sidekiq_options?, <<~PATTERN
+ (send nil :sidekiq_options $...)
+ PATTERN
+
+ def on_send(node)
+ return if in_spec?(node)
+ return unless sidekiq_options?(node)
+
+ node.arguments.first.each_node(:pair) do |pair|
+ key_name = pair.key.children[0]
+
+ add_offense(pair, :expression) if key_name == :queue
+ end
+ end
+ end
+ end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index eb52be3d731..3e3b4c8349a 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -3,10 +3,12 @@ require_relative 'cop/active_record_serialize'
require_relative 'cop/custom_error_class'
require_relative 'cop/gem_fetcher'
require_relative 'cop/in_batches'
+require_relative 'cop/include_sidekiq_worker'
require_relative 'cop/line_break_after_guard_clauses'
require_relative 'cop/polymorphic_associations'
require_relative 'cop/project_path_helper'
require_relative 'cop/redirect_with_status'
+require_relative 'cop/sidekiq_options_queue'
require_relative 'cop/migration/add_column'
require_relative 'cop/migration/add_concurrent_foreign_key'
require_relative 'cop/migration/add_concurrent_index'
diff --git a/spec/rubocop/cop/include_sidekiq_worker_spec.rb b/spec/rubocop/cop/include_sidekiq_worker_spec.rb
new file mode 100644
index 00000000000..7f406535dda
--- /dev/null
+++ b/spec/rubocop/cop/include_sidekiq_worker_spec.rb
@@ -0,0 +1,31 @@
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/include_sidekiq_worker'
+
+describe RuboCop::Cop::IncludeSidekiqWorker do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ context 'when `Sidekiq::Worker` is included' do
+ let(:source) { 'include Sidekiq::Worker' }
+ let(:correct_source) { 'include ApplicationWorker' }
+
+ it 'registers an offense ' do
+ inspect_source(cop, source)
+
+ aggregate_failures do
+ expect(cop.offenses.size).to eq(1)
+ expect(cop.offenses.map(&:line)).to eq([1])
+ expect(cop.highlights).to eq(['Sidekiq::Worker'])
+ end
+ end
+
+ it 'autocorrects to the right version' do
+ autocorrected = autocorrect_source(cop, source)
+
+ expect(autocorrected).to eq(correct_source)
+ end
+ end
+end
diff --git a/spec/rubocop/cop/sidekiq_options_queue_spec.rb b/spec/rubocop/cop/sidekiq_options_queue_spec.rb
new file mode 100644
index 00000000000..a31de381631
--- /dev/null
+++ b/spec/rubocop/cop/sidekiq_options_queue_spec.rb
@@ -0,0 +1,26 @@
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/sidekiq_options_queue'
+
+describe RuboCop::Cop::SidekiqOptionsQueue do
+ include CopHelper
+
+ subject(:cop) { described_class.new }
+
+ it 'registers an offense when `sidekiq_options` is used with the `queue` option' do
+ inspect_source(cop, 'sidekiq_options queue: "some_queue"')
+
+ aggregate_failures do
+ expect(cop.offenses.size).to eq(1)
+ expect(cop.offenses.map(&:line)).to eq([1])
+ expect(cop.highlights).to eq(['queue: "some_queue"'])
+ end
+ end
+
+ it 'does not register an offense when `sidekiq_options` is used with another option' do
+ inspect_source(cop, 'sidekiq_options retry: false')
+
+ expect(cop.offenses).to be_empty
+ end
+end
diff --git a/spec/workers/every_sidekiq_worker_spec.rb b/spec/workers/every_sidekiq_worker_spec.rb
index bf99c24e73c..9e3b99b3502 100644
--- a/spec/workers/every_sidekiq_worker_spec.rb
+++ b/spec/workers/every_sidekiq_worker_spec.rb
@@ -1,10 +1,6 @@
require 'spec_helper'
describe 'Every Sidekiq worker' do
- it 'includes ApplicationWorker' do
- expect(Gitlab::SidekiqConfig.workers).to all(include(ApplicationWorker))
- end
-
it 'does not use the default queue' do
expect(Gitlab::SidekiqConfig.workers.map(&:queue)).not_to include('default')
end