Skip to content

Commit

Permalink
Fix an error for Rails/UniqueValidationWithoutIndex
Browse files Browse the repository at this point in the history
Resolves #1022 (comment).

This PR fixes an error for `Rails/UniqueValidationWithoutIndex`
when the `presence: true` option is used alone for the `validates` method.
  • Loading branch information
koic committed Jun 19, 2023
1 parent 169427d commit 418a64f
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 20 deletions.
Original file line number Diff line number Diff line change
@@ -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][])
32 changes: 15 additions & 17 deletions lib/rubocop/cop/rails/unique_validation_without_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand All @@ -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|
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
22 changes: 19 additions & 3 deletions spec/rubocop/cop/rails/unique_validation_without_index_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 418a64f

Please sign in to comment.