Make drop_table reversible [#8267]

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

@@ -305,6 +305,10 @@ module ActiveRecord
       end
 
       # Drops a table from the database.
+      #
+      # Although this command ignores +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 = {})
         execute "DROP TABLE #{quote_table_name(table_name)}"
       end

activerecord/lib/active_record/migration/command_recorder.rb

@@ -73,7 +73,7 @@ module ActiveRecord
       [:create_table, :create_join_table, :change_table, :rename_table, :add_column, :remove_column,
         :rename_index, :rename_column, :add_index, :remove_index, :add_timestamps, :remove_timestamps,
         :change_column, :change_column_default, :add_reference, :remove_reference, :transaction,
-        :drop_join_table,
+        :drop_join_table, :drop_table
       ].each do |method|
         class_eval <<-EOV, __FILE__, __LINE__ + 1
           def #{method}(*args, &block)          # def create_table(*args, &block)
@@ -90,8 +90,15 @@ module ActiveRecord
         [:transaction, args, block]
       end
 
-      def invert_create_table(args)
-        [:drop_table, [args.first]]
+      def invert_create_table(args, &block)
+        [:drop_table, args, block]
+      end
+
+      def invert_drop_table(args, &block)
+        if args.size == 1 && block == nil
+          raise ActiveRecord::IrreversibleMigration, "To avoid mistakes, drop_table is only reversible if given options or a block (can be empty)."
+        end
+        [:create_table, args, block]
       end
 
       def invert_create_join_table(args, &block)

activerecord/test/cases/invertible_migration_test.rb

@@ -186,10 +186,10 @@ module ActiveRecord
           create_table("grapes")
         end
       end
-      assert_equal [[:create_table, ["apples"], block], [:drop_table, ["elderberries"]],
+      assert_equal [[:create_table, ["apples"], block], [:drop_table, ["elderberries"], nil],
                     [:create_table, ["clementines"], nil], [:create_table, ["dates"], nil],
-                    [:drop_table, ["bananas"]], [:drop_table, ["grapes"]],
-                    [:drop_table, ["figs"]]], recorder.commands
+                    [:drop_table, ["bananas"], block], [:drop_table, ["grapes"], nil],
+                    [:drop_table, ["figs"], nil]], recorder.commands
     end
 
     def test_legacy_up

activerecord/test/cases/migration/command_recorder_test.rb

@@ -50,7 +50,7 @@ module ActiveRecord
           @recorder.record :create_table, [:hello]
           @recorder.record :create_table, [:world]
         end
-        tables = @recorder.commands.map(&:last)
+        tables = @recorder.commands.map{|_cmd, args, _block| args}
         assert_equal [[:world], [:hello]], tables
       end
 
@@ -61,20 +61,20 @@ module ActiveRecord
           revert do
             create_table("bananas", &block)
             revert do
-              create_table("clementines")
+              create_table("clementines", &block)
               create_table("dates")
             end
             create_table("elderberries")
           end
           revert do
-            create_table("figs")
+            create_table("figs", &block)
             create_table("grapes")
           end
         end
-        assert_equal [[:create_table, ["apples"], block], [:drop_table, ["elderberries"]],
-                      [:create_table, ["clementines"], nil], [:create_table, ["dates"], nil],
-                      [:drop_table, ["bananas"]], [:drop_table, ["grapes"]],
-                      [:drop_table, ["figs"]]], @recorder.commands
+        assert_equal [[:create_table, ["apples"], block], [:drop_table, ["elderberries"], nil],
+                      [:create_table, ["clementines"], block], [:create_table, ["dates"], nil],
+                      [:drop_table, ["bananas"], block], [:drop_table, ["grapes"], nil],
+                      [:drop_table, ["figs"], block]], @recorder.commands
       end
 
 
@@ -83,12 +83,25 @@ module ActiveRecord
           @recorder.record :create_table, [:system_settings]
         end
         drop_table = @recorder.commands.first
-        assert_equal [:drop_table, [:system_settings]], drop_table
+        assert_equal [:drop_table, [:system_settings], nil], drop_table
       end
 
-      def test_invert_create_table_with_options
-        drop_table = @recorder.inverse_of :create_table, [:people_reminders, id: false]
-        assert_equal [:drop_table, [:people_reminders]], drop_table
+      def test_invert_create_table_with_options_and_block
+        block = Proc.new{}
+        drop_table = @recorder.inverse_of :create_table, [:people_reminders, id: false], &block
+        assert_equal [:drop_table, [:people_reminders, id: false], block], drop_table
+      end
+
+      def test_invert_drop_table
+        block = Proc.new{}
+        create_table = @recorder.inverse_of :drop_table, [:people_reminders, id: false], &block
+        assert_equal [:create_table, [:people_reminders, id: false], block], create_table
+      end
+
+      def test_invert_drop_table_without_a_block_nor_option
+        assert_raises(ActiveRecord::IrreversibleMigration) do
+          @recorder.inverse_of :drop_table, [:people_reminders]
+        end
       end
 
       def test_invert_create_join_table