NumberHelper methods should now return html_safe strings (when the inputs are valid numbers or are html_safe). Also adds :raise => true (used internaly) to make the number helpers throw InvalidNumberError when the given number is invalid.

Signed-off-by: José Valim <jose.valim@gmail.com>

2 files changed, 207 insertions, 51 deletions

diff --git a/actionpack/lib/action_view/helpers/number_helper.rb b/actionpack/lib/action_view/helpers/number_helper.rb
index 00c54f7644..ca2322499c 100644
--- a/actionpack/lib/action_view/helpers/number_helper.rb
+++ b/actionpack/lib/action_view/helpers/number_helper.rb
@@ -3,6 +3,7 @@ require 'active_support/core_ext/float/rounding'
 
 module ActionView
   module Helpers #:nodoc:
+
     # Provides methods for converting numbers into formatted strings.
     # Methods are provided for phone numbers, currency, percentage,
     # precision, positional notation, file size and pretty printing.
@@ -10,6 +11,16 @@ module ActionView
     # Most methods expect a +number+ argument, and will return it
     # unchanged if can't be converted into a valid number.
     module NumberHelper
+
+      # Raised when argument +number+ param given to the helpers is invalid and
+      # the option :raise is set to  +true+.
+      class InvalidNumberError < StandardError
+        attr_accessor :number
+        def initialize(number)
+          @number = number
+        end
+      end
+
       # Formats a +number+ into a US phone number (e.g., (555) 123-9876). You can customize the format
       # in the +options+ hash.
       #
@@ -33,6 +44,17 @@ module ActionView
       def number_to_phone(number, options = {})
         return nil if number.nil?
 
+        begin
+          Float(number)
+          is_number_html_safe = true
+        rescue ArgumentError, TypeError
+          if options[:raise]
+            raise InvalidNumberError, number
+          else
+            is_number_html_safe = number.to_s.html_safe?
+          end
+        end
+
         number       = number.to_s.strip
         options      = options.symbolize_keys
         area_code    = options[:area_code] || nil
@@ -49,7 +71,7 @@ module ActionView
           number.starts_with?('-') ? number.slice!(1..-1) : number
         end
         str << " x #{extension}" unless extension.blank?
-        str
+        is_number_html_safe ? str.html_safe : str
       end
 
       # Formats a +number+ into a currency string (e.g., $13.65). You can customize the format
@@ -75,6 +97,8 @@ module ActionView
       #  number_to_currency(1234567890.50, :unit => "&pound;", :separator => ",", :delimiter => "", :format => "%n %u")
       #  # => 1234567890,50 &pound;
       def number_to_currency(number, options = {})
+        return nil if number.nil?
+
         options.symbolize_keys!
 
         defaults  = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
@@ -86,13 +110,18 @@ module ActionView
         unit      = options.delete(:unit)
         format    = options.delete(:format)
 
-        value = number_with_precision(number, options)
-
-        if value
+        begin
+          value = number_with_precision(number, options.merge(:raise => true))
           format.gsub(/%n/, value).gsub(/%u/, unit).html_safe
-        else
-          number
+        rescue InvalidNumberError => e
+          if options[:raise]
+            raise
+          else
+            formatted_number = format.gsub(/%n/, e.number).gsub(/%u/, unit)
+            e.number.to_s.html_safe? ? formatted_number.html_safe : formatted_number
+          end
         end
+
       end
 
       # Formats a +number+ as a percentage string (e.g., 65%). You can customize the
@@ -111,6 +140,8 @@ module ActionView
       #  number_to_percentage(1000, :delimiter => '.', :separator => ',') # => 1.000,000%
       #  number_to_percentage(302.24398923423, :precision => 5)           # => 302.24399%
       def number_to_percentage(number, options = {})
+        return nil if number.nil?
+
         options.symbolize_keys!
 
         defaults   = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
@@ -119,8 +150,15 @@ module ActionView
 
         options = options.reverse_merge(defaults)
 
-        value = number_with_precision(number, options)
-        value ? value + "%" : number
+        begin
+          "#{number_with_precision(number, options.merge(:raise => true))}%".html_safe
+        rescue InvalidNumberError => e
+          if options[:raise]
+            raise
+          else
+            e.number.to_s.html_safe? ? "#{e.number}%".html_safe : "#{e.number}%"
+          end
+        end
       end
 
       # Formats a +number+ with grouped thousands using +delimiter+ (e.g., 12,324). You can
@@ -147,6 +185,16 @@ module ActionView
         options = args.extract_options!
         options.symbolize_keys!
 
+        begin
+          Float(number)
+        rescue ArgumentError, TypeError
+          if options[:raise]
+            raise InvalidNumberError, number
+          else
+            return number
+          end
+        end
+
         defaults = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
 
         unless args.empty?
@@ -159,12 +207,8 @@ module ActionView
         options = options.reverse_merge(defaults)
 
         parts = number.to_s.split('.')
-        if parts[0]
-          parts[0].gsub!(/(\d)(?=(\d\d\d)+(?!\d))/, "\\1#{options[:delimiter]}")
-          parts.join(options[:separator])
-        else
-          number
-        end
+        parts[0].gsub!(/(\d)(?=(\d\d\d)+(?!\d))/, "\\1#{options[:delimiter]}")
+        parts.join(options[:separator]).html_safe
 
       end
 
@@ -197,15 +241,20 @@ module ActionView
       # +precision+ as its optional second parameter:
       #   number_with_precision(111.2345, 2)   # => 111.23
       def number_with_precision(number, *args)
+
+        options = args.extract_options!
+        options.symbolize_keys!
+
         number = begin
           Float(number)
         rescue ArgumentError, TypeError
-          return number
+          if options[:raise]
+            raise InvalidNumberError, number
+          else
+            return number
+          end
         end
 
-        options = args.extract_options!
-        options.symbolize_keys!
-
         defaults           = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
         precision_defaults = I18n.translate(:'number.precision.format', :locale => options[:locale], :default => {})
         defaults           = defaults.merge(precision_defaults)
@@ -233,7 +282,7 @@ module ActionView
         formatted_number = number_with_delimiter("%01.#{precision}f" % rounded_number, options)
         if strip_unsignificant_zeros
           escaped_separator = Regexp.escape(options[:separator])
-          formatted_number.sub(/(#{escaped_separator})(\d*[1-9])?0+\z/, '\1\2').sub(/#{escaped_separator}\z/, '')
+          formatted_number.sub(/(#{escaped_separator})(\d*[1-9])?0+\z/, '\1\2').sub(/#{escaped_separator}\z/, '').html_safe
         else
           formatted_number
         end
@@ -276,15 +325,19 @@ module ActionView
       #  number_to_human_size(1234567, 1)    # => 1 MB
       #  number_to_human_size(483989, 2)     # => 470 KB
       def number_to_human_size(number, *args)
+        options = args.extract_options!
+        options.symbolize_keys!
+
         number = begin
           Float(number)
         rescue ArgumentError, TypeError
-          return number
+          if options[:raise]
+            raise InvalidNumberError, number
+          else
+            return number
+          end
         end
 
-        options = args.extract_options!
-        options.symbolize_keys!
-
         defaults = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
         human    = I18n.translate(:'number.human.format', :locale => options[:locale], :default => {})
         defaults = defaults.merge(human)
@@ -303,7 +356,7 @@ module ActionView
 
         if number.to_i < 1024
           unit = I18n.translate(:'number.human.storage_units.units.byte', :locale => options[:locale], :count => number.to_i, :raise => true)
-          storage_units_format.gsub(/%n/, number.to_i.to_s).gsub(/%u/, unit)
+          storage_units_format.gsub(/%n/, number.to_i.to_s).gsub(/%u/, unit).html_safe
         else
           max_exp  = STORAGE_UNITS.size - 1
           exponent = (Math.log(number) / Math.log(1024)).to_i # Convert to base 1024
@@ -314,7 +367,7 @@ module ActionView
           unit = I18n.translate(:"number.human.storage_units.units.#{unit_key}", :locale => options[:locale], :count => number, :raise => true)
 
           formatted_number = number_with_precision(number, options)
-          storage_units_format.gsub(/%n/, formatted_number).gsub(/%u/, unit)
+          storage_units_format.gsub(/%n/, formatted_number).gsub(/%u/, unit).html_safe
         end
       end
 
@@ -395,14 +448,18 @@ module ActionView
       #  number_to_human(0.34, :units => :distance)                                # => "34 centimeters"
       #
       def number_to_human(number, options = {})
+        options.symbolize_keys!
+
         number = begin
           Float(number)
         rescue ArgumentError, TypeError
-          return number
+          if options[:raise]
+            raise InvalidNumberError, number
+          else
+            return number
+          end
         end
 
-        options.symbolize_keys!
-
         defaults = I18n.translate(:'number.format', :locale => options[:locale], :default => {})
         human    = I18n.translate(:'number.human.format', :locale => options[:locale], :default => {})
         defaults = defaults.merge(human)
@@ -438,7 +495,7 @@ module ActionView
 
         decimal_format = options[:format] || I18n.translate(:'number.human.decimal_units.format', :locale => options[:locale], :default => "%n %u")
         formatted_number = number_with_precision(number, options)
-        decimal_format.gsub(/%n/, formatted_number).gsub(/%u/, unit).strip
+        decimal_format.gsub(/%n/, formatted_number).gsub(/%u/, unit).strip.html_safe
       end
 
     end
diff --git a/actionpack/test/template/number_helper_test.rb b/actionpack/test/template/number_helper_test.rb
index c90d89fb61..6adbe9c098 100644
--- a/actionpack/test/template/number_helper_test.rb
+++ b/actionpack/test/template/number_helper_test.rb
@@ -40,8 +40,6 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal("+18005551212", number_to_phone(8005551212, :country_code => 1, :delimiter => ''))
     assert_equal("22-555-1212", number_to_phone(225551212))
     assert_equal("+45-22-555-1212", number_to_phone(225551212, :country_code => 45))
-    assert_equal("x", number_to_phone("x"))
-    assert_nil number_to_phone(nil)
   end
 
   def test_number_to_currency
@@ -52,9 +50,6 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal("&pound;1234567890,50", number_to_currency(1234567890.50, {:unit => "&pound;", :separator => ",", :delimiter => ""}))
     assert_equal("$1,234,567,890.50", number_to_currency("1234567890.50"))
     assert_equal("1,234,567,890.50 K&#269;", number_to_currency("1234567890.50", {:unit => "K&#269;", :format => "%n %u"}))
-    assert_equal("$x.", number_to_currency("x."))
-    assert_equal("$x", number_to_currency("x"))
-    assert_nil number_to_currency(nil)
   end
 
   def test_number_to_percentage
@@ -63,10 +58,8 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal("302.06%", number_to_percentage(302.0574, {:precision => 2}))
     assert_equal("100.000%", number_to_percentage("100"))
     assert_equal("1000.000%", number_to_percentage("1000"))
-    assert_equal("x%", number_to_percentage("x"))
     assert_equal("123.4%", number_to_percentage(123.400, :precision => 3, :strip_unsignificant_zeros => true))
     assert_equal("1.000,000%", number_to_percentage(1000, :delimiter => '.', :separator => ','))
-    assert_nil number_to_percentage(nil)
   end
 
   def test_number_with_delimiter
@@ -80,8 +73,6 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal("123,456,789.78901", number_with_delimiter(123456789.78901))
     assert_equal("0.78901", number_with_delimiter(0.78901))
     assert_equal("123,456.78", number_with_delimiter("123456.78"))
-    assert_equal("x", number_with_delimiter("x"))
-    assert_nil number_with_delimiter(nil)
   end
 
   def test_number_with_delimiter_with_options_hash
@@ -111,11 +102,6 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal("3268", number_with_precision((32.6751 * 100.00), :precision => 0))
     assert_equal("112", number_with_precision(111.50, :precision => 0))
     assert_equal("1234567892", number_with_precision(1234567891.50, :precision => 0))
-
-    # Return non-numeric params unchanged.
-    assert_equal("x.", number_with_precision("x."))
-    assert_equal("x", number_with_precision("x"))
-    assert_nil number_with_precision(nil)
   end
 
   def test_number_with_precision_with_custom_delimiter_and_separator
@@ -183,10 +169,6 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal '10 KB',   number_to_human_size(kilobytes(10.000), :precision => 4)
     assert_equal '1 Byte',   number_to_human_size(1.1)
     assert_equal '10 Bytes', number_to_human_size(10)
-
-    # Return non-numeric params unchanged.
-    assert_equal "x", number_to_human_size('x')
-    assert_nil number_to_human_size(nil)
   end
 
   def test_number_to_human_size_with_options_hash
@@ -234,10 +216,6 @@ class NumberHelperTest < ActionView::TestCase
      assert_equal '1.2346 Million', number_to_human(1234567, :precision => 4, :significant => false)
      assert_equal '1,2 Million', number_to_human(1234567, :precision => 1, :significant => false, :separator => ',')
      assert_equal '1 Million', number_to_human(1234567, :precision => 0, :significant => true, :separator => ',') #significant forced to false
-
-     # Return non-numeric params unchanged.
-     assert_equal "x", number_to_human('x')
-     assert_nil number_to_human(nil)
   end
 
   def test_number_to_human_with_custom_units
@@ -278,4 +256,125 @@ class NumberHelperTest < ActionView::TestCase
     assert_equal '123.lt', number_to_human(123456, :units => volume, :format => "%n.%u")
   end
 
+  def test_number_helpers_should_return_nil_when_given_nil
+    assert_nil number_to_phone(nil)
+    assert_nil number_to_currency(nil)
+    assert_nil number_to_percentage(nil)
+    assert_nil number_with_delimiter(nil)
+    assert_nil number_with_precision(nil)
+    assert_nil number_to_human_size(nil)
+    assert_nil number_to_human(nil)
+  end
+
+  def test_number_helpers_should_return_non_numeric_param_unchanged
+    assert_equal("+1-x x 123", number_to_phone("x", :country_code => 1, :extension => 123))
+    assert_equal("x", number_to_phone("x"))
+    assert_equal("$x.", number_to_currency("x."))
+    assert_equal("$x", number_to_currency("x"))
+    assert_equal("x%", number_to_percentage("x"))
+    assert_equal("x", number_with_delimiter("x"))
+    assert_equal("x.", number_with_precision("x."))
+    assert_equal("x", number_with_precision("x"))
+    assert_equal "x", number_to_human_size('x')
+    assert_equal "x", number_to_human('x')
+  end
+
+  def test_number_helpers_outputs_are_html_safe
+    assert number_to_human(1).html_safe?
+    assert !number_to_human("<script></script>").html_safe?
+    assert number_to_human("asdf".html_safe).html_safe?
+
+    assert number_to_human_size(1).html_safe?
+    assert number_to_human_size(1000000).html_safe?
+    assert !number_to_human_size("<script></script>").html_safe?
+    assert number_to_human_size("asdf".html_safe).html_safe?
+
+    assert number_with_precision(1, :strip_unsignificant_zeros => false).html_safe?
+    assert number_with_precision(1, :strip_unsignificant_zeros => true).html_safe?
+    assert !number_with_precision("<script></script>").html_safe?
+    assert number_with_precision("asdf".html_safe).html_safe?
+
+    assert number_to_currency(1).html_safe?
+    assert !number_to_currency("<script></script>").html_safe?
+    assert number_to_currency("asdf".html_safe).html_safe?
+
+    assert number_to_percentage(1).html_safe?
+    assert !number_to_percentage("<script></script>").html_safe?
+    assert number_to_percentage("asdf".html_safe).html_safe?
+
+    assert number_to_phone(1).html_safe?
+    assert !number_to_phone("<script></script>").html_safe?
+    assert number_to_phone("asdf".html_safe).html_safe?
+
+    assert number_with_delimiter(1).html_safe?
+    assert !number_with_delimiter("<script></script>").html_safe?
+    assert number_with_delimiter("asdf".html_safe).html_safe?
+  end
+
+  def test_number_helpers_should_raise_error_if_invalid_when_specified
+    assert_raise InvalidNumberError do
+      number_to_human("x", :raise => true)
+    end
+    begin
+      number_to_human("x", :raise => true)
+    rescue InvalidNumberError => e
+      assert_equal "x", e.number
+    end
+
+    assert_raise InvalidNumberError do
+      number_to_human_size("x", :raise => true)
+    end
+    begin
+      number_to_human_size("x", :raise => true)
+    rescue InvalidNumberError => e
+      assert_equal "x", e.number
+    end
+
+    assert_raise InvalidNumberError do
+      number_with_precision("x", :raise => true)
+    end
+    begin
+      number_with_precision("x", :raise => true)
+    rescue InvalidNumberError => e
+      assert_equal "x", e.number
+    end
+
+    assert_raise InvalidNumberError do
+      number_to_currency("x", :raise => true)
+    end
+    begin
+      number_with_precision("x", :raise => true)
+    rescue InvalidNumberError => e
+      assert_equal "x", e.number
+    end
+
+    assert_raise InvalidNumberError do
+      number_to_percentage("x", :raise => true)
+    end
+    begin
+      number_to_percentage("x", :raise => true)
+    rescue InvalidNumberError => e
+      assert_equal "x", e.number
+    end
+
+    assert_raise InvalidNumberError do
+     number_with_delimiter("x", :raise => true)
+    end
+    begin
+      number_with_delimiter("x", :raise => true)
+    rescue InvalidNumberError => e
+      assert_equal "x", e.number
+    end
+
+    assert_raise InvalidNumberError do
+     number_to_phone("x", :raise => true)
+    end
+    begin
+      number_to_phone("x", :raise => true)
+    rescue InvalidNumberError => e
+      assert_equal "x", e.number
+    end
+
+  end
+
 end