Fix add_reference options validated on < 7.1

Option validation was [added][1] for 7.1+ Migration classes, and
a compatibility layer was added to ensure that previous Migration
versions do not have their options validated. However, the add_reference
method was missing in the compatibility layer which results in pre 7.1
Migrations validating options passed to add_reference.

This commit fixes the issue by adding add_reference to the compatibility
layer. In addition to adding add_reference to the "no validation"
compatibility test, the test is refactored to run against each previous
migration version to ensure that they all behave consistently.

[1]: e6da3eb

activerecord/CHANGELOG.md

@@ -1,3 +1,8 @@
+*   Fix Migrations with versions older than 7.1 validating options given to
+    `add_reference`.
+
+    *Hartley McGuire*
+
 *   Add `<role>_types` class method to `ActiveRecord::DelegatedType` so that the delegated types can be instrospected
 
     *JP Rosevear*

activerecord/lib/active_record/migration/compatibility.rb

@@ -63,8 +63,10 @@ def expression_column_name?(column_name)
               column_name.is_a?(String) && /\W/.match?(column_name)
             end
         end
+
         module TableDefinition
           include LegacyIndexName
+
           def column(name, type, **options)
             options[:_skip_validate_options] = true
             super
@@ -97,6 +99,12 @@ def add_index(table_name, column_name, **options)
           super
         end
 
+        def add_reference(table_name, ref_name, **options)
+          options[:_skip_validate_options] = true
+          super
+        end
+        alias :add_belongs_to :add_reference
+
         def create_table(table_name, **options)
           options[:_uses_legacy_table_name] = true
           options[:_skip_validate_options] = true

activerecord/test/cases/migration/compatibility_test.rb

@@ -255,33 +255,6 @@ def migrate(x)
         end
       end
 
-      def test_options_are_not_validated
-        migration = Class.new(ActiveRecord::Migration[4.2]) {
-          def migrate(x)
-            create_table :tests, wrong_id: false do |t|
-              t.references :some_table, wrong_primary_key: true
-              t.integer :some_id, wrong_unique: true
-              t.string :some_string_column, wrong_null: false
-            end
-
-            add_column :tests, "last_name", :string, wrong_precision: true
-
-            change_column :tests, :some_id, :float, wrong_index: true
-
-            change_table :tests do |t|
-              t.change :some_id, :float, null: false, wrong_index: true
-              t.integer :another_id, wrong_unique: true
-            end
-          end
-        }.new
-
-        ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
-
-        assert connection.table_exists?(:tests)
-      ensure
-        connection.drop_table :tests, if_exists: true
-      end
-
       def test_create_table_allows_duplicate_column_names
         migration = Class.new(ActiveRecord::Migration[5.2]) {
           def migrate(x)
@@ -681,6 +654,37 @@ def precision_implicit_default
   end
 end
 
+module NoOptionValidationTestCases
+  def test_options_are_not_validated
+    migration = Class.new(migration_class) {
+      def migrate(x)
+        create_table :tests, wrong_id: false do |t|
+          t.references :some_table, wrong_primary_key: true
+          t.integer :some_id, wrong_unique: true
+          t.string :some_string_column, wrong_null: false
+        end
+
+        add_column :tests, "last_name", :string, wrong_precision: true
+
+        change_column :tests, :some_id, :float, wrong_index: true
+
+        add_reference :tests, :another_table, invalid_option: :something
+
+        change_table :tests do |t|
+          t.change :some_id, :float, null: false, wrong_index: true
+          t.integer :another_id, wrong_unique: true
+        end
+      end
+    }.new
+
+    ActiveRecord::Migrator.new(:up, [migration], @schema_migration, @internal_metadata).migrate
+
+    assert connection.table_exists?(:tests)
+  ensure
+    connection.drop_table :tests, if_exists: true
+  end
+end
+
 module DefaultPrecisionImplicitTestCases
   def test_datetime_doesnt_set_precision_on_change_table
     create_migration = Class.new(migration_class) {
@@ -838,6 +842,7 @@ def teardown
 
 class CompatibilityTest7_0 < BaseCompatibilityTest
   include DefaultPrecisionSixTestCases
+  include NoOptionValidationTestCases
 
   private
     def migration_class
@@ -847,6 +852,7 @@ def migration_class
 
 class CompatibilityTest6_1 < BaseCompatibilityTest
   include DefaultPrecisionImplicitTestCases
+  include NoOptionValidationTestCases
 
   private
     def migration_class
@@ -856,6 +862,7 @@ def migration_class
 
 class CompatibilityTest6_0 < BaseCompatibilityTest
   include DefaultPrecisionImplicitTestCases
+  include NoOptionValidationTestCases
 
   private
     def migration_class
@@ -865,6 +872,7 @@ def migration_class
 
 class CompatibilityTest5_2 < BaseCompatibilityTest
   include DefaultPrecisionImplicitTestCases
+  include NoOptionValidationTestCases
 
   private
     def migration_class
@@ -874,6 +882,7 @@ def migration_class
 
 class CompatibilityTest5_1 < BaseCompatibilityTest
   include DefaultPrecisionImplicitTestCases
+  include NoOptionValidationTestCases
 
   private
     def migration_class
@@ -883,6 +892,7 @@ def migration_class
 
 class CompatibilityTest5_0 < BaseCompatibilityTest
   include DefaultPrecisionImplicitTestCases
+  include NoOptionValidationTestCases
 
   private
     def migration_class
@@ -892,6 +902,7 @@ def migration_class
 
 class CompatibilityTest4_2 < BaseCompatibilityTest
   include DefaultPrecisionImplicitTestCases
+  include NoOptionValidationTestCases
 
   private
     def migration_class