Modern Rubocop version and compliance

.codeclimate.yml

@@ -1,8 +1,7 @@
 ---
 engines:
   rubocop:
-    enabled: true
-    channel: rubocop-1-56-3
+    enabled: false
 ratings:
   paths:
   - "**.rb"

.github/workflows/test.yml

@@ -35,3 +35,4 @@ jobs:
         run: sudo systemctl start mysql.service
       - run: bin/setup
       - run: bundle exec rake
+      - run: bundle exec rubocop --parallel

.rubocop.yml

@@ -1,6 +1,11 @@
 ---
+require:
+  - rubocop-performance
+  - rubocop-rake
+
 AllCops:
   TargetRubyVersion: 3.0
+  NewCops: enable
 
 Metrics/AbcSize:
   Enabled: false
@@ -17,12 +22,6 @@ Metrics/BlockNesting:
 Metrics/ClassLength:
   Max: 250
 
-Metrics/LineLength:
-  Max: 120
-  Exclude:
-    - 'departure.gemspec'
-    - 'test_database.rb'
-
 Metrics/MethodLength:
   Max: 30
 
@@ -35,11 +34,6 @@ Metrics/ParameterLists:
 Performance/Casecmp:
   Enabled: false
 
-Style/BracesAroundHashParameters:
-  Exclude:
-    - 'spec/fixtures/migrate/**.rb'
-    - 'spec/integration/**.rb'
-
 Style/CommandLiteral:
   Exclude:
     - 'test_database.rb'
@@ -54,12 +48,18 @@ Style/MultilineBlockChain:
   Exclude:
     - 'spec/integration_spec.rb'
 
-Layout/MultilineMethodCallIndentation:
-  Enabled: false
+Layout/LineLength:
+  Max: 120
+  Exclude:
+    - 'departure.gemspec'
+    - 'test_database.rb'
 
-Style/SymbolArray:
+Layout/MultilineMethodCallIndentation:
   Enabled: false
 
-Style/UnneededPercentQ:
+Style/RedundantPercentQ:
   Exclude:
     - 'departure.gemspec'
+
+Style/SymbolArray:
+  Enabled: false

Gemfile

@@ -2,5 +2,13 @@ source 'https://rubygems.org'
 
 gemspec
 
+gem 'climate_control', '~> 0.0.3'
+gem 'pry-byebug'
+gem 'rake', '>= 10.0'
+gem 'rspec', '~> 3.4', '>= 3.4.0'
+gem 'rspec-its', '~> 1.2'
+
 gem 'codeclimate-test-reporter', '~> 1.0.3', group: :test, require: nil
-gem 'rubocop', '~> 1.56.3', require: false
+gem 'rubocop', '~> 1.59', require: false
+gem 'rubocop-performance'
+gem 'rubocop-rake'

configuration.rb

@@ -7,7 +7,7 @@ class Configuration
   attr_reader :config
 
   def initialize
-    @config = YAML.load(ERB.new(File.read(CONFIG_PATH)).result).freeze
+    @config = YAML.safe_load(ERB.new(File.read(CONFIG_PATH)).result).freeze
   end
 
   def [](key)

departure.gemspec

@@ -1,6 +1,4 @@
-# coding: utf-8
-
-lib = File.expand_path('../lib', __FILE__)
+lib = File.expand_path('lib', __dir__)
 $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 require 'departure/version'
@@ -20,18 +18,14 @@ Gem::Specification.new do |spec|
   spec.homepage      = 'https://github.com/departurerb/departure'
   spec.license       = 'MIT'
 
+  spec.metadata['rubygems_mfa_required'] = 'true'
+
   spec.files         = `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) }
   spec.require_paths = ['lib']
 
   spec.required_ruby_version = '>= 3.0.0'
 
-  spec.add_runtime_dependency 'railties', *Array(RAILS_DEPENDENCY_VERSION)
   spec.add_runtime_dependency 'activerecord', *Array(RAILS_DEPENDENCY_VERSION)
   spec.add_runtime_dependency 'mysql2', '>= 0.4.0', '<= 0.5.5'
-
-  spec.add_development_dependency 'rake', '>= 10.0'
-  spec.add_development_dependency 'rspec', '~> 3.4', '>= 3.4.0'
-  spec.add_development_dependency 'rspec-its', '~> 1.2'
-  spec.add_development_dependency 'pry-byebug'
-  spec.add_development_dependency 'climate_control', '~> 0.0.3'
+  spec.add_runtime_dependency 'railties', *Array(RAILS_DEPENDENCY_VERSION)
 end

lib/active_record/connection_adapters/for_alter.rb

@@ -3,18 +3,19 @@
 module ForAlterStatements
   class << self
     def included(_)
-      STDERR.puts 'Including for_alter statements'
+      warn 'Including for_alter statements'
     end
   end
 
-  def bulk_change_table(table_name, operations) #:nodoc:
+  def bulk_change_table(table_name, operations) # :nodoc:
     sqls = operations.flat_map do |command, args|
       table = args.shift
       arguments = args
 
       method = :"#{command}_for_alter"
 
       raise "Unknown method called : #{method}(#{arguments.inspect})" unless respond_to?(method, true)
+
       public_send(method, table, *arguments)
     end.join(', ')
 
@@ -97,7 +98,7 @@ def remove_column_for_alter(_table_name, column_name, _type = nil, _options = {}
     "DROP COLUMN #{quote_column_name(column_name)}"
   end
 
-  def remove_columns_for_alter(table_name, *column_names, **options)
+  def remove_columns_for_alter(table_name, *column_names, **_options)
     column_names.map { |column_name| remove_column_for_alter(table_name, column_name) }
   end
 end

lib/active_record/connection_adapters/percona_adapter.rb

@@ -64,9 +64,7 @@ def visit_DropForeignKey(name) # rubocop:disable Naming/MethodName
 
       extend Forwardable
 
-      unless method_defined?(:change_column_for_alter)
-        include ForAlterStatements
-      end
+      include ForAlterStatements unless method_defined?(:change_column_for_alter)
 
       ADAPTER_NAME = 'Percona'.freeze
 
@@ -90,7 +88,7 @@ def exec_delete(sql, name, binds)
       end
       alias exec_update exec_delete
 
-      def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) # rubocop:disable Lint/UnusedMethodArgument, Metrics/LineLength
+      def exec_insert(sql, name, binds, pk = nil, sequence_name = nil) # rubocop:disable Lint/UnusedMethodArgument, Naming/MethodParameterName
         execute(to_sql(sql, binds), name)
       end
 
@@ -153,6 +151,7 @@ def add_index(table_name, column_name, options = {})
       def remove_index(table_name, column_name = nil, **options)
         if ActiveRecord::VERSION::STRING >= '6.1'
           return if options[:if_exists] && !index_exists?(table_name, column_name, **options)
+
           index_name = index_name_for_remove(table_name, column_name, options)
         else
           index_name = index_name_for_remove(table_name, options)

lib/departure/alter_argument.rb

@@ -4,7 +4,7 @@ class InvalidAlterStatement < StandardError; end
   # Represents the '--alter' argument of Percona's pt-online-schema-change
   # See https://www.percona.com/doc/percona-toolkit/2.0/pt-online-schema-change.html
   class AlterArgument
-    ALTER_TABLE_REGEX = /\AALTER TABLE [^\s]*[\n]* /
+    ALTER_TABLE_REGEX = /\AALTER TABLE [^\s]*\n* /.freeze
 
     attr_reader :table_name
 
@@ -17,11 +17,12 @@ def initialize(statement)
 
       match = statement.match(ALTER_TABLE_REGEX)
       raise InvalidAlterStatement unless match
+
       # Separates the ALTER TABLE from the table_name
       #
       # Removes the grave marks, if they are there, so we can get the table_name
       @table_name = String(match)
-                      .split(' ')[2]
+                      .split[2]
                       .delete('`')
     end
 
@@ -42,7 +43,7 @@ def parsed_statement
       @parsed_statement ||= statement
         .gsub(ALTER_TABLE_REGEX, '')
         .gsub('`', '\\\`')
-        .gsub(/\\n/, '')
+        .gsub('\n', '')
         .gsub('"', '\\\"')
     end
   end

lib/departure/cli_generator.rb

@@ -9,7 +9,7 @@ module Departure
   # given SQL statement
   #
   # --no-check-alter is used to allow running CHANGE COLUMN statements. For more details, check:
-  # www.percona.com/doc/percona-toolkit/2.2/pt-online-schema-change.html#cmdoption-pt-online-schema-change--[no]check-alter # rubocop:disable Metrics/LineLength
+  # www.percona.com/doc/percona-toolkit/2.2/pt-online-schema-change.html#cmdoption-pt-online-schema-change--[no]check-alter # rubocop:disable Layout/LineLength
   #
   class CliGenerator
     COMMAND_NAME = 'pt-online-schema-change'.freeze

lib/departure/command.rb

@@ -44,11 +44,11 @@ def run_in_process
       Open3.popen3(full_command) do |_stdin, stdout, _stderr, waith_thr|
         begin
           loop do
-            IO.select([stdout])
+            stdout.wait_readable
             data = stdout.read_nonblock(8192)
             logger.write_no_newline(data)
           end
-        rescue EOFError # rubocop:disable Lint/HandleExceptions
+        rescue EOFError
           # noop
         ensure
           @status = waith_thr.value
@@ -89,7 +89,7 @@ def error_message
     # Logs when the execution started
     def log_started
       logger.write("\n")
-      logger.say("Running #{command_line}\n\n", true)
+      logger.say("Running #{command_line}\n\n", subitem: true)
     end
 
     # Prints a line break to keep the logs separate from the execution time

lib/departure/connection_details.rb

@@ -58,9 +58,7 @@ def base_connection
     # @return [String]
     def host_argument
       host_string = host
-      if ssl_ca.present?
-        host_string += ";mysql_ssl=1;mysql_ssl_client_ca=#{ssl_ca}"
-      end
+      host_string += ";mysql_ssl=1;mysql_ssl_client_ca=#{ssl_ca}" if ssl_ca.present?
       "-h \"#{host_string}\""
     end
 

lib/departure/dsn.rb

@@ -15,7 +15,7 @@ def initialize(database, table_name)
     # Returns the pt-online-schema-change DSN string. See
     # https://www.percona.com/doc/percona-toolkit/2.0/pt-online-schema-change.html#dsn-options
     def to_s
-      "D=#{database},t=#{table_name}#{suffix.nil? ? nil : ',' + suffix}"
+      "D=#{database},t=#{table_name}#{suffix.nil? ? nil : ",#{suffix}"}"
     end
 
     private

lib/departure/log_sanitizers/password_sanitizer.rb

@@ -11,6 +11,7 @@ def initialize(connection_details)
 
       def execute(log_statement)
         return log_statement if password_argument.blank?
+
         log_statement.gsub(password_argument, PASSWORD_REPLACEMENT)
       end
 

lib/departure/logger.rb

@@ -13,7 +13,7 @@ def initialize(sanitizers)
     #
     # @param message [String]
     # @param subitem [Boolean] whether to show message as a nested log item
-    def say(message, subitem = false)
+    def say(message, subitem: false)
       write "#{subitem ? '   ->' : '--'} #{message}"
     end
 

lib/departure/migration.rb

@@ -74,31 +74,33 @@ def include_foreigner
     # instead of the current adapter.
     def reconnect_with_percona
       return if connection_config[:adapter] == 'percona'
+
       Departure::ConnectionBase.establish_connection(connection_config.merge(adapter: 'percona'))
     end
 
     # Reconnect without percona adapter when Departure is disabled but was
     # enabled in a previous migration.
     def reconnect_without_percona
       return unless connection_config[:adapter] == 'percona'
+
       Departure::OriginalAdapterConnection.establish_connection(connection_config.merge(adapter: original_adapter))
     end
 
     private
 
-    # Capture the type of the adapter configured by the app if not already set.
-    def connection_config
-      configuration_hash.tap do |config|
-        self.class.original_adapter ||= config[:adapter]
-      end
-    end
-
-    private def configuration_hash
+    def configuration_hash
       if ActiveRecord::VERSION::STRING >= '6.1'
         ActiveRecord::Base.connection_db_config.configuration_hash
       else
         ActiveRecord::Base.connection_config
       end
     end
+
+    # Capture the type of the adapter configured by the app if not already set.
+    def connection_config
+      configuration_hash.tap do |config|
+        self.class.original_adapter ||= config[:adapter]
+      end
+    end
   end
 end

lib/departure/null_logger.rb

@@ -1,6 +1,6 @@
 module Departure
   class NullLogger
-    def say(_message, _subitem = false)
+    def say(_message, subitem: false)
       # noop
     end
 

lib/departure/user_options.rb

@@ -6,7 +6,7 @@ class UserOptions
     # Constructor
     #
     # @param arguments [String]
-    def initialize(arguments = ENV['PERCONA_ARGS'])
+    def initialize(arguments = ENV.fetch('PERCONA_ARGS', nil))
       @arguments = arguments
     end
 

lib/lhm/column_with_sql.rb

@@ -72,15 +72,15 @@ def column
     #
     # @return [String, NilClass]
     def default_value
-      match = if definition =~ /timestamp|datetime/i
+      match = if /timestamp|datetime/i.match?(definition)
                 /default '?(.+[^'])'?/i.match(definition)
               else
                 /default '?(\w+)'?/i.match(definition)
               end
 
       return unless match
 
-      match[1].downcase != 'null' ? match[1] : nil
+      match[1].downcase == 'null' ? nil : match[1]
     end
 
     # Checks whether the column accepts NULL as specified in the definition
@@ -90,7 +90,7 @@ def null_value
       match = /((\w*) NULL)/i.match(definition)
       return true unless match
 
-      match[2].downcase == 'not' ? false : true
+      match[2].downcase != 'not'
     end
   end
 end