Add new Rails/IndexNames cop

This cop enforces the standard index naming provided by Rails, allowing developers to enjoy a standardized schema.

A bonus impact of following this convention is that fully duplicated indexes are prevented by default.

Custom index names were once a required workaround for default names that were too long for various database implementations. That could happen because of long table names, long field names, or long lists of compound index keys.

However, that problem has been fully resolved. The current automated naming approach will deterministically abbreviate indexes that would have been too long before.

changelog/new_merge_pull_request_1378_from.md

@@ -0,0 +1 @@
+* [#1378](https://github.com/rubocop/rubocop-rails/pull/1378): Add new `Rails/IndexNames` cop. ([@corsonknowles][])

config/default.yml

@@ -624,6 +624,15 @@ Rails/IndexBy:
   VersionAdded: '2.5'
   VersionChanged: '2.8'
 
+Rails/IndexNames:
+  Description: 'Checks for and removes custom index names. Since 7.1, Rails can ensure unique index names without exceeding the length limit.'
+  Enabled: pending
+  StartAfterMigrationVersion: null
+  VersionAdded: <<next>>
+  VersionChanged: <<next>>
+  Include:
+    - db/**/*.rb
+
 Rails/IndexWith:
   Description: 'Prefer `index_with` over `each_with_object`, `to_h`, or `map`.'
   Enabled: true

lib/rubocop/cop/mixin/migrations_helper.rb

@@ -12,7 +12,7 @@ module MigrationsHelper
           (send
             (const (const {nil? cbase} :ActiveRecord) :Migration)
             :[]
-            (float _))
+            (float $_))
           _)
       PATTERN
 
@@ -21,6 +21,14 @@ def in_migration?(node)
           migration_class?(class_node)
         end
       end
+
+      def in_supported_migration?(node, minimum_version)
+        node.each_ancestor(:class).any? do |class_node|
+          migration_class?(class_node) do |version|
+            version >= minimum_version
+          end
+        end
+      end
     end
   end
 end

lib/rubocop/cop/rails/index_names.rb

@@ -0,0 +1,114 @@
+# typed: false
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # Checks for custom index names in migrations.
+      #
+      # @example
+      #   # bad
+      #   class ExampleMigration < ActiveRecord::Migration[7.1]
+      #     def change
+      #       change_table :users do |t|
+      #         t.index [:email], name: 'index_custom_name'
+      #       end
+      #     end
+      #   end
+      #
+      #   # good
+      #   class ExampleMigration < ActiveRecord::Migration[7.1]
+      #     def change
+      #       create_table :users do |t|
+      #         t.index [:email]
+      #       end
+      #     end
+      #   end
+      class IndexNames < Base
+        include MigrationsHelper
+        include RangeHelp
+        extend AutoCorrector
+        extend TargetRailsVersion
+
+        minimum_target_rails_version 7.1
+
+        MSG = 'Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.'
+        RESTRICT_ON_SEND = %i[index add_index].freeze
+        # Keys that do not justify a duplicative index or a custom name, as they represent stricter column conditions.:
+        # unique, length, nulls_not_distinct
+        # Refer to: https://github.com/rails/rails/blob/b943622bdc746370ac860bfd3240cc0b8ca59d90/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb#L1477
+        VALID_REASONS_FOR_CUSTOM_NAME = Set.new(%i[where type using comment algorithm include opclass order]).freeze
+
+        def on_send(node)
+          return unless node.last_argument&.hash_type?
+          return unless in_supported_migration?(node, 7.1)
+          return unless (name_pair = find_name_pair(node))
+
+          return if node.last_argument.pairs.any? { |pair| VALID_REASONS_FOR_CUSTOM_NAME.include?(pair.key.value) }
+
+          add_offense(node) do |corrector|
+            remove_name_argument(corrector, name_pair, node)
+          end
+        end
+
+        private
+
+        def find_name_pair(node)
+          node.last_argument.pairs.find { |pair| pair.key.value == :name || pair.key.value == 'name' }
+        end
+
+        def remove_name_argument(corrector, name_pair, node)
+          range = name_argument_range(name_pair, node)
+          corrector.remove(range)
+          remove_extra_comma_and_space(corrector, range, node)
+        end
+
+        def name_argument_range(name_pair, node)
+          hash_node = name_pair.parent
+          if hash_node.pairs.size == 1
+            # If name: is the only argument, remove the entire hash
+            range_between(node.arguments[-2].source_range.end_pos, node.source_range.end_pos)
+          else
+            # Remove the name: argument and any preceding comma and space
+            start_pos = previous_comma_pos(name_pair) || name_pair.source_range.begin_pos
+            range_between(start_pos, name_pair.source_range.end_pos)
+          end
+        end
+
+        def previous_comma_pos(pair)
+          source = pair.parent.source
+          pair_start = pair.source_range.begin_pos - pair.parent.source_range.begin_pos
+          previous_content = source[0...pair_start]
+          comma_index = previous_content.rindex(',')
+          comma_index ? pair.parent.source_range.begin_pos + comma_index : nil
+        end
+
+        def remove_extra_comma_and_space(corrector, removed_range, node)
+          remaining_source = remaining_source(node, removed_range)
+          next_relevant_content = remaining_source.lstrip
+          range_to_remove = if next_relevant_content.start_with?(',')
+                              space_after_comma(removed_range, next_relevant_content)
+                            else
+                              leading_space(remaining_source, removed_range)
+                            end
+
+          corrector.remove(range_to_remove)
+        end
+
+        def remaining_source(node, removed_range)
+          node.source[removed_range.end_pos - node.source_range.begin_pos..]
+        end
+
+        def space_after_comma(removed_range, next_relevant_content)
+          space_after_comma = next_relevant_content[1..].match(/\A\s*/)[0]
+          range_between(removed_range.end_pos, removed_range.end_pos + 1 + space_after_comma.length)
+        end
+
+        def leading_space(remaining_source, removed_range)
+          leading_space = remaining_source.match(/\A\s*/)[0]
+          range_between(removed_range.end_pos, removed_range.end_pos + leading_space.length)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -69,6 +69,7 @@
 require_relative 'rails/ignored_columns_assignment'
 require_relative 'rails/ignored_skip_action_filter_option'
 require_relative 'rails/index_by'
+require_relative 'rails/index_names'
 require_relative 'rails/index_with'
 require_relative 'rails/inquiry'
 require_relative 'rails/inverse_of'

spec/rubocop/cop/rails/index_names_spec.rb

@@ -0,0 +1,291 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::IndexNames, :config do
+  context 'Rails 7.0', :rails70 do
+    context 'when t.index has a custom name argument' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.0]
+            def change
+              change_table :users do |t|
+                t.index [:email], name: 'index_custom_name'
+              end
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when add_index has a name argument' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.0]
+            def change
+              add_index :table, :column, name: 'index_custom_name'
+            end
+          end
+        RUBY
+      end
+    end
+  end
+
+  context 'Rails 7.2', :rails72 do
+    context 'when using an earlier migration version' do
+      context 'when t.index has a custom name argument' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            class ExampleMigration < ActiveRecord::Migration[7.0]
+              def change
+                change_table :users do |t|
+                  t.index [:email], name: 'index_custom_name'
+                end
+              end
+            end
+          RUBY
+        end
+      end
+
+      context 'when add_index has a name argument' do
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            class ExampleMigration < ActiveRecord::Migration[7.0]
+              def change
+                add_index :table, :column, name: 'index_custom_name'
+              end
+            end
+          RUBY
+        end
+      end
+    end
+
+    context 'when t.index has a custom name argument' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              change_table :users do |t|
+                t.index [:email], name: 'index_custom_name'
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.
+              end
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when t.index has no name argument' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email]
+              end
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when add_index has no name argument' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              add_index :table, :column
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when add_index is outside of a migration' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          def change
+            add_index :table, :column, name: 'index_custom_name'
+          end
+        RUBY
+      end
+    end
+
+    context 'when t.index is outside of a migration' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          def change
+            create_table :users do |t|
+              t.index [:email], name: 'index_custom_name'
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when unrelated index method has a name argument' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          index :email, name: 'index_custom_name'
+        RUBY
+      end
+    end
+
+    context 'when t.index has a custom name old style hash argument' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], 'name' => 'index_custom_name'
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.
+              end
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when t.index has multiple arguments and custom name' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], unique: true, name: 'index_custom_name'
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.
+              end
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when t.index has a reason for a custom name and possibly distinct index on the same keys' do
+      it 'does not register an offense' do
+        expect_no_offenses(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], include: :tags, name: 'index_custom_name'
+              end
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when correcting an offense' do
+      it 'removes the custom name argument without removing following arguments' do
+        expect_offense(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], name: 'index_custom_name', unique: true
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.
+              end
+            end
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], unique: true
+              end
+            end
+          end
+        RUBY
+      end
+
+      it 'removes only the name argument when there are other arguments and starts at name:' do
+        expect_offense(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], unique: true, name: 'index_custom_name'
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.
+              end
+            end
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], unique: true
+              end
+            end
+          end
+        RUBY
+      end
+
+      it 'removes the name argument when it is the only keyword argument' do
+        expect_offense(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], name: 'index_custom_name'
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.
+              end
+            end
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email]
+              end
+            end
+          end
+        RUBY
+      end
+
+      it 'removes only the name => argument for classic hash style' do
+        expect_offense(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email], "name" => 'index_custom_name'
+                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.
+              end
+            end
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              create_table :users do |t|
+                t.index [:email]
+              end
+            end
+          end
+        RUBY
+      end
+    end
+
+    context 'when add_index has a name argument' do
+      it 'registers an offense' do
+        expect_offense(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              add_index :table, :column, name: 'index_custom_name'
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Avoid specifying a custom name for common indexes. Let Rails handle the index name automatically.
+            end
+          end
+        RUBY
+
+        expect_correction(<<~RUBY)
+          class ExampleMigration < ActiveRecord::Migration[7.2]
+            def change
+              add_index :table, :column
+            end
+          end
+        RUBY
+      end
+    end
+  end
+end