Merge pull request #517 from theunraveler/polymorphic_unique_validati…

…on_without_index

Fix `UniqueValidationWithoutIndex` cop when validating uniqueness on a polymorphic relation scope

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## master (unreleased)
 
+### Bug fixes
+
+* [#517](https://github.com/rubocop/rubocop-rails/pull/517): Fix an issue for `Rails/UniqueValidationWithoutIndex` when validating uniqueness with a polymorphic scope. ([@theunraveler][])
+
 ## 2.11.2 (2021-07-02)
 
 ### Bug fixes
@@ -424,3 +428,4 @@
 [@nvasilevski]: https://github.com/nvasilevski
 [@skryukov]: https://github.com/skryukov
 [@johnsyweb]: https://github.com/johnsyweb
+[@theunraveler]: https://github.com/theunraveler

lib/rubocop/cop/mixin/active_record_helper.rb

@@ -57,12 +57,13 @@ def table_name(class_node)
       # Resolve relation into column name.
       # It just returns column_name if the column exists.
       # Or it tries to resolve column_name as a relation.
+      # Returns an array of column names if the relation is polymorphic.
       # It returns `nil` if it can't resolve.
       #
       # @param name [String]
       # @param class_node [RuboCop::AST::Node]
       # @param table [RuboCop::Rails::SchemaLoader::Table]
-      # @return [String, nil]
+      # @return [Array, String, nil]
       def resolve_relation_into_column(name:, class_node:, table:)
         return unless table
         return name if table.with_column?(name: name)
@@ -71,7 +72,9 @@ def resolve_relation_into_column(name:, class_node:, table:)
           next unless belongs_to.first_argument.value.to_s == name
 
           fk = foreign_key_of(belongs_to) || "#{name}_id"
-          return fk if table.with_column?(name: fk)
+          next unless table.with_column?(name: fk)
+
+          return polymorphic?(belongs_to) ? [fk, "#{name}_type"] : fk
         end
         nil
       end
@@ -88,6 +91,15 @@ def foreign_key_of(belongs_to)
         end
       end
 
+      def polymorphic?(belongs_to)
+        options = belongs_to.last_argument
+        return false unless options.hash_type?
+
+        options.each_pair.any? do |pair|
+          pair.key.sym_type? && pair.key.value == :polymorphic && pair.value.true_type?
+        end
+      end
+
       def in_where?(node)
         send_node = node.each_ancestor(:send).first
         send_node && WHERE_METHODS.include?(send_node.method_name)

lib/rubocop/cop/rails/unique_validation_without_index.rb

@@ -81,7 +81,7 @@ def column_names(node)
           names_from_scope = column_names_from_scope(node)
           ret.concat(names_from_scope) if names_from_scope
 
-          ret.map! do |name|
+          ret = ret.flat_map do |name|
             klass = class_node(node)
             resolve_relation_into_column(
               name: name.to_s,

spec/rubocop/cop/rails/unique_validation_without_index_spec.rb

@@ -137,6 +137,97 @@ class WrittenArticles
       end
     end
 
+    context 'when the validation is for a polymorphic association' do
+      context 'without proper index' do
+        let(:schema) { <<~RUBY }
+          ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+            create_table "written_articles", force: :cascade do |t|
+              t.string "title", null: false
+              t.string "author_type", null: false
+              t.bigint "author_id", null: false
+              t.index ["title", "author_id"], name: "idx_aid", unique: true
+            end
+          end
+        RUBY
+
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            class WrittenArticles
+              belongs_to :author, polymorphic: true
+              validates :title, uniqueness: { scope: :author }
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Uniqueness validation should be with a unique index.
+            end
+          RUBY
+        end
+      end
+
+      context 'with proper index' do
+        let(:schema) { <<~RUBY }
+          ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+            create_table "written_articles", force: :cascade do |t|
+              t.string "title", null: false
+              t.string "author_type", null: false
+              t.bigint "author_id", null: false
+              t.index ["title", "author_id", "author_type"], name: "idx_aid", unique: true
+            end
+          end
+        RUBY
+
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            class WrittenArticles
+              belongs_to :author, polymorphic: true
+              validates :title, uniqueness: { scope: :author }
+            end
+          RUBY
+        end
+      end
+
+      context 'when `polymorphic: false`' do
+        let(:schema) { <<~RUBY }
+          ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+            create_table "written_articles", force: :cascade do |t|
+              t.string "title", null: false
+              t.string "author_type", null: false
+              t.bigint "author_id", null: false
+              t.index ["title", "author_id"], name: "idx_aid", unique: true
+            end
+          end
+        RUBY
+
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            class WrittenArticles
+              belongs_to :author, polymorphic: false
+              validates :title, uniqueness: { scope: :author }
+            end
+          RUBY
+        end
+      end
+
+      context 'when `polymorphic` option is not specified' do
+        let(:schema) { <<~RUBY }
+          ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+            create_table "written_articles", force: :cascade do |t|
+              t.string "title", null: false
+              t.string "author_type", null: false
+              t.bigint "author_id", null: false
+              t.index ["title", "author_id"], name: "idx_aid", unique: true
+            end
+          end
+        RUBY
+
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            class WrittenArticles
+              belongs_to :author
+              validates :title, uniqueness: { scope: :author }
+            end
+          RUBY
+        end
+      end
+    end
+
     context 'when the validation is for three columns' do
       context 'without proper index' do
         let(:schema) { <<~RUBY }