Merge pull request zdennis#794 from zdennis/update-rubocop

Correct Rubocop offenses

.rubocop.yml

@@ -66,10 +66,10 @@ Style/SymbolArray:
 Style/TrailingCommaInArrayLiteral:
   Enabled: false
 
-Layout/AlignArguments:
+Layout/ArgumentAlignment:
   Enabled: false
 
-Layout/AlignParameters:
+Layout/ParameterAlignment:
   EnforcedStyle: with_fixed_indentation
 
 Layout/EndAlignment:
@@ -81,7 +81,7 @@ Layout/ElseAlignment:
 Layout/EmptyLineAfterGuardClause:
   Enabled: false
 
-Layout/IndentHeredoc:
+Layout/HeredocIndentation:
   Enabled: false
 
 Layout/SpaceInsideParens:
@@ -102,7 +102,7 @@ Lint/PercentStringArray:
 Naming/HeredocDelimiterNaming:
   Enabled: false
 
-Naming/UncommunicativeMethodParamName:
+Naming/MethodParameterName:
   Enabled: false
 
 Security/YAMLLoad:

.rubocop_todo.yml

@@ -1,36 +1,26 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2016-03-17 18:14:55 -0700 using RuboCop version 0.38.0.
+# on 2023-02-15 00:58:14 UTC using RuboCop version 1.45.1.
 # 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
-Lint/HandleExceptions:
-  Exclude:
-    - 'lib/activerecord-import/base.rb'
-    - 'test/import_test.rb'
-
 # Offense count: 2
 Lint/RescueException:
   Exclude:
     - 'benchmarks/lib/cli_parser.rb'
     - 'test/import_test.rb'
 
-# Offense count: 4
-# Cop supports --auto-correct.
-# Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods.
+# Offense count: 3
+# This cop supports safe autocorrection (--autocorrect).
+# Configuration parameters: AllowUnusedKeywordArguments, IgnoreEmptyMethods, IgnoreNotImplementedMethods.
 Lint/UnusedMethodArgument:
   Exclude:
     - 'lib/activerecord-import/adapters/postgresql_adapter.rb'
     - 'lib/activerecord-import/import.rb'
 
 # Offense count: 2
-# Cop supports --auto-correct.
-# Configuration parameters: Keywords.
-# Keywords: TODO, FIXME, OPTIMIZE, HACK, REVIEW
-Style/CommentAnnotation:
+Style/CombinableLoops:
   Exclude:
-    - 'benchmarks/lib/cli_parser.rb'
-    - 'lib/activerecord-import/import.rb'
+    - 'test/support/shared_examples/recursive_import.rb'

Gemfile

@@ -15,7 +15,7 @@ pg_version = '0.9'
 pg_version = '1.1' if version >= 6.1
 
 group :development, :test do
-  gem 'rubocop', '~> 0.71.0'
+  gem 'rubocop'
   gem 'rake'
 end
 

benchmarks/benchmark.rb

@@ -41,7 +41,7 @@
 adapter_schema = File.join(benchmark_dir, "schema/#{options.adapter}_schema.rb")
 require adapter_schema if File.exist?(adapter_schema)
 
-Dir[File.dirname(__FILE__) + "/models/*.rb"].each { |file| require file }
+Dir["#{File.dirname(__FILE__)}/models/*.rb"].sort.each { |file| require file }
 
 require File.join( benchmark_dir, 'lib', "#{options.adapter}_benchmark" )
 
@@ -53,8 +53,8 @@
 end
 
 letter = options.adapter[0].chr
-clazz_str = letter.upcase + options.adapter[1..-1].downcase
-clazz = Object.const_get( clazz_str + "Benchmark" )
+clazz_str = letter.upcase + options.adapter[1..].downcase
+clazz = Object.const_get( "#{clazz_str}Benchmark" )
 
 benchmarks = []
 options.number_of_objects.each do |num|

benchmarks/lib/base.rb

@@ -27,11 +27,11 @@ def benchmark( table_types, num )
   # An OpenStruct object with the following attributes:
   #   * description - the description of the benchmark ran
   #   * tms - a Benchmark::Tms containing the results of the benchmark
-  def bm( description )
+  def bm( description, &block )
     tms = nil
     puts "Benchmarking #{description}"
 
-    Benchmark.bm { |x| tms = x.report { yield } }
+    Benchmark.bm { |x| tms = x.report(&block) }
     delete_all
     failed = false
 

benchmarks/lib/cli_parser.rb

@@ -29,7 +29,7 @@ def self.print_options( options )
     print_valid_table_types( options, prefix: "    " )
   end
 
-  # TODO IMPLEMENT THIS
+  # TODO: IMPLEMENT THIS
   def self.print_valid_table_types( options, hsh = { prefix: '' } )
     if !options.table_types.keys.empty?
       options.table_types.keys.sort.each { |type| puts hsh[:prefix].to_s + type.to_s }

lib/activerecord-import/adapters/abstract_adapter.rb

@@ -9,10 +9,11 @@ def next_value_for_sequence(sequence_name)
     def insert_many( sql, values, _options = {}, *args ) # :nodoc:
       number_of_inserts = 1
 
-      base_sql, post_sql = if sql.is_a?( String )
-        [sql, '']
-      elsif sql.is_a?( Array )
-        [sql.shift, sql.join( ' ' )]
+      base_sql, post_sql = case sql
+                           when String
+                             [sql, '']
+                           when Array
+                             [sql.shift, sql.join( ' ' )]
       end
 
       sql2insert = base_sql + values.join( ',' ) + post_sql

lib/activerecord-import/adapters/mysql_adapter.rb

@@ -13,10 +13,11 @@ def insert_many( sql, values, options = {}, *args ) # :nodoc:
     # the number of inserts default
     number_of_inserts = 0
 
-    base_sql, post_sql = if sql.is_a?( String )
-      [sql, '']
-    elsif sql.is_a?( Array )
-      [sql.shift, sql.join( ' ' )]
+    base_sql, post_sql = case sql
+                         when String
+                           [sql, '']
+                         when Array
+                           [sql.shift, sql.join( ' ' )]
     end
 
     sql_size = QUERY_OVERHEAD + base_sql.size + post_sql.size
@@ -33,7 +34,7 @@ def insert_many( sql, values, options = {}, *args ) # :nodoc:
     max = max_allowed_packet
 
     # if we can insert it all as one statement
-    if NO_MAX_PACKET == max || total_bytes <= max || options[:force_single_insert]
+    if max == NO_MAX_PACKET || total_bytes <= max || options[:force_single_insert]
       number_of_inserts += 1
       sql2insert = base_sql + values.join( ',' ) + post_sql
       insert( sql2insert, *args )
@@ -87,11 +88,12 @@ def sql_for_on_duplicate_key_update( table_name, *args ) # :nodoc:
     sql = ' ON DUPLICATE KEY UPDATE '.dup
     arg = args.first
     locking_column = args.last
-    if arg.is_a?( Array )
+    case arg
+    when Array
       sql << sql_for_on_duplicate_key_update_as_array( table_name, locking_column, arg )
-    elsif arg.is_a?( Hash )
+    when Hash
       sql << sql_for_on_duplicate_key_update_as_hash( table_name, locking_column, arg )
-    elsif arg.is_a?( String )
+    when String
       sql << arg
     else
       raise ArgumentError, "Expected Array or Hash"

lib/activerecord-import/adapters/postgresql_adapter.rb

@@ -12,10 +12,11 @@ def insert_many( sql, values, options = {}, *args ) # :nodoc:
     ids = []
     results = []
 
-    base_sql, post_sql = if sql.is_a?( String )
-      [sql, '']
-    elsif sql.is_a?( Array )
-      [sql.shift, sql.join( ' ' )]
+    base_sql, post_sql = case sql
+                         when String
+                           [sql, '']
+                         when Array
+                           [sql.shift, sql.join( ' ' )]
     end
 
     sql2insert = base_sql + values.join( ',' ) + post_sql
@@ -110,13 +111,14 @@ def returning_selections(options)
   # Add a column to be updated on duplicate key update
   def add_column_for_on_duplicate_key_update( column, options = {} ) # :nodoc:
     arg = options[:on_duplicate_key_update]
-    if arg.is_a?( Hash )
+    case arg
+    when Hash
       columns = arg.fetch( :columns ) { arg[:columns] = [] }
       case columns
       when Array then columns << column.to_sym unless columns.include?( column.to_sym )
       when Hash then columns[column.to_sym] = column.to_sym
       end
-    elsif arg.is_a?( Array )
+    when Array
       arg << column.to_sym unless arg.include?( column.to_sym )
     end
   end
@@ -151,11 +153,12 @@ def sql_for_on_duplicate_key_update( table_name, *args ) # :nodoc:
     end
 
     sql << "#{conflict_target}DO UPDATE SET "
-    if columns.is_a?( Array )
+    case columns
+    when Array
       sql << sql_for_on_duplicate_key_update_as_array( table_name, locking_column, columns )
-    elsif columns.is_a?( Hash )
+    when Hash
       sql << sql_for_on_duplicate_key_update_as_hash( table_name, locking_column, columns )
-    elsif columns.is_a?( String )
+    when String
       sql << columns
     else
       raise ArgumentError, 'Expected :columns to be an Array or Hash'
@@ -192,7 +195,7 @@ def sql_for_conflict_target( args = {} )
     if constraint_name.present?
       "ON CONSTRAINT #{constraint_name} "
     elsif conflict_target.present?
-      sql = '(' + Array( conflict_target ).reject( &:blank? ).join( ', ' ) + ') '
+      sql = "(#{Array( conflict_target ).reject( &:blank? ).join( ', ' )}) "
       sql += "WHERE #{index_predicate} " if index_predicate
       sql
     end

lib/activerecord-import/adapters/sqlite3_adapter.rb

@@ -24,10 +24,11 @@ def supports_on_duplicate_key_update?
   def insert_many( sql, values, _options = {}, *args ) # :nodoc:
     number_of_inserts = 0
 
-    base_sql, post_sql = if sql.is_a?( String )
-      [sql, '']
-    elsif sql.is_a?( Array )
-      [sql.shift, sql.join( ' ' )]
+    base_sql, post_sql = case sql
+                         when String
+                           [sql, '']
+                         when Array
+                           [sql.shift, sql.join( ' ' )]
     end
 
     value_sets = ::ActiveRecord::Import::ValueSetsRecordsParser.parse(values,
@@ -56,11 +57,9 @@ def pre_sql_statements( options )
   def post_sql_statements( table_name, options ) # :nodoc:
     sql = []
 
-    if supports_on_duplicate_key_update?
-      # Options :recursive and :on_duplicate_key_ignore are mutually exclusive
-      if (options[:ignore] || options[:on_duplicate_key_ignore]) && !options[:on_duplicate_key_update]
-        sql << sql_for_on_duplicate_key_ignore( options[:on_duplicate_key_ignore] )
-      end
+    # Options :recursive and :on_duplicate_key_ignore are mutually exclusive
+    if supports_on_duplicate_key_update? && ((options[:ignore] || options[:on_duplicate_key_ignore]) && !options[:on_duplicate_key_update])
+      sql << sql_for_on_duplicate_key_ignore( options[:on_duplicate_key_ignore] )
     end
 
     sql + super
@@ -73,13 +72,14 @@ def next_value_for_sequence(sequence_name)
   # Add a column to be updated on duplicate key update
   def add_column_for_on_duplicate_key_update( column, options = {} ) # :nodoc:
     arg = options[:on_duplicate_key_update]
-    if arg.is_a?( Hash )
+    case arg
+    when Hash
       columns = arg.fetch( :columns ) { arg[:columns] = [] }
       case columns
       when Array then columns << column.to_sym unless columns.include?( column.to_sym )
       when Hash then columns[column.to_sym] = column.to_sym
       end
-    elsif arg.is_a?( Array )
+    when Array
       arg << column.to_sym unless arg.include?( column.to_sym )
     end
   end
@@ -114,11 +114,12 @@ def sql_for_on_duplicate_key_update( table_name, *args ) # :nodoc:
     end
 
     sql << "#{conflict_target}DO UPDATE SET "
-    if columns.is_a?( Array )
+    case columns
+    when Array
       sql << sql_for_on_duplicate_key_update_as_array( table_name, locking_column, columns )
-    elsif columns.is_a?( Hash )
+    when Hash
       sql << sql_for_on_duplicate_key_update_as_hash( table_name, locking_column, columns )
-    elsif columns.is_a?( String )
+    when String
       sql << columns
     else
       raise ArgumentError, 'Expected :columns to be an Array or Hash'
@@ -152,7 +153,7 @@ def sql_for_conflict_target( args = {} )
     conflict_target = args[:conflict_target]
     index_predicate = args[:index_predicate]
     if conflict_target.present?
-      sql = '(' + Array( conflict_target ).reject( &:blank? ).join( ', ' ) + ') '
+      sql = "(#{Array( conflict_target ).reject( &:blank? ).join( ', ' )}) "
       sql += "WHERE #{index_predicate} " if index_predicate
       sql
     end