Merge pull request #3134 from hlindberg/PUP-3366_string-enum-type-calculation

PUP-3366 Fix issues with string/enum type assignable calculations

2 files changed, 39 insertions, 7 deletions

diff --git a/lib/puppet/pops/types/type_calculator.rb b/lib/puppet/pops/types/type_calculator.rb
index 644d007cd..fd1487b78 100644
--- a/lib/puppet/pops/types/type_calculator.rb
+++ b/lib/puppet/pops/types/type_calculator.rb
@@ -1155,11 +1155,13 @@ class Puppet::Pops::Types::TypeCalculator
     case t2
     when Types::PStringType
       # if the set of strings are all found in the set of enums
-      t2.values.all? { |s| t.values.any? { |e| e == s }}
+      !t2.values.empty?() && t2.values.all? { |s| t.values.any? { |e| e == s }}
     when Types::PVariantType
       t2.types.all? {|variant_t| assignable_PEnumType(t, variant_t) }
     when Types::PEnumType
-      t2.values.all? { |s| t.values.any? {|e| e == s }}
+      # empty means any enum
+      return true if t.values.empty?
+      !t2.values.empty? && t2.values.all? { |s| t.values.any? {|e| e == s }}
     else
       false
     end
@@ -1184,7 +1186,7 @@ class Puppet::Pops::Types::TypeCalculator
         assignable_PIntegerType(size_t, @collection_default_size_t)
 
       when Types::PEnumType
-        if t2.values
+        if t2.values && !t2.values.empty?
           # true if all enum values are within range
           min, max = t2.values.map(&:size).minmax
           trange =  from_to_ordered(size_t.from, size_t.to)
@@ -1192,8 +1194,9 @@ class Puppet::Pops::Types::TypeCalculator
           # If t2 min and max are within the range of t
           trange[0] <= t2range[0] && trange[1] >= t2range[1]
         else
-          # no string can match this enum anyway since it does not accept anything
-          false
+          # enum represents all enums, and thus all strings, a sized constrained string can thus not
+          # be assigned any enum (unless it is max size).
+          assignable_PIntegerType(size_t, @collection_default_size_t)
         end
       else
         # no other type matches string
@@ -1539,8 +1542,6 @@ class Puppet::Pops::Types::TypeCalculator
     # translate to string, and skip Unit types
     types = t.param_types.types.map {|t2| string(t2) unless t2.class == Types::PUnitType }.compact
 
-    params_part= types.join(', ')
-
     s = "Callable[" << types.join(', ')
     unless range.empty?
       (s << ', ') unless types.empty?
diff --git a/spec/unit/pops/types/type_calculator_spec.rb b/spec/unit/pops/types/type_calculator_spec.rb
index 0bd475263..b11ed23a9 100644
--- a/spec/unit/pops/types/type_calculator_spec.rb
+++ b/spec/unit/pops/types/type_calculator_spec.rb
@@ -10,6 +10,9 @@ describe 'The type calculator' do
    t.to = to
    t
   end
+  def constrained_t(t, from, to)
+    Puppet::Pops::Types::TypeFactory.constrain_size(t, from, to)
+  end
 
   def pattern_t(*patterns)
     Puppet::Pops::Types::TypeFactory.pattern(*patterns)
@@ -842,12 +845,40 @@ describe 'The type calculator' do
         calculator.assignable?(enum_t('a', 'b'), enum_t('c')).should == false
       end
 
+      it 'non parameterized enum accepts any other enum but not the reverse' do
+        calculator.assignable?(enum_t(), enum_t('a')).should == true
+        calculator.assignable?(enum_t('a'), enum_t()).should == false
+      end
+
       it 'enum should accept a variant where all variants are acceptable' do
         enum = enum_t('a', 'b')
         calculator.assignable?(enum, variant_t(string_t('a'), string_t('b'))).should == true
       end
     end
 
+    context 'when dealing with string and enum combinations' do
+      it 'should accept assigning any enum to unrestricted string' do
+        calculator.assignable?(string_t(), enum_t('blue')).should == true
+        calculator.assignable?(string_t(), enum_t('blue', 'red')).should == true
+      end
+
+      it 'should not accept assigning longer enum value to size restricted string' do
+        calculator.assignable?(constrained_t(string_t(),2,2), enum_t('a','blue')).should == false
+      end
+
+      it 'should accept assigning any string to empty enum' do
+        calculator.assignable?(enum_t(), string_t()).should == true
+      end
+
+      it 'should accept assigning empty enum to any string' do
+        calculator.assignable?(string_t(), enum_t()).should == true
+      end
+
+      it 'should not accept assigning empty enum to size constrained string' do
+        calculator.assignable?(constrained_t(string_t(),2,2), enum_t()).should == false
+      end
+    end
+
     context 'when dealing with tuples' do
       it 'matches empty tuples' do
         tuple1 = tuple_t()