The comparisons operator (and more particularly == and !=) were not treating the undef value as '', like case and selector did since #2818.
This patch makes sure comparison operator uses AST leaf matching. Unfortunately, doing this introduces a behavior change compared to the previous versions: Numbers embedded in strings will now be matched as numbers in case and selector statements instead of string matching. Signed-off-by: Brice Figureau <[email protected]> --- lib/puppet/parser/ast/comparison_operator.rb | 13 ++-- lib/puppet/parser/ast/leaf.rb | 3 + spec/unit/parser/ast/comparison_operator_spec.rb | 80 +++++++++++++++------- spec/unit/parser/ast/leaf_spec.rb | 21 ++++++ 4 files changed, 87 insertions(+), 30 deletions(-) diff --git a/lib/puppet/parser/ast/comparison_operator.rb b/lib/puppet/parser/ast/comparison_operator.rb index c8694bb..bcb4671 100644 --- a/lib/puppet/parser/ast/comparison_operator.rb +++ b/lib/puppet/parser/ast/comparison_operator.rb @@ -16,17 +16,18 @@ class Puppet::Parser::AST def evaluate(scope) # evaluate the operands, should return a boolean value lval = @lval.safeevaluate(scope) - rval = @rval.safeevaluate(scope) # convert to number if operands are number lval = Puppet::Parser::Scope.number?(lval) || lval - rval = Puppet::Parser::Scope.number?(rval) || rval - # return result - unless @operator == '!=' - lval.send(@operator,rval) + case @operator + when "==","!=" + @rval.evaluate_match(lval, scope) ? @operator == '==' : @operator == '!=' else - lval != rval + rval = @rval.safeevaluate(scope) + rval = Puppet::Parser::Scope.number?(rval) || rval + + lval.send(@operator,rval) end end diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb index 3b9163d..49f4302 100644 --- a/lib/puppet/parser/ast/leaf.rb +++ b/lib/puppet/parser/ast/leaf.rb @@ -17,6 +17,9 @@ class Puppet::Parser::AST obj = obj.downcase if obj.respond_to?(:downcase) value = value.downcase if value.respond_to?(:downcase) + obj = Puppet::Parser::Scope.number?(obj) || obj + value = Puppet::Parser::Scope.number?(value) || value + # "" == undef for case/selector/if obj == value or (obj == "" and value == :undef) end diff --git a/spec/unit/parser/ast/comparison_operator_spec.rb b/spec/unit/parser/ast/comparison_operator_spec.rb index 724b6c6..6d6c377 100755 --- a/spec/unit/parser/ast/comparison_operator_spec.rb +++ b/spec/unit/parser/ast/comparison_operator_spec.rb @@ -5,21 +5,22 @@ require File.dirname(__FILE__) + '/../../../spec_helper' describe Puppet::Parser::AST::ComparisonOperator do before :each do @scope = Puppet::Parser::Scope.new - @one = stub 'one', :safeevaluate => "1" - @two = stub 'two', :safeevaluate => "2" + @one = Puppet::Parser::AST::Leaf.new(:value => "1") + @two = Puppet::Parser::AST::Leaf.new(:value => "2") + + @lval = Puppet::Parser::AST::Leaf.new(:value => "one") + @rval = Puppet::Parser::AST::Leaf.new(:value => "two") end - it "should evaluate both branches" do - lval = stub "lval" - lval.expects(:safeevaluate).with(@scope) - rval = stub "rval" - rval.expects(:safeevaluate).with(@scope) + it "should evaluate both values" do + @lval.expects(:safeevaluate).with(@scope) + @rval.expects(:safeevaluate).with(@scope) - operator = Puppet::Parser::AST::ComparisonOperator.new :lval => lval, :operator => "==", :rval => rval + operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "==", :rval => @rval operator.evaluate(@scope) end - it "should convert arguments strings to numbers if they are" do + it "should convert the arguments to numbers if they are numbers in string" do Puppet::Parser::Scope.expects(:number?).with("1").returns(1) Puppet::Parser::Scope.expects(:number?).with("2").returns(2) @@ -27,25 +28,56 @@ describe Puppet::Parser::AST::ComparisonOperator do operator.evaluate(@scope) end - %w{< > <= >= ==}.each do |oper| + %w{< > <= >=}.each do |oper| it "should use string comparison #{oper} if operands are strings" do - lval = stub 'one', :safeevaluate => "one" - rval = stub 'two', :safeevaluate => "two" - Puppet::Parser::Scope.stubs(:number?).with("one").returns(nil) - Puppet::Parser::Scope.stubs(:number?).with("two").returns(nil) - - operator = Puppet::Parser::AST::ComparisonOperator.new :lval => lval, :operator => oper, :rval => rval + operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => oper, :rval => @rval operator.evaluate(@scope).should == "one".send(oper,"two") end end - it "should fail with arguments of different types" do - lval = stub 'one', :safeevaluate => "one" - rval = stub 'two', :safeevaluate => "2" - Puppet::Parser::Scope.stubs(:number?).with("one").returns(nil) - Puppet::Parser::Scope.stubs(:number?).with("2").returns(2) + describe "with string comparison" do + it "should use matching" do + @rval.expects(:evaluate_match).with("one", @scope) + + operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "==", :rval => @rval + operator.evaluate(@scope) + end + + it "should return true for :undef to '' equality" do + astundef = Puppet::Parser::AST::Leaf.new(:value => :undef) + empty = Puppet::Parser::AST::Leaf.new(:value => '') + + operator = Puppet::Parser::AST::ComparisonOperator.new :lval => astundef, :operator => "==", :rval => empty + operator.evaluate(@scope) + end + + [true, false].each do |result| + it "should return #{(result).inspect} with '==' when matching return #{result.inspect}" do + @rval.expects(:evaluate_match).with("one", @scope).returns result - operator = Puppet::Parser::AST::ComparisonOperator.new :lval => lval, :operator => ">", :rval => rval + operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "==", :rval => @rval + operator.evaluate(@scope).should == result + end + + it "should return #{(!result).inspect} with '!=' when matching return #{result.inspect}" do + @rval.expects(:evaluate_match).with("one", @scope).returns result + + operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "!=", :rval => @rval + operator.evaluate(@scope).should == !result + end + end + end + + + it "should return matching result" do + @rval.expects(:evaluate_match).with("one", @scope) + + operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @lval, :operator => "==", :rval => @rval + operator.evaluate(@scope) + end + + it "should fail with arguments of different types" do + operator = Puppet::Parser::AST::ComparisonOperator.new :lval => @one, :operator => ">", :rval => @rval lambda { operator.evaluate(@scope) }.should raise_error(ArgumentError) end @@ -81,8 +113,8 @@ describe Puppet::Parser::AST::ComparisonOperator do # see ticket #1759 %w{< > <= >=}.each do |oper| it "should return the correct result of using '#{oper}' to compare 10 and 9" do - ten = stub 'one', :safeevaluate => "10" - nine = stub 'two', :safeevaluate => "9" + ten = Puppet::Parser::AST::Leaf.new(:value => "10") + nine = Puppet::Parser::AST::Leaf.new(:value => "9") operator = Puppet::Parser::AST::ComparisonOperator.new :lval => ten, :operator => oper, :rval => nine operator.evaluate(@scope).should == 10.send(oper,9) diff --git a/spec/unit/parser/ast/leaf_spec.rb b/spec/unit/parser/ast/leaf_spec.rb index 379cbfd..d21cbf5 100755 --- a/spec/unit/parser/ast/leaf_spec.rb +++ b/spec/unit/parser/ast/leaf_spec.rb @@ -35,6 +35,27 @@ describe Puppet::Parser::AST::Leaf do @leaf.evaluate_match("value", @scope) end + it "should convert values to number" do + @leaf.stubs(:safeevaluate).with(@scope).returns(@value) + Puppet::Parser::Scope.expects(:number?).with(@value).returns(2) + Puppet::Parser::Scope.expects(:number?).with("23").returns(23) + + @leaf.evaluate_match("23", @scope) + end + + it "should compare 'numberized' values" do + @leaf.stubs(:safeevaluate).with(@scope).returns(@value) + two = stub_everything 'two' + one = stub_everything 'one' + + Puppet::Parser::Scope.stubs(:number?).with(@value).returns(one) + Puppet::Parser::Scope.stubs(:number?).with("2").returns(two) + + one.expects(:==).with(two) + + @leaf.evaluate_match("2", @scope) + end + it "should match undef if value is an empty string" do @leaf.stubs(:safeevaluate).with(@scope).returns("") -- 1.6.6.1 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
