Merge pull request #213 from sunny/no-offense-for-unique-validations-…

…with-conditions

Fix false positive for `UniqueValidationWithoutIndex` when using conditions

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### Bug fixes
+
+* Fix a false positive for `Rails/UniqueValidationWithoutIndex` when using conditions. ([@sunny][])
+
 ## 2.5.0 (2020-03-24)
 
 ### New features
@@ -149,3 +153,4 @@
 [@hanachin]: https://github.com/hanachin
 [@joshpencheon]: https://github.com/joshpencheon
 [@djudd]: https://github.com/djudd
+[@sunny]: https://github.com/sunny

lib/rubocop/cop/rails/unique_validation_without_index.rb

@@ -47,6 +47,8 @@ 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
 
@@ -113,6 +115,18 @@ def uniqueness_part(node)
           end
         end
 
+        def condition_part?(node)
+          pairs = node.arguments.last
+          return unless pairs.hash_type?
+
+          pairs.each_pair.any? do |pair|
+            key = pair.key
+            next unless key.sym_type?
+
+            key.value == :if || key.value == :unless
+          end
+        end
+
         def array_node_to_array(node)
           node.values.map do |elm|
             case elm.type

spec/rubocop/cop/rails/unique_validation_without_index_spec.rb

@@ -102,7 +102,7 @@ class User
         end
       RUBY
 
-      it 'registers no offense' do
+      it 'does not register an offense' do
         expect_no_offenses(<<~RUBY)
           class User
             validates :account, uniqueness: true
@@ -145,7 +145,7 @@ class WrittenArticles
           end
         RUBY
 
-        it 'registers an offense' do
+        it 'does not register an offense' do
           expect_no_offenses(<<~RUBY)
             class WrittenArticles
               validates :user_id, uniqueness: { scope: :article_id }
@@ -192,7 +192,7 @@ class WrittenArticles
           end
         RUBY
 
-        it 'registers an offense' do
+        it 'does not register an offense' do
           expect_no_offenses(<<~RUBY)
             class WrittenArticles
               validates :a_id, uniqueness: { scope: [:b_id, :c_id] }
@@ -234,7 +234,7 @@ class Article
           end
         RUBY
 
-        it 'does not register offense' do
+        it 'does not register an offense' do
           expect_no_offenses(<<~RUBY)
             class Article
               belongs_to :user
@@ -244,6 +244,44 @@ class Article
         end
       end
 
+      context 'with an if condition on the validation' do
+        let(:schema) { <<~RUBY }
+          ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+            create_table "articles", force: :cascade do |t|
+              t.bitint "user_id", null: false
+            end
+          end
+        RUBY
+
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            class Article
+              belongs_to :user
+              validates :user, uniqueness: true, if: -> { false }
+            end
+          RUBY
+        end
+      end
+
+      context 'with an unless condition on the validation' do
+        let(:schema) { <<~RUBY }
+          ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+            create_table "articles", force: :cascade do |t|
+              t.bitint "user_id", null: false
+            end
+          end
+        RUBY
+
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            class Article
+              belongs_to :user
+              validates :user, uniqueness: true, unless: -> { true }
+            end
+          RUBY
+        end
+      end
+
       context 'without column definition' do
         let(:schema) { <<~RUBY }
           ActiveRecord::Schema.define(version: 2020_02_02_075409) do
@@ -297,7 +335,7 @@ class Article
           end
         RUBY
 
-        it 'does not register offense' do
+        it 'does not register an offense' do
           expect_no_offenses(<<~RUBY)
             class Article
               belongs_to :member, foreign_key: :user_id