From bb266fd942f756e75209cde0430162659e216e01 Mon Sep 17 00:00:00 2001 From: Sunny Ripert Date: Tue, 24 Mar 2020 15:26:31 +0100 Subject: [PATCH] No UniqueValidationWithoutIndex offense if validation has a condition --- CHANGELOG.md | 5 ++ .../rails/unique_validation_without_index.rb | 14 ++++++ .../unique_validation_without_index_spec.rb | 48 +++++++++++++++++-- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b018f099f4..c0c7b66fd3 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/lib/rubocop/cop/rails/unique_validation_without_index.rb b/lib/rubocop/cop/rails/unique_validation_without_index.rb index ea9a6bf27a..87543a56a0 100644 --- a/lib/rubocop/cop/rails/unique_validation_without_index.rb +++ b/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 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 54e7cc18b9..4f57f2b003 100644 --- a/spec/rubocop/cop/rails/unique_validation_without_index_spec.rb +++ b/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