diff --git a/CHANGELOG.md b/CHANGELOG.md index 7370f15a29..b0e5459e7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/lib/rubocop/cop/rails/unique_validation_without_index.rb b/lib/rubocop/cop/rails/unique_validation_without_index.rb index 87543a56a0..2f1f79b5fd 100644 --- a/lib/rubocop/cop/rails/unique_validation_without_index.rb +++ b/lib/rubocop/cop/rails/unique_validation_without_index.rb @@ -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) @@ -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 diff --git a/spec/rubocop/cop/rails/unique_validation_without_index_spec.rb b/spec/rubocop/cop/rails/unique_validation_without_index_spec.rb index 4f57f2b003..68016c9248 100644 --- a/spec/rubocop/cop/rails/unique_validation_without_index_spec.rb +++ b/spec/rubocop/cop/rails/unique_validation_without_index_spec.rb @@ -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