Merge pull request #16830 from teeparham/schema_dumper

Cleanup SchemaDumper

1 files changed, 28 insertions, 27 deletions

diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index fae6427ea1..b560c58a66 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -105,18 +105,22 @@ HEADER
         end
       end
 
+      def primary_key_for(table)
+        if @connection.respond_to?(:pk_and_sequence_for)
+          pk, _ = @connection.pk_and_sequence_for(table)
+          return pk if pk
+        end
+        return @connection.primary_key(table) if @connection.respond_to?(:primary_key)
+        nil
+      end
+
       def table(table, stream)
         columns = @connection.columns(table)
         begin
           tbl = StringIO.new
 
           # first dump primary key column
-          if @connection.respond_to?(:pk_and_sequence_for)
-            pk, _ = @connection.pk_and_sequence_for(table)
-          end
-          if !pk && @connection.respond_to?(:primary_key)
-            pk = @connection.primary_key(table)
-          end
+          pk = primary_key_for(table)
 
           tbl.print "  create_table #{remove_prefix_and_suffix(table).inspect}"
           pkcol = columns.detect { |c| c.name == pk }
@@ -188,25 +192,22 @@ HEADER
         if (indexes = @connection.indexes(table)).any?
           add_index_statements = indexes.map do |index|
             statement_parts = [
-              ('add_index ' + remove_prefix_and_suffix(index.table).inspect),
+              "add_index #{remove_prefix_and_suffix(index.table).inspect}",
               index.columns.inspect,
-              ('name: ' + index.name.inspect),
+              "name: #{index.name.inspect}",
             ]
             statement_parts << 'unique: true' if index.unique
 
             index_lengths = (index.lengths || []).compact
-            statement_parts << ('length: ' + Hash[index.columns.zip(index.lengths)].inspect) unless index_lengths.empty?
+            statement_parts << "length: #{Hash[index.columns.zip(index.lengths)].inspect}" if index_lengths.any?
 
-            index_orders = (index.orders || {})
-            statement_parts << ('order: ' + index.orders.inspect) unless index_orders.empty?
+            index_orders = index.orders || {}
+            statement_parts << "order: #{index.orders.inspect}" if index_orders.any?
+            statement_parts << "where: #{index.where.inspect}" if index.where
+            statement_parts << "using: #{index.using.inspect}" if index.using
+            statement_parts << "type: #{index.type.inspect}" if index.type
 
-            statement_parts << ('where: ' + index.where.inspect) if index.where
-
-            statement_parts << ('using: ' + index.using.inspect) if index.using
-
-            statement_parts << ('type: ' + index.type.inspect) if index.type
-
-            '  ' + statement_parts.join(', ')
+            "  #{statement_parts.join(', ')}"
           end
 
           stream.puts add_index_statements.sort.join("\n")
@@ -218,26 +219,26 @@ HEADER
         if (foreign_keys = @connection.foreign_keys(table)).any?
           add_foreign_key_statements = foreign_keys.map do |foreign_key|
             parts = [
-                     'add_foreign_key ' + remove_prefix_and_suffix(foreign_key.from_table).inspect,
-                     remove_prefix_and_suffix(foreign_key.to_table).inspect,
-                    ]
+              "add_foreign_key #{remove_prefix_and_suffix(foreign_key.from_table).inspect}",
+              remove_prefix_and_suffix(foreign_key.to_table).inspect,
+            ]
 
             if foreign_key.column != @connection.foreign_key_column_for(foreign_key.to_table)
-              parts << ('column: ' + foreign_key.column.inspect)
+              parts << "column: #{foreign_key.column.inspect}"
             end
 
             if foreign_key.custom_primary_key?
-              parts << ('primary_key: ' + foreign_key.primary_key.inspect)
+              parts << "primary_key: #{foreign_key.primary_key.inspect}"
             end
 
             if foreign_key.name !~ /^fk_rails_[0-9a-f]{10}$/
-              parts << ('name: ' + foreign_key.name.inspect)
+              parts << "name: #{foreign_key.name.inspect}"
             end
 
-            parts << ('on_update: ' + foreign_key.on_update.inspect) if foreign_key.on_update
-            parts << ('on_delete: ' + foreign_key.on_delete.inspect) if foreign_key.on_delete
+            parts << "on_update: #{foreign_key.on_update.inspect}" if foreign_key.on_update
+            parts << "on_delete: #{foreign_key.on_delete.inspect}" if foreign_key.on_delete
 
-            '  ' + parts.join(', ')
+            "  #{parts.join(', ')}"
           end
 
           stream.puts add_foreign_key_statements.sort.join("\n")