Merge pull request #222 from koic/make_unique_validation_without_inde…

…x_aware_of_add_index

[Fix #221] Make `Rails/UniqueValidationWithoutIndex` aware of `add_index`

CHANGELOG.md

@@ -7,6 +7,7 @@
 * [#213](https://github.com/rubocop-hq/rubocop-rails/pull/213): Fix a false positive for `Rails/UniqueValidationWithoutIndex` when using conditions. ([@sunny][])
 * [#215](https://github.com/rubocop-hq/rubocop-rails/issues/215): Fix a false positive for `Rails/UniqueValidationWithoutIndex` when using Expression Indexes. ([@koic][])
 * [#214](https://github.com/rubocop-hq/rubocop-rails/issues/214): Fix an error for `Rails/UniqueValidationWithoutIndex`when a table has no column definition. ([@koic][])
+* [#221](https://github.com/rubocop-hq/rubocop-rails/issues/221): Make `Rails/UniqueValidationWithoutIndex` aware of `add_index` in db/schema.rb. ([@koic][])
 
 ## 2.5.0 (2020-03-24)
 

lib/rubocop/cop/rails/unique_validation_without_index.rb

@@ -51,7 +51,10 @@ def with_index?(node)
           names = column_names(node)
           return true unless names
 
-          table.indices.any? do |index|
+          # Compatibility for Rails 4.2.
+          add_indicies = schema.add_indicies_by(table_name: table_name(klass))
+
+          (table.indices + add_indicies).any? do |index|
             index.unique &&
               (index.columns.to_set == names ||
                include_column_names_in_expression_index?(index, names))

lib/rubocop/rails/schema_loader/schema.rb

@@ -5,10 +5,12 @@ module Rails
     module SchemaLoader
       # Represent db/schema.rb
       class Schema
-        attr_reader :tables
+        attr_reader :tables, :add_indicies
 
         def initialize(ast)
           @tables = []
+          @add_indicies = []
+
           build!(ast)
         end
 
@@ -18,6 +20,12 @@ def table_by(name:)
           end
         end
 
+        def add_indicies_by(table_name:)
+          add_indicies.select do |add_index|
+            add_index.table_name == table_name
+          end
+        end
+
         private
 
         def build!(ast)
@@ -26,6 +34,11 @@ def build!(ast)
           each_table(ast) do |table_def|
             @tables << Table.new(table_def)
           end
+
+          # Compatibility for Rails 4.2.
+          each_add_index(ast) do |add_index_def|
+            @add_indicies << AddIndex.new(add_index_def)
+          end
         end
 
         def each_table(ast)
@@ -40,6 +53,14 @@ def each_table(ast)
             yield ast.body
           end
         end
+
+        def each_add_index(ast)
+          ast.body.children.each do |node|
+            next if !node&.send_type? || !node.method?(:add_index)
+
+            yield(node)
+          end
+        end
       end
 
       # Represent a table
@@ -121,21 +142,19 @@ class Index
         attr_reader :name, :columns, :expression, :unique
 
         def initialize(node)
-          node.first_argument
-          @columns, @expression = build_columns_or_expr(node)
+          @columns, @expression = build_columns_or_expr(node.first_argument)
           @unique = nil
 
           analyze_keywords!(node)
         end
 
         private
 
-        def build_columns_or_expr(node)
-          arg = node.first_argument
-          if arg.array_type?
-            [arg.values.map(&:value), nil]
+        def build_columns_or_expr(columns)
+          if columns.array_type?
+            [columns.values.map(&:value), nil]
           else
-            [[], arg.value]
+            [[], columns.value]
           end
         end
 
@@ -153,6 +172,19 @@ def analyze_keywords!(node)
           end
         end
       end
+
+      # Represent an `add_index`
+      class AddIndex < Index
+        attr_reader :table_name
+
+        def initialize(node)
+          @table_name = node.first_argument.value
+          @columns, @expression = build_columns_or_expr(node.arguments[1])
+          @unique = nil
+
+          analyze_keywords!(node)
+        end
+      end
     end
   end
 end

spec/rubocop/cop/rails/unique_validation_without_index_spec.rb

@@ -464,5 +464,46 @@ class Email < ApplicationRecord
         end
       end
     end
+
+    context 'when db/schema.rb has been dumped using `add_index` for index' do
+      context 'when the table does not have any indices' do
+        let(:schema) { <<~RUBY }
+          ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+            create_table "users", force: :cascade do |t|
+              t.string "account", null: false
+            end
+            add_index "users", "account", name: "index_users_on_account"
+          end
+        RUBY
+
+        it 'registers an offense' do
+          expect_offense(<<~RUBY)
+            class User
+              validates :account, uniqueness: true
+              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Uniqueness validation should be with a unique index.
+            end
+          RUBY
+        end
+      end
+
+      context 'with a unique index' do
+        let(:schema) { <<~RUBY }
+          ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+            create_table "users", force: :cascade do |t|
+              t.string "account", null: false
+            end
+            add_index "users", ["account"], name: "index_users_on_account", unique: true
+          end
+        RUBY
+
+        it 'does not register an offense' do
+          expect_no_offenses(<<~RUBY)
+            class User
+              validates :account, uniqueness: true
+            end
+          RUBY
+        end
+      end
+    end
   end
 end

spec/rubocop/rails/schema_loader_spec.rb

@@ -85,6 +85,28 @@
           expect(table.indices.first.name).to eq 'index_title_lower_unique'
           expect(table.indices.first.unique).to be true
         end
+
+        context 'when an index in users table specified by `add_index`' do
+          let(:schema) { <<~RUBY }
+            ActiveRecord::Schema.define(version: 2020_02_02_075409) do
+              create_table "users", force: :cascade do |t|
+                t.string "account", null: false
+              end
+              add_index "users", ["account"], name: "index_users_on_account", unique: true
+              add_index "users", ["email"], name: "index_users_on_email", unique: true
+              add_index "books", ["isbn"], name: "index_books_on_isbn", unique: true
+            end
+          RUBY
+
+          it 'has an `add_index` for users table' do
+            add_indicies = loaded_schema.add_indicies_by(table_name: 'users')
+            expect(add_indicies.size).to eq 2
+            expect(add_indicies.first.name).to eq 'index_users_on_account'
+            expect(add_indicies.first.table_name).to eq 'users'
+            expect(add_indicies.first.columns).to eq ['account']
+            expect(add_indicies.first.unique).to be true
+          end
+        end
       end
 
       context 'when the current directory is Rails.root' do