From ae39b1a03d0a859be9d5342592c8936f89fcbacf Mon Sep 17 00:00:00 2001
From: Ryuta Kamizono <kamipo@gmail.com>
Date: Fri, 3 Feb 2017 08:50:46 +0900
Subject: [PATCH] Refactor `ColumnDefinition` to contain `options` hash

Column options are passed as an hash args then used as `options` hash in
`add_column_options!`. Converting args to attributes is inconvinient for
using options as an hash.
---
 .../abstract/schema_creation.rb               | 15 +------
 .../abstract/schema_definitions.rb            | 40 +++++++++----------
 .../abstract/schema_statements.rb             |  2 +-
 .../abstract_mysql_adapter.rb                 |  4 +-
 .../mysql/schema_creation.rb                  | 14 +------
 .../mysql/schema_definitions.rb               | 19 ++-------
 .../postgresql/schema_creation.rb             | 15 +++++++
 .../postgresql/schema_definitions.rb          | 16 --------
 .../postgresql/schema_statements.rb           | 24 ++---------
 .../connection_adapters/postgresql_adapter.rb |  1 +
 .../sqlite3/schema_creation.rb                |  9 +----
 .../cases/adapters/mysql2/sql_types_test.rb   |  4 +-
 .../cases/adapters/postgresql/bytea_test.rb   |  4 +-
 .../adapters/postgresql/datatype_test.rb      |  4 +-
 .../test/cases/column_definition_test.rb      | 16 ++------
 15 files changed, 59 insertions(+), 128 deletions(-)
 create mode 100644 activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb
index 81dec97bf7..c48a4acff8 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb
@@ -29,7 +29,7 @@ module ActiveRecord
           end
 
           def visit_ColumnDefinition(o)
-            o.sql_type ||= type_to_sql(o.type, o.limit, o.precision, o.scale)
+            o.sql_type = type_to_sql(o.type, o.options)
             column_sql = "#{quote_column_name(o.name)} #{o.sql_type}"
             add_column_options!(column_sql, column_options(o)) unless o.type == :primary_key
             column_sql
@@ -96,18 +96,7 @@ module ActiveRecord
           end
 
           def column_options(o)
-            column_options = {}
-            column_options[:null] = o.null unless o.null.nil?
-            column_options[:default] = o.default unless o.default.nil?
-            column_options[:column] = o
-            column_options[:first] = o.first
-            column_options[:after] = o.after
-            column_options[:auto_increment] = o.auto_increment
-            column_options[:primary_key] = o.primary_key
-            column_options[:collation] = o.collation
-            column_options[:comment] = o.comment
-            column_options[:as] = o.as
-            column_options
+            o.options.merge(column: o)
           end
 
           def add_column_options!(sql, options)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index 87144fe86f..5eb7787226 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -9,9 +9,21 @@ module ActiveRecord
     # are typically created by methods in TableDefinition, and added to the
     # +columns+ attribute of said TableDefinition object, in order to be used
     # for generating a number of table creation or table changing SQL statements.
-    ColumnDefinition = Struct.new(:name, :type, :limit, :precision, :scale, :default, :null, :first, :after, :auto_increment, :primary_key, :collation, :sql_type, :comment, :as) do # :nodoc:
+    ColumnDefinition = Struct.new(:name, :type, :options, :sql_type) do # :nodoc:
       def primary_key?
-        primary_key || type.to_sym == :primary_key
+        options[:primary_key]
+      end
+
+      [:limit, :precision, :scale, :default, :null, :collation, :comment].each do |option_name|
+        module_eval <<-CODE, __FILE__, __LINE__ + 1
+          def #{option_name}
+            options[:#{option_name}]
+          end
+
+          def #{option_name}=(value)
+            options[:#{option_name}] = value
+          end
+        CODE
       end
     end
 
@@ -360,28 +372,16 @@ module ActiveRecord
       end
       alias :belongs_to :references
 
-      def new_column_definition(name, type, options) # :nodoc:
+      def new_column_definition(name, type, **options) # :nodoc:
         type = aliased_types(type.to_s, type)
-        column = create_column_definition name, type
-
-        column.limit       = options[:limit]
-        column.precision   = options[:precision]
-        column.scale       = options[:scale]
-        column.default     = options[:default]
-        column.null        = options[:null]
-        column.first       = options[:first]
-        column.after       = options[:after]
-        column.auto_increment = options[:auto_increment]
-        column.primary_key = type == :primary_key || options[:primary_key]
-        column.collation   = options[:collation]
-        column.comment     = options[:comment]
-        column.as          = options[:as]
-        column
+        options[:primary_key] ||= type == :primary_key
+        options[:null] = false if options[:primary_key]
+        create_column_definition(name, type, options)
       end
 
       private
-        def create_column_definition(name, type)
-          ColumnDefinition.new name, type
+        def create_column_definition(name, type, options)
+          ColumnDefinition.new(name, type, options)
         end
 
         def aliased_types(name, fallback)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index cf1241bf0c..8de8c05988 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -1052,7 +1052,7 @@ module ActiveRecord
         end
       end
 
-      def type_to_sql(type, limit = nil, precision = nil, scale = nil) #:nodoc:
+      def type_to_sql(type, limit: nil, precision: nil, scale: nil, **) # :nodoc:
         type = type.to_sym if type
         if native = native_database_types[type]
           column_type_sql = (native.is_a?(Hash) ? native[:name] : native).dup
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 18e227b40d..279c232144 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -574,7 +574,7 @@ module ActiveRecord
       end
 
       # Maps logical Rails types to MySQL-specific data types.
-      def type_to_sql(type, limit = nil, precision = nil, scale = nil, unsigned = nil)
+      def type_to_sql(type, limit: nil, precision: nil, scale: nil, unsigned: nil, **) # :nodoc:
         sql = \
           case type.to_s
           when "integer"
@@ -590,7 +590,7 @@ module ActiveRecord
               binary_to_sql(limit)
             end
           else
-            super(type, limit, precision, scale)
+            super
           end
 
         sql << " unsigned" if unsigned && type != :primary_key
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb
index 39c2acbca9..e8358271ab 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb
@@ -1,7 +1,7 @@
 module ActiveRecord
   module ConnectionAdapters
     module MySQL
-      class SchemaCreation < AbstractAdapter::SchemaCreation
+      class SchemaCreation < AbstractAdapter::SchemaCreation # :nodoc:
         delegate :add_sql_comment!, :mariadb?, to: :@conn
         private :add_sql_comment!, :mariadb?
 
@@ -11,11 +11,6 @@ module ActiveRecord
             "DROP FOREIGN KEY #{name}"
           end
 
-          def visit_ColumnDefinition(o)
-            o.sql_type = type_to_sql(o.type, o.limit, o.precision, o.scale, o.unsigned)
-            super
-          end
-
           def visit_AddColumnDefinition(o)
             add_column_position!(super, column_options(o.column))
           end
@@ -29,13 +24,6 @@ module ActiveRecord
             add_sql_comment!(super, options[:comment])
           end
 
-          def column_options(o)
-            column_options = super
-            column_options[:charset] = o.charset
-            column_options[:stored] = o.stored
-            column_options
-          end
-
           def add_column_options!(sql, options)
             if charset = options[:charset]
               sql << " CHARACTER SET #{charset}"
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb
index 76ebd0bf6c..773bbcef4e 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb
@@ -56,14 +56,10 @@ module ActiveRecord
         end
       end
 
-      class ColumnDefinition < ActiveRecord::ConnectionAdapters::ColumnDefinition
-        attr_accessor :charset, :unsigned, :stored
-      end
-
       class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
         include ColumnMethods
 
-        def new_column_definition(name, type, options) # :nodoc:
+        def new_column_definition(name, type, **options) # :nodoc:
           case type
           when :virtual
             type = options[:type]
@@ -76,18 +72,9 @@ module ActiveRecord
             type = $~[:type].to_sym
             options[:unsigned] = true
           end
-          column = super
-          column.unsigned = options[:unsigned]
-          column.charset = options[:charset]
-          column.stored = options[:stored]
-          column
+
+          super
         end
-
-        private
-
-          def create_column_definition(name, type)
-            MySQL::ColumnDefinition.new(name, type)
-          end
       end
 
       class Table < ActiveRecord::ConnectionAdapters::Table
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb
new file mode 100644
index 0000000000..e1d5089115
--- /dev/null
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb
@@ -0,0 +1,15 @@
+module ActiveRecord
+  module ConnectionAdapters
+    module PostgreSQL
+      class SchemaCreation < AbstractAdapter::SchemaCreation # :nodoc:
+        private
+          def add_column_options!(sql, options)
+            if options[:collation]
+              sql << " COLLATE \"#{options[:collation]}\""
+            end
+            super
+          end
+      end
+    end
+  end
+end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
index 0443bd8bdd..7491fb716e 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb
@@ -173,24 +173,8 @@ module ActiveRecord
         end
       end
 
-      class ColumnDefinition < ActiveRecord::ConnectionAdapters::ColumnDefinition
-        attr_accessor :array
-      end
-
       class TableDefinition < ActiveRecord::ConnectionAdapters::TableDefinition
         include ColumnMethods
-
-        def new_column_definition(name, type, options) # :nodoc:
-          column = super
-          column.array = options[:array]
-          column
-        end
-
-        private
-
-          def create_column_definition(name, type)
-            PostgreSQL::ColumnDefinition.new name, type
-          end
       end
 
       class Table < ActiveRecord::ConnectionAdapters::Table
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
index bfda113e40..a4b1723fc7 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb
@@ -3,22 +3,6 @@ require "active_support/core_ext/string/strip"
 module ActiveRecord
   module ConnectionAdapters
     module PostgreSQL
-      class SchemaCreation < AbstractAdapter::SchemaCreation
-        private
-
-          def visit_ColumnDefinition(o)
-            o.sql_type = type_to_sql(o.type, o.limit, o.precision, o.scale, o.array)
-            super
-          end
-
-          def add_column_options!(sql, options)
-            if options[:collation]
-              sql << " COLLATE \"#{options[:collation]}\""
-            end
-            super
-          end
-      end
-
       module SchemaStatements
         # Drops the database specified on the +name+ attribute
         # and creates it again using the provided +options+.
@@ -486,7 +470,7 @@ module ActiveRecord
           clear_cache!
           quoted_table_name = quote_table_name(table_name)
           quoted_column_name = quote_column_name(column_name)
-          sql_type = type_to_sql(type, options[:limit], options[:precision], options[:scale], options[:array])
+          sql_type = type_to_sql(type, options)
           sql = "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}"
           if options[:collation]
             sql << " COLLATE \"#{options[:collation]}\""
@@ -494,7 +478,7 @@ module ActiveRecord
           if options[:using]
             sql << " USING #{options[:using]}"
           elsif options[:cast_as]
-            cast_as_type = type_to_sql(options[:cast_as], options[:limit], options[:precision], options[:scale], options[:array])
+            cast_as_type = type_to_sql(options[:cast_as], options)
             sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})"
           end
           execute sql
@@ -630,7 +614,7 @@ module ActiveRecord
         end
 
         # Maps logical Rails types to PostgreSQL-specific data types.
-        def type_to_sql(type, limit = nil, precision = nil, scale = nil, array = nil)
+        def type_to_sql(type, limit: nil, precision: nil, scale: nil, array: nil, **) # :nodoc:
           sql = \
             case type.to_s
             when "binary"
@@ -655,7 +639,7 @@ module ActiveRecord
               else raise(ActiveRecordError, "No integer type has byte size #{limit}. Use a numeric with scale 0 instead.")
               end
             else
-              super(type, limit, precision, scale)
+              super
             end
 
           sql << "[]" if array && type != :primary_key
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index a2c5ef6817..6b1e870a31 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -9,6 +9,7 @@ require "active_record/connection_adapters/postgresql/explain_pretty_printer"
 require "active_record/connection_adapters/postgresql/oid"
 require "active_record/connection_adapters/postgresql/quoting"
 require "active_record/connection_adapters/postgresql/referential_integrity"
+require "active_record/connection_adapters/postgresql/schema_creation"
 require "active_record/connection_adapters/postgresql/schema_definitions"
 require "active_record/connection_adapters/postgresql/schema_dumper"
 require "active_record/connection_adapters/postgresql/schema_statements"
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb
index 70c0d28830..bc798d1dbb 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3/schema_creation.rb
@@ -1,15 +1,8 @@
 module ActiveRecord
   module ConnectionAdapters
     module SQLite3
-      class SchemaCreation < AbstractAdapter::SchemaCreation
+      class SchemaCreation < AbstractAdapter::SchemaCreation # :nodoc:
         private
-
-          def column_options(o)
-            options = super
-            options[:null] = false if o.primary_key
-            options
-          end
-
           def add_column_options!(sql, options)
             if options[:collation]
               sql << " COLLATE \"#{options[:collation]}\""
diff --git a/activerecord/test/cases/adapters/mysql2/sql_types_test.rb b/activerecord/test/cases/adapters/mysql2/sql_types_test.rb
index bee42d48f1..d6e7f29a5c 100644
--- a/activerecord/test/cases/adapters/mysql2/sql_types_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/sql_types_test.rb
@@ -8,7 +8,7 @@ class Mysql2SqlTypesTest < ActiveRecord::Mysql2TestCase
     assert_equal "blob", type_to_sql(:binary)
   end
 
-  def type_to_sql(*args)
-    ActiveRecord::Base.connection.type_to_sql(*args)
+  def type_to_sql(type, limit = nil)
+    ActiveRecord::Base.connection.type_to_sql(type, limit: limit)
   end
 end
diff --git a/activerecord/test/cases/adapters/postgresql/bytea_test.rb b/activerecord/test/cases/adapters/postgresql/bytea_test.rb
index 5c207116c4..505c297cd4 100644
--- a/activerecord/test/cases/adapters/postgresql/bytea_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/bytea_test.rb
@@ -32,9 +32,9 @@ class PostgresqlByteaTest < ActiveRecord::PostgreSQLTestCase
   end
 
   def test_binary_columns_are_limitless_the_upper_limit_is_one_GB
-    assert_equal "bytea", @connection.type_to_sql(:binary, 100_000)
+    assert_equal "bytea", @connection.type_to_sql(:binary, limit: 100_000)
     assert_raise ActiveRecord::ActiveRecordError do
-      @connection.type_to_sql :binary, 4294967295
+      @connection.type_to_sql(:binary, limit: 4294967295)
     end
   end
 
diff --git a/activerecord/test/cases/adapters/postgresql/datatype_test.rb b/activerecord/test/cases/adapters/postgresql/datatype_test.rb
index 0ac8b7339b..eb61782dc7 100644
--- a/activerecord/test/cases/adapters/postgresql/datatype_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/datatype_test.rb
@@ -61,9 +61,9 @@ class PostgresqlDataTypeTest < ActiveRecord::PostgreSQLTestCase
   end
 
   def test_text_columns_are_limitless_the_upper_limit_is_one_GB
-    assert_equal "text", @connection.type_to_sql(:text, 100_000)
+    assert_equal "text", @connection.type_to_sql(:text, limit: 100_000)
     assert_raise ActiveRecord::ActiveRecordError do
-      @connection.type_to_sql :text, 4294967295
+      @connection.type_to_sql(:text, limit: 4294967295)
     end
   end
 end
diff --git a/activerecord/test/cases/column_definition_test.rb b/activerecord/test/cases/column_definition_test.rb
index 0dffe5e60b..d230700119 100644
--- a/activerecord/test/cases/column_definition_test.rb
+++ b/activerecord/test/cases/column_definition_test.rb
@@ -14,27 +14,17 @@ module ActiveRecord
       # Avoid column definitions in create table statements like:
       # `title` varchar(255) DEFAULT NULL
       def test_should_not_include_default_clause_when_default_is_null
-        column = Column.new("title", nil, SqlTypeMetadata.new(limit: 20))
-        column_def = ColumnDefinition.new(
-          column.name, "string",
-          column.limit, column.precision, column.scale, column.default, column.null)
+        column_def = ColumnDefinition.new("title", "string", limit: 20)
         assert_equal "title varchar(20)", @viz.accept(column_def)
       end
 
       def test_should_include_default_clause_when_default_is_present
-        column = Column.new("title", "Hello", SqlTypeMetadata.new(limit: 20))
-        column_def = ColumnDefinition.new(
-          column.name, "string",
-          column.limit, column.precision, column.scale, column.default, column.null)
+        column_def = ColumnDefinition.new("title", "string", limit: 20, default: "Hello")
         assert_equal "title varchar(20) DEFAULT 'Hello'", @viz.accept(column_def)
       end
 
       def test_should_specify_not_null_if_null_option_is_false
-        type_metadata = SqlTypeMetadata.new(limit: 20)
-        column = Column.new("title", "Hello", type_metadata, false)
-        column_def = ColumnDefinition.new(
-          column.name, "string",
-          column.limit, column.precision, column.scale, column.default, column.null)
+        column_def = ColumnDefinition.new("title", "string", limit: 20, default: "Hello", null: false)
         assert_equal "title varchar(20) DEFAULT 'Hello' NOT NULL", @viz.accept(column_def)
       end
     end