diff --git a/changelog/fix_an_error_for_rails_unique_validation_without_index.md b/changelog/fix_an_error_for_rails_unique_validation_without_index.md new file mode 100644 index 0000000000..3cdda68bcd --- /dev/null +++ b/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][]) diff --git a/lib/rubocop/cop/rails/unique_validation_without_index.rb b/lib/rubocop/cop/rails/unique_validation_without_index.rb index f8ba542883..08f2bec1f4 100644 --- a/lib/rubocop/cop/rails/unique_validation_without_index.rb +++ b/lib/rubocop/cop/rails/unique_validation_without_index.rb @@ -31,11 +31,12 @@ 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 (uniqueness_part = uniqueness_part(node)) + return if uniqueness_part.falsey_literal? + return if condition_part?(node, uniqueness_part) return unless schema - 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 +45,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 +72,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 +91,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 +135,12 @@ 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 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 ae280df549..86acb14537 100644 --- a/spec/rubocop/cop/rails/unique_validation_without_index_spec.rb +++ b/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,7 +72,7 @@ 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 @@ -72,6 +80,14 @@ class User 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