Add new Rails/BulkChangeTable cop (rubocop#5881)

* Add `SupportedDatabases` configurable values

If set `Database` option, `SupportedDatabases` is also used as
configurable values.

* Add new `Rails/BulkChangeTable` cop

This Cop checks whether alter queries are combinable.
If combinable queries are detected, it suggests to you to use
`change_table` with `bulk: true` instead.
When use this method, make combinable alter queries a bulk alter query.

The `bulk` option is only supported on the MySQL and the PostgreSQL
(5.2 later) adapter; thus it will automatically detect an adapter from
`development` environment in `config/database.yml` when the `Database`
option is not set. If the adapter is not `mysql2` or `postgresql`, this
Cop ignores offenses.

For example, the following migration executes 4 SQLs:

```ruby
class BulkExample < ActiveRecord::Migration[5.2]
  def change
    // Output SQL
    ActiveRecord::Base.logger = Logger.new(STDOUT)

    add_reference :users, :team
    add_column :users, :name, :string, null: false
    add_column :users, :nickname, :string
  end
end
```

```
== 20180513070748 BulkExample: migrating ======================================
-- add_reference(:users, :team)
D, [2018-05-13T16:19:09.247167 #88149] DEBUG -- :    (173.7ms)  ALTER TABLE `users` ADD `team_id` bigint
D, [2018-05-13T16:19:09.248363 #88149] DEBUG -- :   ↳ db/migrate/20180513070748_bulk_example.rb:5
D, [2018-05-13T16:19:09.279004 #88149] DEBUG -- :    (29.2ms)  CREATE  INDEX `index_users_on_team_id`  ON `users` (`team_id`)
D, [2018-05-13T16:19:09.279818 #88149] DEBUG -- :   ↳ db/migrate/20180513070748_bulk_example.rb:5
   -> 0.2073s
-- add_column(:users, :name, :string, {:null=>false})
D, [2018-05-13T16:19:09.339642 #88149] DEBUG -- :    (59.1ms)  ALTER TABLE `users` ADD `name` varchar(255) NOT NULL
D, [2018-05-13T16:19:09.339814 #88149] DEBUG -- :   ↳ db/migrate/20180513070748_bulk_example.rb:6
   -> 0.0596s
-- add_column(:users, :nickname, :string)
D, [2018-05-13T16:19:09.386546 #88149] DEBUG -- :    (46.2ms)  ALTER TABLE `users` ADD `nickname` varchar(255)
D, [2018-05-13T16:19:09.387489 #88149] DEBUG -- :   ↳ db/migrate/20180513070748_bulk_example.rb:7
   -> 0.0476s
== 20180513070748 BulkExample: migrated (0.3151s) =============================
```

If you use `bulk: true`, it combines into 3 SQLs.

```ruby
class BulkExample < ActiveRecord::Migration[5.2]
  def change
    ActiveRecord::Base.logger = Logger.new(STDOUT)

    add_reference :users, :team
    change_table :users, bulk: true do |t|
      t.string :name, null: false
      t.string :nickname
    end
  end
end
```

```
== 20180513070748 BulkExample: migrating ======================================
-- add_reference(:users, :team)
D, [2018-05-13T16:26:35.937416 #89967] DEBUG -- :    (70.5ms)  ALTER TABLE `users` ADD `team_id` bigint
D, [2018-05-13T16:26:35.937644 #89967] DEBUG -- :   ↳ db/migrate/20180513070748_bulk_example.rb:5
D, [2018-05-13T16:26:35.974410 #89967] DEBUG -- :    (34.1ms)  CREATE  INDEX `index_users_on_team_id`  ON `users` (`team_id`)
D, [2018-05-13T16:26:35.974609 #89967] DEBUG -- :   ↳ db/migrate/20180513070748_bulk_example.rb:5
   -> 0.1083s
-- change_table(:users, {:bulk=>true})
D, [2018-05-13T16:26:36.030428 #89967] DEBUG -- :    (52.1ms)  ALTER TABLE `users` ADD `name` varchar(255) NOT NULL, ADD `nickname` varchar(255)
D, [2018-05-13T16:26:36.030584 #89967] DEBUG -- :   ↳ db/migrate/20180513070748_bulk_example.rb:6
   -> 0.0557s
== 20180513070748 BulkExample: migrated (0.1644s) =============================
```

By the way, this Cop ignores non-combinable queries such as
`add_references` because including these in bulk changes will
cause an error like the following:

```
== 20180513070748 BulkExample: migrating ======================================
-- change_table(:users, {:bulk=>true})
D, [2018-05-13T16:30:52.348856 #90767] DEBUG -- :    (0.4ms)  SELECT RELEASE_LOCK('5518437444551514000')
D, [2018-05-13T16:30:52.349985 #90767] DEBUG -- :   ↳ /Users/watanabekazuma/.rbenv/versions/2.4.1/bin/rake:22
rake aborted!
StandardError: An error has occurred, all later migrations canceled:

Unknown method called : add_reference_for_alter([:team, {}])
```

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### New features
 
+* [#5881](https://github.com/bbatsov/rubocop/pull/5881): Add new `Rails/BulkChangeTable` cop. ([@wata727][])
 * [#5444](https://github.com/bbatsov/rubocop/pull/5444): Add new `Style/AccessModifierDeclarations` cop. ([@brandonweiss][])
 * [#5803](https://github.com/bbatsov/rubocop/issues/5803): Add new `Style/UnneededCondition` cop. ([@balbesina][])
 * [#5406](https://github.com/bbatsov/rubocop/issues/5406): Add new `Layout/ClosingHeredocIndentation` cop. ([@siggymcfried][])

config/default.yml

@@ -1674,6 +1674,14 @@ Rails/Blank:
   # Convert usages of `unless present?` to `if blank?`
   UnlessPresent: true
 
+Rails/BulkChangeTable:
+  Database: null
+  SupportedDatabases:
+    - mysql
+    - postgresql
+  Include:
+    - db/migrate/*.rb
+
 Rails/CreateTableWithTimestamps:
   Include:
     - db/migrate/*.rb

config/enabled.yml

@@ -1122,6 +1122,10 @@ Rails/Blank:
   Description: 'Enforces use of `blank?`.'
   Enabled: true
 
+Rails/BulkChangeTable:
+  Description: 'Check whether alter queries are combinable.'
+  Enabled: true
+
 Rails/CreateTableWithTimestamps:
   Description: >-
                   Checks the migration for which timestamps are not included

lib/rubocop.rb

@@ -546,6 +546,7 @@
 require_relative 'rubocop/cop/rails/application_record'
 require_relative 'rubocop/cop/rails/assert_not'
 require_relative 'rubocop/cop/rails/blank'
+require_relative 'rubocop/cop/rails/bulk_change_table'
 require_relative 'rubocop/cop/rails/create_table_with_timestamps'
 require_relative 'rubocop/cop/rails/date'
 require_relative 'rubocop/cop/rails/dynamic_find_by'

lib/rubocop/cop/rails/bulk_change_table.rb

@@ -0,0 +1,272 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # This Cop checks whether alter queries are combinable.
+      # If combinable queries are detected, it suggests to you
+      # to use `change_table` with `bulk: true` instead.
+      # When use this method, make combinable alter queries
+      # a bulk alter query.
+      #
+      # The `bulk` option is only supported on the MySQL and
+      # the PostgreSQL (5.2 later) adapter; thus it will
+      # automatically detect an adapter from `development` environment
+      # in `config/database.yml` when the `Database` option is not set.
+      # If the adapter is not `mysql2` or `postgresql`,
+      # this Cop ignores offenses.
+      #
+      # @example
+      #   # bad
+      #   def change
+      #     add_column :users, :name, :string, null: false
+      #     add_column :users, :nickname, :string
+      #
+      #     # ALTER TABLE `users` ADD `name` varchar(255) NOT NULL
+      #     # ALTER TABLE `users` ADD `nickname` varchar(255)
+      #   end
+      #
+      #   # good
+      #   def change
+      #     change_table :users, bulk: true do |t|
+      #       t.string :name, null: false
+      #       t.string :nickname
+      #     end
+      #
+      #     # ALTER TABLE `users` ADD `name` varchar(255) NOT NULL,
+      #     #                     ADD `nickname` varchar(255)
+      #   end
+      #
+      # @example
+      #   # bad
+      #   def change
+      #     change_table :users do |t|
+      #       t.string :name, null: false
+      #       t.string :nickname
+      #     end
+      #   end
+      #
+      #   # good
+      #   def change
+      #     change_table :users, bulk: true do |t|
+      #       t.string :name, null: false
+      #       t.string :nickname
+      #     end
+      #   end
+      #
+      #   # good
+      #   # When you don't want to combine alter queries.
+      #   def change
+      #     change_table :users, bulk: false do |t|
+      #       t.string :name, null: false
+      #       t.string :nickname
+      #     end
+      #   end
+      #
+      # @see http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-change_table
+      # @see http://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/Table.html
+      class BulkChangeTable < Cop
+        MSG_FOR_CHANGE_TABLE = <<-MSG.strip_indent.chomp
+          You can combine alter queries using `bulk: true` options.
+        MSG
+        MSG_FOR_ALTER_METHODS = <<-MSG.strip_indent.chomp
+          You can use `change_table :%<table>s, bulk: true` to combine alter queries.
+        MSG
+
+        MYSQL = 'mysql'.freeze
+        POSTGRESQL = 'postgresql'.freeze
+
+        MIGRATIION_METHODS = %i[change up down].freeze
+
+        COMBINABLE_TRANSFORMATIONS = %i[
+          primary_key
+          column
+          string
+          text
+          integer
+          bigint
+          float
+          decimal
+          numeric
+          datetime
+          timestamp
+          time
+          date
+          binary
+          boolean
+          json
+          virtual
+          remove
+          change
+          timestamps
+          remove_timestamps
+        ].freeze
+
+        COMBINABLE_ALTER_METHODS = %i[
+          add_column
+          remove_column
+          remove_columns
+          change_column
+          add_timestamps
+          remove_timestamps
+        ].freeze
+
+        MYSQL_COMBINABLE_TRANSFORMATIONS = %i[
+          rename
+          index
+          remove_index
+        ].freeze
+
+        MYSQL_COMBINABLE_ALTER_METHODS = %i[
+          rename_column
+          add_index
+          remove_index
+        ].freeze
+
+        POSTGRESQL_COMBINABLE_TRANSFORMATIONS = %i[
+          change_default
+        ].freeze
+
+        POSTGRESQL_COMBINABLE_ALTER_METHODS = %i[
+          change_column_default
+          change_column_null
+        ].freeze
+
+        def on_def(node)
+          return unless support_bulk_alter?
+          return unless MIGRATIION_METHODS.include?(node.method_name)
+
+          recorder = AlterMethodsRecorder.new
+
+          node.body.each_child_node(:send) do |send_node|
+            if combinable_alter_methods.include?(send_node.method_name)
+              recorder.process(send_node)
+            else
+              recorder.flush
+            end
+          end
+
+          recorder.offensive_nodes.each { |n| add_offense_for_alter_methods(n) }
+        end
+
+        def on_send(node)
+          return unless support_bulk_alter?
+          return unless node.command?(:change_table)
+          return if include_bulk_options?(node)
+          return unless node.block_node
+          send_nodes = node.block_node.body.each_child_node(:send).to_a
+
+          transformations = send_nodes.select do |send_node|
+            combinable_transformations.include?(send_node.method_name)
+          end
+
+          add_offense_for_change_table(node) if transformations.size > 1
+        end
+
+        private
+
+        # @param node [RuboCop::AST::SendNode] (send nil? :change_table ...)
+        def include_bulk_options?(node)
+          # arguments: [(sym :table) (hash (pair (sym :bulk) _))]
+          options = node.arguments[1]
+          return false unless options
+          options.hash_type? &&
+            options.keys.any? { |key| key.sym_type? && key.value == :bulk }
+        end
+
+        def database
+          cop_config['Database'] || database_from_yaml
+        end
+
+        def database_from_yaml
+          return nil unless database_yaml
+          case database_yaml['adapter']
+          when 'mysql2'
+            MYSQL
+          when 'postgresql'
+            POSTGRESQL
+          end
+        end
+
+        def database_yaml
+          return nil unless File.exist?('config/database.yml')
+          yaml = YAML.load_file('config/database.yml')
+          return nil unless yaml.is_a? Hash
+          config = yaml['development']
+          return nil unless config.is_a?(Hash)
+          config
+        end
+
+        def support_bulk_alter?
+          case database
+          when MYSQL
+            true
+          when POSTGRESQL
+            # Add bulk alter support for PostgreSQL in 5.2.0
+            # @see https://github.com/rails/rails/pull/31331
+            target_rails_version >= 5.2
+          else
+            false
+          end
+        end
+
+        def combinable_alter_methods
+          case database
+          when MYSQL
+            COMBINABLE_ALTER_METHODS + MYSQL_COMBINABLE_ALTER_METHODS
+          when POSTGRESQL
+            COMBINABLE_ALTER_METHODS + POSTGRESQL_COMBINABLE_ALTER_METHODS
+          end
+        end
+
+        def combinable_transformations
+          case database
+          when MYSQL
+            COMBINABLE_TRANSFORMATIONS + MYSQL_COMBINABLE_TRANSFORMATIONS
+          when POSTGRESQL
+            COMBINABLE_TRANSFORMATIONS + POSTGRESQL_COMBINABLE_TRANSFORMATIONS
+          end
+        end
+
+        # @param node [RuboCop::AST::SendNode]
+        def add_offense_for_alter_methods(node)
+          # arguments: [(sym :table) ...]
+          table_name = node.arguments[0].value
+          message = format(MSG_FOR_ALTER_METHODS, table: table_name)
+          add_offense(node, message: message)
+        end
+
+        # @param node [RuboCop::AST::SendNode]
+        def add_offense_for_change_table(node)
+          add_offense(node, message: MSG_FOR_CHANGE_TABLE)
+        end
+
+        # Record combinable alter methods and register offensive nodes.
+        class AlterMethodsRecorder
+          def initialize
+            @nodes = []
+            @offensive_nodes = []
+          end
+
+          # @param new_node [RuboCop::AST::SendNode]
+          def process(new_node)
+            # arguments: [(sym :table) ...]
+            table_name = new_node.arguments[0]
+            flush unless @nodes.all? { |node| node.arguments[0] == table_name }
+            @nodes << new_node
+          end
+
+          def flush
+            @offensive_nodes << @nodes.first if @nodes.size > 1
+            @nodes = []
+          end
+
+          def offensive_nodes
+            flush
+            @offensive_nodes
+          end
+        end
+      end
+    end
+  end
+end

manual/cops.md

@@ -333,6 +333,7 @@ In the following section you find all available cops:
 * [Rails/ApplicationRecord](cops_rails.md#railsapplicationrecord)
 * [Rails/AssertNot](cops_rails.md#railsassertnot)
 * [Rails/Blank](cops_rails.md#railsblank)
+* [Rails/BulkChangeTable](cops_rails.md#railsbulkchangetable)
 * [Rails/CreateTableWithTimestamps](cops_rails.md#railscreatetablewithtimestamps)
 * [Rails/Date](cops_rails.md#railsdate)
 * [Rails/Delegate](cops_rails.md#railsdelegate)