Merge pull request #612 from fatkodima/multiple-databases-migration-cops

Support multiple databases for `ReversibleMigration` and `ReversibleMigrationMethodDefinition` cops

Author: koic

changelog/change_migration_cops_multiple_databases.md

@@ -0,0 +1 @@
+* [#463](https://github.com/rubocop/rubocop-rails/issues/463): Support multiple databases for `ReversibleMigration` and `ReversibleMigrationMethodDefinition` cops. ([@fatkodima][])

config/default.yml

@@ -702,15 +702,17 @@ Rails/ReversibleMigration:
   Reference: 'https://api.rubyonrails.org/classes/ActiveRecord/Migration/CommandRecorder.html'
   Enabled: true
   VersionAdded: '0.47'
+  VersionChanged: '<<next>>'
   Include:
-    - db/migrate/*.rb
+    - db/**/*.rb
 
 Rails/ReversibleMigrationMethodDefinition:
   Description: 'Checks whether the migration implements either a `change` method or both an `up` and a `down` method.'
   Enabled: false
   VersionAdded: '2.10'
+  VersionChanged: '<<next>>'
   Include:
-    - db/migrate/*.rb
+    - db/**/*.rb
 
 Rails/RootJoinChain:
   Description: 'Use a single `#join` instead of chaining on `Rails.root` or `Rails.public_path`.'

lib/rubocop/cop/mixin/migrations_helper.rb

@@ -0,0 +1,26 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Common functionality for cops working with migrations
+    module MigrationsHelper
+      extend NodePattern::Macros
+
+      def_node_matcher :migration_class?, <<~PATTERN
+        (class
+          (const nil? _)
+          (send
+            (const (const {nil? cbase} :ActiveRecord) :Migration)
+            :[]
+            (float _))
+          _)
+      PATTERN
+
+      def in_migration?(node)
+        node.each_ancestor(:class).any? do |class_node|
+          migration_class?(class_node)
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails/reversible_migration.rb

@@ -176,6 +176,8 @@ module Rails
       #
       # @see https://api.rubyonrails.org/classes/ActiveRecord/Migration/CommandRecorder.html
       class ReversibleMigration < Base
+        include MigrationsHelper
+
         MSG = '%<action>s is not reversible.'
 
         def_node_matcher :irreversible_schema_statement_call, <<~PATTERN
@@ -207,7 +209,7 @@ class ReversibleMigration < Base
         PATTERN
 
         def on_send(node)
-          return unless within_change_method?(node)
+          return unless in_migration?(node) && within_change_method?(node)
           return if within_reversible_or_up_only_block?(node)
 
           check_irreversible_schema_statement_node(node)
@@ -220,7 +222,7 @@ def on_send(node)
         end
 
         def on_block(node)
-          return unless within_change_method?(node)
+          return unless in_migration?(node) && within_change_method?(node)
           return if within_reversible_or_up_only_block?(node)
           return if node.body.nil?
 

lib/rubocop/cop/rails/reversible_migration_method_definition.rb

@@ -43,19 +43,11 @@ module Rails
       #     end
       #   end
       class ReversibleMigrationMethodDefinition < Base
+        include MigrationsHelper
+
         MSG = 'Migrations must contain either a `change` method, or ' \
               'both an `up` and a `down` method.'
 
-        def_node_matcher :migration_class?, <<~PATTERN
-          (class
-            (const nil? _)
-            (send
-              (const (const {nil? cbase} :ActiveRecord) :Migration)
-              :[]
-              (float _))
-            _)
-        PATTERN
-
         def_node_matcher :change_method?, <<~PATTERN
           [ #migration_class? `(def :change (args) _) ]
         PATTERN

lib/rubocop/cop/rails_cops.rb

@@ -4,6 +4,7 @@
 require_relative 'mixin/active_record_migrations_helper'
 require_relative 'mixin/enforce_superclass'
 require_relative 'mixin/index_method'
+require_relative 'mixin/migrations_helper'
 require_relative 'mixin/target_rails_version'
 
 require_relative 'rails/action_filter'

spec/rubocop/cop/rails/reversible_migration_method_definition_spec.rb

@@ -106,4 +106,28 @@ def change
       end
     RUBY
   end
+
+  context 'when multiple databases' do
+    it 'does not register an offense with a change method' do
+      expect_no_offenses(<<~RUBY, 'db/animals_migrate/20211007000002_add_nice_to_animals.rb')
+        class AddNiceToAnimals < ActiveRecord::Migration[7.0]
+          def change
+            add_column :animals, :nice, :boolean, default: true
+          end
+        end
+      RUBY
+    end
+
+    it 'registers an offense with only an up method' do
+      expect_offense(<<~RUBY, 'db/animals_migrate/20211007000002_add_nice_to_animals.rb')
+        class AddNiceToAnimals < ActiveRecord::Migration[7.0]
+        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Migrations must contain either a `change` method, or both an `up` and a `down` method.
+
+          def up
+            add_column :animals, :nice, :boolean, default: true
+          end
+        end
+      RUBY
+    end
+  end
 end

spec/rubocop/cop/rails/reversible_migration_spec.rb

@@ -3,7 +3,7 @@
 RSpec.describe RuboCop::Cop::Rails::ReversibleMigration, :config do
   let(:source) do
     <<~RUBY
-      class ExampleMigration < ActiveRecord::Migration
+      class ExampleMigration < ActiveRecord::Migration[7.0]
         def change
           #{code}
         end
@@ -293,4 +293,27 @@ def change
       remove_index(:posts, name: :index_columns_on_body)
     RUBY
   end
+
+  context 'when multiple databases' do
+    it 'does not register an offense for reversible operation' do
+      expect_no_offenses(<<~RUBY, 'db/animals_migrate/20211007000002_create_animals.rb')
+        class CreateAnimals < ActiveRecord::Migration[7.0]
+          def change
+            create_table :animals
+          end
+        end
+      RUBY
+    end
+
+    it 'registers an offense for irreversible operation' do
+      expect_offense(<<~RUBY, 'db/animals_migrate/20211007000002_remove_animals.rb')
+        class RemoveAnimals < ActiveRecord::Migration[7.0]
+          def change
+            drop_table :animals
+            ^^^^^^^^^^^^^^^^^^^ drop_table(without block) is not reversible.
+          end
+        end
+      RUBY
+    end
+  end
 end