Skip to content

Commit

Permalink
Add new Rails/IndexNames cop
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
corsonknowles committed Oct 23, 2024
1 parent f66b8f7 commit 4b122b8
Show file tree
Hide file tree
Showing 6 changed files with 425 additions and 1 deletion.
1 change: 1 addition & 0 deletions changelog/new_merge_pull_request_1378_from.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1378](https://github.com/rubocop/rubocop-rails/pull/1378): Add new `Rails/IndexNames` cop. ([@corsonknowles][])
9 changes: 9 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion lib/rubocop/cop/mixin/migrations_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module MigrationsHelper
(send
(const (const {nil? cbase} :ActiveRecord) :Migration)
:[]
(float _))
(float $_))
_)
PATTERN

Expand All @@ -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
114 changes: 114 additions & 0 deletions lib/rubocop/cop/rails/index_names.rb
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Loading

0 comments on commit 4b122b8

Please sign in to comment.