Add new `Rails/IndexNames` cop

[corsonknowles]

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.

Background:

• Article: https://www.shakacode.com/blog/rails-7-1-limits-max-length-of-auto-generated-index-names/
• Feature PR: https://github.com/rails/rails/pull/47753

---

Before submitting the PR make sure the following are checked:

• [x] The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
• [x] Wrote good commit messages.
• [x] Commit message starts with [Fix #issue-number] (if the related issue exists).
• [x] Feature branch is up-to-date with master (if not - rebase it).
• [x] Squashed related commits together.
• [x] Added tests.
• [x] Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
• [x] Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.
• [ ] If this is a new cop, consider making a corresponding update to the Rails Style Guide.

[Earlopain]

Hi, thanks for the PR! I don't think I've ever bothered with manually naming my indicies but can you just generalise this?

Generally, I would say let users name their indicies as they wish. Indicies don't have to be only over a column, they can have query constraints, modify the column (lower(column_name)), use different index strategies (gin, gist, etc.). Per the PR you linked, it doesn't look like rails will respect any of these options in the index name.

---

I have some general feedback for your PR. I would wait until more feedback until adressing this:

• This cop should not run over the whole codebase. Limit it only to migrations, same as ThreeStateBooleanColumn and similar cops.
• If only in Rails 7.1 the index is safe to create, this should not run over migrations generated with older rails.
• It might be a good idea to add config StartAfter. It's a bit awkward right now, there are a few PRs wanting to do this but none have been merged so there is no common module yet. https://github.com/rubocop/rubocop-rails/pull/335 https://github.com/rubocop/rubocop-rails/pull/277 https://github.com/rubocop/rubocop-rails/pull/335
• You can also add indicies with add_index, these should be an offense as well.
• adoc is automatically generated. Please drop these changes.

[corsonknowles]

Thanks for the great comments @Earlopain with pointers as well!

All addressed, and I added specs to confirm no offenses both for earlier Rails versions and earlier Migrations versions.

[Earlopain]

That was quick, cool. One additional thing I forgot about: autocorrect will be unsafe since it changes index names which may be referenced later. It might be best to just not support autocorrect. (also remove_index exists).

In particular I'm aware of this pattern that good_job does, where it first checks if a index exists before creating it. I'm not entirely sure why, seems to only matter when a migraiton is run more than once in the same direction, but it's pobably valid in some way: https://github.com/bensheldon/good_job/blob/956e872d321df21738c599aa1a86a8514e05a7ef/demo/db/migrate/20240518175058_create_good_job_process_lock_indexes.rb

I'd also see this as one counter-argument for introducing such a cop. I would be pretty uncomfortable referecing the auto-generated index name without being explicit about it somewhere.

[corsonknowles]

Okay, @Earlopain, much to my surprise it does seem rather possible to generalize and accommodate valid reasons for a custom name that could lead to duplication of indexes with the same column names but different handling or properties (e.g. fulltext as another example where you may want both a fulltext and non fulltext index on the same columns).

I've added an allowlist, and enumerated in a comment the current keys that I think don't justify a custom name as well, for scrutiny.

Comments and improvements welcome!

[corsonknowles]

Thanks @Earlopain, I'm going to mark this unsafe and come up with some relevant description. It's never safe to edit checked in or run migrations. I wonder if I can explain that general principle concisely and with enough detail or perhaps I can find a Rails doc link that's relevant.

Update: Found the doc: https://guides.rubyonrails.org/active_record_migrations.html#changing-existing-migrations