Merge pull request rails#26930 from kamipo/index_name_exists_default

The `default` arg of `index_name_exists?` makes to optional

Author: rafaelfranca

activerecord/CHANGELOG.md

@@ -1,3 +1,7 @@
+*   Deprecate passing `default` to `index_name_exists?`.
+
+    *Ryuta Kamizono*
+
 *   PostgreSQL: schema dumping support for interval and OID columns.
 
     *Ryuta Kamizono*
@@ -12,7 +16,7 @@
 
     *namusyaka*
 
-*   Allow ActiveRecord::Base#as_json to be passed a frozen Hash.
+*   Allow `ActiveRecord::Base#as_json` to be passed a frozen Hash.
 
     *Isaac Betesh*
 
@@ -32,9 +36,9 @@
 
     *Ryuta Kamizono*
 
-*   Fix `association_primary_key_type` for reflections with symbol primary key
+*   Fix `association_primary_key_type` for reflections with symbol primary key.
 
-    Fixes #27864
+    Fixes #27864.
 
     *Daniel Colson*
 

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

@@ -773,11 +773,12 @@ def index_name(table_name, options) #:nodoc:
       end
 
       # Verifies the existence of an index with a given name.
-      #
-      # The default argument is returned if the underlying implementation does not define the indexes method,
-      # as there's no way to determine the correct answer in that case.
-      def index_name_exists?(table_name, index_name, default)
-        return default unless respond_to?(:indexes)
+      def index_name_exists?(table_name, index_name, default = nil)
+        unless default.nil?
+          ActiveSupport::Deprecation.warn(<<-MSG.squish)
+            Passing default to #index_name_exists? is deprecated without replacement.
+          MSG
+        end
         index_name = index_name.to_s
         indexes(table_name).detect { |i| i.name == index_name }
       end
@@ -1149,7 +1150,7 @@ def add_index_options(table_name, column_name, comment: nil, **options) # :nodoc
 
         validate_index_length!(table_name, index_name, options.fetch(:internal, false))
 
-        if data_source_exists?(table_name) && index_name_exists?(table_name, index_name, false)
+        if data_source_exists?(table_name) && index_name_exists?(table_name, index_name)
           raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' already exists"
         end
         index_columns = quoted_columns_for_index(column_names, options).join(", ")

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

@@ -132,7 +132,12 @@ def schema_exists?(name)
         end
 
         # Verifies existence of an index with a given name.
-        def index_name_exists?(table_name, index_name, default)
+        def index_name_exists?(table_name, index_name, default = nil)
+          unless default.nil?
+            ActiveSupport::Deprecation.warn(<<-MSG.squish)
+              Passing default to #index_name_exists? is deprecated without replacement.
+            MSG
+          end
           table = Utils.extract_schema_qualified_name(table_name.to_s)
           index = Utils.extract_schema_qualified_name(index_name.to_s)
 

activerecord/lib/active_record/migration/compatibility.rb

@@ -145,13 +145,13 @@ def remove_index(table_name, options = {})
           def index_name_for_remove(table_name, options = {})
             index_name = index_name(table_name, options)
 
-            unless index_name_exists?(table_name, index_name, true)
+            unless index_name_exists?(table_name, index_name)
               if options.is_a?(Hash) && options.has_key?(:name)
                 options_without_column = options.dup
                 options_without_column.delete :column
                 index_name_without_column = index_name(table_name, options_without_column)
 
-                return index_name_without_column if index_name_exists?(table_name, index_name_without_column, false)
+                return index_name_without_column if index_name_exists?(table_name, index_name_without_column)
               end
 
               raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' does not exist"

activerecord/test/cases/adapters/postgresql/schema_test.rb

@@ -301,13 +301,13 @@ def test_ignore_nil_schema_search_path
 
   def test_index_name_exists
     with_schema_search_path(SCHEMA_NAME) do
-      assert @connection.index_name_exists?(TABLE_NAME, INDEX_A_NAME, true)
-      assert @connection.index_name_exists?(TABLE_NAME, INDEX_B_NAME, true)
-      assert @connection.index_name_exists?(TABLE_NAME, INDEX_C_NAME, true)
-      assert @connection.index_name_exists?(TABLE_NAME, INDEX_D_NAME, true)
-      assert @connection.index_name_exists?(TABLE_NAME, INDEX_E_NAME, true)
-      assert @connection.index_name_exists?(TABLE_NAME, INDEX_E_NAME, true)
-      assert_not @connection.index_name_exists?(TABLE_NAME, "missing_index", true)
+      assert @connection.index_name_exists?(TABLE_NAME, INDEX_A_NAME)
+      assert @connection.index_name_exists?(TABLE_NAME, INDEX_B_NAME)
+      assert @connection.index_name_exists?(TABLE_NAME, INDEX_C_NAME)
+      assert @connection.index_name_exists?(TABLE_NAME, INDEX_D_NAME)
+      assert @connection.index_name_exists?(TABLE_NAME, INDEX_E_NAME)
+      assert @connection.index_name_exists?(TABLE_NAME, INDEX_E_NAME)
+      assert_not @connection.index_name_exists?(TABLE_NAME, "missing_index")
     end
   end
 

activerecord/test/cases/migration/index_test.rb

@@ -31,9 +31,10 @@ def test_rename_index
         connection.add_index(table_name, [:foo], name: "old_idx")
         connection.rename_index(table_name, "old_idx", "new_idx")
 
-        # if the adapter doesn't support the indexes call, pick defaults that let the test pass
-        assert_not connection.index_name_exists?(table_name, "old_idx", false)
-        assert connection.index_name_exists?(table_name, "new_idx", true)
+        assert_deprecated do
+          assert_not connection.index_name_exists?(table_name, "old_idx", false)
+          assert connection.index_name_exists?(table_name, "new_idx", true)
+        end
       end
 
       def test_rename_index_too_long
@@ -45,8 +46,7 @@ def test_rename_index_too_long
         }
         assert_match(/too long; the limit is #{connection.allowed_index_name_length} characters/, e.message)
 
-        # if the adapter doesn't support the indexes call, pick defaults that let the test pass
-        assert connection.index_name_exists?(table_name, "old_idx", false)
+        assert connection.index_name_exists?(table_name, "old_idx")
       end
 
       def test_double_add_index
@@ -63,7 +63,7 @@ def test_remove_nonexistent_index
       def test_add_index_works_with_long_index_names
         connection.add_index(table_name, "foo", name: good_index_name)
 
-        assert connection.index_name_exists?(table_name, good_index_name, false)
+        assert connection.index_name_exists?(table_name, good_index_name)
         connection.remove_index(table_name, name: good_index_name)
       end
 
@@ -75,15 +75,15 @@ def test_add_index_does_not_accept_too_long_index_names
         }
         assert_match(/too long; the limit is #{connection.allowed_index_name_length} characters/, e.message)
 
-        assert_not connection.index_name_exists?(table_name, too_long_index_name, false)
+        assert_not connection.index_name_exists?(table_name, too_long_index_name)
         connection.add_index(table_name, "foo", name: good_index_name)
       end
 
       def test_internal_index_with_name_matching_database_limit
         good_index_name = "x" * connection.index_name_length
         connection.add_index(table_name, "foo", name: good_index_name, internal: true)
 
-        assert connection.index_name_exists?(table_name, good_index_name, false)
+        assert connection.index_name_exists?(table_name, good_index_name)
         connection.remove_index(table_name, name: good_index_name)
       end