Lint sourcecode, setup Rubocop proper and Lint in CI

This is basically linting the codebase, making changes
accordingly (minor) and setting up lint in CI

.circleci/config.yml

@@ -1,6 +1,6 @@
 version: 2
 jobs:
-  build:
+  build_lint_test:
     working_directory: ~/circleci-pg-online-schema-change
 
     docker:
@@ -38,6 +38,10 @@ jobs:
           paths:
             - vendor/bundle
 
+      - run:
+          name: lint
+          command: bundle exec rubocop
+
       - run:
           name: Parallel RSpec
           command: bundle exec rspec

.rubocop.yml

@@ -1,3 +1,5 @@
+inherit_from: .rubocop_todo.yml
+
 require:
   - rubocop-rspec
   - rubocop-packaging
@@ -14,71 +16,70 @@ AllCops:
     - "vendor/**/*"
 
 Layout/HashAlignment:
-  EnforcedColonStyle:
-    - table
-    - key
-  EnforcedHashRocketStyle:
-    - table
-    - key
+  EnforcedColonStyle: key
+  EnforcedHashRocketStyle:  key
 
 Layout/SpaceAroundEqualsInParameterDefault:
-  EnforcedStyle: no_space
+  EnforcedStyle: space
 
 Metrics/AbcSize:
-  Max: 20
+  Enabled: true
+  Max: 40
   Exclude:
-    - "test/**/*"
+    - "spec/**/*"
 
 Metrics/BlockLength:
+  Max: 100
   Exclude:
     - "*.gemspec"
     - "Rakefile"
+    - "spec/**/*"
 
 Metrics/ClassLength:
   Exclude:
     - "test/**/*"
 
 Metrics/MethodLength:
-  Max: 18
+  Max: 30
   Exclude:
     - "test/**/*"
 
 Metrics/ParameterLists:
-  Max: 6
+  Max: 5
 
 Naming/MemoizedInstanceVariableName:
-  Enabled: false
+  Enabled: true
 
 Naming/VariableNumber:
-  Enabled: false
-
-Rake/Desc:
-  Enabled: false
+  Enabled: true
 
 Style/BarePercentLiterals:
   EnforcedStyle: percent_q
 
 Style/ClassAndModuleChildren:
-  Enabled: false
+  Enabled: true
 
 Style/Documentation:
   Enabled: false
 
 Style/DoubleNegation:
-  Enabled: false
+  Enabled: true
 
 Style/EmptyMethod:
-  Enabled: false
+  Enabled: true
 
 Style/FrozenStringLiteralComment:
-  Enabled: false
+  Enabled: true
 
 Style/NumericPredicate:
-  Enabled: false
+  Enabled: true
 
 Style/StringLiterals:
   EnforcedStyle: double_quotes
 
+Style/StringLiteralsInInterpolation:
+  EnforcedStyle: double_quotes
+
 Style/TrivialAccessors:
   AllowPredicates: true
 
@@ -91,9 +92,41 @@ Style/TrailingCommaInArrayLiteral:
 Style/TrailingCommaInHashLiteral:
   EnforcedStyleForMultiline: comma
 
-Style/SpaceAroundEqualsInParameterDefault:
-  EnforcedStyle: space
+Layout/MultilineArrayBraceLayout:
+  Enabled: true
+  EnforcedStyle: symmetrical
 
-Style/MultilineHashBraceLayout:
+Layout/MultilineHashBraceLayout:
   Enabled: true
   EnforcedStyle: symmetrical
+
+Layout/MultilineAssignmentLayout:
+  Enabled: true
+  EnforcedStyle: same_line
+
+Layout/FirstArrayElementIndentation:
+  Enabled: true
+  EnforcedStyle: consistent
+
+Layout/FirstHashElementIndentation:
+  Enabled: true
+  EnforcedStyle: consistent
+
+Layout/MultilineHashKeyLineBreaks:
+  Enabled: true
+
+Layout/LineLength:
+  Enabled: true
+  Max: 250
+
+Style/FormatStringToken:
+  Enabled: true
+  EnforcedStyle: template
+
+RSpec/MessageSpies:
+  Enabled: true
+  EnforcedStyle: receive
+
+RSpec/FilePath:
+  Enabled: true
+  SpecSuffixOnly: true

.rubocop_todo.yml

@@ -0,0 +1,44 @@
+# This configuration was generated by
+# `rubocop --auto-gen-config`
+# on 2022-02-21 22:46:44 UTC using RuboCop version 1.23.0.
+# The point is for the user to remove these configuration records
+# one by one as the offenses are removed from the code base.
+# Note that changes in the inspected code, or installation of new
+# versions of RuboCop, may require this file to be generated again.
+
+# Offense count: 2
+# Configuration parameters: CountComments, CountAsOne.
+Metrics/ClassLength:
+  Max: 233
+
+# Offense count: 2
+# Configuration parameters: IgnoredMethods.
+Metrics/CyclomaticComplexity:
+  Max: 15
+
+# Offense count: 2
+# Configuration parameters: IgnoredMethods.
+Metrics/PerceivedComplexity:
+  Max: 13
+
+# Offense count: 1
+Packaging/GemspecGit:
+  Exclude:
+    - 'pg_online_schema_change.gemspec'
+
+# Offense count: 62
+# Configuration parameters: CountAsOne.
+RSpec/ExampleLength:
+  Max: 55
+
+# Offense count: 38
+RSpec/MultipleExpectations:
+  Max: 14
+
+# Offense count: 6
+# Configuration parameters: AllowedMethods.
+# AllowedMethods: respond_to_missing?
+Style/OptionalBooleanParameter:
+  Exclude:
+    - 'lib/pg_online_schema_change/query.rb'
+    - 'lib/pg_online_schema_change/replay.rb'

lib/pg_online_schema_change.rb

@@ -18,16 +18,12 @@ class Error < StandardError; end
   class CountBelowDelta < StandardError; end
   class AccessExclusiveLockNotAcquired < StandardError; end
 
-  def self.logger=(verbose)
-    @@logger ||= begin
-        logger = Ougai::Logger.new($stdout)
-        logger.level = verbose ? Ougai::Logger::TRACE : Ougai::Logger::INFO
-        logger.with_fields = { version: PgOnlineSchemaChange::VERSION }
-        logger
-      end
-  end
-
-  def self.logger
-    @@logger
+  def self.logger(verbose: false)
+    @logger ||= begin
+      logger = Ougai::Logger.new($stdout)
+      logger.level = verbose ? Ougai::Logger::TRACE : Ougai::Logger::INFO
+      logger.with_fields = { version: PgOnlineSchemaChange::VERSION }
+      logger
+    end
   end
 end

lib/pg_online_schema_change/cli.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "thor"
 
 module PgOnlineSchemaChange
@@ -25,7 +27,7 @@ class CLI < Thor
     def perform
       client_options = Struct.new(*options.keys.map(&:to_sym)).new(*options.values)
 
-      PgOnlineSchemaChange.logger = client_options.verbose
+      PgOnlineSchemaChange.logger(verbose: client_options.verbose)
       PgOnlineSchemaChange::Orchestrate.run!(client_options)
     end
 

lib/pg_online_schema_change/client.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "pg"
 
 module PgOnlineSchemaChange
@@ -16,7 +18,9 @@ def initialize(options)
       @drop = options.drop
       @kill_backends = options.kill_backends
       @wait_time_for_lock = options.wait_time_for_lock
+
       handle_copy_statement(options.copy_statement)
+      handle_validations
 
       @connection = PG.connect(
         dbname: @dbname,
@@ -26,17 +30,19 @@ def initialize(options)
         port: @port,
       )
 
-      raise Error, "Not a valid ALTER statement: #{@alter_statement}" unless Query.alter_statement?(@alter_statement)
-
-      unless Query.same_table?(@alter_statement)
-        raise Error "All statements should belong to the same table: #{@alter_statement}"
-      end
-
       @table = Query.table(@alter_statement)
 
       PgOnlineSchemaChange.logger.debug("Connection established")
     end
 
+    def handle_validations
+      raise Error, "Not a valid ALTER statement: #{@alter_statement}" unless Query.alter_statement?(@alter_statement)
+
+      return if Query.same_table?(@alter_statement)
+
+      raise Error "All statements should belong to the same table: #{@alter_statement}"
+    end
+
     def handle_copy_statement(statement)
       return if statement.nil? || statement == ""
 

lib/pg_online_schema_change/functions.rb

@@ -1,4 +1,6 @@
-FUNC_FIX_SERIAL_SEQUENCE = <<~SQL.freeze
+# frozen_string_literal: true
+
+FUNC_FIX_SERIAL_SEQUENCE = <<~SQL
     CREATE OR REPLACE FUNCTION fix_serial_sequence(_table regclass, _newtable text)
     RETURNS void AS
     $func$
@@ -35,7 +37,7 @@
   $func$  LANGUAGE plpgsql VOLATILE;
 SQL
 
-FUNC_CREATE_TABLE_ALL = <<~SQL.freeze
+FUNC_CREATE_TABLE_ALL = <<~SQL
   CREATE OR REPLACE FUNCTION create_table_all(source_table text, newsource_table text)
     RETURNS void language plpgsql
     as $$

lib/pg_online_schema_change/helper.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 module PgOnlineSchemaChange
   module Helper
     def primary_key
@@ -15,7 +17,14 @@ def method_missing(method, *_args)
       result = Store.send(:get, method)
       return result if result
 
-      raise ArgumentError, "Method `#{method}` doesn't exist."
+      super
+    end
+
+    def respond_to_missing?(method_name, *args)
+      result = Store.send(:get, method)
+      return true if result
+
+      super
     end
   end
 end

lib/pg_online_schema_change/orchestrate.rb

@@ -1,8 +1,10 @@
+# frozen_string_literal: true
+
 require "securerandom"
 
 module PgOnlineSchemaChange
   class Orchestrate
-    SWAP_STATEMENT_TIMEOUT = "5s".freeze
+    SWAP_STATEMENT_TIMEOUT = "5s"
 
     extend Helper
 
@@ -70,7 +72,7 @@ def handle_signals!
         reader = setup_signals!
         signal = reader.gets.chomp
 
-        while !reader.closed? && IO.select([reader])
+        while !reader.closed? && IO.select([reader]) # rubocop:disable Lint/UnreachableLoop
           logger.info "Signal #{signal} received, cleaning up"
 
           client.connection.cancel
@@ -153,7 +155,7 @@ def disable_vacuum!
         # re-uses transaction with serializable
         # Disabling vacuum to avoid any issues during the process
         result = Query.storage_parameters_for(client, client.table, true) || ""
-        primary_table_storage_parameters = Store.set(:primary_table_storage_parameters, result)
+        Store.set(:primary_table_storage_parameters, result)
 
         logger.debug("Disabling vacuum on shadow and audit table",
                      { shadow_table: shadow_table, audit_table: audit_table })
@@ -185,8 +187,7 @@ def copy_data!
         # Begin the process to copy data into copy table
         # depending on the size of the table, this can be a time
         # taking operation.
-        logger.info("Clearing contents of audit table before copy..",
-                    { shadow_table: shadow_table, parent_table: client.table })
+        logger.info("Clearing contents of audit table before copy..", { shadow_table: shadow_table, parent_table: client.table })
         Query.run(client.connection, "DELETE FROM #{audit_table}", true)
 
         logger.info("Copying contents..", { shadow_table: shadow_table, parent_table: client.table })
@@ -272,7 +273,9 @@ def drop_and_cleanup!
         Query.run(client.connection, sql)
       end
 
-      private def random_string
+      private
+
+      def random_string
         @random_string ||= SecureRandom.hex(3)
       end
     end