Invertable methods should have compatible method signature

Otherwise it is hard to invert non-kwargs method to kwargs method.

This makes table and column options to be synchronized as kwargs.

activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb

@@ -364,7 +364,6 @@ def [](name)
       def column(name, type, **options)
         name = name.to_s
         type = type.to_sym if type
-        options = options.dup
 
         if @columns_hash[name]
           if @columns_hash[name].primary_key?
@@ -550,7 +549,7 @@ def column(column_name, type, **options)
       #  t.string(:name) unless t.column_exists?(:name, :string)
       #
       # See {connection.column_exists?}[rdoc-ref:SchemaStatements#column_exists?]
-      def column_exists?(column_name, type = nil, options = {})
+      def column_exists?(column_name, type = nil, **options)
         @base.column_exists?(name, column_name, type, **options)
       end
 
@@ -591,8 +590,8 @@ def rename_index(index_name, new_index_name)
       #  t.timestamps(null: false)
       #
       # See {connection.add_timestamps}[rdoc-ref:SchemaStatements#add_timestamps]
-      def timestamps(options = {})
-        @base.add_timestamps(name, options)
+      def timestamps(**options)
+        @base.add_timestamps(name, **options)
       end
 
       # Changes the column's definition according to the new options.
@@ -622,8 +621,8 @@ def change_default(column_name, default_or_changes)
       #  t.remove(:qualification, :experience)
       #
       # See {connection.remove_columns}[rdoc-ref:SchemaStatements#remove_columns]
-      def remove(*column_names)
-        @base.remove_columns(name, *column_names)
+      def remove(*column_names, **options)
+        @base.remove_columns(name, *column_names, **options)
       end
 
       # Removes the given index from the table.
@@ -643,8 +642,8 @@ def remove_index(column_name = nil, options = {})
       #  t.remove_timestamps
       #
       # See {connection.remove_timestamps}[rdoc-ref:SchemaStatements#remove_timestamps]
-      def remove_timestamps(options = {})
-        @base.remove_timestamps(name, options)
+      def remove_timestamps(**options)
+        @base.remove_timestamps(name, **options)
       end
 
       # Renames a column.

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -392,7 +392,7 @@ def create_join_table(table_1, table_2, column_options: {}, **options)
       # Although this command ignores the block if one is given, it can be helpful
       # to provide one in a migration's +change+ method so it can be reverted.
       # In that case, the block will be used by #create_join_table.
-      def drop_join_table(table_1, table_2, options = {})
+      def drop_join_table(table_1, table_2, **options)
         join_table_name = find_join_table_name(table_1, table_2, options)
         drop_table(join_table_name)
       end
@@ -469,7 +469,7 @@ def drop_join_table(table_1, table_2, options = {})
       #  end
       #
       # See also Table for details on all of the various column transformations.
-      def change_table(table_name, options = {})
+      def change_table(table_name, **options)
         if supports_bulk_alter? && options[:bulk]
           recorder = ActiveRecord::Migration::CommandRecorder.new(self)
           yield update_table_definition(table_name, recorder)
@@ -499,7 +499,7 @@ def rename_table(table_name, new_name)
       # Although this command ignores most +options+ and the block if one is given,
       # it can be helpful to provide these in a migration's +change+ method so it can be reverted.
       # In that case, +options+ and the block will be used by #create_table.
-      def drop_table(table_name, options = {})
+      def drop_table(table_name, **options)
         schema_cache.clear_data_source_cache!(table_name.to_s)
         execute "DROP TABLE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(table_name)}"
       end
@@ -600,12 +600,11 @@ def add_column(table_name, column_name, type, **options)
       # +type+ and other column options can be passed to make migration reversible.
       #
       #    remove_columns(:suppliers, :qualification, :experience, type: :string, null: false)
-      def remove_columns(table_name, *column_names)
+      def remove_columns(table_name, *column_names, **options)
         raise ArgumentError.new("You must specify at least one column name. Example: remove_columns(:people, :first_name)") if column_names.empty?
-        column_options = column_names.extract_options!
-        type = column_options.delete(:type)
+        type = options.delete(:type)
         column_names.each do |column_name|
-          remove_column(table_name, column_name, type, column_options)
+          remove_column(table_name, column_name, type, **options)
         end
       end
 
@@ -617,7 +616,7 @@ def remove_columns(table_name, *column_names)
       # to provide these in a migration's +change+ method so it can be reverted.
       # In that case, +type+ and +options+ will be used by #add_column.
       # Indexes on the column are automatically removed.
-      def remove_column(table_name, column_name, type = nil, options = {})
+      def remove_column(table_name, column_name, type = nil, **options)
         execute "ALTER TABLE #{quote_table_name(table_name)} #{remove_column_for_alter(table_name, column_name, type, options)}"
       end
 
@@ -1156,7 +1155,7 @@ def columns_for_distinct(columns, orders) # :nodoc:
       #
       #   add_timestamps(:suppliers, null: true)
       #
-      def add_timestamps(table_name, options = {})
+      def add_timestamps(table_name, **options)
         options[:null] = false if options[:null].nil?
 
         if !options.key?(:precision) && supports_datetime_with_precision?
@@ -1171,7 +1170,7 @@ def add_timestamps(table_name, options = {})
       #
       #  remove_timestamps(:suppliers)
       #
-      def remove_timestamps(table_name, options = {})
+      def remove_timestamps(table_name, **options)
         remove_column table_name, :updated_at
         remove_column table_name, :created_at
       end

activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb

@@ -70,7 +70,7 @@ def indexes(table_name)
           end
         end
 
-        def remove_column(table_name, column_name, type = nil, options = {})
+        def remove_column(table_name, column_name, type = nil, **options)
           if foreign_key_exists?(table_name, column: column_name)
             remove_foreign_key(table_name, column: column_name)
           end

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -252,7 +252,7 @@ def add_column(table_name, column_name, type, **options) #:nodoc:
         end
       end
 
-      def remove_column(table_name, column_name, type = nil, options = {}) #:nodoc:
+      def remove_column(table_name, column_name, type = nil, **options) #:nodoc:
         alter_table(table_name) do |definition|
           definition.remove_column column_name
           definition.foreign_keys.delete_if do |_, fk_options|

activerecord/lib/active_record/migration.rb

@@ -635,6 +635,7 @@ def maintain_test_schema! #:nodoc:
       def method_missing(name, *args, &block) #:nodoc:
         nearest_delegate.send(name, *args, &block)
       end
+      ruby2_keywords(:method_missing) if respond_to?(:ruby2_keywords, true)
 
       def migrate(direction)
         new.migrate direction
@@ -903,14 +904,10 @@ def method_missing(method, *arguments, &block)
           end
         end
         return super unless connection.respond_to?(method)
-        options = arguments.extract_options!
-        if options.empty?
-          connection.send(method, *arguments, &block)
-        else
-          connection.send(method, *arguments, **options, &block)
-        end
+        connection.send(method, *arguments, &block)
       end
     end
+    ruby2_keywords(:method_missing) if respond_to?(:ruby2_keywords, true)
 
     def copy(destination, sources, options = {})
       copied = []

activerecord/lib/active_record/migration/command_recorder.rb

@@ -32,7 +32,8 @@ class Migration
     # * rename_index
     # * rename_table
     class CommandRecorder
-      ReversibleAndIrreversibleMethods = [:create_table, :create_join_table, :rename_table, :add_column, :remove_column,
+      ReversibleAndIrreversibleMethods = [
+        :create_table, :create_join_table, :rename_table, :add_column, :remove_column,
         :rename_index, :rename_column, :add_index, :remove_index, :add_timestamps, :remove_timestamps,
         :change_column_default, :add_reference, :remove_reference, :transaction,
         :drop_join_table, :drop_table, :execute_block, :enable_extension, :disable_extension,
@@ -113,11 +114,12 @@ def #{method}(*args, &block)          # def create_table(*args, &block)
             record(:"#{method}", args, &block)  #   record(:create_table, args, &block)
           end                                   # end
         EOV
+        ruby2_keywords(method) if respond_to?(:ruby2_keywords, true)
       end
       alias :add_belongs_to :add_reference
       alias :remove_belongs_to :remove_reference
 
-      def change_table(table_name, options = {}) # :nodoc:
+      def change_table(table_name, **options) # :nodoc:
         yield delegate.update_table_definition(table_name, self)
       end
 
@@ -289,6 +291,7 @@ def method_missing(method, *args, &block)
             super
           end
         end
+        ruby2_keywords(:method_missing) if respond_to?(:ruby2_keywords, true)
     end
   end
 end

activerecord/lib/active_record/relation/delegation.rb

@@ -109,7 +109,7 @@ def method_missing(method, *args, &block)
             super
           end
         end
-        ruby2_keywords :method_missing if respond_to?(:ruby2_keywords, true)
+        ruby2_keywords(:method_missing) if respond_to?(:ruby2_keywords, true)
     end
 
     module ClassMethods # :nodoc:

activerecord/test/cases/migration/command_recorder_test.rb

@@ -33,9 +33,13 @@ def create_table(name); end
       end
 
       def test_unknown_commands_delegate
-        recorder = Struct.new(:foo)
-        recorder = CommandRecorder.new(recorder.new("bar"))
-        assert_equal "bar", recorder.foo
+        recorder = Class.new do
+          def foo(kw:)
+            kw
+          end
+        end
+        recorder = CommandRecorder.new(recorder.new)
+        assert_equal "bar", recorder.foo(kw: "bar")
       end
 
       def test_inverse_of_raise_exception_on_unknown_commands
@@ -94,17 +98,10 @@ def test_invert_change_table
             t.rename :kind, :cultivar
           end
         end
-        if RUBY_VERSION < "2.7"
-          assert_equal [
-            [:rename_column, [:fruits, :cultivar, :kind]],
-            [:remove_column, [:fruits, :name, :string, {}], nil],
-          ], @recorder.commands
-        else
-          assert_equal [
-            [:rename_column, [:fruits, :cultivar, :kind]],
-            [:remove_column, [:fruits, :name, :string], nil],
-          ], @recorder.commands
-        end
+        assert_equal [
+          [:rename_column, [:fruits, :cultivar, :kind]],
+          [:remove_column, [:fruits, :name, :string, {}], nil],
+        ], @recorder.commands
 
         assert_raises(ActiveRecord::IrreversibleMigration) do
           @recorder.revert do