(PUP-1195) Fix is_numeric/is_integer when checking non-string parameters

I expect a function called "is_numeric" or "is_integer" to check if a
variable is an integer or a number even if the variable passed by isn't
a string nor a number at all. Otherwise we should call them
is_string_a_number and is_string_an_integer and we have then to remove
the check for .is_a?(Number) and .is_a?(FixNum)

now checking also if it is a hex or octal number

improved/corrected checking for integer

* checking against Integer instead of Fixnum so that
  also Bignum is matching
* now .is_a? Integer is done first so this is quiet fast

Now many types of numerics are recognized.

1. Float/Integer values (signed or unsigned, with exponent or without)
2. octal and hex check
3. except hex numbers and the "0." in a float lower than 1 can be prefixed
   with a '0'.

whitespaces shouldn't be allowed as prefix/suffix

string representation of numbers should not contain any type of
whitespace.. the user is responsible to clean a string before checking
it..

fix documentation and added more checks

tried to be 99.9% backward compatible

* for now the decission is post poned if hex and octal numbers
  should be allowed or not (is_numeric)
* native Bignum is now also a valid integer class

fix problem with old 1.8 ruby and Hash.to_s/Array.to_s

In ruby < 1.9 array and hashes would be recognized as numeric
if they have a special format:

1.8:

  [1,2,3,4].to_s = "1234"
  {1=>2}.to_s    = "12"

1.9:

  [1,2,3,4].to_s = "[1, 2, 3, 4]"
  {1=>2}.to_s    = "{1=>2}"

4 files changed, 184 insertions, 7 deletions

diff --git a/lib/puppet/parser/functions/is_integer.rb b/lib/puppet/parser/functions/is_integer.rb
index 6b29e98..3c4874e 100644
--- a/lib/puppet/parser/functions/is_integer.rb
+++ b/lib/puppet/parser/functions/is_integer.rb
@@ -4,7 +4,10 @@
 
 module Puppet::Parser::Functions
   newfunction(:is_integer, :type => :rvalue, :doc => <<-EOS
-Returns true if the variable returned to this string is an integer.
+Returns true if the variable passed to this function is an integer.
+
+If the variable is a string it has to be in the correct format of an
+integer.
     EOS
   ) do |arguments|
 
@@ -15,12 +18,25 @@ Returns true if the variable returned to this string is an integer.
 
     value = arguments[0]
 
-    if value != value.to_i.to_s and !value.is_a? Fixnum then
-      return false
-    else
+    # Regex is taken from the lexer of puppet
+    # puppet/pops/parser/lexer.rb but modified to match also
+    # negative values and disallow numbers prefixed with multiple
+    # 0's
+    #
+    # TODO these parameter should be a constant but I'm not sure
+    # if there is no risk to declare it inside of the module
+    # Puppet::Parser::Functions
+
+    # Integer numbers like
+    # -1234568981273
+    # 47291
+    numeric = %r{^-?(?:(?:[1-9]\d*)|0)$}
+
+    if value.is_a? Integer or (value.is_a? String and value.match numeric)
       return true
+    else
+      return false
     end
-
   end
 end
 
diff --git a/lib/puppet/parser/functions/is_numeric.rb b/lib/puppet/parser/functions/is_numeric.rb
index abf0321..f2417f3 100644
--- a/lib/puppet/parser/functions/is_numeric.rb
+++ b/lib/puppet/parser/functions/is_numeric.rb
@@ -5,6 +5,20 @@
 module Puppet::Parser::Functions
   newfunction(:is_numeric, :type => :rvalue, :doc => <<-EOS
 Returns true if the variable passed to this function is a number.
+
+The function recognizes only integer and float but not hex or octal
+numbers (for now) until a decision is made how to handle these types.
+
+The parameter can be in the native format or given as string representation
+of a number.
+
+Valid examples:
+
+  77435
+  10e-12
+  -8475
+  0.2343
+  -23.561e3
     EOS
   ) do |arguments|
 
@@ -15,12 +29,44 @@ Returns true if the variable passed to this function is a number.
 
     value = arguments[0]
 
-    if value == value.to_f.to_s or value == value.to_i.to_s or value.is_a? Numeric then
+    # Regex is taken from the lexer of puppet
+    # puppet/pops/parser/lexer.rb but modified to match also
+    # negative values and disallow invalid octal numbers or
+    # numbers prefixed with multiple 0's (except in hex numbers)
+    #
+    # TODO these parameters should be constants but I'm not sure
+    # if there is no risk to declare them inside of the module
+    # Puppet::Parser::Functions
+
+    # TODO decide if this should be used
+    # HEX numbers like
+    # 0xaa230F
+    # 0X1234009C
+    # 0x0012
+    # -12FcD
+    #numeric_hex = %r{^-?0[xX][0-9A-Fa-f]+$}
+
+    # TODO decide if this should be used
+    # OCTAL numbers like
+    # 01234567
+    # -045372
+    #numeric_oct = %r{^-?0[1-7][0-7]*$}
+
+    # Integer/Float numbers like
+    # -0.1234568981273
+    # 47291
+    # 42.12345e-12
+    numeric = %r{^-?(?:(?:[1-9]\d*)|0)(?:\.\d+)?(?:[eE]-?\d+)?$}
+
+    if value.is_a? Numeric or (value.is_a? String and (
+      value.match(numeric) #or
+    #  value.match(numeric_hex) or
+    #  value.match(numeric_oct)
+    ))
       return true
     else
       return false
     end
-
   end
 end
 
diff --git a/spec/unit/puppet/parser/functions/is_integer_spec.rb b/spec/unit/puppet/parser/functions/is_integer_spec.rb
index 4335795..24141cc 100644
--- a/spec/unit/puppet/parser/functions/is_integer_spec.rb
+++ b/spec/unit/puppet/parser/functions/is_integer_spec.rb
@@ -17,6 +17,11 @@ describe "the is_integer function" do
     result.should(eq(true))
   end
 
+  it "should return true if a negative integer" do
+    result = scope.function_is_integer(["-7"])
+    result.should(eq(true))
+  end
+
   it "should return false if a float" do
     result = scope.function_is_integer(["3.2"])
     result.should(eq(false))
@@ -31,4 +36,34 @@ describe "the is_integer function" do
     result = scope.function_is_integer([3*2])
     result.should(eq(true))
   end
+
+  it "should return false if an array" do
+    result = scope.function_is_numeric([["asdf"]])
+    result.should(eq(false))
+  end
+
+  it "should return false if a hash" do
+    result = scope.function_is_numeric([{"asdf" => false}])
+    result.should(eq(false))
+  end
+
+  it "should return false if a boolean" do
+    result = scope.function_is_numeric([true])
+    result.should(eq(false))
+  end
+
+  it "should return false if a whitespace is in the string" do
+    result = scope.function_is_numeric([" -1324"])
+    result.should(eq(false))
+  end
+
+  it "should return false if it is zero prefixed" do
+    result = scope.function_is_numeric(["0001234"])
+    result.should(eq(false))
+  end
+
+  it "should return false if it is wrapped inside an array" do
+    result = scope.function_is_numeric([[1234]])
+    result.should(eq(false))
+  end
 end
diff --git a/spec/unit/puppet/parser/functions/is_numeric_spec.rb b/spec/unit/puppet/parser/functions/is_numeric_spec.rb
index d7440fb..1df1497 100644
--- a/spec/unit/puppet/parser/functions/is_numeric_spec.rb
+++ b/spec/unit/puppet/parser/functions/is_numeric_spec.rb
@@ -36,4 +36,84 @@ describe "the is_numeric function" do
     result = scope.function_is_numeric(["asdf"])
     result.should(eq(false))
   end
+
+  it "should return false if an array" do
+    result = scope.function_is_numeric([["asdf"]])
+    result.should(eq(false))
+  end
+
+  it "should return false if an array of integers" do
+    result = scope.function_is_numeric([[1,2,3,4]])
+    result.should(eq(false))
+  end
+
+  it "should return false if a hash" do
+    result = scope.function_is_numeric([{"asdf" => false}])
+    result.should(eq(false))
+  end
+
+  it "should return false if a hash with numbers in it" do
+    result = scope.function_is_numeric([{1 => 2}])
+    result.should(eq(false))
+  end
+
+  it "should return false if a boolean" do
+    result = scope.function_is_numeric([true])
+    result.should(eq(false))
+  end
+
+  it "should return true if a negative float with exponent" do
+    result = scope.function_is_numeric(["-342.2315e-12"])
+    result.should(eq(true))
+  end
+
+  it "should return false if a negative integer with whitespaces before/after the dash" do
+    result = scope.function_is_numeric([" -  751"])
+    result.should(eq(false))
+  end
+
+#  it "should return true if a hexadecimal" do
+#    result = scope.function_is_numeric(["0x52F8c"])
+#    result.should(eq(true))
+#  end
+#
+#  it "should return true if a hexadecimal with uppercase 0X prefix" do
+#    result = scope.function_is_numeric(["0X52F8c"])
+#    result.should(eq(true))
+#  end
+#
+#  it "should return false if a hexadecimal without a prefix" do
+#    result = scope.function_is_numeric(["52F8c"])
+#    result.should(eq(false))
+#  end
+#
+#  it "should return true if a octal" do
+#    result = scope.function_is_numeric(["0751"])
+#    result.should(eq(true))
+#  end
+#
+#  it "should return true if a negative hexadecimal" do
+#    result = scope.function_is_numeric(["-0x52F8c"])
+#    result.should(eq(true))
+#  end
+#
+#  it "should return true if a negative octal" do
+#    result = scope.function_is_numeric(["-0751"])
+#    result.should(eq(true))
+#  end
+#
+#  it "should return false if a negative octal with whitespaces before/after the dash" do
+#    result = scope.function_is_numeric([" -  0751"])
+#    result.should(eq(false))
+#  end
+#
+#  it "should return false if a bad hexadecimal" do
+#    result = scope.function_is_numeric(["0x23d7g"])
+#    result.should(eq(false))
+#  end
+#
+#  it "should return false if a bad octal" do
+#    result = scope.function_is_numeric(["0287"])
+#    result.should(eq(false))
+#  end
 end