Raise an exception when using unrecognized options in change_table block

In a database migration, the expressions `add_column`, `remove_index`,
etc. accept as keyword options `if_exists: true`/`if_not_exists: true`
which will skip that table alteration if the column or index does or
does not already exist.

This might lead some to think that within a change_table block,
```
change_table(:table) do |t|
	t.column :new_column, if_not_exists: true
	t.remove_index :old_column, if_exists: true
end
```
also works, but it doesn't. Or rather, it is silently ignored when
change_table is called with `bulk: true`, and it works accidentally
otherwise.

This commit raises an exception when these options are used in a
change_table block, which suggests the similar syntax:
`t.column :new_column unless t.column_exists?(:new_column)`. This
suggestion is already made in the documentation to
`ActiveRecord::ConnectionAdapters::Table`.
https://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/Table.html#method-i-column_exists-3F

Do not raise these new exceptions on migrations before 7.0

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

@@ -622,6 +622,7 @@ module ActiveRecord
       #
       # See TableDefinition#column for details of the options you can use.
       def column(column_name, type, index: nil, **options)
+        raise_on_if_exist_options(options)
         @base.add_column(name, column_name, type, **options)
         if index
           index_options = index.is_a?(Hash) ? index : {}
@@ -647,6 +648,7 @@ module ActiveRecord
       #
       # See {connection.add_index}[rdoc-ref:SchemaStatements#add_index] for details of the options you can use.
       def index(column_name, **options)
+        raise_on_if_exist_options(options)
         @base.add_index(name, column_name, **options)
       end
 
@@ -657,8 +659,8 @@ module ActiveRecord
       #  end
       #
       # See {connection.index_exists?}[rdoc-ref:SchemaStatements#index_exists?]
-      def index_exists?(column_name, options = {})
+      def index_exists?(column_name, **options)
-        @base.index_exists?(name, column_name, options)
+        @base.index_exists?(name, column_name, **options)
       end
 
       # Renames the given index on the table.
@@ -676,6 +678,7 @@ module ActiveRecord
       #
       # See {connection.add_timestamps}[rdoc-ref:SchemaStatements#add_timestamps]
       def timestamps(**options)
+        raise_on_if_exist_options(options)
         @base.add_timestamps(name, **options)
       end
 
@@ -686,6 +689,7 @@ module ActiveRecord
       #
       # See TableDefinition#column for details of the options you can use.
       def change(column_name, type, **options)
+        raise_on_if_exist_options(options)
         @base.change_column(name, column_name, type, **options)
       end
 
@@ -717,6 +721,7 @@ module ActiveRecord
       #
       # See {connection.remove_columns}[rdoc-ref:SchemaStatements#remove_columns]
       def remove(*column_names, **options)
+        raise_on_if_exist_options(options)
         @base.remove_columns(name, *column_names, **options)
       end
 
@@ -729,6 +734,7 @@ module ActiveRecord
       #
       # See {connection.remove_index}[rdoc-ref:SchemaStatements#remove_index]
       def remove_index(column_name = nil, **options)
+        raise_on_if_exist_options(options)
         @base.remove_index(name, column_name, **options)
       end
 
@@ -757,6 +763,7 @@ module ActiveRecord
       #
       # See {connection.add_reference}[rdoc-ref:SchemaStatements#add_reference] for details of the options you can use.
       def references(*args, **options)
+        raise_on_if_exist_options(options)
         args.each do |ref_name|
           @base.add_reference(name, ref_name, **options)
         end
@@ -770,6 +777,7 @@ module ActiveRecord
       #
       # See {connection.remove_reference}[rdoc-ref:SchemaStatements#remove_reference]
       def remove_references(*args, **options)
+        raise_on_if_exist_options(options)
         args.each do |ref_name|
           @base.remove_reference(name, ref_name, **options)
         end
@@ -783,6 +791,7 @@ module ActiveRecord
       #
       # See {connection.add_foreign_key}[rdoc-ref:SchemaStatements#add_foreign_key]
       def foreign_key(*args, **options)
+        raise_on_if_exist_options(options)
         @base.add_foreign_key(name, *args, **options)
       end
 
@@ -793,6 +802,7 @@ module ActiveRecord
       #
       # See {connection.remove_foreign_key}[rdoc-ref:SchemaStatements#remove_foreign_key]
       def remove_foreign_key(*args, **options)
+        raise_on_if_exist_options(options)
         @base.remove_foreign_key(name, *args, **options)
       end
 
@@ -822,6 +832,22 @@ module ActiveRecord
       def remove_check_constraint(*args)
         @base.remove_check_constraint(name, *args)
       end
+
+      private
+        def raise_on_if_exist_options(options)
+          unrecognized_option = options.keys.find do |key|
+            key == :if_exists || key == :if_not_exists
+          end
+          if unrecognized_option
+            conditional = unrecognized_option == :if_exists ? "if" : "unless"
+            message = <<~TXT
+              Option #{unrecognized_option} will be ignored. If you are calling an expression like
+              `t.column(.., #{unrecognized_option}: true)` from inside a change_table block, try a
+              conditional clause instead, as in `t.column(..) #{conditional} t.column_exists?(..)`
+            TXT
+            raise ArgumentError.new(message)
+          end
+        end
     end
   end
 end

activerecord/lib/active_record/migration/compatibility.rb

@@ -67,6 +67,14 @@ module ActiveRecord
           end
         end
 
+        def change_table(table_name, **options)
+          if block_given?
+            super { |t| yield compatible_table_definition(t) }
+          else
+            super
+          end
+        end
+
         module TableDefinition
           def new_column_definition(name, type, **options)
             type = PostgreSQLCompat.compatible_timestamp_type(type, @conn)
@@ -77,6 +85,10 @@ module ActiveRecord
             options[:precision] ||= nil
             super
           end
+
+          private
+            def raise_on_if_exist_options(options)
+            end
         end
 
         private

activerecord/test/cases/invertible_migration_test.rb

@@ -96,7 +96,7 @@ module ActiveRecord
       def change
         change_table("horses") do |t|
           t.remove_index [:name, :color]
-          t.remove_index [:color], if_exists: true
+          t.remove_index [:color] if t.index_exists?(:color)
         end
       end
     end

activerecord/test/cases/migration/change_table_test.rb

@@ -188,7 +188,7 @@ module ActiveRecord
 
       def test_index_exists
         with_change_table do |t|
-          @connection.expect :index_exists?, nil, [:delete_me, :bar, {}]
+          @connection.expect :index_exists?, nil, [:delete_me, :bar]
           t.index_exists?(:bar)
         end
       end
@@ -289,6 +289,22 @@ module ActiveRecord
           t.remove_check_constraint name: "price_check"
         end
       end
+
+      def test_remove_column_with_if_exists_raises_error
+        assert_raises(ArgumentError) do
+          with_change_table do |t|
+            t.remove :name, if_exists: true
+          end
+        end
+      end
+
+      def test_add_column_with_if_not_exists_raises_error
+        assert_raises(ArgumentError) do
+          with_change_table do |t|
+            t.string :nickname, if_not_exists: true
+          end
+        end
+      end
     end
   end
 end

activerecord/test/cases/migration/compatibility_test.rb

@@ -519,6 +519,20 @@ module ActiveRecord
         assert connection.column_exists?(:testings, :published_at, **precision_implicit_default)
       end
 
+      def test_change_table_allows_if_exists_option_on_6_1
+        migration = Class.new(ActiveRecord::Migration[6.1]) {
+          def migrate(x)
+            change_table(:testings) do |t|
+              t.remove :foo, if_exists: true
+            end
+          end
+        }.new
+
+        ActiveRecord::Migrator.new(:up, [migration], @schema_migration).migrate
+
+        assert_not connection.column_exists?(:testings, :foo)
+      end
+
       private
         def precision_implicit_default
           if current_adapter?(:Mysql2Adapter)