Merge pull request #3142 from hlindberg/PUP-3365_remove-deep-undef-mapping-in-3x-func-api

(PUP-3365) Change 3x deep map of undef to only do surface map

2 files changed, 53 insertions, 2 deletions

diff --git a/lib/puppet/pops/evaluator/runtime3_support.rb b/lib/puppet/pops/evaluator/runtime3_support.rb
index 127c7d6f5..518b5216e 100644
--- a/lib/puppet/pops/evaluator/runtime3_support.rb
+++ b/lib/puppet/pops/evaluator/runtime3_support.rb
@@ -425,6 +425,7 @@ module Puppet::Pops::Evaluator::Runtime3Support
 
   def initialize
     @@convert_visitor   ||= Puppet::Pops::Visitor.new(self, "convert", 2, 2)
+    @@convert2_visitor   ||= Puppet::Pops::Visitor.new(self, "convert2", 2, 2)
   end
 
   # Converts 4x supported values to 3x values. This is required because
@@ -436,35 +437,53 @@ module Puppet::Pops::Evaluator::Runtime3Support
     @@convert_visitor.visit_this_2(self, o, scope, undef_value)
   end
 
+  # Converts nested 4x supported values to 3x values. This is required because
+  # resources and other objects do not know about the new type system, and does not support
+  # regular expressions. Unfortunately this has to be done for array and hash as well.
+  # A complication is that catalog types needs to be resolved against the scope.
+  #
+  def convert2(o, scope, undef_value)
+    @@convert2_visitor.visit_this_2(self, o, scope, undef_value)
+  end
+
 
   def convert_NilClass(o, scope, undef_value)
     undef_value
   end
 
+  def convert2_NilClass(o, scope, undef_value)
+    :undef
+  end
+
   def convert_String(o, scope, undef_value)
     # although wasteful, needed because user code may mutate these strings in Resources
     o.frozen? ? o.dup : o
   end
+  alias convert2_String :convert_String
 
   def convert_Object(o, scope, undef_value)
     o
   end
+  alias :convert2_Object :convert_Object
 
   def convert_Array(o, scope, undef_value)
-    o.map {|x| convert(x, scope, undef_value) }
+    o.map {|x| convert2(x, scope, undef_value) }
   end
+  alias :convert2_Array :convert_Array
 
   def convert_Hash(o, scope, undef_value)
     result = {}
-    o.each {|k,v| result[convert(k, scope, undef_value)] = convert(v, scope, undef_value) }
+    o.each {|k,v| result[convert2(k, scope, undef_value)] = convert2(v, scope, undef_value) }
     result
   end
+  alias :convert2_Hash :convert_Hash
 
   def convert_Regexp(o, scope, undef_value)
     # Puppet 3x cannot handle parameter values that are reqular expressions. Turn into regexp string in
     # source form
     o.inspect
   end
+  alias :convert2_Regexp :convert_Regexp
 
   def convert_Symbol(o, scope, undef_value)
     case o
@@ -476,9 +495,15 @@ module Puppet::Pops::Evaluator::Runtime3Support
     end
   end
 
+  # The :undef symbol should not be converted when nested in arrays or hashes
+  def convert2_Symbol(o, scope, undef_value)
+    o
+  end
+
   def convert_PAnyType(o, scope, undef_value)
     o
   end
+  alias :convert2_PAnyType :convert_PAnyType
 
   def convert_PCatalogEntryType(o, scope, undef_value)
     # Since 4x does not support dynamic scoping, all names are absolute and can be
@@ -489,6 +514,7 @@ module Puppet::Pops::Evaluator::Runtime3Support
 
     Puppet::Resource.new(*catalog_type_to_split_type_title(o))
   end
+  alias :convert2_PCatalogEntryType :convert_PCatalogEntryType
 
   private
 
diff --git a/spec/unit/pops/evaluator/evaluating_parser_spec.rb b/spec/unit/pops/evaluator/evaluating_parser_spec.rb
index 5e80e4076..bff009ddf 100644
--- a/spec/unit/pops/evaluator/evaluating_parser_spec.rb
+++ b/spec/unit/pops/evaluator/evaluating_parser_spec.rb
@@ -963,6 +963,31 @@ describe 'Puppet::Pops::Evaluator::EvaluatorImpl' do
       expect{parser.evaluate_string(scope, "assert_no_undef({undef => 1})")}.to_not raise_error()
       expect{parser.evaluate_string(scope, "assert_no_undef({1 => undef})")}.to_not raise_error()
     end
+
+    context 'using the 3x function api' do
+      it 'can call a 3x function' do
+        Puppet::Parser::Functions.newfunction("bazinga", :type => :rvalue) { |args| args[0] }
+        parser.evaluate_string(scope, "bazinga(42)", __FILE__).should == 42
+      end
+
+      it 'maps :undef to empty string' do
+        Puppet::Parser::Functions.newfunction("bazinga", :type => :rvalue) { |args| args[0] }
+        parser.evaluate_string(scope, "$a = {} bazinga($a[nope])", __FILE__).should == ''
+        parser.evaluate_string(scope, "bazinga(undef)", __FILE__).should == ''
+      end
+
+      it 'does not map :undef to empty string in arrays' do
+        Puppet::Parser::Functions.newfunction("bazinga", :type => :rvalue) { |args| args[0][0] }
+        parser.evaluate_string(scope, "$a = {} $b = [$a[nope]] bazinga($b)", __FILE__).should == :undef
+        parser.evaluate_string(scope, "bazinga([undef])", __FILE__).should == :undef
+      end
+
+      it 'does not map :undef to empty string in hashes' do
+        Puppet::Parser::Functions.newfunction("bazinga", :type => :rvalue) { |args| args[0]['a'] }
+        parser.evaluate_string(scope, "$a = {} $b = {a => $a[nope]} bazinga($b)", __FILE__).should == :undef
+        parser.evaluate_string(scope, "bazinga({a => undef})", __FILE__).should == :undef
+      end
+    end
   end
 
   context "When evaluator performs string interpolation" do