Skip to content

Commit

Permalink
Merge pull request #217 from koic/fix_a_false_positive_for_unique_val…
Browse files Browse the repository at this point in the history
…idation_without_index

[Fix #215] Fix a false positive for `Rails/UniqueValidationWithoutIndex`
  • Loading branch information
koic authored Mar 27, 2020
2 parents e952888 + ac766e3 commit 789a8f2
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Bug fixes

* [#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][])

## 2.5.0 (2020-03-24)

Expand Down
15 changes: 12 additions & 3 deletions lib/rubocop/cop/rails/unique_validation_without_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class UniqueValidationWithoutIndex < Cop
def on_send(node)
return unless node.method?(:validates)
return unless uniqueness_part(node)
return if condition_part?(node)
return unless schema
return if with_index?(node)

Expand All @@ -47,13 +48,21 @@ def with_index?(node)
table = schema.table_by(name: table_name(klass))
return true unless table # Skip analysis if it can't find the table

return true if condition_part?(node)

names = column_names(node)
return true unless names

table.indices.any? do |index|
index.unique && index.columns.to_set == names
index.unique &&
(index.columns.to_set == names ||
include_column_names_in_expression_index?(index, names))
end
end

def include_column_names_in_expression_index?(index, column_names)
return false unless (expression_index = index.expression)

column_names.all? do |column_name|
expression_index.include?(column_name)
end
end

Expand Down
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 @@ -406,5 +406,46 @@ class User
RUBY
end
end

context 'with expression indexes' do
context 'when column name is included in expression index' do
let(:schema) { <<~RUBY }
ActiveRecord::Schema.define(version: 2020_02_02_075409) do
create_table 'emails', force: :cascade do |t|
t.string 'address', null: false
t.index 'lower(address)', name: 'index_emails_on_lower_address', unique: true
end
end
RUBY

it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
class Email < ApplicationRecord
validates :address, presence: true, uniqueness: { case_sensitive: false }, email: true
end
RUBY
end
end

context 'when column name is not included in expression index' do
let(:schema) { <<~RUBY }
ActiveRecord::Schema.define(version: 2020_02_02_075409) do
create_table 'emails', force: :cascade do |t|
t.string 'address', null: false
t.index 'lower(unexpected_column_name)', name: 'index_emails_on_lower_address', unique: true
end
end
RUBY

it 'registers an offense' do
expect_offense(<<~RUBY)
class Email < ApplicationRecord
validates :address, presence: true, uniqueness: { case_sensitive: false }, email: true
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Uniqueness validation should be with a unique index.
end
RUBY
end
end
end
end
end

0 comments on commit 789a8f2

Please sign in to comment.