Add rubocop check to CI (#106)

* Add GitHub action to run rubocop.

* Fix rubocop violations.

* Update .ruby-version to 3.0.7.

Fixes an annoying issue with rubygems which spams warnings when running
certain commands.

See rubygems/rubygems#5016

.github/workflows/tests.yml

@@ -4,6 +4,21 @@ on:
   push:
 
 jobs:
+  rubocop:
+    runs-on: ubuntu-latest
+    env:
+      BUNDLE_RUBYGEMS__PKG__GITHUB__COM: gocardless-robot-readonly:${{ secrets.GITHUB_TOKEN }}
+    steps:
+      - uses: actions/checkout@v4
+      - name: Set up Ruby
+        uses: ruby/setup-ruby@v1
+        with:
+          bundler-cache: true
+          ruby-version: "3.0"
+      - name: Run rubocop
+        run: |
+          bundle exec rubocop --extra-details --display-style-guide --no-server --parallel
+
   rspec:
     strategy:
       fail-fast: false
@@ -33,7 +48,7 @@ jobs:
       PGHOST: localhost
       BUNDLE_RUBYGEMS__PKG__GITHUB__COM: gocardless-robot-readonly:${{ secrets.GITHUB_TOKEN }}
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - name: Set up Ruby
         uses: ruby/setup-ruby@v1
         with:

.rubocop.yml

@@ -1,26 +1,27 @@
 inherit_gem:
   gc_ruboconfig: rubocop.yml
 
-AllCops:
-  Exclude: [legacy_spec/**/*]
+require:
+  - rubocop-performance
+  - rubocop-rake
+  - rubocop-rspec
+  - rubocop-sequel
 
-Metrics/MethodLength:
-  Max: 20
+AllCops:
+  NewCops: enable
+  Exclude:
+    - "vendor/**/*"
+    - "legacy_spec/**/*"
+    - "lib/generators/que/templates/*.rb"
 
 RSpec/MultipleExpectations:
   Enabled: false
 
-RSpec/NestedGroups:
-  Max: 5
-
-RSpec/NamedSubject:
-  Enabled: false
-
 RSpec/ExampleLength:
   Enabled: false
 
-RSpec/DescribeClass:
+RSpec/IndexedLet:
   Enabled: false
 
-Metrics/CyclomaticComplexity:
-  Max: 10
+RSpec/NestedGroups:
+  Max: 5

.ruby-version

@@ -1 +1 @@
-3.0.2
+3.0.7

Gemfile

@@ -11,7 +11,11 @@ group :development, :test do
   gem 'pg', require: nil, platform: :ruby
   gem 'pg_jruby', require: nil, platform: :jruby
   gem 'pond', require: nil
-  gem 'rubocop'
+  gem 'rubocop', '~> 1.64.1'
+  gem 'rubocop-performance', '~> 1.21.1'
+  gem 'rubocop-rake', '~> 0.6.0'
+  gem 'rubocop-rspec', '~> 3.0.2'
+  gem 'rubocop-sequel', '~> 0.3.4'
   gem 'sequel', require: nil
 end
 

Rakefile

@@ -2,4 +2,4 @@
 
 require "bundler/gem_tasks"
 
-Dir["./tasks/*.rb"].sort.each { |f| require f }
+Dir["./tasks/*.rb"].each { |f| require f }

benchmark/seed-jobs

@@ -19,7 +19,7 @@ unless ARGV.count == 3
 end
 
 def parse_range(token)
-  [*Range.new(*token.split("..").map(&:to_i))]
+  Array(Range.new(*token.split("..").map(&:to_i)))
 rescue StandardError
   [token.to_i]
 end

benchmark/setup.rb

@@ -9,7 +9,7 @@
 Que.connection = ActiveRecord
 Que.migrate!
 
-Que.logger = Logger.new(STDOUT)
+Que.logger = Logger.new($stdout)
 Que.logger.formatter = proc do |severity, datetime, _progname, payload|
   { ts: datetime, tid: Thread.current.object_id, level: severity }.
     merge(payload).to_json + "\n"

bin/que

@@ -13,7 +13,7 @@ $stdout.sync = true
 
 options = OpenStruct.new
 
-# rubocop:disable Metrics/LineLength
+# rubocop:disable Layout/LineLength
 OptionParser.new do |opts|
   opts.banner = "usage: que [options] file/to/require ..."
 
@@ -59,7 +59,7 @@ OptionParser.new do |opts|
 
   opts.on("-v", "--version", "Show Que version") do
     require "que"
-    $stdout.puts "Que version #{Que::Version}"
+    $stdout.puts "Que version #{Que::VERSION}"
     exit 0
   end
 
@@ -68,9 +68,9 @@ OptionParser.new do |opts|
     exit 0
   end
 end.parse!(ARGV)
-# rubocop:enable Metrics/LineLength
+# rubocop:enable Layout/LineLength
 
-if ARGV.length.zero?
+if ARGV.empty?
   $stdout.puts <<~OUTPUT
     You didn't include any Ruby files to require!
     Que needs to be able to load your application before it can process jobs.
@@ -92,9 +92,9 @@ wake_interval    = options.wake_interval || ENV["QUE_WAKE_INTERVAL"]&.to_f || Qu
 cursor_expiry    = options.cursor_expiry || wake_interval
 worker_count     = options.worker_count  || 1
 timeout          = options.timeout
-secondary_queues = options.secondary_queues  || []
+secondary_queues = options.secondary_queues || []
 
-Que.logger ||= Logger.new(STDOUT)
+Que.logger ||= Logger.new($stdout)
 
 begin
   Que.logger.level = Logger.const_get(log_level.upcase) if log_level

lib/que.rb

@@ -42,7 +42,7 @@ module Que
   SYMBOLIZER = proc do |object|
     case object
     when Hash
-      object.keys.each do |key|
+      object.each_key do |key|
         object[key.to_sym] = SYMBOLIZER.call(object.delete(key))
       end
       object
@@ -145,11 +145,11 @@ def transaction
           begin
             execute "BEGIN"
             yield
-          rescue StandardError => error
+          rescue StandardError => e
             raise
           ensure
             # Handle a raised error or a killed thread.
-            if error || Thread.current.status == "aborting"
+            if e || Thread.current.status == "aborting"
               execute "ROLLBACK"
             else
               execute "COMMIT"

lib/que/adapters/base.rb

@@ -82,7 +82,7 @@ def execute_prepared(name, params)
             end
 
             conn.exec_prepared("que_#{name}", params)
-          rescue ::PG::InvalidSqlStatementName => error
+          rescue ::PG::InvalidSqlStatementName => e
             # Reconnections on ActiveRecord can cause the same connection
             # objects to refer to new backends, so recover as well as we can.
 
@@ -92,7 +92,7 @@ def execute_prepared(name, params)
               retry
             end
 
-            raise error
+            raise e
           end
         end
       end
@@ -102,7 +102,7 @@ def execute_prepared(name, params)
         16 => ->(value) {
           case value
           when String then value == "t"
-          else !!value
+          else !value.nil?
           end
         },
         # bigint

lib/que/job.rb

@@ -84,15 +84,17 @@ def self.custom_log_context(custom_proc)
       if custom_proc.is_a?(Proc)
         self.log_context_proc = custom_proc
       else
-        raise ArgumentError.new "Custom log context must be a Proc " \
-                                "which receives the job as an argument and " \
-                                "returns a hash"
+        raise ArgumentError, "Custom log context must be a Proc " \
+                             "which receives the job as an argument and " \
+                             "returns a hash"
       end
     end
 
+    # rubocop:disable Naming/AccessorMethodName
     def get_custom_log_context
       self.class.log_context_proc&.call(@attrs) || {}
     end
+    # rubocop:enable Naming/AccessorMethodName
 
     # This is accepting JOB_OPTIONS and args as keyword parameters. In future we want to
     # set instance variables instead of using a grab bag of parameters, which would allow

lib/que/leaky_bucket.rb

@@ -56,11 +56,11 @@ def refill
 
     private
 
-    def catch_error(&block)
-      result = block.call
+    def catch_error
+      result = yield
       [result, nil]
-    rescue StandardError => err
-      [result, err]
+    rescue StandardError => e
+      [result, e]
     end
   end
 end

lib/que/middleware/queue_collector.rb

@@ -41,7 +41,6 @@ def initialize(app, options = {})
         end
       end
 
-      # rubocop:disable Metrics/AbcSize
       def call(env)
         # Reset all the previously observed values back to zero, ensuring we only ever
         # report metric values that are current in every scrape.
@@ -72,7 +71,6 @@ def call(env)
 
         @app.call(env)
       end
-      # rubocop:enable Metrics/AbcSize
 
       def refresh_materialized_view
         # Ensure generating metrics never take more than 5000ms to execute. If we can't

lib/que/middleware/worker_collector.rb

@@ -28,8 +28,7 @@ def call(env)
 
       private
 
-      # rubocop:disable Lint/HandleExceptions
-      # rubocop:disable Style/RedundantBegin
+      # rubocop:disable Style/RedundantBegin, Lint/SuppressedException
       def register(*metrics)
         begin
           metrics.each do |metric|
@@ -38,8 +37,7 @@ def register(*metrics)
         rescue Prometheus::Client::Registry::AlreadyRegisteredError
         end
       end
-      # rubocop:enable Style/RedundantBegin
-      # rubocop:enable Lint/HandleExceptions
+      # rubocop:enable Style/RedundantBegin, Lint/SuppressedException
     end
   end
 end

lib/que/migrations.rb

@@ -8,7 +8,6 @@ module Migrations
     CURRENT_VERSION = 6
 
     class << self
-      # rubocop:disable Metrics/AbcSize
       def migrate!(options = { version: CURRENT_VERSION })
         Que.transaction do
           version = options[:version]
@@ -32,7 +31,6 @@ def migrate!(options = { version: CURRENT_VERSION })
           self.db_version = version
         end
       end
-      # rubocop:enable Metrics/AbcSize
 
       def db_version
         result = Que.execute <<-SQL

lib/que/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Que
-  Version = "1.0.0"
+  VERSION = "1.0.0"
 end

lib/que/worker.rb

@@ -145,7 +145,7 @@ def work_loop
 
       @tracer.trace(RunningSecondsTotal, queue: @queue, primary_queue: @queue) do
         loop do
-          case event = work
+          case work
           when :postgres_error
             Que.logger&.info(event: "que.postgres_error", wake_interval: @wake_interval)
             @tracer.trace(SleepingSecondsTotal, queue: @queue, primary_queue: @queue) do
@@ -252,7 +252,7 @@ def work
 
             # For compatibility with que-failure, we need to allow failure handlers to be
             # defined on the job class.
-            if klass&.respond_to?(:handle_job_failure)
+            if klass.respond_to?(:handle_job_failure)
               klass.handle_job_failure(e, job)
             else
               handle_job_failure(e, job)
@@ -302,7 +302,7 @@ def class_for(string)
     end
 
     def actual_job_class_name(class_name, args)
-      return args.first["job_class"] if /ActiveJob::QueueAdapters/.match?(class_name)
+      return args.first["job_class"] if class_name.include?("ActiveJob::QueueAdapters")
 
       class_name
     end

que.gemspec

@@ -6,7 +6,7 @@ require "que/version"
 
 Gem::Specification.new do |spec|
   spec.name          = "que"
-  spec.version       = Que::Version
+  spec.version       = Que::VERSION
   spec.authors       = ["Chris Hanks"]
   spec.email         = ["[email protected]"]
   spec.description   =
@@ -15,9 +15,9 @@ Gem::Specification.new do |spec|
   spec.homepage      = "https://github.com/chanks/que"
   spec.license       = "MIT"
 
+  spec.required_ruby_version = ">= 3.0"
   spec.files         = `git ls-files`.split($INPUT_RECORD_SEPARATOR)
   spec.executables   = ["que"]
-  spec.test_files    = spec.files.grep(%r{^(test|spec|features)/})
   spec.require_paths = ["lib"]
 
   # We're pointing to our own branch of the Prometheus Client.
@@ -31,4 +31,5 @@ Gem::Specification.new do |spec|
   spec.add_dependency "webrick", "~> 1.7"
 
   spec.add_runtime_dependency "activesupport"
+  spec.metadata["rubygems_mfa_required"] = "true"
 end

spec/integration/integration_spec.rb

@@ -3,6 +3,7 @@
 require "spec_helper"
 require "que/worker" # required to prevent autoload races
 
+# rubocop:disable RSpec/DescribeClass
 RSpec.describe "multiple workers" do
   def with_workers(num, stop_timeout: 5, secondary_queues: [], &block)
     Que::WorkerGroup.start(
@@ -150,3 +151,4 @@ def wait_for_jobs_to_be_worked(timeout: 10)
     end
   end
 end
+# rubocop:enable RSpec/DescribeClass