Ensure I18n format values always have precedence over defaults

Always merge I18n format values, namespaced or not, over the default
ones, to ensure I18n format defaults will have precedence over our
namespaced values.

Precedence should happen like this:

    default :format
    default :namespace :format
    i18n    :format
    i18n    :namespace :format

Because we cannot allow our namespaced default to override a I18n
:format config - ie precision in I18n :format should always have higher
precedence than our default precision for a particular :namespace.

Also simplify default format options logic.

3 files changed, 47 insertions, 43 deletions

diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb
index 89a58f3701..3849f94a31 100644
--- a/activesupport/lib/active_support/number_helper.rb
+++ b/activesupport/lib/active_support/number_helper.rb
@@ -8,21 +8,6 @@ module ActiveSupport
     extend self
 
     DEFAULTS = {
-      # Used in number_to_currency
-      currency: {
-        format: {
-          # Where is the currency sign? %u is the currency unit, %n the number (default: $5.00)
-          format: "%u%n",
-          unit: "$",
-          # These five are to override number.format and are optional
-          separator: ".",
-          delimiter: ",",
-          precision: 2,
-          significant: false,
-          strip_insignificant_zeros: false
-        }
-      },
-
       # Used in number_to_delimited
       # These are also the defaults for 'currency', 'percentage', 'precision', and 'human'
       format: {
@@ -39,6 +24,21 @@ module ActiveSupport
         strip_insignificant_zeros: false
       },
 
+      # Used in number_to_currency
+      currency: {
+        format: {
+          format: "%u%n",
+          negative_format: "-%u%n",
+          unit: "$",
+          # These five are to override number.format and are optional
+          separator: ".",
+          delimiter: ",",
+          precision: 2,
+          significant: false,
+          strip_insignificant_zeros: false
+        }
+      },
+
       # Used in number_to_percentage
       percentage: {
         format: {
@@ -202,10 +202,10 @@ module ActiveSupport
       return unless number
       options = options.symbolize_keys
 
-      currency = translations_for(:currency, options[:locale])
+      currency = i18n_format_options(options[:locale], :currency)
       currency[:negative_format] ||= "-" + currency[:format] if currency[:format]
 
-      defaults  = defaults_translations(options[:locale]).merge(currency)
+      defaults  = default_format_options(:currency).merge!(currency)
       defaults[:negative_format] = "-" + options[:format] if options[:format]
       options   = defaults.merge!(options)
 
@@ -256,7 +256,7 @@ module ActiveSupport
       return unless number
       options = options.symbolize_keys
 
-      defaults = format_translations(:percentage, options[:locale])
+      defaults = format_options(options[:locale], :percentage)
       options  = defaults.merge!(options)
 
       format = options[:format] || "%n%"
@@ -293,7 +293,7 @@ module ActiveSupport
 
       return number unless valid_float?(number)
 
-      options = defaults_translations(options[:locale]).merge(options)
+      options = format_options(options[:locale]).merge!(options)
 
       parts = number.to_s.to_str.split('.')
       parts[0].gsub!(/(\d)(?=(\d\d\d)+(?!\d))/, "\\1#{options[:delimiter]}")
@@ -344,7 +344,7 @@ module ActiveSupport
       number  = Float(number)
       options = options.symbolize_keys
 
-      defaults = format_translations(:precision, options[:locale])
+      defaults = format_options(options[:locale], :precision)
       options  = defaults.merge!(options)
 
       precision = options.delete :precision
@@ -424,7 +424,7 @@ module ActiveSupport
       return number unless valid_float?(number)
       number = Float(number)
 
-      defaults = format_translations(:human, options[:locale])
+      defaults = format_options(options[:locale], :human)
       options  = defaults.merge!(options)
 
       #for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files
@@ -554,7 +554,7 @@ module ActiveSupport
       return number unless valid_float?(number)
       number = Float(number)
 
-      defaults = format_translations(:human, options[:locale])
+      defaults = format_options(options[:locale], :human)
       options  = defaults.merge!(options)
 
       #for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files
@@ -598,33 +598,31 @@ module ActiveSupport
     end
     private_class_method :private_module_and_instance_method
 
-    def format_translations(namespace, locale) #:nodoc:
-      defaults_translations(locale).merge!(translations_for(namespace, locale))
+    def format_options(locale, namespace = nil) #:nodoc:
+      default_format_options(namespace).merge!(i18n_format_options(locale, namespace))
     end
-    private_module_and_instance_method :format_translations
+    private_module_and_instance_method :format_options
 
-    def defaults_translations(locale) #:nodoc:
-      defaults      = DEFAULTS[:format].dup
-      i18n_defaults = I18n.translate(:'number.format', :locale => locale, :default => {})
-      defaults.merge!(i18n_defaults)
+    def default_format_options(namespace = nil) #:nodoc:
+      options = DEFAULTS[:format].dup
+      options.merge!(DEFAULTS[namespace][:format]) if namespace
+      options
     end
-    private_module_and_instance_method :defaults_translations
+    private_module_and_instance_method :default_format_options
 
-    def translations_for(namespace, locale) #:nodoc:
-      defaults      = DEFAULTS[namespace][:format].dup
-      i18n_defaults = I18n.translate(:"number.#{namespace}.format", :locale => locale, :default => {})
-      defaults.merge!(i18n_defaults)
+    def i18n_format_options(locale, namespace = nil) #:nodoc:
+      options = I18n.translate(:'number.format', locale: locale, default: {}).dup
+      if namespace
+        options.merge!(I18n.translate(:"number.#{namespace}.format", locale: locale, default: {}))
+      end
+      options
     end
-    private_module_and_instance_method :translations_for
+    private_module_and_instance_method :i18n_format_options
 
-    def translate_number_value_with_default(key, i18n_options = {})
-      defaults_keys = key.split('.')
-      default       = DEFAULTS
-      while default_key = defaults_keys.shift
-        default = default[default_key.to_sym]
-      end
+    def translate_number_value_with_default(key, i18n_options = {}) #:nodoc:
+      default = key.split('.').reduce(DEFAULTS) { |defaults, k| defaults[k.to_sym] }
 
-      I18n.translate(key, { :default => default, scope: :number }.merge!(i18n_options))
+      I18n.translate(key, { default: default, scope: :number }.merge!(i18n_options))
     end
     private_module_and_instance_method :translate_number_value_with_default
 
diff --git a/activesupport/test/number_helper_i18n_test.rb b/activesupport/test/number_helper_i18n_test.rb
index 4d0d5e8631..65aecece71 100644
--- a/activesupport/test/number_helper_i18n_test.rb
+++ b/activesupport/test/number_helper_i18n_test.rb
@@ -56,6 +56,13 @@ module ActiveSupport
       assert_equal("-$10.00", number_to_currency(-10, :locale => 'empty'))
     end
 
+    def test_locale_default_format_has_precedence_over_helper_defaults
+      I18n.backend.store_translations 'ts',
+        { :number => { :format => { :separator => ";" } } }
+
+      assert_equal("&$ - 10;00", number_to_currency(10, :locale => 'ts'))
+    end
+
     def test_number_to_currency_without_currency_negative_format
       I18n.backend.store_translations 'no_negative_format', :number => {
         :currency => { :format => { :unit => '@', :format => '%n %u' } }
diff --git a/activesupport/test/number_helper_test.rb b/activesupport/test/number_helper_test.rb
index f26d75edfb..5f54587f93 100644
--- a/activesupport/test/number_helper_test.rb
+++ b/activesupport/test/number_helper_test.rb
@@ -369,7 +369,6 @@ module ActiveSupport
           assert number_helper.respond_to?(:valid_float?, true)
         end
       end
-
     end
   end
 end