Skip to content

Commit

Permalink
[Fix rubocop#221] Make Rails/UniqueValidationWithoutIndex aware of …
Browse files Browse the repository at this point in the history
…`add_index`

Fixes rubocop#221.

This PR makes `Rails/UniqueValidationWithoutIndex` aware of `add_index`
in db/schema.rb.

Rails 4.0 and Rails 4.1 support will drop shortly, but Rails 4.2
may be a bit ahead. There may be changes to these support after
the bug fix release of RoboCop Rails 2.5 series.
  • Loading branch information
koic committed Mar 28, 2020
1 parent 789a8f2 commit 41c6eaa
Show file tree
Hide file tree
Showing 5 changed files with 108 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

* [#213](https://github.com/rubocop-hq/rubocop-rails/pull/213): Fix a false positive for `Rails/UniqueValidationWithoutIndex` when using conditions. ([@sunny][])
* [#215](https://github.com/rubocop-hq/rubocop-rails/issues/215): Fix a false positive for `Rails/UniqueValidationWithoutIndex` when using Expression Indexes. ([@koic][])
* [#221](https://github.com/rubocop-hq/rubocop-rails/issues/221): Make `Rails/UniqueValidationWithoutIndex` aware of `add_index` in db/schema.rb. ([@koic][])

## 2.5.0 (2020-03-24)

Expand Down
5 changes: 4 additions & 1 deletion lib/rubocop/cop/rails/unique_validation_without_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,10 @@ def with_index?(node)
names = column_names(node)
return true unless names

table.indices.any? do |index|
# Compatibility for Rails 4.2.
add_indicies = schema.add_indicies_by(table_name: table_name(klass))

(table.indices + add_indicies).any? do |index|
index.unique &&
(index.columns.to_set == names ||
include_column_names_in_expression_index?(index, names))
Expand Down
48 changes: 40 additions & 8 deletions lib/rubocop/rails/schema_loader/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ module Rails
module SchemaLoader
# Represent db/schema.rb
class Schema
attr_reader :tables
attr_reader :tables, :add_indicies

def initialize(ast)
@tables = []
@add_indicies = []

build!(ast)
end

Expand All @@ -18,6 +20,12 @@ def table_by(name:)
end
end

def add_indicies_by(table_name:)
add_indicies.select do |add_index|
add_index.table_name == table_name
end
end

private

def build!(ast)
Expand All @@ -26,6 +34,11 @@ def build!(ast)
each_table(ast) do |table_def|
@tables << Table.new(table_def)
end

# Compatibility for Rails 4.2.
each_add_index(ast) do |add_index_def|
@add_indicies << AddIndex.new(add_index_def)
end
end

def each_table(ast)
Expand All @@ -40,6 +53,14 @@ def each_table(ast)
yield ast.body
end
end

def each_add_index(ast)
ast.body.children.each do |node|
next unless node.send_type? && node.method?(:add_index)

yield(node)
end
end
end

# Represent a table
Expand Down Expand Up @@ -121,21 +142,19 @@ class Index
attr_reader :name, :columns, :expression, :unique

def initialize(node)
node.first_argument
@columns, @expression = build_columns_or_expr(node)
@columns, @expression = build_columns_or_expr(node.first_argument)
@unique = nil

analyze_keywords!(node)
end

private

def build_columns_or_expr(node)
arg = node.first_argument
if arg.array_type?
[arg.values.map(&:value), nil]
def build_columns_or_expr(columns)
if columns.array_type?
[columns.values.map(&:value), nil]
else
[[], arg.value]
[[], columns.value]
end
end

Expand All @@ -153,6 +172,19 @@ def analyze_keywords!(node)
end
end
end

# Represent an `add_index`
class AddIndex < Index
attr_reader :table_name

def initialize(node)
@table_name = node.first_argument.str_content
@columns, @expression = build_columns_or_expr(node.arguments[1])
@unique = nil

analyze_keywords!(node)
end
end
end
end
end
41 changes: 41 additions & 0 deletions spec/rubocop/cop/rails/unique_validation_without_index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -447,5 +447,46 @@ class Email < ApplicationRecord
end
end
end

context 'when db/schema.rb has been dumped using `add_index` for index' do
context 'when the table does not have any indices' do
let(:schema) { <<~RUBY }
ActiveRecord::Schema.define(version: 2020_02_02_075409) do
create_table "users", force: :cascade do |t|
t.string "account", null: false
end
add_index "users", "account", name: "index_users_on_account"
end
RUBY

it 'registers an offense' do
expect_offense(<<~RUBY)
class User
validates :account, uniqueness: true
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Uniqueness validation should be with a unique index.
end
RUBY
end
end

context 'with a unique index' do
let(:schema) { <<~RUBY }
ActiveRecord::Schema.define(version: 2020_02_02_075409) do
create_table "users", force: :cascade do |t|
t.string "account", null: false
end
add_index "users", ["account"], name: "index_users_on_account", unique: true
end
RUBY

it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class User
validates :account, uniqueness: true
end
RUBY
end
end
end
end
end
22 changes: 22 additions & 0 deletions spec/rubocop/rails/schema_loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,28 @@
expect(table.indices.first.name).to eq 'index_title_lower_unique'
expect(table.indices.first.unique).to be true
end

context 'when an index in users table specified by `add_index`' do
let(:schema) { <<~RUBY }
ActiveRecord::Schema.define(version: 2020_02_02_075409) do
create_table "users", force: :cascade do |t|
t.string "account", null: false
end
add_index "users", ["account"], name: "index_users_on_account", unique: true
add_index "users", ["email"], name: "index_users_on_email", unique: true
add_index "books", ["isbn"], name: "index_books_on_isbn", unique: true
end
RUBY

it 'has an `add_index` for users table' do
add_indicies = loaded_schema.add_indicies_by(table_name: 'users')
expect(add_indicies.size).to eq 2
expect(add_indicies.first.name).to eq 'index_users_on_account'
expect(add_indicies.first.table_name).to eq 'users'
expect(add_indicies.first.columns).to eq ['account']
expect(add_indicies.first.unique).to be true
end
end
end

context 'when the current directory is Rails.root' do
Expand Down

0 comments on commit 41c6eaa

Please sign in to comment.