From cf056ef3b3c50cfb80f924dd831f0cd5016d6abd Mon Sep 17 00:00:00 2001 From: Koichi ITO Date: Thu, 26 Mar 2020 19:03:52 +0900 Subject: [PATCH] [Fix #215] Fix a false positive for `Rails/UniqueValidationWithoutIndex` Fixes #215. This PR fixes a false positive for `Rails/UniqueValidationWithoutIndex` when using Expression Indexes for PostgreSQL. Since it is difficult to predict a kind of string described in Expression Indexes, this implementation judges based on whether or not an Expression Indexes include a column name. There is a possibility of false negative, but in most cases I expect it works. ## References - https://www.postgresql.org/docs/12/indexes-expressional.html - https://github.com/rails/rails/commit/edc2b7718725016e988089b5fb6d6fb9d6e16882 --- CHANGELOG.md | 1 + .../rails/unique_validation_without_index.rb | 12 +++++- .../unique_validation_without_index_spec.rb | 41 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) 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..7281455bfc 100644 --- a/lib/rubocop/cop/rails/unique_validation_without_index.rb +++ b/lib/rubocop/cop/rails/unique_validation_without_index.rb @@ -53,7 +53,17 @@ def with_index?(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