Merge pull request #1028 from koic/fix_an_error_for_rails_unique_validation_without_index

Fix an error for `Rails/UniqueValidationWithoutIndex`

Author: koic

changelog/fix_an_error_for_rails_unique_validation_without_index.md

@@ -0,0 +1 @@
+* [#1028](https://github.com/rubocop/rubocop-rails/pull/1028): Fix an error for `Rails/UniqueValidationWithoutIndex` when the `presence: true` option is used alone for the `validates` method. ([@koic][])

lib/rubocop/cop/rails/unique_validation_without_index.rb

@@ -31,11 +31,11 @@ class UniqueValidationWithoutIndex < Base
         RESTRICT_ON_SEND = %i[validates].freeze
 
         def on_send(node)
-          return if uniqueness_part(node)&.falsey_literal?
-          return if condition_part?(node)
           return unless schema
+          return unless (uniqueness_part = uniqueness_part(node))
+          return if uniqueness_part.falsey_literal? || condition_part?(node, uniqueness_part)
 
-          klass, table, names = find_schema_information(node)
+          klass, table, names = find_schema_information(node, uniqueness_part)
           return unless names
           return if with_index?(klass, table, names)
 
@@ -44,12 +44,12 @@ def on_send(node)
 
         private
 
-        def find_schema_information(node)
+        def find_schema_information(node, uniqueness_part)
           klass = class_node(node)
           return unless klass
 
           table = schema.table_by(name: table_name(klass))
-          names = column_names(node)
+          names = column_names(node, uniqueness_part)
 
           [klass, table, names]
         end
@@ -71,12 +71,12 @@ def include_column_names_in_expression_index?(index, column_names)
           end
         end
 
-        def column_names(node)
+        def column_names(node, uniqueness_part)
           arg = node.first_argument
           return unless arg.str_type? || arg.sym_type?
 
           ret = [arg.value]
-          names_from_scope = column_names_from_scope(node)
+          names_from_scope = column_names_from_scope(uniqueness_part)
           ret.concat(names_from_scope) if names_from_scope
 
           ret = ret.flat_map do |name|
@@ -90,11 +90,10 @@ def column_names(node)
           ret.include?(nil) ? nil : ret.to_set
         end
 
-        def column_names_from_scope(node)
-          uniq = uniqueness_part(node)
-          return unless uniq.hash_type?
+        def column_names_from_scope(uniqueness_part)
+          return unless uniqueness_part.hash_type?
 
-          scope = find_scope(uniq)
+          scope = find_scope(uniqueness_part)
           return unless scope
 
           scope = unfreeze_scope(scope)
@@ -135,14 +134,11 @@ def uniqueness_part(node)
           end
         end
 
-        def condition_part?(node)
-          pairs = node.arguments.last
-          return unless pairs.hash_type?
-
+        def condition_part?(node, uniqueness_node)
+          pairs = node.last_argument
+          return false unless pairs.hash_type?
           return true if condition_hash_part?(pairs, keys: %i[if unless])
-
-          uniqueness_node = uniqueness_part(node)
-          return unless uniqueness_node&.hash_type?
+          return false unless uniqueness_node.hash_type?
 
           condition_hash_part?(uniqueness_node, keys: %i[if unless conditions])
         end

spec/rubocop/cop/rails/unique_validation_without_index_spec.rb

@@ -36,7 +36,7 @@ class User < ApplicationRecord
         end
       RUBY
 
-      it 'registers an offense' do
+      it 'registers an offense when `uniqueness: true`' do
         expect_offense(<<~RUBY)
           class User
             validates :account, uniqueness: true
@@ -45,13 +45,21 @@ class User
         RUBY
       end
 
-      it 'does not register an offense' do
+      it 'does not register an offense when `uniqueness: false`' do
         expect_no_offenses(<<~RUBY)
           class User
             validates :account, uniqueness: false
           end
         RUBY
       end
+
+      it 'does not register an offense when `uniqueness: nil`' do
+        expect_no_offenses(<<~RUBY)
+          class User
+            validates :account, uniqueness: nil
+          end
+        RUBY
+      end
     end
 
     context 'when the table has an index but it is not unique' do
@@ -64,14 +72,22 @@ class User
         end
       RUBY
 
-      it 'registers an offense' do
+      it 'registers an offense when the `uniqueness: true` option is used alone' do
         expect_offense(<<~RUBY)
           class User
             validates :account, uniqueness: true
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Uniqueness validation should have a unique index on the database column.
           end
         RUBY
       end
+
+      it 'does not register an offense when the `presence: true` option is used alone' do
+        expect_no_offenses(<<~RUBY)
+          class User
+            validates :account, presence: true
+          end
+        RUBY
+      end
     end
 
     context 'with a unique index' do