Fix #2818 - scope variable assigned with undef are not "undef"

The following manifest doesn't work:
$foo = undef
case $foo {
  undef: { notice("undef") }
  default: { notice("defined") }
}

This is because "undef" scope variable are returned as an empty
string.

This patch introduces a behavior change:
Now, unassigned variable usage returns also undef.

This might produce some issues in existing manifests, although
care has been taken to allow correct behavior in the most commonly
used patterns.

For instance:
case $bar {
  undef: { notice("undef") }
  default: { notice("defined") }
}
will print "undef".

But matching undef in case/selector/if will also match "".
case $bar {
  "": { notice("empty") }
  default: { notice("defined") }
}
will print "empty".

Of course "" doesn't match undef :-)

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

3 files changed, 47 insertions, 10 deletions

diff --git a/spec/unit/parser/ast/arithmetic_operator.rb b/spec/unit/parser/ast/arithmetic_operator.rb
index 4a0be483b..ad8d9947b 100755
--- a/spec/unit/parser/ast/arithmetic_operator.rb
+++ b/spec/unit/parser/ast/arithmetic_operator.rb
@@ -61,8 +61,8 @@ describe Puppet::Parser::AST::ArithmeticOperator do
     end
 
     it "should work for variables too" do
-        @scope.expects(:lookupvar).with("one").returns(1)
-        @scope.expects(:lookupvar).with("two").returns(2)
+        @scope.expects(:lookupvar).with("one", false).returns(1)
+        @scope.expects(:lookupvar).with("two", false).returns(2)
         one = ast::Variable.new( :value => "one" )
         two = ast::Variable.new( :value => "two" )
 
diff --git a/spec/unit/parser/ast/comparison_operator.rb b/spec/unit/parser/ast/comparison_operator.rb
index 26311797f..97402433f 100755
--- a/spec/unit/parser/ast/comparison_operator.rb
+++ b/spec/unit/parser/ast/comparison_operator.rb
@@ -71,8 +71,8 @@ describe Puppet::Parser::AST::ComparisonOperator do
         one = Puppet::Parser::AST::Variable.new( :value => "one" )
         two = Puppet::Parser::AST::Variable.new( :value => "two" )
 
-        @scope.expects(:lookupvar).with("one").returns(1)
-        @scope.expects(:lookupvar).with("two").returns(2)
+        @scope.expects(:lookupvar).with("one", false).returns(1)
+        @scope.expects(:lookupvar).with("two", false).returns(2)
 
         operator = Puppet::Parser::AST::ComparisonOperator.new :lval => one, :operator => "<", :rval => two
         operator.evaluate(@scope).should == true
diff --git a/spec/unit/parser/ast/leaf.rb b/spec/unit/parser/ast/leaf.rb
index ee1ee04a7..640c25237 100755
--- a/spec/unit/parser/ast/leaf.rb
+++ b/spec/unit/parser/ast/leaf.rb
@@ -21,6 +21,7 @@ describe Puppet::Parser::AST::Leaf do
         end
 
         it "should match values by equality" do
+            @value.stubs(:==).returns(false)
             @leaf.stubs(:safeevaluate).with(@scope).returns(@value)
             @value.expects(:==).with("value")
 
@@ -33,6 +34,12 @@ describe Puppet::Parser::AST::Leaf do
 
             @leaf.evaluate_match("value", @scope, :insensitive => true)
         end
+
+        it "should match undef if value is an empty string" do
+            @leaf.stubs(:safeevaluate).with(@scope).returns("")
+
+            @leaf.evaluate_match(:undef, @scope).should be_true
+        end
     end
 
     describe "when converting to string" do
@@ -72,12 +79,18 @@ describe Puppet::Parser::AST::String do
     end
 end
 
-describe Puppet::Parser::AST::Variable do
-    describe "when converting to string" do
-        it "should transform its value to a variable" do
-            value = stub 'value', :is_a? => true, :to_s => "myvar"
-            Puppet::Parser::AST::Variable.new( :value => value ).to_s.should == "\$myvar"
-        end
+describe Puppet::Parser::AST::Undef do
+    before :each do
+        @scope = stub 'scope'
+        @undef = Puppet::Parser::AST::Undef.new(:value => :undef)
+    end
+
+    it "should match undef with undef" do
+        @undef.evaluate_match(:undef, @scope).should be_true
+    end
+
+    it "should not match undef with an empty string" do
+        @undef.evaluate_match("", @scope).should be_false
     end
 end
 
@@ -158,6 +171,30 @@ describe Puppet::Parser::AST::Regex do
     end
 end
 
+describe Puppet::Parser::AST::Variable do
+    before :each do
+        @scope = stub 'scope'
+        @var = Puppet::Parser::AST::Variable.new(:value => "myvar")
+    end
+
+    it "should lookup the variable in scope" do
+        @scope.expects(:lookupvar).with("myvar", false).returns(:myvalue)
+        @var.safeevaluate(@scope).should == :myvalue
+    end
+
+    it "should return undef if the variable wasn't set" do
+        @scope.expects(:lookupvar).with("myvar", false).returns(:undefined)
+        @var.safeevaluate(@scope).should == :undef
+    end
+
+    describe "when converting to string" do
+        it "should transform its value to a variable" do
+            value = stub 'value', :is_a? => true, :to_s => "myvar"
+            Puppet::Parser::AST::Variable.new( :value => value ).to_s.should == "\$myvar"
+        end
+    end
+end
+
 describe Puppet::Parser::AST::HostName do
     before :each do
         @scope = stub 'scope'