AS core_ext refactoring pt.2

20 files changed, 110 insertions, 86 deletions

diff --git a/activesupport/lib/active_support/core_ext/array/conversions.rb b/activesupport/lib/active_support/core_ext/array/conversions.rb
index 258688772b..24aa28b895 100644
--- a/activesupport/lib/active_support/core_ext/array/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/array/conversions.rb
@@ -9,28 +9,32 @@ class Array
   # * <tt>:two_words_connector</tt> - The sign or word used to join the elements in arrays with two elements (default: " and ")
   # * <tt>:last_word_connector</tt> - The sign or word used to join the last element in arrays with three or more elements (default: ", and ")
   def to_sentence(options = {})
+    options.assert_valid_keys(:words_connector, :two_words_connector, :last_word_connector, :locale)
+
+    default_connectors = {
+      :words_connector     => ', ',
+      :two_words_connector => ' and ',
+      :last_word_connector => ', and '
+    }
     if defined?(I18n)
-      default_words_connector     = I18n.translate(:'support.array.words_connector',     :locale => options[:locale])
-      default_two_words_connector = I18n.translate(:'support.array.two_words_connector', :locale => options[:locale])
-      default_last_word_connector = I18n.translate(:'support.array.last_word_connector', :locale => options[:locale])
-    else
-      default_words_connector     = ", "
-      default_two_words_connector = " and "
-      default_last_word_connector = ", and "
+      namespace = 'support.array.'
+      default_connectors.each_key do |name|
+        i18n_key = (namespace + name.to_s).to_sym
+        default_connectors[name] = I18n.translate i18n_key, :locale => options[:locale]
+      end
     end
 
-    options.assert_valid_keys(:words_connector, :two_words_connector, :last_word_connector, :locale)
-    options.reverse_merge! :words_connector => default_words_connector, :two_words_connector => default_two_words_connector, :last_word_connector => default_last_word_connector
+    options.reverse_merge! default_connectors
 
     case length
-      when 0
-        ""
-      when 1
-        self[0].to_s.dup
-      when 2
-        "#{self[0]}#{options[:two_words_connector]}#{self[1]}"
-      else
-        "#{self[0...-1].join(options[:words_connector])}#{options[:last_word_connector]}#{self[-1]}"
+    when 0
+      ''
+    when 1
+      self[0].to_s.dup
+    when 2
+      "#{self[0]}#{options[:two_words_connector]}#{self[1]}"
+    else
+      "#{self[0...-1].join(options[:words_connector])}#{options[:last_word_connector]}#{self[-1]}"
     end
   end
 
@@ -45,14 +49,14 @@ class Array
   #   Blog.all.to_formatted_s(:db) # => "1,2,3"
   def to_formatted_s(format = :default)
     case format
-      when :db
-        if respond_to?(:empty?) && empty?
-          "null"
-        else
-          collect { |element| element.id }.join(",")
-        end
+    when :db
+      if empty?
+        'null'
       else
-        to_default_s
+        collect { |element| element.id }.join(',')
+      end
+    else
+      to_default_s
     end
   end
   alias_method :to_default_s, :to_s
@@ -86,20 +90,20 @@ class Array
   #     </project>
   #   </projects>
   #
-  # Otherwise the root element is "records":
+  # Otherwise the root element is "objects":
   #
   #   [{:foo => 1, :bar => 2}, {:baz => 3}].to_xml
   #
   #   <?xml version="1.0" encoding="UTF-8"?>
-  #   <records type="array">
-  #     <record>
+  #   <objects type="array">
+  #     <object>
   #       <bar type="integer">2</bar>
   #       <foo type="integer">1</foo>
-  #     </record>
-  #     <record>
+  #     </object>
+  #     <object>
   #       <baz type="integer">3</baz>
-  #     </record>
-  #   </records>
+  #     </object>
+  #   </objects>
   #
   # If the collection is empty the root element is "nil-classes" by default:
   #
@@ -139,26 +143,28 @@ class Array
     options = options.dup
     options[:indent]  ||= 2
     options[:builder] ||= Builder::XmlMarkup.new(:indent => options[:indent])
-    options[:root]    ||= if first.class.to_s != "Hash" && all? { |e| e.is_a?(first.class) }
-      underscored = ActiveSupport::Inflector.underscore(first.class.name)
-      ActiveSupport::Inflector.pluralize(underscored).tr('/', '_')
-    else
-      "objects"
-    end
+    options[:root]    ||= \
+      if first.class != Hash && all? { |e| e.is_a?(first.class) }
+        underscored = ActiveSupport::Inflector.underscore(first.class.name)
+        ActiveSupport::Inflector.pluralize(underscored).tr('/', '_')
+      else
+        'objects'
+      end
 
     builder = options[:builder]
     builder.instruct! unless options.delete(:skip_instruct)
 
     root = ActiveSupport::XmlMini.rename_key(options[:root].to_s, options)
     children = options.delete(:children) || root.singularize
+    attributes = options[:skip_types] ? {} : {:type => 'array'}
 
-    attributes = options[:skip_types] ? {} : {:type => "array"}
-    return builder.tag!(root, attributes) if empty?
-
-    builder.__send__(:method_missing, root, attributes) do
-      each { |value| ActiveSupport::XmlMini.to_tag(children, value, options) }
-      yield builder if block_given?
+    if empty?
+      builder.tag!(root, attributes)
+    else
+      builder.__send__(:method_missing, root, attributes) do
+        each { |value| ActiveSupport::XmlMini.to_tag(children, value, options) }
+        yield builder if block_given?
+      end
     end
   end
-
 end
diff --git a/activesupport/lib/active_support/core_ext/array/grouping.rb b/activesupport/lib/active_support/core_ext/array/grouping.rb
index 2b3f639cb1..ac1ae53db0 100644
--- a/activesupport/lib/active_support/core_ext/array/grouping.rb
+++ b/activesupport/lib/active_support/core_ext/array/grouping.rb
@@ -82,11 +82,9 @@ class Array
   #
   #   [1, 2, 3, 4, 5].split(3)                # => [[1, 2], [4, 5]]
   #   (1..10).to_a.split { |i| i % 3 == 0 }   # => [[1, 2], [4, 5], [7, 8], [10]]
-  def split(value = nil)
-    using_block = block_given?
-
+  def split(value = nil, &block)
     inject([[]]) do |results, element|
-      if (using_block && yield(element)) || (value == element)
+      if block && block.call(element) || value == element
         results << []
       else
         results.last << element
diff --git a/activesupport/lib/active_support/core_ext/array/uniq_by.rb b/activesupport/lib/active_support/core_ext/array/uniq_by.rb
index ac3dedc0e3..3bedfa9a61 100644
--- a/activesupport/lib/active_support/core_ext/array/uniq_by.rb
+++ b/activesupport/lib/active_support/core_ext/array/uniq_by.rb
@@ -6,8 +6,7 @@ class Array
   #   [1, 2, 3, 4].uniq_by { |i| i.odd? } # => [1, 2]
   #
   def uniq_by(&block)
-    ActiveSupport::Deprecation.warn "uniq_by " \
-      "is deprecated. Use Array#uniq instead", caller
+    ActiveSupport::Deprecation.warn 'uniq_by is deprecated. Use Array#uniq instead', caller
     uniq(&block)
   end
 
@@ -15,8 +14,7 @@ class Array
   #
   # Same as +uniq_by+, but modifies +self+.
   def uniq_by!(&block)
-    ActiveSupport::Deprecation.warn "uniq_by! " \
-      "is deprecated. Use Array#uniq! instead", caller
+    ActiveSupport::Deprecation.warn 'uniq_by! is deprecated. Use Array#uniq! instead', caller
     uniq!(&block)
   end
 end
diff --git a/activesupport/lib/active_support/core_ext/date_time/conversions.rb b/activesupport/lib/active_support/core_ext/date_time/conversions.rb
index dc55e9c33c..f09ba9e8cc 100644
--- a/activesupport/lib/active_support/core_ext/date_time/conversions.rb
+++ b/activesupport/lib/active_support/core_ext/date_time/conversions.rb
@@ -61,7 +61,11 @@ class DateTime
   # Attempts to convert self to a Ruby Time object; returns self if out of range of Ruby Time class.
   # If self has an offset other than 0, self will just be returned unaltered, since there's no clean way to map it to a Time.
   def to_time
-    self.offset == 0 ? ::Time.utc_time(year, month, day, hour, min, sec, sec_fraction * 1000000) : self
+    if offset == 0
+      ::Time.utc_time(year, month, day, hour, min, sec, sec_fraction * 1000000)
+    else
+      self
+    end
   end
 
   # Returns DateTime with local offset for given year if format is local else offset is zero
diff --git a/activesupport/lib/active_support/core_ext/date_time/zones.rb b/activesupport/lib/active_support/core_ext/date_time/zones.rb
index 6fa55a9255..823735d3e2 100644
--- a/activesupport/lib/active_support/core_ext/date_time/zones.rb
+++ b/activesupport/lib/active_support/core_ext/date_time/zones.rb
@@ -14,8 +14,10 @@ class DateTime
   #
   #   DateTime.new(2000).in_time_zone('Alaska')  # => Fri, 31 Dec 1999 15:00:00 AKST -09:00
   def in_time_zone(zone = ::Time.zone)
-    return self unless zone
-
-    ActiveSupport::TimeWithZone.new(utc? ? self : getutc, ::Time.find_zone!(zone))
+    if zone
+      ActiveSupport::TimeWithZone.new(utc? ? self : getutc, ::Time.find_zone!(zone))
+    else
+      self
+    end
   end
 end
diff --git a/activesupport/lib/active_support/core_ext/enumerable.rb b/activesupport/lib/active_support/core_ext/enumerable.rb
index 8c6c1b6658..48b5a570bf 100644
--- a/activesupport/lib/active_support/core_ext/enumerable.rb
+++ b/activesupport/lib/active_support/core_ext/enumerable.rb
@@ -34,8 +34,11 @@ module Enumerable
   #     => { "Chade- Fowlersburg-e" => <Person ...>, "David Heinemeier Hansson" => <Person ...>, ...}
   #
   def index_by
-    return to_enum :index_by unless block_given?
-    Hash[map { |elem| [yield(elem), elem] }]
+    if block_given?
+      Hash[map { |elem| [yield(elem), elem] }]
+    else
+      to_enum :index_by
+    end
   end
 
   # Returns true if the enumerable has more than 1 element. Functionally equivalent to enum.to_a.size > 1.
@@ -48,7 +51,7 @@ module Enumerable
         cnt > 1
       end
     else
-      any?{ (cnt += 1) > 1 }
+      any? { (cnt += 1) > 1 }
     end
   end
 
diff --git a/activesupport/lib/active_support/core_ext/file/atomic.rb b/activesupport/lib/active_support/core_ext/file/atomic.rb
index fc3277f4d2..99c9435275 100644
--- a/activesupport/lib/active_support/core_ext/file/atomic.rb
+++ b/activesupport/lib/active_support/core_ext/file/atomic.rb
@@ -26,8 +26,14 @@ class File
       old_stat = stat(file_name)
     rescue Errno::ENOENT
       # No old permissions, write a temp file to determine the defaults
-      check_name = join(dirname(file_name), ".permissions_check.#{Thread.current.object_id}.#{Process.pid}.#{rand(1000000)}")
-      open(check_name, "w") { }
+      temp_file_name = [
+        '.permissions_check',
+        Thread.current.object_id,
+        Process.pid,
+        rand(1000000)
+      ].join('.')
+      check_name = join(dirname(file_name), temp_file_name)
+      open(check_name, 'w') { }
       old_stat = stat(check_name)
       unlink(check_name)
     end
diff --git a/activesupport/lib/active_support/core_ext/hash/diff.rb b/activesupport/lib/active_support/core_ext/hash/diff.rb
index b904f49fa8..855dcb38bc 100644
--- a/activesupport/lib/active_support/core_ext/hash/diff.rb
+++ b/activesupport/lib/active_support/core_ext/hash/diff.rb
@@ -7,7 +7,9 @@ class Hash
   #   {1 => 2}.diff(1 => 3)         # => {1 => 2}
   #   {}.diff(1 => 2)               # => {1 => 2}
   #   {1 => 2, 3 => 4}.diff(1 => 2) # => {3 => 4}
-  def diff(h2)
-    dup.delete_if { |k, v| h2[k] == v }.merge!(h2.dup.delete_if { |k, v| has_key?(k) })
+  def diff(other)
+    dup.
+      delete_if { |k, v| other[k] == v }.
+      merge!(other.dup.delete_if { |k, v| has_key?(k) })
   end
 end
diff --git a/activesupport/lib/active_support/core_ext/hash/keys.rb b/activesupport/lib/active_support/core_ext/hash/keys.rb
index 65c3736593..8f8c24aeee 100644
--- a/activesupport/lib/active_support/core_ext/hash/keys.rb
+++ b/activesupport/lib/active_support/core_ext/hash/keys.rb
@@ -25,6 +25,7 @@ class Hash
     end
     result
   end
+  alias_method :to_options,  :symbolize_keys
 
   # Destructively convert all keys to symbols, as long as they respond
   # to +to_sym+.
@@ -34,8 +35,6 @@ class Hash
     end
     self
   end
-
-  alias_method :to_options,  :symbolize_keys
   alias_method :to_options!, :symbolize_keys!
 
   # Validate all keys in a hash match *valid keys, raising ArgumentError on a mismatch.
@@ -49,7 +48,7 @@ class Hash
   def assert_valid_keys(*valid_keys)
     valid_keys.flatten!
     each_key do |k|
-      raise(ArgumentError, "Unknown key: #{k}") unless valid_keys.include?(k)
+      raise ArgumentError.new("Unknown key: #{k}") unless valid_keys.include?(k)
     end
   end
 end
diff --git a/activesupport/lib/active_support/core_ext/hash/reverse_merge.rb b/activesupport/lib/active_support/core_ext/hash/reverse_merge.rb
index 01863a162b..6074103484 100644
--- a/activesupport/lib/active_support/core_ext/hash/reverse_merge.rb
+++ b/activesupport/lib/active_support/core_ext/hash/reverse_merge.rb
@@ -18,6 +18,5 @@ class Hash
     # right wins if there is no left
     merge!( other_hash ){|key,left,right| left }
   end
-
   alias_method :reverse_update, :reverse_merge!
 end
diff --git a/activesupport/lib/active_support/core_ext/hash/slice.rb b/activesupport/lib/active_support/core_ext/hash/slice.rb
index 45181f0e16..fdeea4c148 100644
--- a/activesupport/lib/active_support/core_ext/hash/slice.rb
+++ b/activesupport/lib/active_support/core_ext/hash/slice.rb
@@ -31,6 +31,6 @@ class Hash
   # Removes and returns the key/value pairs matching the given keys.
   #   {:a => 1, :b => 2, :c => 3, :d => 4}.extract!(:a, :b) # => {:a => 1, :b => 2}
   def extract!(*keys)
-    keys.each_with_object({}) {|key, result| result[key] = delete(key) }
+    keys.each_with_object({}) { |key, result| result[key] = delete(key) }
   end
 end
diff --git a/activesupport/lib/active_support/core_ext/kernel/reporting.rb b/activesupport/lib/active_support/core_ext/kernel/reporting.rb
index 526b8378a5..8f1636c8da 100644
--- a/activesupport/lib/active_support/core_ext/kernel/reporting.rb
+++ b/activesupport/lib/active_support/core_ext/kernel/reporting.rb
@@ -1,4 +1,5 @@
 require 'rbconfig'
+
 module Kernel
   # Sets $VERBOSE to nil for the duration of the block and back to its original value afterwards.
   #
diff --git a/activesupport/lib/active_support/core_ext/module/aliasing.rb b/activesupport/lib/active_support/core_ext/module/aliasing.rb
index ce481f0e84..382156ecd8 100644
--- a/activesupport/lib/active_support/core_ext/module/aliasing.rb
+++ b/activesupport/lib/active_support/core_ext/module/aliasing.rb
@@ -26,18 +26,19 @@ class Module
     aliased_target, punctuation = target.to_s.sub(/([?!=])$/, ''), $1
     yield(aliased_target, punctuation) if block_given?
 
-    with_method, without_method = "#{aliased_target}_with_#{feature}#{punctuation}", "#{aliased_target}_without_#{feature}#{punctuation}"
+    with_method = "#{aliased_target}_with_#{feature}#{punctuation}"
+    without_method = "#{aliased_target}_without_#{feature}#{punctuation}"
 
     alias_method without_method, target
     alias_method target, with_method
 
     case
-      when public_method_defined?(without_method)
-        public target
-      when protected_method_defined?(without_method)
-        protected target
-      when private_method_defined?(without_method)
-        private target
+    when public_method_defined?(without_method)
+      public target
+    when protected_method_defined?(without_method)
+      protected target
+    when private_method_defined?(without_method)
+      private target
     end
   end
 
diff --git a/activesupport/lib/active_support/core_ext/module/attr_internal.rb b/activesupport/lib/active_support/core_ext/module/attr_internal.rb
index 00db75bfec..db07d549b0 100644
--- a/activesupport/lib/active_support/core_ext/module/attr_internal.rb
+++ b/activesupport/lib/active_support/core_ext/module/attr_internal.rb
@@ -15,7 +15,6 @@ class Module
     attr_internal_reader(*attrs)
     attr_internal_writer(*attrs)
   end
-
   alias_method :attr_internal, :attr_internal_accessor
 
   class << self; attr_accessor :attr_internal_naming_format end
diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb
index ee8adae1cb..1933d90bb7 100644
--- a/activesupport/lib/active_support/core_ext/module/delegation.rb
+++ b/activesupport/lib/active_support/core_ext/module/delegation.rb
@@ -114,7 +114,7 @@ class Module
       raise ArgumentError, "Can only automatically set the delegation prefix when delegating to a method."
     end
 
-    method_prefix =
+    method_prefix = \
       if prefix
         "#{prefix == true ? to : prefix}_"
       else
diff --git a/activesupport/lib/active_support/core_ext/module/introspection.rb b/activesupport/lib/active_support/core_ext/module/introspection.rb
index 743db47bac..3c8e811fa4 100644
--- a/activesupport/lib/active_support/core_ext/module/introspection.rb
+++ b/activesupport/lib/active_support/core_ext/module/introspection.rb
@@ -5,10 +5,11 @@ class Module
   #
   #   M::N.parent_name # => "M"
   def parent_name
-    unless defined? @parent_name
+    if defined? @parent_name
+      @parent_name
+    else
       @parent_name = name =~ /::[^:]+\Z/ ? $`.freeze : nil
     end
-    @parent_name
   end
 
   # Returns the module which contains this one according to its name.
@@ -73,7 +74,7 @@ class Module
   # This method is useful for forward compatibility, since Ruby 1.8 returns
   # constant names as strings, whereas 1.9 returns them as symbols.
   def local_constant_names
-    ActiveSupport::Deprecation.warn('Module#local_constant_names is deprecated, use Module#local_constants instead', caller)
+    ActiveSupport::Deprecation.warn 'Module#local_constant_names is deprecated, use Module#local_constants instead', caller
     local_constants.map { |c| c.to_s }
   end
 end
diff --git a/activesupport/lib/active_support/core_ext/module/qualified_const.rb b/activesupport/lib/active_support/core_ext/module/qualified_const.rb
index 8adf050b6b..7d7ae38f68 100644
--- a/activesupport/lib/active_support/core_ext/module/qualified_const.rb
+++ b/activesupport/lib/active_support/core_ext/module/qualified_const.rb
@@ -5,7 +5,7 @@ require 'active_support/core_ext/string/inflections'
 #++
 module QualifiedConstUtils
   def self.raise_if_absolute(path)
-    raise NameError, "wrong constant name #$&" if path =~ /\A::[^:]+/
+    raise NameError.new("wrong constant name #$&") if path =~ /\A::[^:]+/
   end
 
   def self.names(path)
diff --git a/activesupport/lib/active_support/core_ext/string/access.rb b/activesupport/lib/active_support/core_ext/string/access.rb
index 9b5266c58c..23aaee9c43 100644
--- a/activesupport/lib/active_support/core_ext/string/access.rb
+++ b/activesupport/lib/active_support/core_ext/string/access.rb
@@ -1,4 +1,4 @@
-require "active_support/multibyte"
+require 'active_support/multibyte'
 
 class String
   def at(position)
diff --git a/activesupport/lib/active_support/core_ext/string/inflections.rb b/activesupport/lib/active_support/core_ext/string/inflections.rb
index 2194dafe4d..639ebc58e5 100644
--- a/activesupport/lib/active_support/core_ext/string/inflections.rb
+++ b/activesupport/lib/active_support/core_ext/string/inflections.rb
@@ -77,8 +77,10 @@ class String
   #   "active_record/errors".camelize(:lower) # => "activeRecord::Errors"
   def camelize(first_letter = :upper)
     case first_letter
-      when :upper then ActiveSupport::Inflector.camelize(self, true)
-      when :lower then ActiveSupport::Inflector.camelize(self, false)
+    when :upper
+      ActiveSupport::Inflector.camelize(self, true)
+    when :lower
+      ActiveSupport::Inflector.camelize(self, false)
     end
   end
   alias_method :camelcase, :camelize
diff --git a/activesupport/lib/active_support/core_ext/time/calculations.rb b/activesupport/lib/active_support/core_ext/time/calculations.rb
index 2ff2ca1dbc..1434e186c3 100644
--- a/activesupport/lib/active_support/core_ext/time/calculations.rb
+++ b/activesupport/lib/active_support/core_ext/time/calculations.rb
@@ -22,8 +22,11 @@ class Time
     # Return the number of days in the given month.
     # If no year is specified, it will use the current year.
     def days_in_month(month, year = now.year)
-      return 29 if month == 2 && ::Date.gregorian_leap?(year)
-      COMMON_YEAR_DAYS_IN_MONTH[month]
+      if month == 2 && ::Date.gregorian_leap?(year)
+        29
+      else
+        COMMON_YEAR_DAYS_IN_MONTH[month]
+      end
     end
 
     # Returns a new Time if requested year can be accommodated by Ruby's Time class